From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47682) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b2br8-0003AB-Q6 for qemu-devel@nongnu.org; Tue, 17 May 2016 06:04:03 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1b2br2-0007Pn-P4 for qemu-devel@nongnu.org; Tue, 17 May 2016 06:04:01 -0400 References: <3f8edd94-280c-8aa7-0f07-ab674ee4667b@gmail.com> <20160513084548.GA5529@noname.redhat.com> From: =?UTF-8?Q?Sebastian_F=c3=a4rber?= Message-ID: Date: Tue, 17 May 2016 12:03:53 +0200 MIME-Version: 1.0 In-Reply-To: <20160513084548.GA5529@noname.redhat.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH] block/rbd: add .bdrv_reopen_prepare() stub List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: qemu-block@nongnu.org, qemu-devel@nongnu.org, jdurgin@redhat.com, jcody@redhat.com, mreitz@redhat.com Hi Kevin, > A correct reopen implementation must consider all options and flags that > .bdrv_open() looked at. > > The options are okay, as both "filename" and "password-secret" aren't > things that we want to allow a reopen to change. However, in the flags > BDRV_O_NOCACHE makes a difference: > > if (flags & BDRV_O_NOCACHE) { > rados_conf_set(s->cluster, "rbd_cache", "false"); > } else { > rados_conf_set(s->cluster, "rbd_cache", "true"); > } > > A reopen must either update the setting, or if it can't (e.g. because > librbd doesn't support it) any attempt to change the flag must fail. Thanks for the feedback. As far as i can tell it's not possible to update the cache settings without reconnecting. I've added a check in the following patch. Would be great if someone who knows the internals of ceph/rbd could have a look as well. Sebastian -- >8 -- Subject: [PATCH] block/rbd: add .bdrv_reopen_prepare() stub Add support for reopen() by adding the .bdrv_reopen_prepare() stub Signed-off-by: Sebastian Färber --- block/rbd.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/block/rbd.c b/block/rbd.c index 5bc5b32..8ecf096 100644 --- a/block/rbd.c +++ b/block/rbd.c @@ -577,6 +577,19 @@ failed_opts: return r; } +/* Note that this will not re-establish a connection with the Ceph cluster + - it is effectively a NOP. */ +static int qemu_rbd_reopen_prepare(BDRVReopenState *state, + BlockReopenQueue *queue, Error **errp) +{ + if (state->flags & BDRV_O_NOCACHE && + ((state->bs->open_flags & BDRV_O_NOCACHE) == 0)) { + error_setg(errp, "Cannot turn off rbd_cache during reopen"); + return -EINVAL; + } + return 0; +} + static void qemu_rbd_close(BlockDriverState *bs) { BDRVRBDState *s = bs->opaque; @@ -976,6 +989,7 @@ static BlockDriver bdrv_rbd = { .instance_size = sizeof(BDRVRBDState), .bdrv_needs_filename = true, .bdrv_file_open = qemu_rbd_open, + .bdrv_reopen_prepare = qemu_rbd_reopen_prepare, .bdrv_close = qemu_rbd_close, .bdrv_create = qemu_rbd_create, .bdrv_has_zero_init = bdrv_has_zero_init_1, -- 1.8.3.1