[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH] block/rbd: add .bdrv_reopen_prepare() stub

From: Josh Durgin
Subject: Re: [Qemu-devel] [PATCH] block/rbd: add .bdrv_reopen_prepare() stub
Date: Tue, 17 May 2016 11:48:15 -0700
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.7.1

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

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?


-- >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 <address@hidden>
  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,

reply via email to

[Prev in Thread] Current Thread [Next in Thread]