diff mbox

[8/8] block: Implement 'block_disconnect' HMP command

Message ID 1448636346-24641-9-git-send-email-hare@suse.de
State New
Headers show

Commit Message

Hannes Reinecke Nov. 27, 2015, 2:59 p.m. UTC 
Implement a 'block_disconnect' HMP command to simulate a device
communication / link failure.

Signed-off-by: Hannes Reinecke <hare@suse.com>
---
 block.c                        |  5 +++++
 block/block-backend.c          |  8 ++++++++
 blockdev.c                     | 18 ++++++++++++++++++
 hmp-commands.hx                | 14 ++++++++++++++
 hmp.c                          | 10 ++++++++++
 hmp.h                          |  1 +
 hw/scsi/megasas.c              |  4 ++++
 hw/scsi/virtio-scsi.c          |  5 +++++
 include/block/block.h          |  1 +
 include/block/block_int.h      |  1 +
 include/sysemu/block-backend.h |  1 +
 qapi/block-core.json           | 21 +++++++++++++++++++++
 qmp-commands.hx                | 26 ++++++++++++++++++++++++++
 13 files changed, 115 insertions(+)

Comments

Eric Blake Nov. 27, 2015, 6 p.m. UTC | #1 
On 11/27/2015 07:59 AM, Hannes Reinecke wrote:
> Implement a 'block_disconnect' HMP command to simulate a device
> communication / link failure.
> 
> Signed-off-by: Hannes Reinecke <hare@suse.com>
> ---

> +++ b/qapi/block-core.json
> @@ -754,6 +754,27 @@
>                                         'size': 'int' }}
>  
>  ##
> +# @block_disconnect

New QMP commands should favor '-' rather than '_'; this should be
'block-disconnect'.

> +#
> +# Simulate block device disconnect while a guest is running.
> +#
> +# Either @device or @node-name must be set but not both.
> +#
> +# @device: #optional the name of the device to get the image resized

Bad copy-and-paste?  We aren't resizing anything here.

> +#
> +# @node-name: #optional graph node name to get the image resized (Since 2.0)

And again. And since the command is new, you don't need a '(Since 2.0)'.

> +#
> +# @disconnect:  true for disconnecting the device
> +#
> +# Returns: nothing on success
> +#          If @device is not a valid block device, DeviceNotFound
> +#
> +##

Missing a '# Since 2.6' line.

> +{ 'command': 'block_disconnect', 'data': { '*device': 'str',
> +                                           '*node-name': 'str',
> +                                           'disconnect': 'bool' }}

Mutually-exclusive 'device' vs. 'node-name' is awkward.  For new
commands, it is sufficient to just use 'node' (and accept both node
names for the direct node to disconnect, or a device name to detach the
BDS node plugged in to that device).


> +block_disconnect
> +----------------
> +
> +Simulate a block device disconnect while a guest is running.
> +
> +Arguments:
> +
> +- "device": the device's ID, must be unique (json-string)
> +- "node-name": the node name in the block driver state graph (json-string)

Awkward that you aren't mentioning the mutual exclusion above; and again
I think that a single parameter is better than two mutually exclusive ones.

> +- "disconnect": whether to simulate a device disconnect (json-bool)

Do I again call the command with 'disconnect':false to undo the
disconnect?  That sounds like a double-negative.  It might make more
sense to have:

{ 'command':'block-set-connection', 'data': { 'node':'str',
'connected':'bool' } }

where I pass 'connected':false to disconnect, and 'connected':true to
reconnect.
diff mbox

Patch

diff --git a/block.c b/block.c
index 3a7324b..0b48c8f 100644
--- a/block.c
+++ b/block.c
@@ -3130,6 +3130,11 @@  void bdrv_lock_medium(BlockDriverState *bs, bool locked)
     }
 }
 
+bool bdrv_is_disconnected(BlockDriverState *bs)
+{
+    return bs->disconnected;
+}
+
 BdrvDirtyBitmap *bdrv_find_dirty_bitmap(BlockDriverState *bs, const char *name)
 {
     BdrvDirtyBitmap *bm;
diff --git a/block/block-backend.c b/block/block-backend.c
index 9889e81..da55bf8 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1062,6 +1062,14 @@  void blk_op_unblock_all(BlockBackend *blk, Error *reason)
     }
 }
 
+bool blk_is_disconnected(BlockBackend *blk)
+{
+    if (blk->bs) {
+        return bdrv_is_disconnected(blk->bs);
+    }
+    return false;
+}
+
 AioContext *blk_get_aio_context(BlockBackend *blk)
 {
     if (blk->bs) {
diff --git a/blockdev.c b/blockdev.c
index fc85128..2dbd895 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2821,6 +2821,24 @@  out:
     aio_context_release(aio_context);
 }
 
+void qmp_block_disconnect(bool has_device, const char *device,
+                          bool has_node_name, const char *node_name,
+                          bool disconnect, Error **errp)
+{
+    Error *local_err = NULL;
+    BlockDriverState *bs;
+
+    bs = bdrv_lookup_bs(has_device ? device : NULL,
+                        has_node_name ? node_name : NULL,
+                        &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        return;
+    }
+
+    bs->disconnected = disconnect;
+}
+
 static void block_job_cb(void *opaque, int ret)
 {
     /* Note that this function may be executed from another AioContext besides
diff --git a/hmp-commands.hx b/hmp-commands.hx
index bb52e4d..992e10c 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -74,6 +74,20 @@  resizes image files, it can not resize block devices like LVM volumes.
 ETEXI
 
     {
+        .name       = "block_disconnect",
+        .args_type  = "device:B,disconnect:b",
+        .params     = "disconnect",
+        .help       = "disconnect a block device",
+        .mhandler.cmd = hmp_block_disconnect,
+    },
+
+STEXI
+@item block_disconnect
+@findex block_disconnect
+Simulate a block device disconnect while a guest is running.
+ETEXI
+
+    {
         .name       = "block_stream",
         .args_type  = "device:B,speed:o?,base:s?",
         .params     = "device [speed [base]]",
diff --git a/hmp.c b/hmp.c
index 2140605..60d11b0 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1049,6 +1049,16 @@  void hmp_block_resize(Monitor *mon, const QDict *qdict)
     hmp_handle_error(mon, &err);
 }
 
+void hmp_block_disconnect(Monitor *mon, const QDict *qdict)
+{
+    const char *device = qdict_get_str(qdict, "device");
+    bool disconnect = qdict_get_try_bool(qdict, "disconnect", false);
+    Error *err = NULL;
+
+    qmp_block_disconnect(true, device, false, NULL, disconnect, &err);
+    hmp_handle_error(mon, &err);
+}
+
 void hmp_drive_mirror(Monitor *mon, const QDict *qdict)
 {
     const char *device = qdict_get_str(qdict, "device");
diff --git a/hmp.h b/hmp.h
index a8c5b5a..e2f5e81 100644
--- a/hmp.h
+++ b/hmp.h
@@ -56,6 +56,7 @@  void hmp_set_link(Monitor *mon, const QDict *qdict);
 void hmp_block_passwd(Monitor *mon, const QDict *qdict);
 void hmp_balloon(Monitor *mon, const QDict *qdict);
 void hmp_block_resize(Monitor *mon, const QDict *qdict);
+void hmp_block_disconnect(Monitor *mon, const QDict *qdict);
 void hmp_snapshot_blkdev(Monitor *mon, const QDict *qdict);
 void hmp_snapshot_blkdev_internal(Monitor *mon, const QDict *qdict);
 void hmp_snapshot_delete_blkdev_internal(Monitor *mon, const QDict *qdict);
diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c
index d7dc667..b2942b7 100644
--- a/hw/scsi/megasas.c
+++ b/hw/scsi/megasas.c
@@ -1678,6 +1678,10 @@  static int megasas_handle_scsi(MegasasState *s, MegasasCmd *cmd,
         return MFI_STAT_DEVICE_NOT_FOUND;
     }
 
+    if (blk_is_disconnected(sdev->conf.blk)) {
+        return MFI_STAT_LD_OFFLINE;
+    }
+
     if (cmd->frame->header.cdb_len > 16) {
         trace_megasas_scsi_invalid_cdb_len(
                 mfi_frame_desc[cmd->frame->header.frame_cmd], is_logical,
diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index 7655401..748b777 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -534,6 +534,11 @@  bool virtio_scsi_handle_cmd_req_prepare(VirtIOSCSI *s, VirtIOSCSIReq *req)
         virtio_scsi_complete_cmd_req(req);
         return false;
     }
+    if (blk_is_disconnected(d->conf.blk)) {
+        req->resp.cmd.response = VIRTIO_SCSI_S_TRANSPORT_FAILURE;
+        virtio_scsi_complete_cmd_req(req);
+        return false;
+    }
     if (s->dataplane_started) {
         assert(blk_get_aio_context(d->conf.blk) == s->ctx);
     }
diff --git a/include/block/block.h b/include/block/block.h
index 73edb1a..ea5ebf3 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -398,6 +398,7 @@  int bdrv_is_sg(BlockDriverState *bs);
 int bdrv_enable_write_cache(BlockDriverState *bs);
 void bdrv_set_enable_write_cache(BlockDriverState *bs, bool wce);
 bool bdrv_is_inserted(BlockDriverState *bs);
+bool bdrv_is_disconnected(BlockDriverState *bs);
 int bdrv_media_changed(BlockDriverState *bs);
 void bdrv_lock_medium(BlockDriverState *bs, bool locked);
 void bdrv_eject(BlockDriverState *bs, bool eject_flag);
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 4012e36..60ff193 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -371,6 +371,7 @@  struct BlockDriverState {
     int sg;        /* if true, the device is a /dev/sg* */
     int copy_on_read; /* if true, copy read backing sectors into image
                          note this is a reference count */
+    bool disconnected;
     bool probed;
 
     BlockDriver *drv; /* NULL means no media */
diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index f4a68e2..2578996 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -144,6 +144,7 @@  bool blk_is_inserted(BlockBackend *blk);
 bool blk_is_available(BlockBackend *blk);
 void blk_lock_medium(BlockBackend *blk, bool locked);
 void blk_eject(BlockBackend *blk, bool eject_flag);
+bool blk_is_disconnected(BlockBackend *blk);
 int blk_get_flags(BlockBackend *blk);
 int blk_get_max_transfer_length(BlockBackend *blk);
 void blk_set_guest_block_size(BlockBackend *blk, int align);
diff --git a/qapi/block-core.json b/qapi/block-core.json
index f97c250..e3009bf 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -754,6 +754,27 @@ 
                                        'size': 'int' }}
 
 ##
+# @block_disconnect
+#
+# Simulate block device disconnect while a guest is running.
+#
+# Either @device or @node-name must be set but not both.
+#
+# @device: #optional the name of the device to get the image resized
+#
+# @node-name: #optional graph node name to get the image resized (Since 2.0)
+#
+# @disconnect:  true for disconnecting the device
+#
+# Returns: nothing on success
+#          If @device is not a valid block device, DeviceNotFound
+#
+##
+{ 'command': 'block_disconnect', 'data': { '*device': 'str',
+                                           '*node-name': 'str',
+                                           'disconnect': 'bool' }}
+
+##
 # @NewImageMode
 #
 # An enumeration that tells QEMU how to set the backing file path in
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 9d8b42f..5181d54 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -1052,6 +1052,32 @@  Example:
 
 EQMP
 
+
+    {
+        .name       = "block_disconnect",
+        .args_type  = "device:s?,node-name:s?,disconnect:b",
+        .mhandler.cmd_new = qmp_marshal_block_disconnect,
+    },
+
+SQMP
+block_disconnect
+----------------
+
+Simulate a block device disconnect while a guest is running.
+
+Arguments:
+
+- "device": the device's ID, must be unique (json-string)
+- "node-name": the node name in the block driver state graph (json-string)
+- "disconnect": whether to simulate a device disconnect (json-bool)
+
+Example:
+
+-> { "execute": "block_disconnect", "arguments": { "device": "scratch", "disconnect": true } }
+<- { "return": {} }
+
+EQMP
+
     {
         .name       = "block-stream",
         .args_type  = "device:B,base:s?,speed:o?,backing-file:s?,on-error:s?",