[libvirt] [PATCH v2.1 08/21] Change virDomainVcpuPinDefParseXML to support parsing emulatorpin

[Daniel P. Berrange]

On Tue, Aug 21, 2012 at 05:18:31PM +0800, Hu Tao wrote:
> ---
>  src/conf/domain_conf.c |   22 +++++++++++++++-------
>  1 file changed, 15 insertions(+), 7 deletions(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index ff27bc7..62ba9de 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -7860,7 +7860,19 @@ cleanup:
>      return ret;
>  }
>  
> -/* Parse the XML definition for a vcpupin */
> +/* Parse the XML definition for a vcpupin or emulatorpin.
> + *
> + * vcpupin has the form of
> + *
> + *   <vcpupin vcpu='0' cpuset='0'/>
> + *
> + * and emulatorpin has the form of
> + *
> + *   <emulatorpin cpuset='0'/>
> + *
> + * A vcpuid of -1 is valid and only valid for emulatorpin. So callers
> + * have to check the returned cpuid for validity.

You didn't modify the existing caller to check this, nor does the
new caller you added in the next patch check this. I'm not really
a fan of this style of API. IMHO, you should pass in a parameter
indicating whether 'vcpu' is allowed in the XML or not and then
keep validation in this method.

>  static virDomainVcpuPinDefPtr
>  virDomainVcpuPinDefParseXML(const xmlNodePtr node,
>                              xmlXPathContextPtr ctxt,
> @@ -7868,7 +7880,7 @@ virDomainVcpuPinDefParseXML(const xmlNodePtr node,
>  {
>      virDomainVcpuPinDefPtr def;
>      xmlNodePtr oldnode = ctxt->node;
> -    unsigned int vcpuid;
> +    unsigned int vcpuid = -1;
>      char *tmp = NULL;
>      int ret;
>  
> @@ -7884,13 +7896,9 @@ virDomainVcpuPinDefParseXML(const xmlNodePtr node,
>          virReportError(VIR_ERR_INTERNAL_ERROR,
>                         "%s", _("vcpu id must be an unsigned integer"));
>          goto error;
> -    } else if (ret == -1) {
> -        virReportError(VIR_ERR_INTERNAL_ERROR,
> -                       "%s", _("can't parse vcpupin node"));
> -        goto error;
>      }
>  
> -    if (vcpuid >= maxvcpus) {
> +    if (vcpuid != -1 && vcpuid >= maxvcpus) {
>          virReportError(VIR_ERR_INTERNAL_ERROR,
>                         "%s", _("vcpu id must be less than maxvcpus"));
>          goto error;



Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|