[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCHv2 08/13] snapshot: simplify indentation of cpu features



On 10/20/2011 06:35 AM, Peter Krempa wrote:
On 09/29/2011 06:22 PM, Eric Blake wrote:
Auto-indent makes life a bit easier; this patch also drops unused
arguments and fixes a flag name.

* src/conf/cpu_conf.h (virCPUFormatFlags): Fix typo.
(virCPUDefFormat, virCPUDefFormatBuf): Drop unused arguments.
* src/conf/cpu_conf.c (virCPUDefFormat, virCPUDefFormatBuf): Simplify
indentation.
* src/conf/domain_conf.c (virDomainDefFormatInternal): Adjust
caller.
* src/conf/capabilities.c (virCapabilitiesFormatXML): Likewise.
* src/cpu/cpu.c (cpuBaselineXML): Likewise.
* tests/cputest.c (cpuTestCompareXML): Likewise.
---
src/conf/capabilities.c | 8 +++++---
src/conf/cpu_conf.c | 42 +++++++++++++++++-------------------------
src/conf/cpu_conf.h | 9 +++------
src/conf/domain_conf.c | 4 +++-
src/cpu/cpu.c | 2 +-
tests/cputest.c | 2 +-
6 files changed, 30 insertions(+), 37 deletions(-)

diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
index 2f243ae..5f7f768 100644
--- a/src/conf/capabilities.c
+++ b/src/conf/capabilities.c

@@ -681,8 +681,10 @@ virCapabilitiesFormatXML(virCapsPtr caps)
virBufferAddLit(&xml, "</features>\n");
}

- virCPUDefFormatBuf(&xml, caps->host.cpu, " ",
- VIR_CPU_FORMAT_EMBEDED);
+ /* virCPUDefFormatBuf with EMBEDDED uses indent of 2, we want 4 more */
+ virBufferAdjustIndent(&xml, 4);
+ virCPUDefFormatBuf(&xml, caps->host.cpu, VIR_CPU_FORMAT_EMBEDDED);
+ virBufferAdjustIndent(&xml, -4);


Oh well. I don't like this very much, but removing things like this
would ultimately end in having a flat XML output structure and using
indentation adjustment to have correct indentation across the xml, which
is somewhat controversial. Well, it doesn't affect functionality, so
it's not a show-stoping issue.

I don't like how VIR_CPU_FORMAT_EMBED[D]ED was used either. So on review, I think it's better to split this one into multiple functions, specifically catering to each caller, with each embeddable call-point starting at 0 indentation.


Otherwise, this patch works correct, so ACK.

I went ahead and squashed this in before pushing.

diff --git i/src/conf/capabilities.c w/src/conf/capabilities.c
index 5f7f768..40e2976 100644
--- i/src/conf/capabilities.c
+++ w/src/conf/capabilities.c
@@ -681,10 +681,9 @@ virCapabilitiesFormatXML(virCapsPtr caps)
         virBufferAddLit(&xml, "      </features>\n");
     }

-    /* virCPUDefFormatBuf with EMBEDDED uses indent of 2, we want 4 more */
-    virBufferAdjustIndent(&xml, 4);
-    virCPUDefFormatBuf(&xml, caps->host.cpu, VIR_CPU_FORMAT_EMBEDDED);
-    virBufferAdjustIndent(&xml, -4);
+    virBufferAdjustIndent(&xml, 6);
+    virCPUDefFormatBuf(&xml, caps->host.cpu);
+    virBufferAdjustIndent(&xml, -6);

     virBufferAddLit(&xml, "    </cpu>\n");

diff --git i/src/conf/cpu_conf.c w/src/conf/cpu_conf.c
index 49ea392..41e997e 100644
--- i/src/conf/cpu_conf.c
+++ w/src/conf/cpu_conf.c
@@ -309,7 +309,7 @@ virCPUDefFormat(virCPUDefPtr def)
 {
     virBuffer buf = VIR_BUFFER_INITIALIZER;

-    if (virCPUDefFormatBuf(&buf, def, 0) < 0)
+    if (virCPUDefFormatBufFull(&buf, def) < 0)
         goto cleanup;

     if (virBufferError(&buf))
@@ -326,9 +326,41 @@ cleanup:


 int
+virCPUDefFormatBufFull(virBufferPtr buf,
+                       virCPUDefPtr def)
+{
+    if (!def)
+        return 0;
+
+    if (def->type == VIR_CPU_TYPE_GUEST && def->model) {
+        const char *match;
+        if (!(match = virCPUMatchTypeToString(def->match))) {
+            virCPUReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Unexpected CPU match policy %d"), def->match);
+            return -1;
+        }
+
+        virBufferAsprintf(buf, "<cpu match='%s'>\n", match);
+    } else {
+        virBufferAddLit(buf, "<cpu>\n");
+    }
+
+    if (def->arch)
+        virBufferAsprintf(buf, "  <arch>%s</arch>\n", def->arch);
+
+    virBufferAdjustIndent(buf, 2);
+    if (virCPUDefFormatBuf(buf, def) < 0)
+        return -1;
+    virBufferAdjustIndent(buf, -2);
+
+    virBufferAddLit(buf, "</cpu>\n");
+
+    return 0;
+}
+
+int
 virCPUDefFormatBuf(virBufferPtr buf,
-                   virCPUDefPtr def,
-                   unsigned int flags)
+                   virCPUDefPtr def)
 {
     unsigned int i;

@@ -341,33 +373,15 @@ virCPUDefFormatBuf(virBufferPtr buf,
         return -1;
     }

-    if (!(flags & VIR_CPU_FORMAT_EMBEDDED)) {
-        if (def->type == VIR_CPU_TYPE_GUEST && def->model) {
-            const char *match;
-            if (!(match = virCPUMatchTypeToString(def->match))) {
-                virCPUReportError(VIR_ERR_INTERNAL_ERROR,
-                        _("Unexpected CPU match policy %d"), def->match);
-                return -1;
-            }
-
-            virBufferAsprintf(buf, "<cpu match='%s'>\n", match);
-        } else {
-            virBufferAddLit(buf, "<cpu>\n");
-        }
-
-        if (def->arch)
-            virBufferAsprintf(buf, "  <arch>%s</arch>\n", def->arch);
-    }
-
     if (def->model)
-        virBufferAsprintf(buf, "  <model>%s</model>\n", def->model);
+        virBufferAsprintf(buf, "<model>%s</model>\n", def->model);

     if (def->vendor) {
-        virBufferAsprintf(buf, "  <vendor>%s</vendor>\n", def->vendor);
+        virBufferAsprintf(buf, "<vendor>%s</vendor>\n", def->vendor);
     }

     if (def->sockets && def->cores && def->threads) {
-        virBufferAddLit(buf, "  <topology");
+        virBufferAddLit(buf, "<topology");
         virBufferAsprintf(buf, " sockets='%u'", def->sockets);
         virBufferAsprintf(buf, " cores='%u'", def->cores);
         virBufferAsprintf(buf, " threads='%u'", def->threads);
@@ -392,17 +406,14 @@ virCPUDefFormatBuf(virBufferPtr buf,
_("Unexpected CPU feature policy %d"), feature->policy);
                 return -1;
             }
-            virBufferAsprintf(buf, "  <feature policy='%s' name='%s'/>\n",
+            virBufferAsprintf(buf, "<feature policy='%s' name='%s'/>\n",
                               policy, feature->name);
         } else {
-            virBufferAsprintf(buf, "  <feature name='%s'/>\n",
+            virBufferAsprintf(buf, "<feature name='%s'/>\n",
                               feature->name);
         }
     }

-    if (!(flags & VIR_CPU_FORMAT_EMBEDDED))
-        virBufferAddLit(buf, "</cpu>\n");
-
     return 0;
 }

diff --git i/src/conf/cpu_conf.h w/src/conf/cpu_conf.h
index 409bbca..4406cba 100644
--- i/src/conf/cpu_conf.h
+++ w/src/conf/cpu_conf.h
@@ -95,11 +95,6 @@ virCPUDefParseXML(const xmlNodePtr node,
                   xmlXPathContextPtr ctxt,
                   enum virCPUType mode);

-enum virCPUFormatFlags {
- VIR_CPU_FORMAT_EMBEDDED = (1 << 0) /* embed into existing <cpu/> element
-                                          * in host capabilities */
-};
-
 bool
 virCPUDefIsEqual(virCPUDefPtr src,
                  virCPUDefPtr dst);
@@ -109,8 +104,10 @@ virCPUDefFormat(virCPUDefPtr def);

 int
 virCPUDefFormatBuf(virBufferPtr buf,
-                   virCPUDefPtr def,
-                   unsigned int flags);
+                   virCPUDefPtr def);
+int
+virCPUDefFormatBufFull(virBufferPtr buf,
+                       virCPUDefPtr def);

 int
 virCPUDefAddFeature(virCPUDefPtr cpu,
diff --git i/src/conf/domain_conf.c w/src/conf/domain_conf.c
index 314e4fc..304d1a8 100644
--- i/src/conf/domain_conf.c
+++ w/src/conf/domain_conf.c
@@ -10826,7 +10826,7 @@ virDomainDefFormatInternal(virDomainDefPtr def,
     }

     virBufferAdjustIndent(buf, 2);
-    if (virCPUDefFormatBuf(buf, def->cpu, 0) < 0)
+    if (virCPUDefFormatBufFull(buf, def->cpu) < 0)
         goto cleanup;
     virBufferAdjustIndent(buf, -2);



--
Eric Blake   eblake redhat com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]