From: Max Reitz <mreitz@redhat.com>
To: "Benoît Canet" <benoit.canet@irqsave.net>
Cc: Kevin Wolf <kwolf@redhat.com>,
famz@redhat.com, qemu-devel@nongnu.org, armbru@redhat.com,
stefanha@redhat.com
Subject: Re: [Qemu-devel] [PATCH V6 8/8] block: Use graph node name as reference in bdrv_file_open().
Date: Mon, 27 Jan 2014 20:11:59 +0100 [thread overview]
Message-ID: <52E6AF7F.90000@redhat.com> (raw)
In-Reply-To: <20140127143644.GH7415@irqsave.net>
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
next prev parent reply other threads:[~2014-01-27 19:10 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-01-23 20:31 [Qemu-devel] [PATCH V6 0/8] Giving names to graph's BlockDriverState Benoît Canet
2014-01-23 20:31 ` [Qemu-devel] [PATCH V6 1/8] block: Add bs->node_name to hold the name of a bs node of the bs graph Benoît Canet
2014-01-23 20:31 ` [Qemu-devel] [PATCH V6 2/8] block: Allow the user to define "node-name" option both on command line and QMP Benoît Canet
2014-01-24 14:51 ` Benoît Canet
2014-01-24 15:10 ` Kevin Wolf
2014-01-23 20:31 ` [Qemu-devel] [PATCH V6 3/8] qmp: Add QMP query-named-block-nodes to list the named BlockDriverState nodes Benoît Canet
2014-01-23 20:31 ` [Qemu-devel] [PATCH V6 4/8] qmp: Allow to change password on named block driver states Benoît Canet
2014-01-23 20:31 ` [Qemu-devel] [PATCH V6 5/8] block: Create authorizations mechanism for external snapshot and resize Benoît Canet
2014-02-04 0:15 ` Jeff Cody
2014-02-04 10:25 ` Kevin Wolf
2014-02-04 13:03 ` Jeff Cody
2014-02-10 19:39 ` Benoît Canet
2014-02-10 19:45 ` Benoît Canet
2014-02-10 20:18 ` Benoît Canet
2014-01-23 20:31 ` [Qemu-devel] [PATCH V6 6/8] qmp: Allow block_resize to manipulate bs graph nodes Benoît Canet
2014-01-23 20:31 ` [Qemu-devel] [PATCH V6 7/8] qmp: Allow to take external snapshots on bs graphs node Benoît Canet
2014-01-23 20:31 ` [Qemu-devel] [PATCH V6 8/8] block: Use graph node name as reference in bdrv_file_open() Benoît Canet
2014-01-24 13:26 ` Kevin Wolf
2014-01-24 13:37 ` Max Reitz
2014-01-24 14:48 ` Kevin Wolf
2014-01-24 14:54 ` Max Reitz
2014-01-27 14:36 ` Benoît Canet
2014-01-27 19:11 ` Max Reitz [this message]
2014-01-28 0:04 ` Benoît Canet
2014-01-31 20:32 ` Max Reitz
2014-01-31 21:37 ` Benoît Canet
2014-02-03 9:43 ` Kevin Wolf
2014-02-04 13:02 ` Eric Blake
2014-01-24 13:27 ` [Qemu-devel] [PATCH V6 0/8] Giving names to graph's BlockDriverState Kevin Wolf
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=52E6AF7F.90000@redhat.com \
--to=mreitz@redhat.com \
--cc=armbru@redhat.com \
--cc=benoit.canet@irqsave.net \
--cc=famz@redhat.com \
--cc=kwolf@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=stefanha@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.