[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/2] block: Add bs->node_name to hold the name o
From: |
Benoît Canet |
Subject: |
Re: [Qemu-devel] [PATCH 1/2] block: Add bs->node_name to hold the name of a bs node of the bs graph. |
Date: |
Thu, 7 Nov 2013 22:09:48 +0100 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
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 <address@hidden>
> > ---
>
> > + /* 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
>