On 12/03/2013 06:26 AM, Benoît Canet wrote: > Signed-off-by: Benoit Canet <ben...@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?
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
