Patchwork [13/14] qapi: convert change

login
register
mail settings
Submitter Anthony Liguori
Date Aug. 24, 2011, 6:43 p.m.
Message ID <1314211389-28915-14-git-send-email-aliguori@us.ibm.com>
Download mbox | patch
Permalink /patch/111417/
State New
Headers show

Comments

Anthony Liguori - Aug. 24, 2011, 6:43 p.m.
Convert the change command to QAPI and document all of its gloriousness.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 blockdev.c       |   27 ++++++++----------
 blockdev.h       |    6 +++-
 hmp-commands.hx  |    3 +-
 hmp.c            |   59 ++++++++++++++++++++++++++++++++++++++++
 hmp.h            |    1 +
 monitor.c        |   78 +++++++----------------------------------------------
 qapi-schema.json |   38 ++++++++++++++++++++++++++
 qmp-commands.hx  |    3 +-
 8 files changed, 127 insertions(+), 88 deletions(-)
Luiz Capitulino - Aug. 25, 2011, 2:43 p.m.
On Wed, 24 Aug 2011 13:43:08 -0500
Anthony Liguori <aliguori@us.ibm.com> wrote:

> Convert the change command to QAPI and document all of its gloriousness.
> 
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> ---
>  blockdev.c       |   27 ++++++++----------
>  blockdev.h       |    6 +++-
>  hmp-commands.hx  |    3 +-
>  hmp.c            |   59 ++++++++++++++++++++++++++++++++++++++++
>  hmp.h            |    1 +
>  monitor.c        |   78 +++++++----------------------------------------------
>  qapi-schema.json |   38 ++++++++++++++++++++++++++
>  qmp-commands.hx  |    3 +-
>  8 files changed, 127 insertions(+), 88 deletions(-)
> 
> diff --git a/blockdev.c b/blockdev.c
> index c00c69d..3832118 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -785,8 +785,9 @@ void qmp_change_blockdev(const char *device, const char *filename,
>      qmp_bdrv_open_encrypted(bs, filename, bdrv_flags, drv, password, errp);
>  }
>  
> -int do_change_block(Monitor *mon, const char *device,
> -                    const char *filename, const char *fmt)
> +void deprecated_qmp_change_blockdev(const char *device, const char *filename,
> +                                    bool has_format, const char *format,
> +                                    Error **errp)
>  {
>      BlockDriverState *bs;
>      BlockDriver *drv = NULL;
> @@ -795,14 +796,14 @@ int do_change_block(Monitor *mon, const char *device,
>  
>      bs = bdrv_find(device);
>      if (!bs) {
> -        qerror_report(QERR_DEVICE_NOT_FOUND, device);
> -        return -1;
> +        error_set(errp, QERR_DEVICE_NOT_FOUND, device);
> +        return;
>      }
> -    if (fmt) {
> -        drv = bdrv_find_whitelisted_format(fmt);
> +    if (has_format) {
> +        drv = bdrv_find_whitelisted_format(format);
>          if (!drv) {
> -            qerror_report(QERR_INVALID_BLOCK_FORMAT, fmt);
> -            return -1;
> +            error_set(errp, QERR_INVALID_BLOCK_FORMAT, format);
> +            return;
>          }
>      }
>  
> @@ -811,17 +812,13 @@ int do_change_block(Monitor *mon, const char *device,
>  
>      eject_device(bs, 0, &err);
>      if (err) {
> -        qerror_report_err(err);
> -        return -1;
> +        error_propagate(errp, err);
> +        return;
>      }
>  
>      qmp_bdrv_open_encrypted(bs, filename, bdrv_flags, drv, NULL, &err);
> -    if (err) {
> -        qerror_report_err(err);
> -        return -1;
> -    }
>  
> -    return 0;
> +    error_propagate(errp, err);
>  }
>  
>  int do_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data)
> diff --git a/blockdev.h b/blockdev.h
> index badbf01..54b142f 100644
> --- a/blockdev.h
> +++ b/blockdev.h
> @@ -12,6 +12,7 @@
>  
>  #include "block.h"
>  #include "qemu-queue.h"
> +#include "error.h"
>  
>  void blockdev_mark_auto_del(BlockDriverState *bs);
>  void blockdev_auto_del(BlockDriverState *bs);
> @@ -59,8 +60,9 @@ DriveInfo *add_init_drive(const char *opts);
>  
>  void do_commit(Monitor *mon, const QDict *qdict);
>  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);
> +void deprecated_qmp_change_blockdev(const char *device, const char *filename,
> +                                    bool has_format, const char *format,
> +                                    Error **errp);
>  int do_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data);
>  int do_snapshot_blkdev(Monitor *mon, const QDict *qdict, QObject **ret_data);
>  int do_block_resize(Monitor *mon, const QDict *qdict, QObject **ret_data);
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index 2f0ffa3..700d2f1 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -108,8 +108,7 @@ ETEXI
>          .args_type  = "device:B,target:F,arg:s?",
>          .params     = "device filename [format]",
>          .help       = "change a removable medium, optional format",
> -        .user_print = monitor_user_noop,
> -        .mhandler.cmd_new = do_change,
> +        .mhandler.cmd = hmp_change,
>      },
>  
>  STEXI
> diff --git a/hmp.c b/hmp.c
> index a8ae36b..e8bca20 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -50,3 +50,62 @@ void hmp_block_passwd(Monitor *mon, const QDict *qdict)
>          error_free(err);
>      }
>  }
> +
> +static void cb_hmp_change_bdrv_pwd(Monitor *mon, const char *password,
> +                                   void *opaque)
> +{
> +    Error *encryption_err = opaque;
> +    Error *err = NULL;
> +    const char *device;
> +
> +    device = error_get_field(encryption_err, "device");
> +
> +    qmp_block_passwd(device, password, &err);
> +    if (err) {
> +        monitor_printf(mon, "invalid password\n");
> +        error_free(err);
> +    }
> +
> +    error_free(encryption_err);
> +
> +    monitor_read_command(mon, 1);
> +}
> +
> +static void hmp_change_read_arg(Monitor *mon, const char *password,
> +                                void *opaque)
> +{
> +    qmp_change_vnc_password(password, NULL);
> +    monitor_read_command(mon, 1);
> +}
> +
> +void hmp_change(Monitor *mon, const QDict *qdict)
> +{
> +    const char *device = qdict_get_str(qdict, "device");
> +    const char *target = qdict_get_str(qdict, "target");
> +    const char *arg = qdict_get_try_str(qdict, "arg");
> +    Error *err = NULL;
> +
> +    if (strcmp(device, "vnc") == 0 &&
> +        (strcmp(target, "passwd") == 0 ||
> +         strcmp(target, "password") == 0)) {
> +        if (arg == NULL) {
> +            monitor_read_password(mon, hmp_change_read_arg, NULL);
> +            return;
> +        }
> +    }
> +
> +    qmp_change(device, target, !!arg, arg, &err);
> +    if (error_is_type(err, QERR_DEVICE_ENCRYPTED)) {

You need to do:

if (err && error_is_type(err, QERR_DEVICE_ENCRYPTED)) {

Otherwise it segfaults. I'd also add a 'assert(err);' in error_is_type().
Or return false in error_is_type() is 'err' is NULL.

> +        monitor_printf(mon, "%s (%s) is encrypted.\n",
> +                       error_get_field(err, "device"),
> +                       error_get_field(err, "encrypted_filename"));
> +        if (!monitor_get_rs(mon)) {
> +            monitor_printf(mon,
> +                           "terminal does not support password prompting\n");
> +            error_free(err);
> +            return;
> +        }
> +        readline_start(monitor_get_rs(mon), "Password: ", 1,
> +                       cb_hmp_change_bdrv_pwd, err);
> +    }
> +}
> diff --git a/hmp.h b/hmp.h
> index 8f72ef2..9df6ccc 100644
> --- a/hmp.h
> +++ b/hmp.h
> @@ -20,5 +20,6 @@
>  void hmp_info_name(Monitor *mon);
>  void hmp_eject(Monitor *mon, const QDict *args);
>  void hmp_block_passwd(Monitor *mon, const QDict *qdict);
> +void hmp_change(Monitor *mon, const QDict *qdict);
>  
>  #endif
> diff --git a/monitor.c b/monitor.c
> index 5cb36cd..9801a2d 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -1005,78 +1005,22 @@ static int do_quit(Monitor *mon, const QDict *qdict, QObject **ret_data)
>      return 0;
>  }
>  
> -#ifdef CONFIG_VNC
> -static int change_vnc_password(const char *password)
> -{
> -    if (!password || !password[0]) {
> -        if (vnc_display_disable_login(NULL)) {
> -            qerror_report(QERR_SET_PASSWD_FAILED);
> -            return -1;
> -        }
> -        return 0;
> -    }
> -
> -    if (vnc_display_password(NULL, password) < 0) {
> -        qerror_report(QERR_SET_PASSWD_FAILED);
> -        return -1;
> -    }
> -
> -    return 0;
> -}
> -
> -static void change_vnc_password_cb(Monitor *mon, const char *password,
> -                                   void *opaque)
> +void qmp_change(const char *device, const char *target,
> +                bool has_arg, const char *arg, Error **err)
>  {
> -    change_vnc_password(password);
> -    monitor_read_command(mon, 1);
> -}
> -
> -static int do_change_vnc(Monitor *mon, const char *target, const char *arg)
> -{
> -    if (strcmp(target, "passwd") == 0 ||
> -        strcmp(target, "password") == 0) {
> -        if (arg) {
> -            char password[9];
> -            strncpy(password, arg, sizeof(password));
> -            password[sizeof(password) - 1] = '\0';
> -            return change_vnc_password(password);
> +    if (strcmp(device, "vnc") == 0) {
> +        if (strcmp(target, "passwd") == 0 || strcmp(target, "password") == 0) {
> +            if (!has_arg || !arg[0]) {
> +                vnc_display_disable_login(NULL);
> +            } else {
> +                qmp_change_vnc_password(arg, err);
> +            }
>          } else {
> -            return monitor_read_password(mon, change_vnc_password_cb, NULL);
> +            qmp_change_vnc_listen(target, err);
>          }
>      } else {
> -        if (vnc_display_open(NULL, target) < 0) {
> -            qerror_report(QERR_VNC_SERVER_FAILED, target);
> -            return -1;
> -        }
> +        deprecated_qmp_change_blockdev(device, target, has_arg, arg, err);
>      }
> -
> -    return 0;
> -}
> -#else
> -static int do_change_vnc(Monitor *mon, const char *target, const char *arg)
> -{
> -    qerror_report(QERR_FEATURE_DISABLED, "vnc");
> -    return -ENODEV;
> -}
> -#endif
> -
> -/**
> - * do_change(): Change a removable medium, or VNC configuration
> - */
> -static int do_change(Monitor *mon, const QDict *qdict, QObject **ret_data)
> -{
> -    const char *device = qdict_get_str(qdict, "device");
> -    const char *target = qdict_get_str(qdict, "target");
> -    const char *arg = qdict_get_try_str(qdict, "arg");
> -    int ret;
> -
> -    if (strcmp(device, "vnc") == 0) {
> -        ret = do_change_vnc(mon, target, arg);
> -    } else {
> -        ret = do_change_block(mon, device, target, arg);
> -    }
> -
> -    return ret;
>  }
>  
>  static int set_password(Monitor *mon, const QDict *qdict, QObject **ret_data)
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 139c6e3..f108d20 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -84,6 +84,44 @@
>    'data': {'device': 'str', 'password': 'str'} }
>  
>  ##
> +# @change:
> +#
> +# This command is multiple commands multiplexed together.  Generally speaking,
> +# it should not be used in favor of the single purpose alternatives such as
> +# @change-vnc-listen, @change-vnc-password, and @change-blockdev.
> +#
> +# @device: This is normally the name of a block device but it may also be 'vnc'.
> +#          when it's 'vnc', then sub command depends on @target
> +#
> +# @target: If @device is a block device, then this is the new filename.
> +#          If @device is 'vnc', then if the value 'password' selects the vnc
> +#          change password command.   Otherwise, this specifies a new server URI
> +#          address to listen to for VNC connections.
> +#
> +# @arg:    If @device is a block device, then this is an optional format to open
> +#          the device with.
> +#          If @device is 'vnc' and @target is 'password', this is the new VNC
> +#          password to set.  If this argument is an empty string, then no future
> +#          logins will be allowed.
> +#
> +# Returns: Nothing on success.
> +#          If @device is not a valid block device, DeviceNotFound
> +#          If @format is not a valid block format, InvalidBlockFormat
> +#          If the new block device is encrypted, DeviceEncrypted.  Note that
> +#          if this error is returned, the device has been opened successfully
> +#          and an additional call to @block_passwd is required to set the
> +#          device's password.  The behavior of reads and writes to the block
> +#          device between when these calls are executed is undefined.
> +#
> +# Notes:  It is strongly recommended that this interface is not used especially
> +#         for changing block devices.

I'd drop the 'especially' part.

> +#
> +# Since: 0.14.0
> +##
> +{ 'command': 'change',
> +  'data': {'device': 'str', 'target': 'str', '*arg': 'str'} }
> +
> +##
>  # @set-blockdev-password:
>  #
>  # Alias for @block_passwd.
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index cec7135..89b8d00 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -117,8 +117,7 @@ EQMP
>          .args_type  = "device:B,target:F,arg:s?",
>          .params     = "device filename [format]",
>          .help       = "change a removable medium, optional format",
> -        .user_print = monitor_user_noop,
> -        .mhandler.cmd_new = do_change,
> +        .mhandler.cmd_new = qmp_marshal_input_change,
>      },
>  
>      {

Patch

diff --git a/blockdev.c b/blockdev.c
index c00c69d..3832118 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -785,8 +785,9 @@  void qmp_change_blockdev(const char *device, const char *filename,
     qmp_bdrv_open_encrypted(bs, filename, bdrv_flags, drv, password, errp);
 }
 
-int do_change_block(Monitor *mon, const char *device,
-                    const char *filename, const char *fmt)
+void deprecated_qmp_change_blockdev(const char *device, const char *filename,
+                                    bool has_format, const char *format,
+                                    Error **errp)
 {
     BlockDriverState *bs;
     BlockDriver *drv = NULL;
@@ -795,14 +796,14 @@  int do_change_block(Monitor *mon, const char *device,
 
     bs = bdrv_find(device);
     if (!bs) {
-        qerror_report(QERR_DEVICE_NOT_FOUND, device);
-        return -1;
+        error_set(errp, QERR_DEVICE_NOT_FOUND, device);
+        return;
     }
-    if (fmt) {
-        drv = bdrv_find_whitelisted_format(fmt);
+    if (has_format) {
+        drv = bdrv_find_whitelisted_format(format);
         if (!drv) {
-            qerror_report(QERR_INVALID_BLOCK_FORMAT, fmt);
-            return -1;
+            error_set(errp, QERR_INVALID_BLOCK_FORMAT, format);
+            return;
         }
     }
 
@@ -811,17 +812,13 @@  int do_change_block(Monitor *mon, const char *device,
 
     eject_device(bs, 0, &err);
     if (err) {
-        qerror_report_err(err);
-        return -1;
+        error_propagate(errp, err);
+        return;
     }
 
     qmp_bdrv_open_encrypted(bs, filename, bdrv_flags, drv, NULL, &err);
-    if (err) {
-        qerror_report_err(err);
-        return -1;
-    }
 
-    return 0;
+    error_propagate(errp, err);
 }
 
 int do_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data)
diff --git a/blockdev.h b/blockdev.h
index badbf01..54b142f 100644
--- a/blockdev.h
+++ b/blockdev.h
@@ -12,6 +12,7 @@ 
 
 #include "block.h"
 #include "qemu-queue.h"
+#include "error.h"
 
 void blockdev_mark_auto_del(BlockDriverState *bs);
 void blockdev_auto_del(BlockDriverState *bs);
@@ -59,8 +60,9 @@  DriveInfo *add_init_drive(const char *opts);
 
 void do_commit(Monitor *mon, const QDict *qdict);
 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);
+void deprecated_qmp_change_blockdev(const char *device, const char *filename,
+                                    bool has_format, const char *format,
+                                    Error **errp);
 int do_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data);
 int do_snapshot_blkdev(Monitor *mon, const QDict *qdict, QObject **ret_data);
 int do_block_resize(Monitor *mon, const QDict *qdict, QObject **ret_data);
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 2f0ffa3..700d2f1 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -108,8 +108,7 @@  ETEXI
         .args_type  = "device:B,target:F,arg:s?",
         .params     = "device filename [format]",
         .help       = "change a removable medium, optional format",
-        .user_print = monitor_user_noop,
-        .mhandler.cmd_new = do_change,
+        .mhandler.cmd = hmp_change,
     },
 
 STEXI
diff --git a/hmp.c b/hmp.c
index a8ae36b..e8bca20 100644
--- a/hmp.c
+++ b/hmp.c
@@ -50,3 +50,62 @@  void hmp_block_passwd(Monitor *mon, const QDict *qdict)
         error_free(err);
     }
 }
+
+static void cb_hmp_change_bdrv_pwd(Monitor *mon, const char *password,
+                                   void *opaque)
+{
+    Error *encryption_err = opaque;
+    Error *err = NULL;
+    const char *device;
+
+    device = error_get_field(encryption_err, "device");
+
+    qmp_block_passwd(device, password, &err);
+    if (err) {
+        monitor_printf(mon, "invalid password\n");
+        error_free(err);
+    }
+
+    error_free(encryption_err);
+
+    monitor_read_command(mon, 1);
+}
+
+static void hmp_change_read_arg(Monitor *mon, const char *password,
+                                void *opaque)
+{
+    qmp_change_vnc_password(password, NULL);
+    monitor_read_command(mon, 1);
+}
+
+void hmp_change(Monitor *mon, const QDict *qdict)
+{
+    const char *device = qdict_get_str(qdict, "device");
+    const char *target = qdict_get_str(qdict, "target");
+    const char *arg = qdict_get_try_str(qdict, "arg");
+    Error *err = NULL;
+
+    if (strcmp(device, "vnc") == 0 &&
+        (strcmp(target, "passwd") == 0 ||
+         strcmp(target, "password") == 0)) {
+        if (arg == NULL) {
+            monitor_read_password(mon, hmp_change_read_arg, NULL);
+            return;
+        }
+    }
+
+    qmp_change(device, target, !!arg, arg, &err);
+    if (error_is_type(err, QERR_DEVICE_ENCRYPTED)) {
+        monitor_printf(mon, "%s (%s) is encrypted.\n",
+                       error_get_field(err, "device"),
+                       error_get_field(err, "encrypted_filename"));
+        if (!monitor_get_rs(mon)) {
+            monitor_printf(mon,
+                           "terminal does not support password prompting\n");
+            error_free(err);
+            return;
+        }
+        readline_start(monitor_get_rs(mon), "Password: ", 1,
+                       cb_hmp_change_bdrv_pwd, err);
+    }
+}
diff --git a/hmp.h b/hmp.h
index 8f72ef2..9df6ccc 100644
--- a/hmp.h
+++ b/hmp.h
@@ -20,5 +20,6 @@ 
 void hmp_info_name(Monitor *mon);
 void hmp_eject(Monitor *mon, const QDict *args);
 void hmp_block_passwd(Monitor *mon, const QDict *qdict);
+void hmp_change(Monitor *mon, const QDict *qdict);
 
 #endif
diff --git a/monitor.c b/monitor.c
index 5cb36cd..9801a2d 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1005,78 +1005,22 @@  static int do_quit(Monitor *mon, const QDict *qdict, QObject **ret_data)
     return 0;
 }
 
-#ifdef CONFIG_VNC
-static int change_vnc_password(const char *password)
-{
-    if (!password || !password[0]) {
-        if (vnc_display_disable_login(NULL)) {
-            qerror_report(QERR_SET_PASSWD_FAILED);
-            return -1;
-        }
-        return 0;
-    }
-
-    if (vnc_display_password(NULL, password) < 0) {
-        qerror_report(QERR_SET_PASSWD_FAILED);
-        return -1;
-    }
-
-    return 0;
-}
-
-static void change_vnc_password_cb(Monitor *mon, const char *password,
-                                   void *opaque)
+void qmp_change(const char *device, const char *target,
+                bool has_arg, const char *arg, Error **err)
 {
-    change_vnc_password(password);
-    monitor_read_command(mon, 1);
-}
-
-static int do_change_vnc(Monitor *mon, const char *target, const char *arg)
-{
-    if (strcmp(target, "passwd") == 0 ||
-        strcmp(target, "password") == 0) {
-        if (arg) {
-            char password[9];
-            strncpy(password, arg, sizeof(password));
-            password[sizeof(password) - 1] = '\0';
-            return change_vnc_password(password);
+    if (strcmp(device, "vnc") == 0) {
+        if (strcmp(target, "passwd") == 0 || strcmp(target, "password") == 0) {
+            if (!has_arg || !arg[0]) {
+                vnc_display_disable_login(NULL);
+            } else {
+                qmp_change_vnc_password(arg, err);
+            }
         } else {
-            return monitor_read_password(mon, change_vnc_password_cb, NULL);
+            qmp_change_vnc_listen(target, err);
         }
     } else {
-        if (vnc_display_open(NULL, target) < 0) {
-            qerror_report(QERR_VNC_SERVER_FAILED, target);
-            return -1;
-        }
+        deprecated_qmp_change_blockdev(device, target, has_arg, arg, err);
     }
-
-    return 0;
-}
-#else
-static int do_change_vnc(Monitor *mon, const char *target, const char *arg)
-{
-    qerror_report(QERR_FEATURE_DISABLED, "vnc");
-    return -ENODEV;
-}
-#endif
-
-/**
- * do_change(): Change a removable medium, or VNC configuration
- */
-static int do_change(Monitor *mon, const QDict *qdict, QObject **ret_data)
-{
-    const char *device = qdict_get_str(qdict, "device");
-    const char *target = qdict_get_str(qdict, "target");
-    const char *arg = qdict_get_try_str(qdict, "arg");
-    int ret;
-
-    if (strcmp(device, "vnc") == 0) {
-        ret = do_change_vnc(mon, target, arg);
-    } else {
-        ret = do_change_block(mon, device, target, arg);
-    }
-
-    return ret;
 }
 
 static int set_password(Monitor *mon, const QDict *qdict, QObject **ret_data)
diff --git a/qapi-schema.json b/qapi-schema.json
index 139c6e3..f108d20 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -84,6 +84,44 @@ 
   'data': {'device': 'str', 'password': 'str'} }
 
 ##
+# @change:
+#
+# This command is multiple commands multiplexed together.  Generally speaking,
+# it should not be used in favor of the single purpose alternatives such as
+# @change-vnc-listen, @change-vnc-password, and @change-blockdev.
+#
+# @device: This is normally the name of a block device but it may also be 'vnc'.
+#          when it's 'vnc', then sub command depends on @target
+#
+# @target: If @device is a block device, then this is the new filename.
+#          If @device is 'vnc', then if the value 'password' selects the vnc
+#          change password command.   Otherwise, this specifies a new server URI
+#          address to listen to for VNC connections.
+#
+# @arg:    If @device is a block device, then this is an optional format to open
+#          the device with.
+#          If @device is 'vnc' and @target is 'password', this is the new VNC
+#          password to set.  If this argument is an empty string, then no future
+#          logins will be allowed.
+#
+# Returns: Nothing on success.
+#          If @device is not a valid block device, DeviceNotFound
+#          If @format is not a valid block format, InvalidBlockFormat
+#          If the new block device is encrypted, DeviceEncrypted.  Note that
+#          if this error is returned, the device has been opened successfully
+#          and an additional call to @block_passwd is required to set the
+#          device's password.  The behavior of reads and writes to the block
+#          device between when these calls are executed is undefined.
+#
+# Notes:  It is strongly recommended that this interface is not used especially
+#         for changing block devices.
+#
+# Since: 0.14.0
+##
+{ 'command': 'change',
+  'data': {'device': 'str', 'target': 'str', '*arg': 'str'} }
+
+##
 # @set-blockdev-password:
 #
 # Alias for @block_passwd.
diff --git a/qmp-commands.hx b/qmp-commands.hx
index cec7135..89b8d00 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -117,8 +117,7 @@  EQMP
         .args_type  = "device:B,target:F,arg:s?",
         .params     = "device filename [format]",
         .help       = "change a removable medium, optional format",
-        .user_print = monitor_user_noop,
-        .mhandler.cmd_new = do_change,
+        .mhandler.cmd_new = qmp_marshal_input_change,
     },
 
     {