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@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/