On Thu, May 15, 2014 at 09:59:01AM -0600, Eric Blake wrote: > On 05/14/2014 09:20 PM, Jeff Cody wrote: > > Currently, node_name is only filled in when done so explicitly by the > > user. If no node_name is specified, then the node name field is not > > populated. > > > > If node_names are automatically generated when not specified, that means > > that all block job operations can be done by reference to the unique > > node_name field. This eliminates ambiguity in filename pathing > > (relative filenames, or file descriptors, symlinks, mounts, etc..) that > > qemu currently needs to deal with. > > > > If a node name is specified, then it will not be automatically > > generated for that BDS entry. > > > > If it is automatically generated, it will be prefaced with "__qemu##", > > followed by 8 characters of a unique number, followed by 8 random > > ASCII characters in the range of 'A-Z'. Some sample generated node-name > > strings: > > __qemu##00000000IAIYNXXR > > __qemu##00000002METXTRBQ > > __qemu##00000001FMBORDWG > > > > The prefix is to aid in identifying it as a qemu-generated name, the > > numeric portion is to guarantee uniqueness in a given qemu session, and > > the random characters are to further avoid any accidental collisions > > with user-specified node-names. > > > > Signed-off-by: Jeff Cody <[email protected]> > > --- > > block.c | 16 +++++++++++++++- > > 1 file changed, 15 insertions(+), 1 deletion(-) > > This patch feels incomplete. We probably also ought to modify > qapi-schema.json to make BlockDeviceInfo's output of 'node-name' by > unconditional (as an output-only struct, changing from optional to > mandatory is a safe change for back-compat API considerations); which > implies further modifying code to get rid of has_node_name arguments in > all places that generate BlockDeviceInfo details.
I think it should be a separate patch (but still in this series). Strictly speaking, this patch doesn't force the argument to be mandatory, the value just happens to always be populated now. > See also my thoughts on patch 5/5 - can this patch be rebased to be LAST > in the series, rather than first, so that it serves as a reliable > witness that everything else related to block operations on node-names > is complete? > How about this becomes the penultimate patch, and the patch to make BlockDeviceInfo's node-name field to be unconditional becomes the last patch? The only thing I don't like about moving this further back in the patch series is it makes the earlier patches untestable; I can't easily test the usage of the node-names for various intermediate BDS because they don't have node-names set. So that means I'll just need to rebase the patches prior to sending. So let me revise my above statement - how about this patch stays at the beginning, which makes development / testing easier, with the patch that modifies BlockDeviceInfo as the final (not including tests) patch?
