Re: [PATCH v3 09/13] monitor/hmp: move remaining hmp_block* functions to block-hmp-cmds.c

2020-01-28 Thread Dr. David Alan Gilbert
* Maxim Levitsky (mlevi...@redhat.com) wrote:
> Signed-off-by: Maxim Levitsky 

Reviewed-by: Dr. David Alan Gilbert 

> ---
>  block/monitor/block-hmp-cmds.c | 138 +
>  include/block/block-hmp-commands.h |   9 ++
>  include/monitor/hmp.h  |   6 --
>  monitor/hmp-cmds.c | 137 
>  4 files changed, 147 insertions(+), 143 deletions(-)
> 
> diff --git a/block/monitor/block-hmp-cmds.c b/block/monitor/block-hmp-cmds.c
> index df0178d0f9..60d63bfe18 100644
> --- a/block/monitor/block-hmp-cmds.c
> +++ b/block/monitor/block-hmp-cmds.c
> @@ -29,6 +29,7 @@
>  #include "block/block_int.h"
>  #include "block/block-hmp-commands.h"
>  #include "monitor/hmp.h"
> +#include "qemu-io.h"
>  
>  void hmp_drive_add(Monitor *mon, const QDict *qdict)
>  {
> @@ -415,3 +416,140 @@ void hmp_nbd_server_stop(Monitor *mon, const QDict 
> *qdict)
>  qmp_nbd_server_stop();
>  hmp_handle_error(mon, err);
>  }
> +
> +void hmp_block_resize(Monitor *mon, const QDict *qdict)
> +{
> +const char *device = qdict_get_str(qdict, "device");
> +int64_t size = qdict_get_int(qdict, "size");
> +Error *err = NULL;
> +
> +qmp_block_resize(true, device, false, NULL, size, );
> +hmp_handle_error(mon, err);
> +}
> +
> +void hmp_block_stream(Monitor *mon, const QDict *qdict)
> +{
> +Error *error = NULL;
> +const char *device = qdict_get_str(qdict, "device");
> +const char *base = qdict_get_try_str(qdict, "base");
> +int64_t speed = qdict_get_try_int(qdict, "speed", 0);
> +
> +qmp_block_stream(true, device, device, base != NULL, base, false, NULL,
> + false, NULL, qdict_haskey(qdict, "speed"), speed, true,
> + BLOCKDEV_ON_ERROR_REPORT, false, false, false, false,
> + );
> +
> +hmp_handle_error(mon, error);
> +}
> +
> +void hmp_block_passwd(Monitor *mon, const QDict *qdict)
> +{
> +const char *device = qdict_get_str(qdict, "device");
> +const char *password = qdict_get_str(qdict, "password");
> +Error *err = NULL;
> +
> +qmp_block_passwd(true, device, false, NULL, password, );
> +hmp_handle_error(mon, err);
> +}
> +
> +void hmp_block_set_io_throttle(Monitor *mon, const QDict *qdict)
> +{
> +Error *err = NULL;
> +char *device = (char *) qdict_get_str(qdict, "device");
> +BlockIOThrottle throttle = {
> +.bps = qdict_get_int(qdict, "bps"),
> +.bps_rd = qdict_get_int(qdict, "bps_rd"),
> +.bps_wr = qdict_get_int(qdict, "bps_wr"),
> +.iops = qdict_get_int(qdict, "iops"),
> +.iops_rd = qdict_get_int(qdict, "iops_rd"),
> +.iops_wr = qdict_get_int(qdict, "iops_wr"),
> +};
> +
> +/* qmp_block_set_io_throttle has separate parameters for the
> + * (deprecated) block device name and the qdev ID but the HMP
> + * version has only one, so we must decide which one to pass. */
> +if (blk_by_name(device)) {
> +throttle.has_device = true;
> +throttle.device = device;
> +} else {
> +throttle.has_id = true;
> +throttle.id = device;
> +}
> +
> +qmp_block_set_io_throttle(, );
> +hmp_handle_error(mon, err);
> +}
> +
> +void hmp_eject(Monitor *mon, const QDict *qdict)
> +{
> +bool force = qdict_get_try_bool(qdict, "force", false);
> +const char *device = qdict_get_str(qdict, "device");
> +Error *err = NULL;
> +
> +qmp_eject(true, device, false, NULL, true, force, );
> +hmp_handle_error(mon, err);
> +}
> +
> +void hmp_qemu_io(Monitor *mon, const QDict *qdict)
> +{
> +BlockBackend *blk;
> +BlockBackend *local_blk = NULL;
> +bool qdev = qdict_get_try_bool(qdict, "qdev", false);
> +const char* device = qdict_get_str(qdict, "device");
> +const char* command = qdict_get_str(qdict, "command");
> +Error *err = NULL;
> +int ret;
> +
> +if (qdev) {
> +blk = blk_by_qdev_id(device, );
> +if (!blk) {
> +goto fail;
> +}
> +} else {
> +blk = blk_by_name(device);
> +if (!blk) {
> +BlockDriverState *bs = bdrv_lookup_bs(NULL, device, );
> +if (bs) {
> +blk = local_blk = blk_new(bdrv_get_aio_context(bs),
> +  0, BLK_PERM_ALL);
> +ret = blk_insert_bs(blk, bs, );
> +if (ret < 0) {
> +goto fail;
> +}
> +} else {
> +goto fail;
> +}
> +}
> +}
> +
> +/*
> + * Notably absent: Proper permission management. This is sad, but it 
> seems
> + * almost impossible to achieve without changing the semantics and 
> thereby
> + * limiting the use cases of the qemu-io HMP command.
> + *
> + * In an ideal world we would unconditionally create a new BlockBackend 
> for
> + * qemuio_command(), but we have commands like 'reopen' and want them to
> + * 

Re: [PATCH v3 09/13] monitor/hmp: move remaining hmp_block* functions to block-hmp-cmds.c

2020-01-28 Thread Dr. David Alan Gilbert
* Maxim Levitsky (mlevi...@redhat.com) wrote:
> Signed-off-by: Maxim Levitsky 

Reviewed-by: Dr. David Alan Gilbert 

> ---
>  block/monitor/block-hmp-cmds.c | 138 +
>  include/block/block-hmp-commands.h |   9 ++
>  include/monitor/hmp.h  |   6 --
>  monitor/hmp-cmds.c | 137 
>  4 files changed, 147 insertions(+), 143 deletions(-)
> 
> diff --git a/block/monitor/block-hmp-cmds.c b/block/monitor/block-hmp-cmds.c
> index df0178d0f9..60d63bfe18 100644
> --- a/block/monitor/block-hmp-cmds.c
> +++ b/block/monitor/block-hmp-cmds.c
> @@ -29,6 +29,7 @@
>  #include "block/block_int.h"
>  #include "block/block-hmp-commands.h"
>  #include "monitor/hmp.h"
> +#include "qemu-io.h"
>  
>  void hmp_drive_add(Monitor *mon, const QDict *qdict)
>  {
> @@ -415,3 +416,140 @@ void hmp_nbd_server_stop(Monitor *mon, const QDict 
> *qdict)
>  qmp_nbd_server_stop();
>  hmp_handle_error(mon, err);
>  }
> +
> +void hmp_block_resize(Monitor *mon, const QDict *qdict)
> +{
> +const char *device = qdict_get_str(qdict, "device");
> +int64_t size = qdict_get_int(qdict, "size");
> +Error *err = NULL;
> +
> +qmp_block_resize(true, device, false, NULL, size, );
> +hmp_handle_error(mon, err);
> +}
> +
> +void hmp_block_stream(Monitor *mon, const QDict *qdict)
> +{
> +Error *error = NULL;
> +const char *device = qdict_get_str(qdict, "device");
> +const char *base = qdict_get_try_str(qdict, "base");
> +int64_t speed = qdict_get_try_int(qdict, "speed", 0);
> +
> +qmp_block_stream(true, device, device, base != NULL, base, false, NULL,
> + false, NULL, qdict_haskey(qdict, "speed"), speed, true,
> + BLOCKDEV_ON_ERROR_REPORT, false, false, false, false,
> + );
> +
> +hmp_handle_error(mon, error);
> +}
> +
> +void hmp_block_passwd(Monitor *mon, const QDict *qdict)
> +{
> +const char *device = qdict_get_str(qdict, "device");
> +const char *password = qdict_get_str(qdict, "password");
> +Error *err = NULL;
> +
> +qmp_block_passwd(true, device, false, NULL, password, );
> +hmp_handle_error(mon, err);
> +}
> +
> +void hmp_block_set_io_throttle(Monitor *mon, const QDict *qdict)
> +{
> +Error *err = NULL;
> +char *device = (char *) qdict_get_str(qdict, "device");
> +BlockIOThrottle throttle = {
> +.bps = qdict_get_int(qdict, "bps"),
> +.bps_rd = qdict_get_int(qdict, "bps_rd"),
> +.bps_wr = qdict_get_int(qdict, "bps_wr"),
> +.iops = qdict_get_int(qdict, "iops"),
> +.iops_rd = qdict_get_int(qdict, "iops_rd"),
> +.iops_wr = qdict_get_int(qdict, "iops_wr"),
> +};
> +
> +/* qmp_block_set_io_throttle has separate parameters for the
> + * (deprecated) block device name and the qdev ID but the HMP
> + * version has only one, so we must decide which one to pass. */
> +if (blk_by_name(device)) {
> +throttle.has_device = true;
> +throttle.device = device;
> +} else {
> +throttle.has_id = true;
> +throttle.id = device;
> +}
> +
> +qmp_block_set_io_throttle(, );
> +hmp_handle_error(mon, err);
> +}
> +
> +void hmp_eject(Monitor *mon, const QDict *qdict)
> +{
> +bool force = qdict_get_try_bool(qdict, "force", false);
> +const char *device = qdict_get_str(qdict, "device");
> +Error *err = NULL;
> +
> +qmp_eject(true, device, false, NULL, true, force, );
> +hmp_handle_error(mon, err);
> +}
> +
> +void hmp_qemu_io(Monitor *mon, const QDict *qdict)
> +{
> +BlockBackend *blk;
> +BlockBackend *local_blk = NULL;
> +bool qdev = qdict_get_try_bool(qdict, "qdev", false);
> +const char* device = qdict_get_str(qdict, "device");
> +const char* command = qdict_get_str(qdict, "command");
> +Error *err = NULL;
> +int ret;
> +
> +if (qdev) {
> +blk = blk_by_qdev_id(device, );
> +if (!blk) {
> +goto fail;
> +}
> +} else {
> +blk = blk_by_name(device);
> +if (!blk) {
> +BlockDriverState *bs = bdrv_lookup_bs(NULL, device, );
> +if (bs) {
> +blk = local_blk = blk_new(bdrv_get_aio_context(bs),
> +  0, BLK_PERM_ALL);
> +ret = blk_insert_bs(blk, bs, );
> +if (ret < 0) {
> +goto fail;
> +}
> +} else {
> +goto fail;
> +}
> +}
> +}
> +
> +/*
> + * Notably absent: Proper permission management. This is sad, but it 
> seems
> + * almost impossible to achieve without changing the semantics and 
> thereby
> + * limiting the use cases of the qemu-io HMP command.
> + *
> + * In an ideal world we would unconditionally create a new BlockBackend 
> for
> + * qemuio_command(), but we have commands like 'reopen' and want them to
> + * 

[PATCH v3 09/13] monitor/hmp: move remaining hmp_block* functions to block-hmp-cmds.c

2020-01-27 Thread Maxim Levitsky
Signed-off-by: Maxim Levitsky 
---
 block/monitor/block-hmp-cmds.c | 138 +
 include/block/block-hmp-commands.h |   9 ++
 include/monitor/hmp.h  |   6 --
 monitor/hmp-cmds.c | 137 
 4 files changed, 147 insertions(+), 143 deletions(-)

diff --git a/block/monitor/block-hmp-cmds.c b/block/monitor/block-hmp-cmds.c
index df0178d0f9..60d63bfe18 100644
--- a/block/monitor/block-hmp-cmds.c
+++ b/block/monitor/block-hmp-cmds.c
@@ -29,6 +29,7 @@
 #include "block/block_int.h"
 #include "block/block-hmp-commands.h"
 #include "monitor/hmp.h"
+#include "qemu-io.h"
 
 void hmp_drive_add(Monitor *mon, const QDict *qdict)
 {
@@ -415,3 +416,140 @@ void hmp_nbd_server_stop(Monitor *mon, const QDict *qdict)
 qmp_nbd_server_stop();
 hmp_handle_error(mon, err);
 }
+
+void hmp_block_resize(Monitor *mon, const QDict *qdict)
+{
+const char *device = qdict_get_str(qdict, "device");
+int64_t size = qdict_get_int(qdict, "size");
+Error *err = NULL;
+
+qmp_block_resize(true, device, false, NULL, size, );
+hmp_handle_error(mon, err);
+}
+
+void hmp_block_stream(Monitor *mon, const QDict *qdict)
+{
+Error *error = NULL;
+const char *device = qdict_get_str(qdict, "device");
+const char *base = qdict_get_try_str(qdict, "base");
+int64_t speed = qdict_get_try_int(qdict, "speed", 0);
+
+qmp_block_stream(true, device, device, base != NULL, base, false, NULL,
+ false, NULL, qdict_haskey(qdict, "speed"), speed, true,
+ BLOCKDEV_ON_ERROR_REPORT, false, false, false, false,
+ );
+
+hmp_handle_error(mon, error);
+}
+
+void hmp_block_passwd(Monitor *mon, const QDict *qdict)
+{
+const char *device = qdict_get_str(qdict, "device");
+const char *password = qdict_get_str(qdict, "password");
+Error *err = NULL;
+
+qmp_block_passwd(true, device, false, NULL, password, );
+hmp_handle_error(mon, err);
+}
+
+void hmp_block_set_io_throttle(Monitor *mon, const QDict *qdict)
+{
+Error *err = NULL;
+char *device = (char *) qdict_get_str(qdict, "device");
+BlockIOThrottle throttle = {
+.bps = qdict_get_int(qdict, "bps"),
+.bps_rd = qdict_get_int(qdict, "bps_rd"),
+.bps_wr = qdict_get_int(qdict, "bps_wr"),
+.iops = qdict_get_int(qdict, "iops"),
+.iops_rd = qdict_get_int(qdict, "iops_rd"),
+.iops_wr = qdict_get_int(qdict, "iops_wr"),
+};
+
+/* qmp_block_set_io_throttle has separate parameters for the
+ * (deprecated) block device name and the qdev ID but the HMP
+ * version has only one, so we must decide which one to pass. */
+if (blk_by_name(device)) {
+throttle.has_device = true;
+throttle.device = device;
+} else {
+throttle.has_id = true;
+throttle.id = device;
+}
+
+qmp_block_set_io_throttle(, );
+hmp_handle_error(mon, err);
+}
+
+void hmp_eject(Monitor *mon, const QDict *qdict)
+{
+bool force = qdict_get_try_bool(qdict, "force", false);
+const char *device = qdict_get_str(qdict, "device");
+Error *err = NULL;
+
+qmp_eject(true, device, false, NULL, true, force, );
+hmp_handle_error(mon, err);
+}
+
+void hmp_qemu_io(Monitor *mon, const QDict *qdict)
+{
+BlockBackend *blk;
+BlockBackend *local_blk = NULL;
+bool qdev = qdict_get_try_bool(qdict, "qdev", false);
+const char* device = qdict_get_str(qdict, "device");
+const char* command = qdict_get_str(qdict, "command");
+Error *err = NULL;
+int ret;
+
+if (qdev) {
+blk = blk_by_qdev_id(device, );
+if (!blk) {
+goto fail;
+}
+} else {
+blk = blk_by_name(device);
+if (!blk) {
+BlockDriverState *bs = bdrv_lookup_bs(NULL, device, );
+if (bs) {
+blk = local_blk = blk_new(bdrv_get_aio_context(bs),
+  0, BLK_PERM_ALL);
+ret = blk_insert_bs(blk, bs, );
+if (ret < 0) {
+goto fail;
+}
+} else {
+goto fail;
+}
+}
+}
+
+/*
+ * Notably absent: Proper permission management. This is sad, but it seems
+ * almost impossible to achieve without changing the semantics and thereby
+ * limiting the use cases of the qemu-io HMP command.
+ *
+ * In an ideal world we would unconditionally create a new BlockBackend for
+ * qemuio_command(), but we have commands like 'reopen' and want them to
+ * take effect on the exact BlockBackend whose name the user passed instead
+ * of just on a temporary copy of it.
+ *
+ * Another problem is that deleting the temporary BlockBackend involves
+ * draining all requests on it first, but some qemu-iotests cases want to
+ * issue multiple aio_read/write requests and expect them to complete in
+ * the