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