[Qemu-devel] [PATCH 0/2] v5 Decouple block device removal from device removal

[Qemu-devel] [PATCH 0/2] v5 Decouple block device removal from device removal

[Ryan Harper]

This patch series decouples the detachment of a block device from the removal
of the backing pci-device.  Removal of a hotplugged pci device requires the
guest to respond before qemu tears down the block device. In some cases, the
guest may not respond leaving the guest with continued access to the block
device.  

The new monitor command, drive_unplug, will revoke a guests access to the
block device independently of the removal of the pci device.

The first patch adds a new drive find method, the second patch implements the
monitor command and block layer changes.

Changes since v4:
- Droppped drive_get_by_id patch and use bdrv_find() instead
- Added additional details about drive_unplug to hmp/qmp interface

Changes since v3:
- Moved QMP command for drive_unplug() to separate patch

Changes since v2:
- Added QMP command for drive_unplug()

Changes since v1:
- CodingStyle fixes
- Added qemu_aio_flush() to bdrv_unplug()

Signed-off-by: Ryan Harper <ryanh@us.ibm.com>

[Qemu-devel] [PATCH 1/2] Fix Block Hotplug race with drive_unplug()

[Ryan Harper]

Block hot unplug is racy since the guest is required to acknowlege the ACPI
unplug event; this may not happen synchronously with the device removal command

This series aims to close a gap where by mgmt applications that assume the
block resource has been removed without confirming that the guest has
acknowledged the removal may re-assign the underlying device to a second guest
leading to data leakage.

This series introduces a new montor command to decouple asynchornous device
removal from restricting guest access to a block device.  We do this by creating
a new monitor command drive_unplug which maps to a bdrv_unplug() command which
does a qemu_aio_flush; bdrv_flush() and bdrv_close().  Once complete, subsequent
IO is rejected from the device and the guest will get IO errors but continue to
function.

A subsequent device removal command can be issued to remove the device, to which
the guest may or maynot respond, but as long as the unplugged bit is set, no IO
will be sumbitted.

Signed-off-by: Ryan Harper <ryanh@us.ibm.com>
---
 block.c         |    7 +++++++
 block.h         |    1 +
 blockdev.c      |   17 +++++++++++++++++
 blockdev.h      |    1 +
 hmp-commands.hx |   20 ++++++++++++++++++++
 5 files changed, 46 insertions(+), 0 deletions(-)

diff --git a/block.c b/block.c
index 985d0b7..2320e9d 100644
--- a/block.c
+++ b/block.c
@@ -1328,6 +1328,13 @@ void bdrv_set_removable(BlockDriverState *bs, int removable)
     }
 }
 
+void bdrv_unplug(BlockDriverState *bs)
+{
+    qemu_aio_flush();
+    bdrv_flush(bs);
+    bdrv_close(bs);
+}
+
 int bdrv_is_removable(BlockDriverState *bs)
 {
     return bs->removable;
diff --git a/block.h b/block.h
index a4facf2..608fd83 100644
--- a/block.h
+++ b/block.h
@@ -171,6 +171,7 @@ void bdrv_set_on_error(BlockDriverState *bs, BlockErrorAction on_read_error,
                        BlockErrorAction on_write_error);
 BlockErrorAction bdrv_get_on_error(BlockDriverState *bs, int is_read);
 void bdrv_set_removable(BlockDriverState *bs, int removable);
+void bdrv_unplug(BlockDriverState *bs);
 int bdrv_is_removable(BlockDriverState *bs);
 int bdrv_is_read_only(BlockDriverState *bs);
 int bdrv_is_sg(BlockDriverState *bs);
diff --git a/blockdev.c b/blockdev.c
index ff7602b..7bbdb65 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -597,3 +597,20 @@ int do_change_block(Monitor *mon, const char *device,
     }
     return monitor_read_bdrv_key_start(mon, bs, NULL, NULL);
 }
+
+int do_drive_unplug(Monitor *mon, const QDict *qdict, QObject **ret_data)
+{
+    const char *id = qdict_get_str(qdict, "id");
+    BlockDriverState *bs;
+
+    bs = bdrv_find(id);
+    if (!bs) {
+        qerror_report(QERR_DEVICE_NOT_FOUND, id);
+        return -1;
+    }
+
+    bdrv_unplug(bs);
+
+    return 0;
+}
+ 
diff --git a/blockdev.h b/blockdev.h
index 653affc..a454853 100644
--- a/blockdev.h
+++ b/blockdev.h
@@ -51,5 +51,6 @@ int do_eject(Monitor *mon, const QDict *qdict, QObject **ret_data);
 int do_block_set_passwd(Monitor *mon, const QDict *qdict, QObject **ret_data);
 int do_change_block(Monitor *mon, const char *device,
                     const char *filename, const char *fmt);
+int do_drive_unplug(Monitor *mon, const QDict *qdict, QObject **ret_data);
 
 #endif
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 81999aa..f6d3c85 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -68,6 +68,26 @@ Eject a removable medium (use -f to force it).
 ETEXI
 
     {
+        .name       = "drive_unplug",
+        .args_type  = "id:s",
+        .params     = "device",
+        .help       = "unplug block device",
+        .user_print = monitor_user_noop,
+        .mhandler.cmd_new = do_drive_unplug,
+    },
+
+STEXI
+@item unplug @var{device}
+@findex unplug
+Unplug block device.  The result is that guest generated IO is no longer
+submitted against the host device underlying the disk.  Once a drive has
+been unplugged, the QEMU Block layer returns -EIO which results in IO 
+errors in the guest for applications that are reading/writing to the device
+when it is unplugged.  Unplugged block devices can be safely deleted along with
+the associated pci devices (if present).
+ETEXI
+
+    {
         .name       = "change",
         .args_type  = "device:B,target:F,arg:s?",
         .params     = "device filename [format]",
-- 
1.6.3.3

[Qemu-devel] [PATCH 2/2] Add qmp version of drive_unplug

[Ryan Harper]

Signed-off-by: Ryan Harper <ryanh@us.ibm.com>
---
 qmp-commands.hx |   31 +++++++++++++++++++++++++++++++
 1 files changed, 31 insertions(+), 0 deletions(-)

diff --git a/qmp-commands.hx b/qmp-commands.hx
index 793cf1c..a1f7b2f 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -338,6 +338,37 @@ Example:
 EQMP
 
     {
+        .name       = "drive_unplug",
+        .args_type  = "id:s",
+        .params     = "device",
+        .help       = "unplug block device",
+        .user_print = monitor_user_noop,
+        .mhandler.cmd_new = do_drive_unplug,
+    },
+
+SQMP
+drive unplug
+----------
+
+Unplug a block device.  The result is that guest generated IO is no longer 
+submitted against the host device underlying the disk.  Once a drive has
+been unplugged, the QEMU Block layer returns -EIO which results in IO 
+errors in the guest for applications that are reading/writing to the device
+when it is unplugged.  Unplugged block devices can be safely deleted along with 
+the associated pci devices (if present).
+
+Arguments:
+
+- "id": the device's ID (json-string)
+
+Example:
+
+-> { "execute": "drive_unplug", "arguments": { "id": "drive-virtio-blk1" } }
+<- { "return": {} }
+
+EQMP
+
+    {
         .name       = "cpu",
         .args_type  = "index:i",
         .params     = "index",
-- 
1.6.3.3