Am 12.12.2013 um 16:33 hat BenoƮt Canet geschrieben: > There was two candidate ways to implement named node manipulation: > > 1) > { 'command': 'block_passwd', 'data': {'*device': 'str', > '*node-name': 'str', 'password': 'str'} > } > > 2) > > { 'command': 'block_passwd', 'data': {'device': 'str', > '*device-is-node': 'bool', > 'password': 'str'} } > > Luiz proposed 1 and says 2 was an abuse of the QMP interface and proposed to > rewrite the QMP block interface for 2.0. > > Luiz does not like in 1 the fact that 2 fields are optional but one of them > must > be specified leading to an abuse of the QMP semantic. > > Kevin argumented that 2 what a clear abuse of the device field and would not > be > practical when reading fast some log file because the user would read "device" > and think that a device is manipulated when it's in fact a node name. > Documentation of 1 make it pretty clear what to do for the user. > > Kevin argued that all bs are node including devices ones so 2 does not make > sense. > > Kevin also argued that rewriting the QMP block interface would not make > disapear > the current one. > > Kevin pushed the argument that making the QAPI generator compatible with the > semantic of the operation would need a rewrite that no one has done yet. > > A vote has been done on the list to elect the version to use and 1 won. > > For reference the complete thread is: > "[Qemu-devel] [PATCH V4 4/7] qmp: Allow to change password on names block > driver > states." > > Signed-off-by: Benoit Canet <ben...@irqsave.net> > --- > block.c | 32 ++++++++++++++++++++++++++++++++ > blockdev.c | 13 +++++++++---- > hmp.c | 2 +- > include/block/block.h | 3 +++ > qapi-schema.json | 9 +++++++-- > qmp-commands.hx | 3 ++- > 6 files changed, 54 insertions(+), 8 deletions(-) > > diff --git a/block.c b/block.c > index 78d13e5..22190a4 100644 > --- a/block.c > +++ b/block.c > @@ -3207,6 +3207,38 @@ BlockDeviceInfoList *bdrv_named_nodes_list(void) > return list; > } > > +BlockDriverState *bdrv_lookup_bs(bool has_device, const char *device, > + bool has_node_name, const char *node_name, > + Error **errp)
This is a normal function without generated callers, so can we get rid of has_device/has_node_name and just switch to passing NULL if it's not present? That would make it more convenient to use this function in other places. Kevin