diff mbox

[RFC,v2,2/3] block: call the snapshot handlers of the protocol drivers

Message ID 1273830676-2349-3-git-send-email-morita.kazutaka@lab.ntt.co.jp
State New
Headers show

Commit Message

MORITA Kazutaka May 14, 2010, 9:51 a.m. UTC 
When snapshot handlers of the format driver is not defined, it is
better to call the ones of the protocol driver.

This enables us to implement snapshot support in the protocol driver.

Signed-off-by: MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp>
---
 block.c |   48 ++++++++++++++++++++++++++++++------------------
 1 files changed, 30 insertions(+), 18 deletions(-)

Comments

Kevin Wolf May 14, 2010, 12:55 p.m. UTC | #1 
Am 14.05.2010 11:51, schrieb MORITA Kazutaka:
> When snapshot handlers of the format driver is not defined, it is
> better to call the ones of the protocol driver.
> 
> This enables us to implement snapshot support in the protocol driver.
> 
> Signed-off-by: MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp>

>  int bdrv_snapshot_goto(BlockDriverState *bs,
> @@ -1737,9 +1743,11 @@ int bdrv_snapshot_goto(BlockDriverState *bs,
>      BlockDriver *drv = bs->drv;
>      if (!drv)
>          return -ENOMEDIUM;
> -    if (!drv->bdrv_snapshot_goto)
> -        return -ENOTSUP;
> -    return drv->bdrv_snapshot_goto(bs, snapshot_id);
> +    if (drv->bdrv_snapshot_goto)
> +        return drv->bdrv_snapshot_goto(bs, snapshot_id);
> +    if (bs->file)
> +        return bdrv_snapshot_goto(bs->file, snapshot_id);
> +    return -ENOTSUP;
>  }

During lunch one more thing came to my mind...

For bdrv_snapshot_goto it's probably not that easy for formats other
than raw. Usually the drivers keep some state in memory, and with this
code they won't re-read it after the snapshot has been loaded - which
will lead to an inconsistent state very easily.

I think the right thing to do here is:

1. Call drv->bdrv_close() so that the format driver cleans up, but the
underlying protocol stays open
2. Load the snapshot on the protocol level
3. Call drv->bdrv_open() to load the new state

The other functions in this patch should be okay without re-opening
because they don't change the current state.

Kevin
diff mbox

Patch

diff --git a/block.c b/block.c
index 988a94a..d1866be 100644
--- a/block.c
+++ b/block.c
@@ -1689,9 +1689,11 @@  int bdrv_save_vmstate(BlockDriverState *bs, const uint8_t *buf,
     BlockDriver *drv = bs->drv;
     if (!drv)
         return -ENOMEDIUM;
-    if (!drv->bdrv_save_vmstate)
-        return -ENOTSUP;
-    return drv->bdrv_save_vmstate(bs, buf, pos, size);
+    if (drv->bdrv_save_vmstate)
+        return drv->bdrv_save_vmstate(bs, buf, pos, size);
+    if (bs->file)
+        return bdrv_save_vmstate(bs->file, buf, pos, size);
+    return -ENOTSUP;
 }
 
 int bdrv_load_vmstate(BlockDriverState *bs, uint8_t *buf,
@@ -1700,9 +1702,11 @@  int bdrv_load_vmstate(BlockDriverState *bs, uint8_t *buf,
     BlockDriver *drv = bs->drv;
     if (!drv)
         return -ENOMEDIUM;
-    if (!drv->bdrv_load_vmstate)
-        return -ENOTSUP;
-    return drv->bdrv_load_vmstate(bs, buf, pos, size);
+    if (drv->bdrv_load_vmstate)
+        return drv->bdrv_load_vmstate(bs, buf, pos, size);
+    if (bs->file)
+        return bdrv_load_vmstate(bs->file, buf, pos, size);
+    return -ENOTSUP;
 }
 
 void bdrv_debug_event(BlockDriverState *bs, BlkDebugEvent event)
@@ -1726,9 +1730,11 @@  int bdrv_snapshot_create(BlockDriverState *bs,
     BlockDriver *drv = bs->drv;
     if (!drv)
         return -ENOMEDIUM;
-    if (!drv->bdrv_snapshot_create)
-        return -ENOTSUP;
-    return drv->bdrv_snapshot_create(bs, sn_info);
+    if (drv->bdrv_snapshot_create)
+        return drv->bdrv_snapshot_create(bs, sn_info);
+    if (bs->file)
+        return bdrv_snapshot_create(bs->file, sn_info);
+    return -ENOTSUP;
 }
 
 int bdrv_snapshot_goto(BlockDriverState *bs,
@@ -1737,9 +1743,11 @@  int bdrv_snapshot_goto(BlockDriverState *bs,
     BlockDriver *drv = bs->drv;
     if (!drv)
         return -ENOMEDIUM;
-    if (!drv->bdrv_snapshot_goto)
-        return -ENOTSUP;
-    return drv->bdrv_snapshot_goto(bs, snapshot_id);
+    if (drv->bdrv_snapshot_goto)
+        return drv->bdrv_snapshot_goto(bs, snapshot_id);
+    if (bs->file)
+        return bdrv_snapshot_goto(bs->file, snapshot_id);
+    return -ENOTSUP;
 }
 
 int bdrv_snapshot_delete(BlockDriverState *bs, const char *snapshot_id)
@@ -1747,9 +1755,11 @@  int bdrv_snapshot_delete(BlockDriverState *bs, const char *snapshot_id)
     BlockDriver *drv = bs->drv;
     if (!drv)
         return -ENOMEDIUM;
-    if (!drv->bdrv_snapshot_delete)
-        return -ENOTSUP;
-    return drv->bdrv_snapshot_delete(bs, snapshot_id);
+    if (drv->bdrv_snapshot_delete)
+        return drv->bdrv_snapshot_delete(bs, snapshot_id);
+    if (bs->file)
+        return bdrv_snapshot_delete(bs->file, snapshot_id);
+    return -ENOTSUP;
 }
 
 int bdrv_snapshot_list(BlockDriverState *bs,
@@ -1758,9 +1768,11 @@  int bdrv_snapshot_list(BlockDriverState *bs,
     BlockDriver *drv = bs->drv;
     if (!drv)
         return -ENOMEDIUM;
-    if (!drv->bdrv_snapshot_list)
-        return -ENOTSUP;
-    return drv->bdrv_snapshot_list(bs, psn_info);
+    if (drv->bdrv_snapshot_list)
+        return drv->bdrv_snapshot_list(bs, psn_info);
+    if (bs->file)
+        return bdrv_snapshot_list(bs->file, psn_info);
+    return -ENOTSUP;
 }
 
 #define NB_SUFFIXES 4