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

[Daniel Veillard <veillard@xxxxxxxxxx>]

On Tue, Aug 21, 2012 at 05:44:10PM +0100, Daniel P. Berrange wrote:
> 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.
> 

  Done, i reworked that the following way

Daniel

    Change virDomainVcpuPinDefParseXML to support parsing emulatorpin

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 5533355..ee247f6 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -7902,15 +7902,28 @@ 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.
+ */
 static virDomainVcpuPinDefPtr
 virDomainVcpuPinDefParseXML(const xmlNodePtr node,
                             xmlXPathContextPtr ctxt,
-                            int maxvcpus)
+                            int maxvcpus,
+                            int emulator)
 {
     virDomainVcpuPinDefPtr def;
     xmlNodePtr oldnode = ctxt->node;
-    unsigned int vcpuid;
+    int vcpuid;
     char *tmp = NULL;
     int ret;
 
@@ -7921,14 +7934,14 @@ virDomainVcpuPinDefParseXML(const xmlNodePtr node,
 
     ctxt->node = node;
 
-    ret = virXPathUInt("string(./@vcpu)", ctxt, &vcpuid);
-    if (ret == -2) {
+    ret = virXPathInt("string(./@vcpu)", ctxt, &vcpuid);
+    if ((ret == -2) || (vcpuid < -1)) {
         virReportError(VIR_ERR_INTERNAL_ERROR,
-                       "%s", _("vcpu id must be an unsigned integer"));
+                       "%s", _("vcpu id must be an unsigned integer or -1"));
         goto error;
-    } else if (ret == -1) {
+    } else if ((vcpuid == -1) && (emulator == 0)) {
         virReportError(VIR_ERR_INTERNAL_ERROR,
-                       "%s", _("can't parse vcpupin node"));
+                       "%s", _("vcpu id value -1 is not allowed for vcpupin"));
         goto error;
     }
 
@@ -8346,7 +8359,7 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps,
 
     for (i = 0 ; i < n ; i++) {
         virDomainVcpuPinDefPtr vcpupin = NULL;
-        vcpupin = virDomainVcpuPinDefParseXML(nodes[i], ctxt, def->maxvcpus);
+        vcpupin = virDomainVcpuPinDefParseXML(nodes[i], ctxt, def->maxvcpus, 0);
 
         if (!vcpupin)
             goto error;

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel@xxxxxxxxxxxx  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list