Skip to content

Commit

Permalink
savevm: Survive hot-unplug of snapshot device
Browse files Browse the repository at this point in the history
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 becomes unusable.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
  • Loading branch information
Markus Armbruster authored and kevmw committed Jul 2, 2010
1 parent 8db520c commit f9092b1
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 27 deletions.
26 changes: 26 additions & 0 deletions block.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -629,6 +632,9 @@ int bdrv_open(BlockDriverState *bs, const char *filename, int flags,
void bdrv_close(BlockDriverState *bs)
{
if (bs->drv) {
if (bs == bs_snapshots) {
bs_snapshots = NULL;
}
if (bs->backing_hd) {
bdrv_delete(bs->backing_hd);
bs->backing_hd = NULL;
Expand Down Expand Up @@ -677,6 +683,7 @@ void bdrv_delete(BlockDriverState *bs)
bdrv_delete(bs->file);
}

assert(bs != bs_snapshots);
qemu_free(bs);
}

Expand Down Expand Up @@ -1778,6 +1785,25 @@ int bdrv_can_snapshot(BlockDriverState *bs)
return 1;
}

BlockDriverState *bdrv_snapshots(void)
{
BlockDriverState *bs;

if (bs_snapshots)
return bs_snapshots;

bs = NULL;
while ((bs = bdrv_next(bs))) {
if (bdrv_can_snapshot(bs)) {
goto ok;
}
}
return NULL;
ok:
bs_snapshots = bs;
return bs;
}

int bdrv_snapshot_create(BlockDriverState *bs,
QEMUSnapshotInfo *sn_info)
{
Expand Down
1 change: 1 addition & 0 deletions block.h
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,7 @@ const char *bdrv_get_encrypted_filename(BlockDriverState *bs);
void bdrv_get_backing_filename(BlockDriverState *bs,
char *filename, int filename_size);
int bdrv_can_snapshot(BlockDriverState *bs);
BlockDriverState *bdrv_snapshots(void);
int bdrv_snapshot_create(BlockDriverState *bs,
QEMUSnapshotInfo *sn_info);
int bdrv_snapshot_goto(BlockDriverState *bs,
Expand Down
31 changes: 4 additions & 27 deletions savevm.c
Original file line number Diff line number Diff line change
Expand Up @@ -83,9 +83,6 @@
#include "qemu_socket.h"
#include "qemu-queue.h"

/* point to the block driver where the snapshots are managed */
static BlockDriverState *bs_snapshots;

#define SELF_ANNOUNCE_ROUNDS 5

#ifndef ETH_P_RARP
Expand Down Expand Up @@ -1575,26 +1572,6 @@ int qemu_loadvm_state(QEMUFile *f)
return ret;
}

static BlockDriverState *get_bs_snapshots(void)
{
BlockDriverState *bs;

if (bs_snapshots)
return bs_snapshots;
/* FIXME what if bs_snapshots gets hot-unplugged? */

bs = NULL;
while ((bs = bdrv_next(bs))) {
if (bdrv_can_snapshot(bs)) {
goto ok;
}
}
return NULL;
ok:
bs_snapshots = bs;
return bs;
}

static int bdrv_snapshot_find(BlockDriverState *bs, QEMUSnapshotInfo *sn_info,
const char *name)
{
Expand Down Expand Up @@ -1674,7 +1651,7 @@ void do_savevm(Monitor *mon, const QDict *qdict)
}
}

bs = get_bs_snapshots();
bs = bdrv_snapshots();
if (!bs) {
monitor_printf(mon, "No block device can accept snapshots\n");
return;
Expand Down Expand Up @@ -1769,7 +1746,7 @@ int load_vmstate(const char *name)
}
}

bs = get_bs_snapshots();
bs = bdrv_snapshots();
if (!bs) {
error_report("No block device supports snapshots");
return -EINVAL;
Expand Down Expand Up @@ -1833,7 +1810,7 @@ void do_delvm(Monitor *mon, const QDict *qdict)
int ret;
const char *name = qdict_get_str(qdict, "name");

bs = get_bs_snapshots();
bs = bdrv_snapshots();
if (!bs) {
monitor_printf(mon, "No block device supports snapshots\n");
return;
Expand Down Expand Up @@ -1863,7 +1840,7 @@ void do_info_snapshots(Monitor *mon)
int nb_sns, i;
char buf[256];

bs = get_bs_snapshots();
bs = bdrv_snapshots();
if (!bs) {
monitor_printf(mon, "No available block device supports snapshots\n");
return;
Expand Down

0 comments on commit f9092b1

Please sign in to comment.