Re: [libvirt] [PATCH v2.1 08/21] Change virDomainVcpuPinDefParseXML to support parsing emulatorpin
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(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/