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

[Kevin Wolf <kwolf@redhat.com>]

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