Patchwork [09/14] qapi: convert block_passwd and add set-blockdev-password

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

Comments

Anthony Liguori - Aug. 24, 2011, 6:43 p.m.
block_passwd is unfortunately named so while converting block_passwd to QAPI,
introduce a more properly named alias.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 blockdev.c       |   29 +++++++++++++++--------------
 hmp-commands.hx  |    2 +-
 hmp.c            |   12 ++++++++++++
 hmp.h            |    1 +
 qapi-schema.json |   47 +++++++++++++++++++++++++++++++++++++++++++++++
 qmp-commands.hx  |   11 +++++++++--
 6 files changed, 85 insertions(+), 17 deletions(-)
Kevin Wolf - Aug. 25, 2011, 12:29 p.m.
Am 24.08.2011 20:43, schrieb Anthony Liguori:
> block_passwd is unfortunately named so while converting block_passwd to QAPI,
> introduce a more properly named alias.
> 
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>

I'm not sure if I like the set-blockdev-password alias, for two reasons.

The first one is that blockdev_* is supposed to be the new, clean
interface, so I wouldn't add similarly named commands without having a
look at the big picture. For example, what again was the reason for
having the password separate from other block device options?

The second one is that names like set-foo, get-foo and frob-foo make it
very hard to see what I can do with a foo. If they are called as today
(drive_add, drive_del) and ideally use a consistent prefix (unlike
drive_*, block_*, snapshot_blkdev today), you'll find all of them in the
same place in the documentation and in HMP typing blockdev_<TAB> will
show you everything you can do with a block device.

Kevin

Patch

diff --git a/blockdev.c b/blockdev.c
index 6b7fc41..37b2f29 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -672,28 +672,29 @@  void qmp_eject(const char *device, bool has_force, bool force, Error **errp)
     eject_device(bs, force, errp);
 }
 
-int do_block_set_passwd(Monitor *mon, const QDict *qdict,
-                        QObject **ret_data)
+void qmp_set_blockdev_password(const char *device, const char *password,
+                               Error **err)
 {
     BlockDriverState *bs;
-    int err;
+    int ret;
 
-    bs = bdrv_find(qdict_get_str(qdict, "device"));
+    bs = bdrv_find(device);
     if (!bs) {
-        qerror_report(QERR_DEVICE_NOT_FOUND, qdict_get_str(qdict, "device"));
-        return -1;
+        error_set(err, QERR_DEVICE_NOT_FOUND, device);
+        return;
     }
 
-    err = bdrv_set_key(bs, qdict_get_str(qdict, "password"));
-    if (err == -EINVAL) {
-        qerror_report(QERR_DEVICE_NOT_ENCRYPTED, bdrv_get_device_name(bs));
-        return -1;
-    } else if (err < 0) {
-        qerror_report(QERR_INVALID_PASSWORD);
-        return -1;
+    ret = bdrv_set_key(bs, password);
+    if (ret == -EINVAL) {
+        error_set(err, QERR_DEVICE_NOT_ENCRYPTED, bdrv_get_device_name(bs));
+    } else if (ret < 0) {
+        error_set(err, QERR_INVALID_PASSWORD);
     }
+}
 
-    return 0;
+void qmp_block_passwd(const char *device, const char *password, Error **err)
+{
+    qmp_set_blockdev_password(device, password, err);
 }
 
 int do_change_block(Monitor *mon, const char *device,
diff --git a/hmp-commands.hx b/hmp-commands.hx
index bcb789b..2f0ffa3 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1205,7 +1205,7 @@  ETEXI
         .params     = "block_passwd device password",
         .help       = "set the password of encrypted block devices",
         .user_print = monitor_user_noop,
-        .mhandler.cmd_new = do_block_set_passwd,
+        .mhandler.cmd = hmp_block_passwd,
     },
 
 STEXI
diff --git a/hmp.c b/hmp.c
index 36eb5b9..a8ae36b 100644
--- a/hmp.c
+++ b/hmp.c
@@ -38,3 +38,15 @@  void hmp_eject(Monitor *mon, const QDict *qdict)
     }
 }
 
+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_set_blockdev_password(device, password, &err);
+    if (err) {
+        monitor_printf(mon, "block_passwd: %s\n", error_get_pretty(err));
+        error_free(err);
+    }
+}
diff --git a/hmp.h b/hmp.h
index 6a552c1..8f72ef2 100644
--- a/hmp.h
+++ b/hmp.h
@@ -19,5 +19,6 @@ 
 
 void hmp_info_name(Monitor *mon);
 void hmp_eject(Monitor *mon, const QDict *args);
+void hmp_block_passwd(Monitor *mon, const QDict *qdict);
 
 #endif
diff --git a/qapi-schema.json b/qapi-schema.json
index 934ea81..f159d81 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -49,3 +49,50 @@ 
 ##
 { 'command': 'eject', 'data': {'device': 'str', '*force': 'bool'} }
 
+##
+# @block_passwd:
+#
+# This command sets the password of a block device that has not been open
+# with a password and requires one.
+#
+# The two cases where this can happen are a block device is created through
+# QEMU's initial command line or a block device is changed through the legacy
+# @change interface.
+#
+# In the event that the block device is created through the initial command
+# line, the VM will start in the stopped state regardless of whether '-S' is
+# used.  The intention is for a management tool to query the block devices to
+# determine which ones are encrypted, set the passwords with this command, and
+# then start the guest with the @cont command.
+#
+# @device:   the name of the device to set the password on
+#
+# @password: the password to use for the device
+#
+# Returns: nothing on success
+#          If @device is not a valid block device, DeviceNotFound
+#          If @device is not encrypted, DeviceNotEncrypted
+#          If @password is not valid for this device, InvalidPassword
+#
+# Notes:  Not all block formats support encryption and some that do are not
+#         able to validate that a password is correct.  Disk corruption may
+#         occur if an invalid password is specified.
+#
+# Since: 0.14.0
+##
+{ 'command': 'block_passwd',
+  'data': {'device': 'str', 'password': 'str'} }
+
+##
+# @set-blockdev-password:
+#
+# Alias for @block_passwd.
+#
+# @device:   see @block_passwd
+# @password: see @block_passwd
+#
+# Since: 1.0
+##
+{ 'command': 'set-blockdev-password',
+  'data': {'device': 'str', 'password': 'str'} }
+
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 81d1800..909c778 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -837,8 +837,15 @@  EQMP
         .args_type  = "device:B,password:s",
         .params     = "block_passwd device password",
         .help       = "set the password of encrypted block devices",
-        .user_print = monitor_user_noop,
-        .mhandler.cmd_new = do_block_set_passwd,
+        .mhandler.cmd_new = qmp_marshal_input_block_passwd,
+    },
+
+    {
+        .name       = "set-blockdev-password",
+        .args_type  = "device:B,password:s",
+        .params     = "block_passwd device password",
+        .help       = "set the password of encrypted block devices",
+        .mhandler.cmd_new = qmp_marshal_input_set_blockdev_password,
     },
 
 SQMP