On 05/17/2016 03:03 AM, Sebastian Färber wrote:
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.
Updating this setting on an open image won't do anything, but if you
rbd_close() and rbd_open() it again the setting will take effect.
rbd_close() will force a flush of any pending I/O in librbd and
free the memory for librbd's ImageCtx, which may or may not be desired
here.
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.
There's no need to reset the librados state, so connections to the
cluster can stick around. I'm a bit unclear on the bdrv_reopen_*
functions though - what is their intended use and semantics?
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 <sfaerbe...@gmail.com>
---
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