[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] Re: [PATCH 09/12] savevm: Survive hot-unplug of snapshot de
From: |
Kevin Wolf |
Subject: |
[Qemu-devel] Re: [PATCH 09/12] savevm: Survive hot-unplug of snapshot device |
Date: |
Wed, 30 Jun 2010 15:37:28 +0200 |
User-agent: |
Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1.9) Gecko/20100430 Fedora/3.0.4-2.fc12 Thunderbird/3.0.4 |
Am 25.06.2010 18:53, schrieb Markus Armbruster:
> savevm.c keeps a pointer to the snapshot block device. If you manage
> to get that device deleted, the pointer dangles, and the next snapshot
> operation will crash & burn. Unplugging a guest device that uses it
> does the trick:
>
> $ MALLOC_PERTURB_=234 qemu-system-x86_64 [...]
> QEMU 0.12.50 monitor - type 'help' for more information
> (qemu) info snapshots
> No available block device supports snapshots
> (qemu) drive_add auto if=none,file=tmp.qcow2
> OK
> (qemu) device_add usb-storage,id=foo,drive=none1
> (qemu) info snapshots
> Snapshot devices: none1
> Snapshot list (from none1):
> ID TAG VM SIZE DATE VM CLOCK
> (qemu) device_del foo
> (qemu) info snapshots
> Snapshot devices:
> Segmentation fault (core dumped)
>
> Move management of that pointer to block.c, and zap it when the device
> it points to goes away.
>
> Signed-off-by: Markus Armbruster <address@hidden>
> ---
> block.c | 25 +++++++++++++++++++++++++
> block.h | 1 +
> savevm.c | 31 ++++---------------------------
> 3 files changed, 30 insertions(+), 27 deletions(-)
>
> diff --git a/block.c b/block.c
> index 5e0ffa0..34055e0 100644
> --- a/block.c
> +++ b/block.c
> @@ -63,6 +63,9 @@ 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;
>
> @@ -660,6 +663,9 @@ void bdrv_close_all(void)
> void bdrv_delete(BlockDriverState *bs)
> {
> assert(!bs->peer);
> + if (bs == bs_snapshots) {
> + bs_snapshots = NULL;
> + }
This should probably be in bdrv_close() instead. A BlockDriverState can
be closed, but not deleted yet; it can't handle snapshots in this state,
though.
> /* remove from list, if necessary */
> if (bs->device_name[0] != '\0') {
> @@ -1772,6 +1778,25 @@ int bdrv_can_snapshot(BlockDriverState *bs)
> return 1;
> }
>
> +BlockDriverState *bdrv_snapshots(void)
> +{
> + BlockDriverState *bs;
> +
> + if (bs_snapshots)
> + return bs_snapshots;
I know that this function is just moved with no changes, but while we're
at it and you need to respin anyway, can we add braces here?
> +
> + bs = NULL;
> + while ((bs = bdrv_next(bs))) {
> + if (bdrv_can_snapshot(bs)) {
> + goto ok;
> + }
> + }
> + return NULL;
> + ok:
> + bs_snapshots = bs;
> + return bs;
> +}
And instead of a goto we could do the right thing directly in the if block.
Kevin
- Re: [Qemu-devel] Re: [PATCH 08/12] block: Catch attempt to attach multiple devices to a blockdev, (continued)
- [Qemu-devel] [PATCH 03/12] blockdev: Remove drive_get_serial(), Markus Armbruster, 2010/06/25
- [Qemu-devel] [PATCH 09/12] savevm: Survive hot-unplug of snapshot device, Markus Armbruster, 2010/06/25
- [Qemu-devel] [PATCH 10/12] block: Fix virtual media change for if=none, Markus Armbruster, 2010/06/25
- [Qemu-devel] [PATCH 11/12] ide: Make PIIX and ISA IDE init functions return the qdev, Markus Armbruster, 2010/06/25
- [Qemu-devel] [PATCH 07/12] blockdev: drive_get_by_id() is no longer used, remove, Markus Armbruster, 2010/06/25
- [Qemu-devel] [PATCH 02/12] ide: Make it explicit that ide_create_drive() can't fail, Markus Armbruster, 2010/06/25
- [Qemu-devel] [PATCH 04/12] blockdev: New drive_of_blockdev(), Markus Armbruster, 2010/06/25