Re: [Qemu-devel] [PATCH 1/2] block: Add bs->node_name to hold the name of a bs node of the bs graph.

["Benoît Canet" <benoit.canet@irqsave.net>]

Le Thursday 07 Nov 2013 à 13:23:43 (-0700), Eric Blake a écrit :
> On 11/07/2013 08:01 AM, Benoît Canet wrote:
> > Add the minimum of code to prepare the followings patches.
> > 
> > If no node_name is provided to bdrv_new the bs->node_name is set to "undefined".
> > This will allow to have some default string to communicate in QMP and HMP.
> > This also make "undefined" a reserved string for bs->node_name.
> > 
> > Signed-off-by: Benoit Canet <benoit@irqsave.net>
> > ---
> 
> > +    /* if node name is given store it in bs and insert bs in the graph bs list
> > +     * note: undefined is a reserved node name
> > +     */
> > +    if (node_name &&
> > +        node_name[0] != '\0' &&
> > +        strcmp(node_name, "undefined")) {
> > +        pstrcpy(bs->node_name, sizeof(bs->node_name), node_name);
> > +        QTAILQ_INSERT_TAIL(&graph_bdrv_states, bs, node_list);
> > +    /* else set the bs node name to undefined for QMP and HMP */
> > +    } else {
> > +        sprintf(bs->node_name, "undefined");
> 
> strcpy() is more efficient than sprintf() with no % in the format string.
> 
> 
> >  
> > +/* This function is to find a node in the bs graph */
> > +BlockDriverState *bdrv_find_node(const char *node_name)
> > +{
> > +    BlockDriverState *bs;
> > +
> 
> Should this function assert that node_name is not "undefined"?
> 
> 
> > +++ b/include/block/block_int.h
> > @@ -297,11 +297,18 @@ struct BlockDriverState {
> >      BlockdevOnError on_read_error, on_write_error;
> >      bool iostatus_enabled;
> >      BlockDeviceIoStatus iostatus;
> > +
> > +    /* the following member give a name to every node on the BlockDriverState
> 
> s/give/gives/
> 
> > +     * graph.
> > +     */
> > +    char node_name[32];
> > +    QTAILQ_ENTRY(BlockDriverState) node_list;
> > +    /* Device name is the name associated with the "drive" the guest see */
> 
> s/see/sees/
> 
> >      char device_name[32];
> > +    QTAILQ_ENTRY(BlockDriverState) device_list;
> 
> Maybe I missed it, but is there code that ensures there are no duplicate
> node names (other than the magic "undefined")?
> 
Your are right I will add some checkings.

> Seems like it's moving in the right direction, although I'm not sure
> it's worth applying this until we know the qapi for working with node
> names makes sense.

I am not seeking for fast commit of these patch, I am only trying to avoid
posting a long series at once.

Best regards

Benoît

> 
> -- 
> Eric Blake   eblake redhat com    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
>