[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH V3 1/4] block: drop bs_snapshots global variable
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [PATCH V3 1/4] block: drop bs_snapshots global variable |
Date: |
Mon, 27 May 2013 17:25:16 +0200 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
Am 25.05.2013 um 05:09 hat Wenchao Xia geschrieben:
> From: Stefan Hajnoczi <address@hidden>
>
> The bs_snapshots global variable points to the BlockDriverState which
> will be used to save vmstate. This is really a savevm.c concept but was
> moved into block.c:bdrv_snapshots() when it became clear that hotplug
> could result in a dangling pointer.
>
> While auditing the block layer's global state I came upon bs_snapshots
> and realized that a variable is not necessary here. Simply find the
> first BlockDriverState capable of internal snapshots each time this is
> needed.
>
> The behavior of bdrv_snapshots() is preserved across hotplug because new
> drives are always appended to the bdrv_states list. This means that
> calling the new find_vmstate_bs() function is idempotent - it returns
> the same BlockDriverState unless it was hot-unplugged.
>
> Signed-off-by: Stefan Hajnoczi <address@hidden>
> Reviewed-by: Eric Blake <address@hidden>
> Reviewed-by: Wenchao Xia <address@hidden>
> Signed-off-by: Wenchao Xia <address@hidden>
> ---
> block.c | 28 ----------------------------
> include/block/block.h | 1 -
> savevm.c | 19 +++++++++++++++----
> 3 files changed, 15 insertions(+), 33 deletions(-)
>
> diff --git a/block.c b/block.c
> index 3f87489..478a3b2 100644
> --- a/block.c
> +++ b/block.c
> @@ -99,9 +99,6 @@ static QTAILQ_HEAD(, BlockDriverState) bdrv_states =
> static QLIST_HEAD(, BlockDriver) bdrv_drivers =
> QLIST_HEAD_INITIALIZER(bdrv_drivers);
>
> -/* The device to use for VM snapshots */
> -static BlockDriverState *bs_snapshots;
> -
> /* If non-zero, use only whitelisted block drivers */
> static int use_bdrv_whitelist;
>
> @@ -1357,9 +1354,6 @@ void bdrv_close(BlockDriverState *bs)
> notifier_list_notify(&bs->close_notifiers, bs);
>
> if (bs->drv) {
> - if (bs == bs_snapshots) {
> - bs_snapshots = NULL;
> - }
> if (bs->backing_hd) {
> bdrv_delete(bs->backing_hd);
> bs->backing_hd = NULL;
> @@ -1591,7 +1585,6 @@ void bdrv_delete(BlockDriverState *bs)
>
> bdrv_close(bs);
>
> - assert(bs != bs_snapshots);
> g_free(bs);
> }
>
> @@ -1635,9 +1628,6 @@ void bdrv_set_dev_ops(BlockDriverState *bs, const
> BlockDevOps *ops,
> {
> bs->dev_ops = ops;
> bs->dev_opaque = opaque;
> - if (bdrv_dev_has_removable_media(bs) && bs == bs_snapshots) {
> - bs_snapshots = NULL;
> - }
This hunk isn't replaced by any other code. If I understand correctly
what it's doing, it prevented you from saving the VM state to a
removable device, which would be allowed after this patch.
Is this a change we want to make? Why isn't it described in the commit
message?
Kevin
- [Qemu-devel] [PATCH V3 0/4] qapi and snapshot code clean up in block layer, Wenchao Xia, 2013/05/24
- [Qemu-devel] [PATCH V3 1/4] block: drop bs_snapshots global variable, Wenchao Xia, 2013/05/24
- Re: [Qemu-devel] [PATCH V3 1/4] block: drop bs_snapshots global variable, Kevin Wolf, 2013/05/27
- Re: [Qemu-devel] [PATCH V3 1/4] block: drop bs_snapshots global variable,
Kevin Wolf <=
- Re: [Qemu-devel] [PATCH V3 1/4] block: drop bs_snapshots global variable, Wenchao Xia, 2013/05/27
- Re: [Qemu-devel] [PATCH V3 1/4] block: drop bs_snapshots global variable, Stefan Hajnoczi, 2013/05/28
- Re: [Qemu-devel] [PATCH V3 1/4] block: drop bs_snapshots global variable, Wenchao Xia, 2013/05/29
- Re: [Qemu-devel] [PATCH V3 1/4] block: drop bs_snapshots global variable, Stefan Hajnoczi, 2013/05/29
- Re: [Qemu-devel] [PATCH V3 1/4] block: drop bs_snapshots global variable, Wenchao Xia, 2013/05/29
[Qemu-devel] [PATCH V3 2/4] block: move snapshot code in block.c to block/snapshot.c, Wenchao Xia, 2013/05/24
[Qemu-devel] [PATCH V3 4/4] block: dump snapshot and image info to specified output, Wenchao Xia, 2013/05/24