[Qemu-devel] [PATCH v3 09/13] savevm: Survive hot-unplug of snapshot device

[Markus Armbruster <armbru@redhat.com>]

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>
---
 block.c  |   26 ++++++++++++++++++++++++++
 block.h  |    1 +
 savevm.c |   31 ++++---------------------------
 3 files changed, 31 insertions(+), 27 deletions(-)

diff --git a/block.c b/block.c
index 5e0ffa0..acb4182 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;
 
@@ -623,6 +626,9 @@ unlink_and_fail:
 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;
@@ -671,6 +677,7 @@ void bdrv_delete(BlockDriverState *bs)
         bdrv_delete(bs->file);
     }
 
+    assert(bs != bs_snapshots);
     qemu_free(bs);
 }
 
@@ -1772,6 +1779,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)
 {
diff --git a/block.h b/block.h
index 88ac06e..012c2a1 100644
--- a/block.h
+++ b/block.h
@@ -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,
diff --git a/savevm.c b/savevm.c
index 20354a8..f1f450e 100644
--- a/savevm.c
+++ b/savevm.c
@@ -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
@@ -1575,26 +1572,6 @@ out:
     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)
 {
@@ -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;
@@ -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;
@@ -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;
@@ -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;
-- 
1.6.6.1