QEMU code uses "abort()" in certain cases where image locking acquisition options are not expected to fail. This is risky. One example is bug 1588873. Another is 1584982. Although when the abort happens, it is often a misbehave of some other parts of the system rather than QEMU itself, effectively asserting about the external resources is not robust program logic. Specifically, the code in question is usually written as: blk_set_perm(blk, 0, BLK_PERM_ALL, &error_abort); where error_abort should be replaced by local_err and error_report().
Also, it's fairly easy to avoid the locked bits re-acquire waste as described in bug 1588873, like this: commit 1986f53e8958cd8e893d5658f9cb7209551cf98b (image-lock-skip-bits) Author: Fam Zheng <famz> Date: Tue Jun 5 13:11:36 2018 +0800 file-posix: Skip already locked/unlocked bits Signed-off-by: Fam Zheng <famz> diff --git a/block/file-posix.c b/block/file-posix.c index 5a602cfe37..5cc9caaa23 100644 --- a/block/file-posix.c +++ b/block/file-posix.c @@ -650,13 +650,14 @@ static int raw_apply_lock_bytes(BDRVRawState *s, PERM_FOREACH(i) { int off = RAW_LOCK_PERM_BASE + i; - if (perm_lock_bits & (1ULL << i)) { + uint64_t bit = (1ULL << i); + if ((perm_lock_bits & bit) && !(s->perm & bit)) { ret = qemu_lock_fd(s->lock_fd, off, 1, false); if (ret) { error_setg(errp, "Failed to lock byte %d", off); return ret; } - } else if (unlock) { + } else if (unlock && (s->perm & bit)) { ret = qemu_unlock_fd(s->lock_fd, off, 1); if (ret) { error_setg(errp, "Failed to unlock byte %d", off); @@ -666,13 +667,14 @@ static int raw_apply_lock_bytes(BDRVRawState *s, } PERM_FOREACH(i) { int off = RAW_LOCK_SHARED_BASE + i; - if (shared_perm_lock_bits & (1ULL << i)) { + uint64_t bit = (1ULL << i); + if ((shared_perm_lock_bits & bit) && !(s->shared_perm & bit)) { ret = qemu_lock_fd(s->lock_fd, off, 1, false); if (ret) { error_setg(errp, "Failed to lock byte %d", off); return ret; } - } else if (unlock) { + } else if (unlock && (s->shared_perm & bit)) { ret = qemu_unlock_fd(s->lock_fd, off, 1); if (ret) { error_setg(errp, "Failed to unlock byte %d", off);
See also Bug 1526313