All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: "Benoît Canet" <benoit@irqsave.net>, qemu-devel@nongnu.org
Cc: kwolf@redhat.com, famz@redhat.com, armbru@redhat.com,
	stefanha@redhat.com
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, 07 Nov 2013 13:23:43 -0700	[thread overview]
Message-ID: <527BF6CF.8070407@redhat.com> (raw)
In-Reply-To: <1383836503-25447-2-git-send-email-benoit@irqsave.net>

[-- Attachment #1: Type: text/plain, Size: 2247 bytes --]

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")?

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.

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

  reply	other threads:[~2013-11-07 20:23 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-11-07 15:01 [Qemu-devel] [PATCH 0/2] Giving names to graph's BlockDriverState Benoît Canet
2013-11-07 15:01 ` [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
2013-11-07 20:23   ` Eric Blake [this message]
2013-11-07 21:09     ` Benoît Canet
2013-11-08  9:51     ` Kevin Wolf
2013-11-07 20:54   ` Jeff Cody
2013-11-07 21:11     ` Benoît Canet
2013-11-08  8:19   ` Fam Zheng
2013-11-07 15:01 ` [Qemu-devel] [PATCH 2/2] block: Allow the user to define "node-name" option Benoît Canet
2013-11-07 20:31   ` Eric Blake
2013-11-08  9:55   ` Kevin Wolf
2013-11-07 20:34 ` [Qemu-devel] [PATCH 0/2] Giving names to graph's BlockDriverState Eric Blake

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=527BF6CF.8070407@redhat.com \
    --to=eblake@redhat.com \
    --cc=armbru@redhat.com \
    --cc=benoit@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.