| Message ID | 1386077165-19577-5-git-send-email-benoit@irqsave.net |
|---|---|
| State | New |
| Headers | show |
On 12/03/2013 06:26 AM, Benoît Canet wrote: > Signed-off-by: Benoit Canet <benoit@irqsave.net> > --- > > +BlockDriverState * bdrv_lookup_bs(bool has_device, const char * device, > + bool has_node_name, const char * node_name, Style: no space after * (3 instances) > + Error **errp) > +{ > + BlockDriverState *bs = NULL; > + > + if ((has_device && has_node_name) || > + (!has_device && !has_node_name)) { Could be shortened to: if (has_device == has_node_name) { > + error_setg(errp, "Use either device or node-name but not both."); We tend to avoid trailing '.' on error messages > > -void qmp_block_passwd(const char *device, const char *password, Error **errp) > +void qmp_block_passwd(bool has_device, const char * device, > + bool has_node_name, const char * node_name, > + const char * password, Error **errp) Again, no space after '*' > +++ b/include/block/block.h > @@ -371,6 +371,9 @@ void bdrv_eject(BlockDriverState *bs, bool eject_flag); > const char *bdrv_get_format_name(BlockDriverState *bs); > BlockDriverState *bdrv_find(const char *name); > BlockDriverState *bdrv_find_node(const char *node_name); > +BlockDriverState * bdrv_lookup_bs(bool has_device, const char * device, > + bool has_node_name, const char * node_name, > + Error **errp); And again > +++ b/qapi-schema.json > @@ -1675,7 +1675,11 @@ > # 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 > +# Either @device or @node-name must be set but not both. > +# > +# @device: #optional the name of the block backend device to set the password on > +# > +# @node-name: #optional graph node name to set the password on (Since 1.8) 2.0 > # > # @password: the password to use for the device > # > @@ -1689,7 +1693,8 @@ > # > # Since: 0.14.0 > ## > -{ 'command': 'block_passwd', 'data': {'device': 'str', 'password': 'str'} } > +{ 'command': 'block_passwd', 'data': {'*device': 'str', > + '*node-name': 'str', 'password': 'str'} } Seems like a reasonable addition; shouldn't cause any back-compat problems (older management tools will always provide the now-optional 'device'). Is it intentional that you are not exposing this new functionality in HMP?
Le Wednesday 04 Dec 2013 à 16:56:05 (-0700), Eric Blake a écrit : > On 12/03/2013 06:26 AM, Benoît Canet wrote: > > Signed-off-by: Benoit Canet <benoit@irqsave.net> > > --- > > > > +BlockDriverState * bdrv_lookup_bs(bool has_device, const char * device, > > + bool has_node_name, const char * node_name, > > Style: no space after * (3 instances) > > > + Error **errp) > > +{ > > + BlockDriverState *bs = NULL; > > + > > + if ((has_device && has_node_name) || > > + (!has_device && !has_node_name)) { > > Could be shortened to: > > if (has_device == has_node_name) { > > > + error_setg(errp, "Use either device or node-name but not both."); > > We tend to avoid trailing '.' on error messages > > > > > -void qmp_block_passwd(const char *device, const char *password, Error **errp) > > +void qmp_block_passwd(bool has_device, const char * device, > > + bool has_node_name, const char * node_name, > > + const char * password, Error **errp) > > Again, no space after '*' > > > +++ b/include/block/block.h > > @@ -371,6 +371,9 @@ void bdrv_eject(BlockDriverState *bs, bool eject_flag); > > const char *bdrv_get_format_name(BlockDriverState *bs); > > BlockDriverState *bdrv_find(const char *name); > > BlockDriverState *bdrv_find_node(const char *node_name); > > +BlockDriverState * bdrv_lookup_bs(bool has_device, const char * device, > > + bool has_node_name, const char * node_name, > > + Error **errp); > > And again > > > +++ b/qapi-schema.json > > @@ -1675,7 +1675,11 @@ > > # 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 > > +# Either @device or @node-name must be set but not both. > > +# > > +# @device: #optional the name of the block backend device to set the password on > > +# > > +# @node-name: #optional graph node name to set the password on (Since 1.8) > > 2.0 > > > # > > # @password: the password to use for the device > > # > > @@ -1689,7 +1693,8 @@ > > # > > # Since: 0.14.0 > > ## > > -{ 'command': 'block_passwd', 'data': {'device': 'str', 'password': 'str'} } > > +{ 'command': 'block_passwd', 'data': {'*device': 'str', > > + '*node-name': 'str', 'password': 'str'} } > > Seems like a reasonable addition; shouldn't cause any back-compat > problems (older management tools will always provide the now-optional > 'device'). > > Is it intentional that you are not exposing this new functionality in HMP? Yes, I don't foresee any way to print the graph in HMP so I am limiting the changes to QMP. Best regards Benoît > > -- > Eric Blake eblake redhat com +1-919-301-3266 > Libvirt virtualization library http://libvirt.org >
diff --git a/block.c b/block.c index 3a0cb30..8016ff2 100644 --- a/block.c +++ b/block.c @@ -3176,6 +3176,39 @@ BlockDriverState *bdrv_find_node(const char *node_name) return NULL; } +BlockDriverState * bdrv_lookup_bs(bool has_device, const char * device, + bool has_node_name, const char * node_name, + Error **errp) +{ + BlockDriverState *bs = NULL; + + if ((has_device && has_node_name) || + (!has_device && !has_node_name)) { + error_setg(errp, "Use either device or node-name but not both."); + return NULL; + } + + if (has_device) { + bs = bdrv_find(device); + + if (!bs) { + error_set(errp, QERR_DEVICE_NOT_FOUND, device); + return NULL; + } + + return bs; + } + + bs = bdrv_find_node(node_name); + + if (!bs) { + error_set(errp, QERR_DEVICE_NOT_FOUND, node_name); + return NULL; + } + + return bs; +} + BlockDriverState *bdrv_next(BlockDriverState *bs) { if (!bs) { diff --git a/blockdev.c b/blockdev.c index 824e718..aab370f 100644 --- a/blockdev.c +++ b/blockdev.c @@ -1474,14 +1474,19 @@ void qmp_eject(const char *device, bool has_force, bool force, Error **errp) eject_device(bs, force, errp); } -void qmp_block_passwd(const char *device, const char *password, Error **errp) +void qmp_block_passwd(bool has_device, const char * device, + bool has_node_name, const char * node_name, + const char * password, Error **errp) { BlockDriverState *bs; + Error *local_err = NULL; int err; - bs = bdrv_find(device); - if (!bs) { - error_set(errp, QERR_DEVICE_NOT_FOUND, device); + bs = bdrv_lookup_bs(has_device, device, + has_node_name, node_name, + &local_err); + if (error_is_set(&local_err)) { + error_propagate(errp, local_err); return; } diff --git a/hmp.c b/hmp.c index 32ee285..3820fbe 100644 --- a/hmp.c +++ b/hmp.c @@ -870,7 +870,7 @@ void hmp_block_passwd(Monitor *mon, const QDict *qdict) const char *password = qdict_get_str(qdict, "password"); Error *errp = NULL; - qmp_block_passwd(device, password, &errp); + qmp_block_passwd(true, device, false, NULL, password, &errp); hmp_handle_error(mon, &errp); } diff --git a/include/block/block.h b/include/block/block.h index 6426ca6..26c48e7 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -371,6 +371,9 @@ void bdrv_eject(BlockDriverState *bs, bool eject_flag); const char *bdrv_get_format_name(BlockDriverState *bs); BlockDriverState *bdrv_find(const char *name); BlockDriverState *bdrv_find_node(const char *node_name); +BlockDriverState * bdrv_lookup_bs(bool has_device, const char * device, + bool has_node_name, const char * node_name, + Error **errp); BlockDriverState *bdrv_next(BlockDriverState *bs); void bdrv_iterate(void (*it)(void *opaque, BlockDriverState *bs), void *opaque); diff --git a/qapi-schema.json b/qapi-schema.json index 938f8b9..60d3bd9 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -1675,7 +1675,11 @@ # 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 +# Either @device or @node-name must be set but not both. +# +# @device: #optional the name of the block backend device to set the password on +# +# @node-name: #optional graph node name to set the password on (Since 1.8) # # @password: the password to use for the device # @@ -1689,7 +1693,8 @@ # # Since: 0.14.0 ## -{ 'command': 'block_passwd', 'data': {'device': 'str', 'password': 'str'} } +{ 'command': 'block_passwd', 'data': {'*device': 'str', + '*node-name': 'str', 'password': 'str'} } ## # @balloon: diff --git a/qmp-commands.hx b/qmp-commands.hx index fba15cd..a73b08f 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -1452,7 +1452,7 @@ EQMP { .name = "block_passwd", - .args_type = "device:B,password:s", + .args_type = "device:s?,node-name:s?,password:s", .mhandler.cmd_new = qmp_marshal_input_block_passwd, }, @@ -1465,6 +1465,7 @@ Set the password of encrypted block devices. Arguments: - "device": device name (json-string) +- "node-name": name in the block driver state graph (json-string) - "password": password (json-string) Example:
Signed-off-by: Benoit Canet <benoit@irqsave.net> --- block.c | 33 +++++++++++++++++++++++++++++++++ blockdev.c | 13 +++++++++---- hmp.c | 2 +- include/block/block.h | 3 +++ qapi-schema.json | 9 +++++++-- qmp-commands.hx | 3 ++- 6 files changed, 55 insertions(+), 8 deletions(-)