From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59397) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aIeyZ-0002LL-6x for qemu-devel@nongnu.org; Mon, 11 Jan 2016 11:05:53 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aIeyT-0007Sg-GL for qemu-devel@nongnu.org; Mon, 11 Jan 2016 11:05:47 -0500 Date: Mon, 11 Jan 2016 17:05:33 +0100 From: Kevin Wolf Message-ID: <20160111160533.GF9454@noname.redhat.com> References: <1450802786-20893-1-git-send-email-kwolf@redhat.com> <1450802786-20893-7-git-send-email-kwolf@redhat.com> <5679BB6A.6080902@redhat.com> <87twmkt8u9.fsf@blackfin.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <87twmkt8u9.fsf@blackfin.pond.sub.org> Subject: Re: [Qemu-devel] [PATCH 06/10] qemu-img: Prepare for locked images List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: qemu-devel@nongnu.org, qemu-block@nongnu.org, mreitz@redhat.com Am 11.01.2016 um 16:49 hat Markus Armbruster geschrieben: > Eric Blake 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 > >> --- > >> 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