6:03 p.m.
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(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org