Patchwork [V2,4/4] block: Optionally block drivers to optionally reopen images after snapshot creation.

login
register
mail settings
Submitter Benoît Canet
Date Jan. 28, 2013, 5:04 p.m.
Message ID <1359392680-15838-5-git-send-email-benoit@irqsave.net>
Download mbox | patch
Permalink /patch/216253/
State New
Headers show

Comments

Benoît Canet - Jan. 28, 2013, 5:04 p.m.
Protocols like quorum will be able to queue multiple reopens.

Signed-off-by: Benoit Canet <benoit@irqsave.net>
---
 block.c                   |   25 +++++++++++++++++++++++++
 blockdev.c                |    5 +++--
 include/block/block.h     |    2 ++
 include/block/block_int.h |    3 +++
 4 files changed, 33 insertions(+), 2 deletions(-)
Kevin Wolf - Jan. 29, 2013, 12:22 p.m.
Am 28.01.2013 18:04, schrieb Benoît Canet:
> Protocols like quorum will be able to queue multiple reopens.
> 
> Signed-off-by: Benoit Canet <benoit@irqsave.net>

-EPARSE for the subject line.

Also, what's the difference between this and a normal reopen?

Kevin
Benoît Canet - Jan. 29, 2013, 1:09 p.m.
Le Tuesday 29 Jan 2013 à 13:22:12 (+0100), Kevin Wolf a écrit :
> Am 28.01.2013 18:04, schrieb Benoît Canet:
> > Protocols like quorum will be able to queue multiple reopens.
> > 
> > Signed-off-by: Benoit Canet <benoit@irqsave.net>
> 
> -EPARSE for the subject line.
> 
> Also, what's the difference between this and a normal reopen?

With this patch protocols like quorum can do something like :

+static int quorum_snapshot_reopen(BlockDriverState *bs, int bdrv_flags,
+                                  Error **errp)
+{
+    BDRVQuorumState *s = bs->file->opaque;
+    int i, ret = -1;
+    Error *local_err = NULL;
+    BlockReopenQueue *queue = NULL;
+
+    for (i = 0; i < s->total; i++) {
+        queue = bdrv_reopen_queue(queue, s->bs[i], bdrv_flags);
+    }
+
+    ret = bdrv_reopen_multiple(queue, &local_err);
+    if (local_err != NULL) {
+        error_propagate(errp, local_err);
+    }
+    return ret;
+}

It allows to handle the reopening of multiple bs without duplicating a part of
bdrv_reopen_multiple's code.

Benoît


> 
> Kevin
>
Kevin Wolf - Jan. 29, 2013, 1:30 p.m.
Am 29.01.2013 14:09, schrieb Benoît Canet:
> Le Tuesday 29 Jan 2013 à 13:22:12 (+0100), Kevin Wolf a écrit :
>> Am 28.01.2013 18:04, schrieb Benoît Canet:
>>> Protocols like quorum will be able to queue multiple reopens.
>>>
>>> Signed-off-by: Benoit Canet <benoit@irqsave.net>
>>
>> -EPARSE for the subject line.
>>
>> Also, what's the difference between this and a normal reopen?
> 
> With this patch protocols like quorum can do something like :
> 
> +static int quorum_snapshot_reopen(BlockDriverState *bs, int bdrv_flags,
> +                                  Error **errp)
> +{
> +    BDRVQuorumState *s = bs->file->opaque;
> +    int i, ret = -1;
> +    Error *local_err = NULL;
> +    BlockReopenQueue *queue = NULL;
> +
> +    for (i = 0; i < s->total; i++) {
> +        queue = bdrv_reopen_queue(queue, s->bs[i], bdrv_flags);
> +    }
> +
> +    ret = bdrv_reopen_multiple(queue, &local_err);
> +    if (local_err != NULL) {
> +        error_propagate(errp, local_err);
> +    }
> +    return ret;
> +}
> 
> It allows to handle the reopening of multiple bs without duplicating a part of
> bdrv_reopen_multiple's code.

Have a look at VMDK to see how it's supposed to be done.

Kevin
Benoît Canet - Jan. 29, 2013, 1:47 p.m.
Le Tuesday 29 Jan 2013 à 14:30:39 (+0100), Kevin Wolf a écrit :
> Am 29.01.2013 14:09, schrieb Benoît Canet:
> > Le Tuesday 29 Jan 2013 à 13:22:12 (+0100), Kevin Wolf a écrit :
> >> Am 28.01.2013 18:04, schrieb Benoît Canet:
> >>> Protocols like quorum will be able to queue multiple reopens.
> >>>
> >>> Signed-off-by: Benoit Canet <benoit@irqsave.net>
> >>
> >> -EPARSE for the subject line.
> >>
> >> Also, what's the difference between this and a normal reopen?
> > 
> > With this patch protocols like quorum can do something like :
> > 
> > +static int quorum_snapshot_reopen(BlockDriverState *bs, int bdrv_flags,
> > +                                  Error **errp)
> > +{
> > +    BDRVQuorumState *s = bs->file->opaque;
> > +    int i, ret = -1;
> > +    Error *local_err = NULL;
> > +    BlockReopenQueue *queue = NULL;
> > +
> > +    for (i = 0; i < s->total; i++) {
> > +        queue = bdrv_reopen_queue(queue, s->bs[i], bdrv_flags);
> > +    }
> > +
> > +    ret = bdrv_reopen_multiple(queue, &local_err);
> > +    if (local_err != NULL) {
> > +        error_propagate(errp, local_err);
> > +    }
> > +    return ret;
> > +}
> > 
> > It allows to handle the reopening of multiple bs without duplicating a part of
> > bdrv_reopen_multiple's code.
> 
> Have a look at VMDK to see how it's supposed to be done.

ok,
Thanks

> 
> Kevin
>

Patch

diff --git a/block.c b/block.c
index 843583f..e46a106 100644
--- a/block.c
+++ b/block.c
@@ -4652,3 +4652,28 @@  void bdrv_ext_snapshot_img_create(BlockDriverState *old_bs,
                                                     options, img_size,
                                                     flags, errp);
 }
+
+int bdrv_ext_snapshot_reopen(BlockDriverState *bs, int bdrv_flags,
+                             Error **errp)
+{
+    Error *local_err = NULL;
+    int ret = -ENOTSUP;
+
+    if (!bs->file || !bs->file->drv) {
+        error_setg(errp, "Block driver not reachable.");
+        return ret;
+    }
+
+    if (!bs->file->drv->bdrv_ext_snapshot_reopen) {
+        ret = bdrv_reopen(bs, bdrv_flags, &local_err);
+    } else {
+        ret = bs->file->drv->bdrv_ext_snapshot_reopen(bs, bdrv_flags,
+                                                      &local_err);
+    }
+
+    if (error_is_set(&local_err)) {
+        error_propagate(errp, local_err);
+    }
+
+    return ret;
+}
diff --git a/blockdev.c b/blockdev.c
index b1f388b..e9d235b 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -817,8 +817,9 @@  void qmp_transaction(BlockdevActionList *dev_list, Error **errp)
         /* We don't need (or want) to use the transactional
          * bdrv_reopen_multiple() across all the entries at once, because we
          * don't want to abort all of them if one of them fails the reopen */
-        bdrv_reopen(states->new_bs, states->new_bs->open_flags & ~BDRV_O_RDWR,
-                    NULL);
+        bdrv_ext_snapshot_reopen(states->new_bs,
+                                 states->new_bs->open_flags & ~BDRV_O_RDWR,
+                                 NULL);
     }
 
     /* success */
diff --git a/include/block/block.h b/include/block/block.h
index b7be2d2..d7e7334 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -141,6 +141,8 @@  BlockReopenQueue *bdrv_reopen_queue(BlockReopenQueue *bs_queue,
                                     BlockDriverState *bs, int flags);
 int bdrv_reopen_multiple(BlockReopenQueue *bs_queue, Error **errp);
 int bdrv_reopen(BlockDriverState *bs, int bdrv_flags, Error **errp);
+int bdrv_ext_snapshot_reopen(BlockDriverState *bs, int bdrv_flags,
+                             Error **errp);
 int bdrv_reopen_prepare(BDRVReopenState *reopen_state,
                         BlockReopenQueue *queue, Error **errp);
 void bdrv_reopen_commit(BDRVReopenState *reopen_state);
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 5bab830..996fd9b 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -209,6 +209,9 @@  struct BlockDriver {
                                          const char *base_fmt,
                                          char *options, uint64_t img_size,
                                          int flags, Error **errp);
+    /* optional field */
+    int (*bdrv_ext_snapshot_reopen)(BlockDriverState *bs, int bdrv_flags,
+                                    Error **errp);
 
     QLIST_ENTRY(BlockDriver) list;
 };