Message ID | 1314211389-28915-14-git-send-email-aliguori@us.ibm.com |
---|---|
State | New |
Headers | show |
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, > }, > > {
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, }, {
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(-)