Re: [Qemu-devel] [PATCH 06/10] qemu-img: Prepare for locked images

[Kevin Wolf <kwolf@redhat.com>]

Am 11.01.2016 um 16:49 hat Markus Armbruster geschrieben:
> Eric Blake <eblake@redhat.com> writes:
> 
> > On 12/22/2015 09:46 AM, Kevin Wolf wrote:
> >> This patch extends qemu-img for working with locked images. It prints a
> >> helpful error message when trying to access a locked image read-write,
> >> and adds a 'qemu-img force-unlock' command as well as a 'qemu-img check
> >> -r all --force' option in order to override a lock left behind after a
> >> qemu crash.
> >> 
> >> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> >> ---
> >>  include/block/block.h |  1 +
> >>  include/qapi/error.h  |  1 +
> >>  qapi/common.json      |  3 +-
> >>  qemu-img-cmds.hx      | 10 ++++--
> >>  qemu-img.c            | 96 +++++++++++++++++++++++++++++++++++++++++++--------
> >>  qemu-img.texi         | 20 ++++++++++-
> >>  6 files changed, 113 insertions(+), 18 deletions(-)
> >> 
> >
> >> +++ b/include/qapi/error.h
> >> @@ -102,6 +102,7 @@ typedef enum ErrorClass {
> >>      ERROR_CLASS_DEVICE_NOT_ACTIVE = QAPI_ERROR_CLASS_DEVICENOTACTIVE,
> >>      ERROR_CLASS_DEVICE_NOT_FOUND = QAPI_ERROR_CLASS_DEVICENOTFOUND,
> >>      ERROR_CLASS_KVM_MISSING_CAP = QAPI_ERROR_CLASS_KVMMISSINGCAP,
> >> +    ERROR_CLASS_IMAGE_FILE_LOCKED = QAPI_ERROR_CLASS_IMAGEFILELOCKED,
> >>  } ErrorClass;
> >
> > Wow - a new ErrorClass.  It's been a while since we could justify one of
> > these, but I think you might have found a case.
> 
> Spell out the rationale for the new ErrorClass, please.

Action to be taken for this error class: Decide whether the lock is a
leftover from a previous qemu run that ended in an unclean shutdown. If
so, retry with overriding the lock.

Currently used by qemu-img when ordered to override a lock. libvirt
will need to do the same.

Alternative design without a new error class: Accept the
override-lock=on option on the block.c level (QAPI BlockdevOptionsBase)
and ignore it silently for image formats that don't support locking
(i.e. anything but qcow2), so that qemu-img and libvirt can set this
option unconditionally even if they don't know that the image is locked
(but libvirt would have to check first if qemu supports the option at
all in order to maintain compatibility with old qemu versions).

In my opinion, this would be worse design than adding an error class. It
would also prevent libvirt from logging when it forcefully unlocks an
image.

Kevin