[libvirt] [PATCH 06/10] linkstate: Add parsing code for new XML element

[Eric Blake]

On 08/11/2011 09:27 AM, Peter Krempa wrote:
> ---
>   src/conf/domain_conf.c |   19 +++++++++++++++++++
>   src/conf/domain_conf.h |    1 +
>   2 files changed, 20 insertions(+), 0 deletions(-)

This patch is useful, even without the new API.

> @@ -3077,6 +3081,14 @@ virDomainNetDefParseXML(virCapsPtr caps,
>           }
>       }
>
> +    def->linkstate = VIR_LINK_STATE_DEFAULT;
> +    if (linkstate != NULL) {
> +        if (STREQ(linkstate, "down"))
> +            def->linkstate = VIR_LINK_STATE_DOWN;
> +        else
> +            def->linkstate = VIR_LINK_STATE_UP;

Do we really want to convert all other strings to STATE_UP, or should we 
specifically check for "up" and reject unknown strings?

> @@ -9019,6 +9032,12 @@ virDomainNetDefFormat(virBufferPtr buf,
>           virBufferAddLit(buf,   "</tune>\n");
>       }
>
> +
> +    if (def->linkstate == VIR_LINK_STATE_DOWN)
> +        virBufferAddLit(buf,   "<link state='down'/>\n");
> +    if (def->linkstate == VIR_LINK_STATE_UP)

This could be 'else if', but not a big deal to micro-optimize.

> +        virBufferAddLit(buf,   "<link state='up'/>\n");
> +
>       if (virBandwidthDefFormat(buf, def->bandwidth, "      ")<  0)
>           return -1;
>
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index abf9cbd..4655563 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -427,6 +427,7 @@ struct _virDomainNetDef {
>       char *filter;
>       virNWFilterHashTablePtr filterparams;
>       virBandwidthPtr bandwidth;
> +    unsigned int linkstate;

Why unsigned?  As currently used, you could get away with a bool.  Would 
it be better to introduce a VIR_ENUM for link states, especially if we 
ever think it might be worth adding other link states? (I know that some 
10/100/1000 NIC hardware has a state representing autonegotiation, where 
a remote end has been detected but where the link has not yet settled on 
which speed to use, although I have no idea if that hardware state is 
ever exposed to the userspace by the kernel, let alone something that 
can be emulated).  If you do use VIR_ENUM, then our convention has been 
to use 'int', not 'unsigned int'.

-- 
Eric Blake   eblake at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org