Re: [Qemu-devel] [PATCH V6 8/8] block: Use graph node name as reference in bdrv_file_open().

[Benoît Canet]

Le Monday 27 Jan 2014 à 20:11:59 (+0100), Max Reitz a écrit :
> On 27.01.2014 15:36, Benoît Canet wrote:
> >Le Friday 24 Jan 2014 à 15:54:39 (+0100), Max Reitz a écrit :
> >>On 24.01.2014 15:48, Kevin Wolf wrote:
> >>>Am 24.01.2014 um 14:37 hat Max Reitz geschrieben:
> >>>>On 24.01.2014 14:26, Kevin Wolf wrote:
> >>>>>Am 23.01.2014 um 21:31 hat Benoît Canet geschrieben:
> >>>>>>Signed-off-by: Benoit Canet <address@hidden>
> >>>>>>---
> >>>>>>  block.c | 6 +++---
> >>>>>>  1 file changed, 3 insertions(+), 3 deletions(-)
> >>>>>I'm not going to merge this one yet. It breaks qemu-iotests case 071,
> >>>>>which would have to be adapted.
> >>>>>
> >>>>>However, first of all I'd like to hear the opinions of at least Eric and
> >>>>>Max on what BlockRef should really refer to. I think node names make
> >>>>>most sense, but perhaps it's a bit inconvenient and the command line
> >>>>>should default to node-name = id when id is set, but node-name isn't?
> >>>>The QAPI schema is pretty clear about this: “references the ID of an
> >>>>existing block device.”
> >>>Sure, that's because I wrote that text before we had a node name.
> >>>
> >>>However, in 1.7 references didn't work yet, so we still have all freedom
> >>>to change the interface as we like.
> >>Yes, that's right.
> >>
> >>>>However, if the ID cannot be found, I think
> >>>>we should interpret it as a reference to the node name.
> >>>>
> >>>>Therefore, I'd first try bdrv_find() and if that returns NULL, try
> >>>>again with bdrv_find_node().
> >>>I think I would prefer to avoid such ambiguities. Otherwise a management
> >>>tool that wants to use the node name needs to check first if it's not
> >>>already used as a device name somewhere else and would therefore operate
> >>>on the wrong device.
> >>>
> >>>On the other hand, a management tool using the same names for devices
> >>>and nodes just gets what it deserves.
> >>>
> >>>Perhaps we should use a common namespace for both, i.e. you get an error
> >>>if you try to assign a node name that is already a device name and vice
> >>>versa?
> >>This is what I would go for. However, then I don't really know why
> >>we should separate the ID and the node name in the first place
> >>(although that's probably because I haven't followed the discussion
> >>around node names).
> >>
> >>Max
> >Ping,
> >
> >I still want to make quorum merge.
> >What should be done for the references ?
> >
> >Best regards
> >
> >Benoît
> 
> My only problem is that I don't really know what IDs are for, then. ;-)
> 

>From the understanding I have ID are for block backend top level bds and
node-name naming all the bds burried in the graph.

So my personal opinion would be to relax the constraint on bdrv_lookup_bs
and use it for references.

Kevin && Max: what do you think of this scheme ?

Best regards

Benoît

> Currently, I think using the node name is probably (more) correct
> and it can't hurt anyone; thus, I'm okay with this patch.
> 
> Max