| Message ID | 1390509099-695-9-git-send-email-benoit.canet@irqsave.net |
|---|---|
| State | New |
| Headers | show |
Am 23.01.2014 um 21:31 hat Benoît Canet geschrieben: > Signed-off-by: Benoit Canet <benoit@irqsave.net> > --- > 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? Kevin > diff --git a/block.c b/block.c > index 3e0994b..7726636 100644 > --- a/block.c > +++ b/block.c > @@ -908,15 +908,15 @@ int bdrv_file_open(BlockDriverState **pbs, const char *filename, > > if (reference) { > if (filename || qdict_size(options)) { > - error_setg(errp, "Cannot reference an existing block device with " > + error_setg(errp, "Cannot reference an existing graph node with " > "additional options or a new filename"); > return -EINVAL; > } > QDECREF(options); > > - bs = bdrv_find(reference); > + bs = bdrv_find_node(reference); > if (!bs) { > - error_setg(errp, "Cannot find block device '%s'", reference); > + error_setg(errp, "Cannot find graph node '%s'", reference); > return -ENODEV; > } > bdrv_ref(bs); > -- > 1.8.3.2 >
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 <benoit@irqsave.net> >> --- >> 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.” 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(). Max
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 <benoit@irqsave.net> > >>--- > >> 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. > 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? Kevin
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 <benoit@irqsave.net> >>>> --- >>>> 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
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 <benoit@irqsave.net> > >>>>--- > >>>> 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
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 <benoit@irqsave.net> >>>>>> --- >>>>>> 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. ;-) 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
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 <benoit@irqsave.net> > >>>>>>--- > >>>>>> 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
On 28.01.2014 01:04, Benoît Canet wrote: > 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 <benoit@irqsave.net> >>>>>>>> --- >>>>>>>> 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 ? I agree. For example, we could change the constraint to report an error only if both ID and node name are actually valid (and point to different devices), that is, bdrv_find() and bdrv_find_node() return different non-NULL values. Max
Le Friday 31 Jan 2014 à 21:32:34 (+0100), Max Reitz a écrit : > On 28.01.2014 01:04, Benoît Canet wrote: > >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 <benoit@irqsave.net> > >>>>>>>>--- > >>>>>>>> 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 ? > > I agree. For example, we could change the constraint to report an > error only if both ID and node name are actually valid (and point to > different devices), that is, bdrv_find() and bdrv_find_node() return > different non-NULL values. Ok I will write patch doing this on top of quorum patches. Best regards Benoît > > Max
Am 31.01.2014 um 22:37 hat Benoît Canet geschrieben: > Le Friday 31 Jan 2014 à 21:32:34 (+0100), Max Reitz a écrit : > > On 28.01.2014 01:04, Benoît Canet wrote: > > >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 <benoit@irqsave.net> > > >>>>>>>>--- > > >>>>>>>> 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 ? > > > > I agree. For example, we could change the constraint to report an > > error only if both ID and node name are actually valid (and point to > > different devices), that is, bdrv_find() and bdrv_find_node() return > > different non-NULL values. > > Ok I will write patch doing this on top of quorum patches. Yes, I think allowing bdrv_lookup_bs() to find both node names and device names makes sense. I would still use a common namespace and forbid using the same name for a device and a node. Kevin
On 02/03/2014 02:43 AM, Kevin Wolf wrote: > > Yes, I think allowing bdrv_lookup_bs() to find both node names and > device names makes sense. > > I would still use a common namespace and forbid using the same name for > a device and a node. That makes sense for libvirt as well (libvirt has every intention of using a different prefix when referring to a device than when referring to a node, so a shared namespace won't hurt libvirt, and makes the most sense for other operations).
On 01/24/2014 07:48 AM, Kevin Wolf wrote: Revisiting this: >> 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. Is this statement still true of 2.0? I'm trying to figure out if it is possible to create a node name not attached to a device, and then later attach it by reference to another device, from either the command line or via blockdev-add. More precisely, I'm trying to figure out if Jeff's patches have to worry about the user ever doing: / file1 base < \ file2 and having to identify the overlay of node-name base, which gets more interesting if a node-name can belong to more than one device. > >> 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? > > Kevin > >
The Friday 30 May 2014 à 13:10:56 (-0600), Eric Blake wrote : > On 01/24/2014 07:48 AM, Kevin Wolf wrote: > > Revisiting this: > > >> 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. > > Is this statement still true of 2.0? I'm trying to figure out if it is > possible to create a node name not attached to a device, and then later > attach it by reference to another device Seems possible. > , from either the command line > or via blockdev-add. More precisely, I'm trying to figure out if Jeff's > patches have to worry about the user ever doing: > > / file1 > base < > \ file2 > > and having to identify the overlay of node-name base, which gets more > interesting if a node-name can belong to more than one device. > I think a bdrv_swap would occur and prevent double parenting. > > > >> 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? > > > > Kevin > > > > > > -- > Eric Blake eblake redhat com +1-919-301-3266 > Libvirt virtualization library http://libvirt.org >
diff --git a/block.c b/block.c index 3e0994b..7726636 100644 --- a/block.c +++ b/block.c @@ -908,15 +908,15 @@ int bdrv_file_open(BlockDriverState **pbs, const char *filename, if (reference) { if (filename || qdict_size(options)) { - error_setg(errp, "Cannot reference an existing block device with " + error_setg(errp, "Cannot reference an existing graph node with " "additional options or a new filename"); return -EINVAL; } QDECREF(options); - bs = bdrv_find(reference); + bs = bdrv_find_node(reference); if (!bs) { - error_setg(errp, "Cannot find block device '%s'", reference); + error_setg(errp, "Cannot find graph node '%s'", reference); return -ENODEV; } bdrv_ref(bs);
Signed-off-by: Benoit Canet <benoit@irqsave.net> --- block.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)