[PATCH] Fix parsing of vendor_id

[Ken ICHIKAWA <ichikawa.ken@xxxxxxxxxxxxxx>]

I have a problem about parsing vendor_id of domain XML.
When define vendor_id attribute,
why definition of fallback attribute is needed?
I explain below for example.
(I used virsh edit.)

Current domain xml state is like below,
<domain>
  <cpu>
  </cpu>
</domain>

And I redefine domain xml like below,
<domain>
  <cpu>
    <model vendor_id='aaaabbbbcccc'>core2duo</model>
  </cpu>
</domain>

Then, do dumpxml,
vendor_id is not reflected like below.
<domain>
  <cpu mode='custom' match='exact'>
    <model fallback='allow'>core2duo</model>
  </cpu>
</domain>

I think this is not right behavior. It should be defined like
below.
<domain>
  <cpu mode='custom' match='exact'>
    <model fallback='allow' vendor_id='aaaabbbbcccc'>core2duo</model>
  </cpu>
</domain>

And if I define fallback attribute and vendor_id attribute at the
same time, or define vendor_id attribute after fallback attribute
is defined, vendor_id attribute is reflected normally.
Is it a bug? or is there some reason?

I read past mailing list's thread about the patch adding vendor_id
but I could not find the reason.
https://www.redhat.com/archives/libvir-list/2012-June/thread.html#00917
It seems that patch v1 doesn't need definition of fallback attribute
but v2 needs it.

If it's a bug, please consider to apply this patch.

This patch fixes a problem that vendor_id attribute can not be
defined when fallback attribute is not defined.

Signed-off-by: Ken ICHIKAWA <ichikawa.ken@xxxxxxxxxxxxxx>
---
 src/conf/cpu_conf.c | 42 +++++++++++++++++++++---------------------
 1 file changed, 21 insertions(+), 21 deletions(-)

diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c
index 8cb54a3..6157ed7 100644
--- a/src/conf/cpu_conf.c
+++ b/src/conf/cpu_conf.c
@@ -300,32 +300,32 @@ virCPUDefParseXML(const xmlNodePtr node,
                     goto error;
                 }
             }
+        }

-            if (virXPathBoolean("boolean(./model[1]/@vendor_id)", ctxt)) {
-                char *vendor_id;
-
-                vendor_id = virXPathString("string(./model[1]/@vendor_id)",
-                                           ctxt);
-                if (!vendor_id ||
-                    strlen(vendor_id) != VIR_CPU_VENDOR_ID_LENGTH) {
-                    virReportError(VIR_ERR_XML_ERROR,
-                                   _("vendor_id must be exactly"
-                                     " %d characters long"),
-                                   VIR_CPU_VENDOR_ID_LENGTH);
+        if (virXPathBoolean("boolean(./model[1]/@vendor_id)", ctxt)) {
+            char *vendor_id;
+
+            vendor_id = virXPathString("string(./model[1]/@vendor_id)",
+                                       ctxt);
+            if (!vendor_id ||
+                strlen(vendor_id) != VIR_CPU_VENDOR_ID_LENGTH) {
+                virReportError(VIR_ERR_XML_ERROR,
+                               _("vendor_id must be exactly"
+                                 " %d characters long"),
+                               VIR_CPU_VENDOR_ID_LENGTH);
+                VIR_FREE(vendor_id);
+                goto error;
+            }
+            /* ensure that the string can be passed to qemu*/
+            for (i = 0; i < strlen(vendor_id); i++) {
+                if (vendor_id[i]==',') {
+                    virReportError(VIR_ERR_XML_ERROR, "%s",
+                                   _("vendor id is invalid"));
                     VIR_FREE(vendor_id);
                     goto error;
                 }
-                /* ensure that the string can be passed to qemu*/
-                for (i = 0; i < strlen(vendor_id); i++) {
-                    if (vendor_id[i]==',') {
-                        virReportError(VIR_ERR_XML_ERROR, "%s",
-                                       _("vendor id is invalid"));
-                        VIR_FREE(vendor_id);
-                        goto error;
-                    }
-                }
-                def->vendor_id = vendor_id;
             }
+            def->vendor_id = vendor_id;
         }
     }

-- 
1.7.11.7

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