[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH v2 2/4] conf: add optional attribte primary to video <model> element



On 12/11/2012 07:14 AM, Guannan Ren wrote:
> If there are multiple video devices
> primary = 'yes' marks this video device as the primary one.
> The rest are secondary video devices. No more than one could be
> mark as primary. If none of them has primary attribute, the first
> one will be the primary by default like what it was.
> The reason of this changing is that for qemu, only one primary video
> device is permitted which can be of any type. For secondary video
> devices, only qxl is allowd. Primary attribute removes the restriction
> that the first have to be the primary one.
> 
> We always put the primary video device into the first position of
> video device structure array after parsing.
> ---
>  src/conf/domain_conf.c | 25 +++++++++++++++++++++++++
>  src/conf/domain_conf.h |  1 +
>  2 files changed, 26 insertions(+)

Incomplete.  I can't approve this without a change to
docs/schemas/domaincommon.rng to parse the new attribute, and to
docs/formatdomain.html.in to document it.

> @@ -7289,6 +7290,11 @@ virDomainVideoDefParseXML(const xmlNodePtr node,
>                  type = virXMLPropString(cur, "type");
>                  vram = virXMLPropString(cur, "vram");
>                  heads = virXMLPropString(cur, "heads");
> +
> +                if ((primary = virXMLPropString(cur, "primary"))!= NULL)

Space before !=.

> @@ -9961,6 +9968,22 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps,
>                                                                 flags);
>          if (!video)
>              goto error;
> +
> +        if (!primaryVideo && video->primary) {
> +            if (def->nvideos != 0)
> +                memmove(def->videos + 1,
> +                        def->videos,
> +                        (sizeof(def->videos[0]) * def->nvideos));
> +
> +            def->videos[0] = video;
> +            def->nvideos++;
> +            primaryVideo = true;

This would be a good candidate to use Laine's new VIR_INSERT_ELEMENT macro.

> +++ b/src/conf/domain_conf.h
> @@ -1120,6 +1120,7 @@ struct _virDomainVideoDef {
>      int type;
>      unsigned int vram;
>      unsigned int heads;
> +    unsigned int primary : 1;

It's probably easier to use 'bool primary' instead of a bitfield here.

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

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]