From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45604) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1arQBs-0005Mt-E3 for qemu-devel@nongnu.org; Sat, 16 Apr 2016 09:23:13 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1arQBr-0000Zv-75 for qemu-devel@nongnu.org; Sat, 16 Apr 2016 09:23:12 -0400 References: <1460690887-32751-1-git-send-email-famz@redhat.com> <1460690887-32751-5-git-send-email-famz@redhat.com> From: "Denis V. Lunev" Message-ID: <57123CB3.9070803@openvz.org> Date: Sat, 16 Apr 2016 16:22:59 +0300 MIME-Version: 1.0 In-Reply-To: <1460690887-32751-5-git-send-email-famz@redhat.com> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH for-2.7 v2 04/17] block: Introduce image file locking List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Fam Zheng , qemu-devel@nongnu.org Cc: Kevin Wolf , Max Reitz , Jeff Cody , Markus Armbruster , Eric Blake , John Snow , qemu-block@nongnu.org, berrange@redhat.com, pbonzini@redhat.com On 04/15/2016 06:27 AM, Fam Zheng wrote: > Block drivers can implement this new operation .bdrv_lockf to actually lock the > image in the protocol specific way. > > Signed-off-by: Fam Zheng > --- > block.c | 42 ++++++++++++++++++++++++++++++++++++++++++ > include/block/block_int.h | 12 ++++++++++++ > 2 files changed, 54 insertions(+) > > diff --git a/block.c b/block.c > index 1c575e4..7971a25 100644 > --- a/block.c > +++ b/block.c > @@ -846,6 +846,34 @@ out: > g_free(gen_node_name); > } > > +static int bdrv_lock_unlock_image_do(BlockDriverState *bs, bool lock_image) > +{ > + int cmd = BDRV_LOCKF_UNLOCK; > + > + if (bs->image_locked == lock_image) { > + return 0; > + } else if (!bs->drv) { > + return -ENOMEDIUM; > + } else if (!bs->drv->bdrv_lockf) { > + return 0; > + } > + if (lock_image) { > + cmd = bs->open_flags & BDRV_O_RDWR ? BDRV_LOCKF_RWLOCK : > + BDRV_LOCKF_ROLOCK; > + } > + return bs->drv->bdrv_lockf(bs, cmd); should we handle ENOTSUP specially? f.e. this would fire with raw-posix.c on a filesystem which does not support locking. from my POW this situations is equivalent to the absence of bs->drv->bdrv_lockf > +} > + > +static int bdrv_lock_image(BlockDriverState *bs) > +{ > + return bdrv_lock_unlock_image_do(bs, true); > +} > + > +static int bdrv_unlock_image(BlockDriverState *bs) > +{ > + return bdrv_lock_unlock_image_do(bs, false); > +} > + > static QemuOptsList bdrv_runtime_opts = { > .name = "bdrv_common", > .head = QTAILQ_HEAD_INITIALIZER(bdrv_runtime_opts.head), > @@ -995,6 +1023,14 @@ static int bdrv_open_common(BlockDriverState *bs, BdrvChild *file, > goto free_and_fail; > } > > + if (!(open_flags & (BDRV_O_NO_LOCK | BDRV_O_INACTIVE))) { > + ret = bdrv_lock_image(bs); > + if (ret) { > + error_setg(errp, "Failed to lock image"); > + goto free_and_fail; > + } > + } > + > ret = refresh_total_sectors(bs, bs->total_sectors); > if (ret < 0) { > error_setg_errno(errp, -ret, "Could not refresh total sector count"); > @@ -2144,6 +2180,7 @@ static void bdrv_close(BlockDriverState *bs) > if (bs->drv) { > BdrvChild *child, *next; > > + bdrv_unlock_image(bs); > bs->drv->bdrv_close(bs); > bs->drv = NULL; > > @@ -3230,6 +3267,9 @@ void bdrv_invalidate_cache(BlockDriverState *bs, Error **errp) > error_setg_errno(errp, -ret, "Could not refresh total sector count"); > return; > } > + if (!(bs->open_flags & BDRV_O_NO_LOCK)) { > + bdrv_lock_image(bs); > + } > } > > void bdrv_invalidate_cache_all(Error **errp) > @@ -3262,6 +3302,7 @@ static int bdrv_inactivate(BlockDriverState *bs) > } > > bs->open_flags |= BDRV_O_INACTIVE; > + ret = bdrv_unlock_image(bs); I'd better move unlock a line above. Though this is personal. This could be useful for debugging purposes. > return 0; > } > > @@ -3981,3 +4022,4 @@ void bdrv_refresh_filename(BlockDriverState *bs) > QDECREF(json); > } > } > + this hunk is extra > diff --git a/include/block/block_int.h b/include/block/block_int.h > index 10d8759..ffa30b0 100644 > --- a/include/block/block_int.h > +++ b/include/block/block_int.h > @@ -85,6 +85,12 @@ typedef struct BdrvTrackedRequest { > struct BdrvTrackedRequest *waiting_for; > } BdrvTrackedRequest; > > +typedef enum { > + BDRV_LOCKF_RWLOCK, > + BDRV_LOCKF_ROLOCK, > + BDRV_LOCKF_UNLOCK, > +} BdrvLockfCmd; > + > struct BlockDriver { > const char *format_name; > int instance_size; > @@ -317,6 +323,11 @@ struct BlockDriver { > */ > void (*bdrv_drain)(BlockDriverState *bs); > > + /** > + * Lock/unlock the image. > + */ > + int (*bdrv_lockf)(BlockDriverState *bs, BdrvLockfCmd cmd); > + > QLIST_ENTRY(BlockDriver) list; > }; > > @@ -485,6 +496,7 @@ struct BlockDriverState { > NotifierWithReturn write_threshold_notifier; > > int quiesce_counter; > + bool image_locked; > }; > > struct BlockBackendRootState {