[libvirt] [PATCH 00/14] snapshot: improve dumpxml output
This series fixes 'virsh snapshot-dumpxml' to use nicer formatting. Patch 1 adds some nice helper routines, patches 2-13 are mostly mechanical conversions to use the helpers and pass indentation levels through the entire call chain, and patch 14 adds a test which uncovered a couple minor issues in how formatting was done. Eric Blake (14): snapshot: indent domain xml when nesting, round 1 snapshot: indent domain xml when nesting, round 2 snapshot: indent domain xml when nesting, round 3 snapshot: indent domain xml when nesting, round 4 snapshot: indent domain xml when nesting, round 5 snapshot: indent domain xml when nesting, round 6 snapshot: indent domain xml when nesting, round 7 snapshot: indent domain xml when nesting, round 8 snapshot: indent domain xml when nesting, round 9 snapshot: indent domain xml when nesting, round 10 snapshot: indent domain xml when nesting, round 11 snapshot: indent domain xml when nesting, round 12 snapshot: indent domain xml when nesting, round 13 snapshot: test domainsnapshot indentation .gitignore | 1 + src/conf/capabilities.c | 6 +- src/conf/capabilities.h | 6 +- src/conf/cpu_conf.c | 43 +- src/conf/cpu_conf.h | 7 +- src/conf/domain_conf.c | 778 +++++++++++---------- src/conf/domain_conf.h | 6 +- src/conf/network_conf.c | 8 +- src/conf/nwfilter_conf.c | 16 +- src/conf/nwfilter_params.c | 41 +- src/conf/nwfilter_params.h | 8 +- src/conf/storage_encryption_conf.c | 4 +- src/cpu/cpu.c | 2 +- src/libvirt_private.syms | 3 + src/qemu/qemu_domain.c | 39 +- src/qemu/qemu_driver.c | 2 +- src/qemu/qemu_migration.c | 23 +- src/util/buf.c | 70 ++- src/util/buf.h | 19 +- src/util/network.c | 49 +- src/util/network.h | 9 +- src/util/sysinfo.c | 407 ++++------- src/util/sysinfo.h | 4 +- tests/Makefile.am | 14 +- tests/cputest.c | 2 +- tests/domainsnapshotxml2xmlout/all_parameters.xml | 2 +- tests/domainsnapshotxml2xmlout/disk_snapshot.xml | 102 ++-- tests/domainsnapshotxml2xmlout/full_domain.xml | 52 +- tests/domainsnapshotxml2xmltest.c | 128 ++++ tests/testutils.c | 2 +- tests/virbuftest.c | 32 +- 31 files changed, 1033 insertions(+), 852 deletions(-) create mode 100644 tests/domainsnapshotxml2xmltest.c -- 1.7.4.4
Future patches can take advantage of this to generate nicer XML output with parameterizable indentation. On the side, I had some temporary test failures as I was using these functions in later patches, with output that looked like: Expected [<] Actual [ <] which is pretty hard to figure out. Adding an Offset designation made it much easier to find which particular '<' was at the wrong indentation, to fix the right part of the code. * src/util/buf.h (virBufferIndentAdd, virBufferIndentAddLit) (virBufferIndentEscapeString): New prototypes and macro. * src/libvirt_private.syms (buf.h): Export new functions. * src/util/buf.c (virBufferAdd): Move body... (virBufferIndentAdd): ...to new function. (virBufferIndentEscapeString): New function. * tests/virbuftest.c (testBufIndentation): Test it. * tests/testutils.c (virtTestDifference): Make it easier to diagnose test failures. --- src/libvirt_private.syms | 2 + src/util/buf.c | 70 ++++++++++++++++++++++++++++++++++++--------- src/util/buf.h | 19 ++++++++++-- tests/testutils.c | 2 +- tests/virbuftest.c | 32 ++++++++++++++++++++- 5 files changed, 105 insertions(+), 20 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 8235ea1..1523289 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -28,6 +28,8 @@ virBufferError; virBufferEscapeSexpr; virBufferEscapeString; virBufferFreeAndReset; +virBufferIndentAdd; +virBufferIndentEscapeString; virBufferStrcat; virBufferURIEncodeString; virBufferUse; diff --git a/src/util/buf.c b/src/util/buf.c index 5002486..061d83b 100644 --- a/src/util/buf.c +++ b/src/util/buf.c @@ -78,21 +78,23 @@ virBufferGrow(virBufferPtr buf, unsigned int len) } /** - * virBufferAdd: - * @buf: the buffer to add to - * @str: the string - * @len: the number of bytes to add + * virBufferIndentAdd: + * @buf: the buffer to add to + * @indent: amount of indentation + * @str: the string, or NULL to skip indentation + * @len: the number of bytes to add * - * Add a string range to an XML buffer. if len == -1, the length of - * str is recomputed to the full string. + * Add indentation, then a string range to an XML buffer. if len == -1, the + * length of str is recomputed to the full string. * */ void -virBufferAdd(const virBufferPtr buf, const char *str, int len) +virBufferIndentAdd(const virBufferPtr buf, int indent, + const char *str, int len) { unsigned int needSize; - if ((str == NULL) || (buf == NULL) || (len == 0)) + if (!str || !buf || (len == 0 && indent == 0)) return; if (buf->error) @@ -101,17 +103,34 @@ virBufferAdd(const virBufferPtr buf, const char *str, int len) if (len < 0) len = strlen(str); - needSize = buf->use + len + 2; + needSize = buf->use + indent + len + 2; if (needSize > buf->size && virBufferGrow(buf, needSize - buf->use) < 0) return; - memcpy (&buf->content[buf->use], str, len); - buf->use += len; + memset (&buf->content[buf->use], ' ', indent); + memcpy (&buf->content[buf->use + indent], str, len); + buf->use += indent + len; buf->content[buf->use] = '\0'; } /** + * virBufferAdd: + * @buf: the buffer to add to + * @str: the string + * @len: the number of bytes to add + * + * Add a string range to an XML buffer. if len == -1, the length of + * str is recomputed to the full string. + * + */ +void +virBufferAdd(const virBufferPtr buf, const char *str, int len) +{ + virBufferIndentAdd(buf, 0, str, len); +} + +/** * virBufferAddChar: * @buf: the buffer to add to * @c: the character to add @@ -120,7 +139,7 @@ virBufferAdd(const virBufferPtr buf, const char *str, int len) * */ void -virBufferAddChar (virBufferPtr buf, char c) +virBufferAddChar(virBufferPtr buf, char c) { unsigned int needSize; @@ -290,10 +309,12 @@ virBufferVasprintf(const virBufferPtr buf, const char *format, va_list argptr) * @str: the string argument which need to be escaped * * Do a formatted print with a single string to an XML buffer. The string - * is escaped to avoid generating a not well-formed XML instance. + * is escaped to avoid generating a not well-formed XML instance. If + * @str is NULL, nothing is added (not even the rest of @format). */ void -virBufferEscapeString(const virBufferPtr buf, const char *format, const char *str) +virBufferEscapeString(const virBufferPtr buf, const char *format, + const char *str) { int len; char *escaped, *out; @@ -369,6 +390,27 @@ virBufferEscapeString(const virBufferPtr buf, const char *format, const char *st } /** + * virBufferIndentEscapeString: + * @buf: the buffer to dump + * @indent: amount of indentation + * @format: a printf like format string but with only one %s parameter + * @str: the string argument which need to be escaped, or NULL for no action + * + * Do a formatted print with a single string to an XML buffer, with leading + * indentation. The single %s string is escaped to avoid generating a not + * well-formed XML instance. + */ +void +virBufferIndentEscapeString(const virBufferPtr buf, int indent, + const char *format, const char *str) +{ + if (str) { + virBufferIndentAdd(buf, indent, "", 0); + virBufferEscapeString(buf, format, str); + } +} + +/** * virBufferEscapeSexpr: * @buf: the buffer to dump * @format: a printf like format string but with only one %s parameter diff --git a/src/util/buf.h b/src/util/buf.h index 06d01ba..c5e2874 100644 --- a/src/util/buf.h +++ b/src/util/buf.h @@ -48,11 +48,22 @@ void virBufferVasprintf(const virBufferPtr buf, const char *format, va_list ap) ATTRIBUTE_FMT_PRINTF(2, 0); void virBufferStrcat(const virBufferPtr buf, ...) ATTRIBUTE_SENTINEL; -void virBufferEscapeString(const virBufferPtr buf, const char *format, const char *str); -void virBufferEscapeSexpr(const virBufferPtr buf, const char *format, const char *str); -void virBufferURIEncodeString (const virBufferPtr buf, const char *str); +void virBufferEscapeString(const virBufferPtr buf, const char *format, + const char *str); +void virBufferEscapeSexpr(const virBufferPtr buf, const char *format, + const char *str); +void virBufferURIEncodeString(const virBufferPtr buf, const char *str); # define virBufferAddLit(buf_, literal_string_) \ - virBufferAdd (buf_, "" literal_string_ "", sizeof literal_string_ - 1) + virBufferAdd(buf_, "" literal_string_ "", sizeof literal_string_ - 1) + +void virBufferIndentAdd(const virBufferPtr buf, int indent, + const char *str, int len); +void virBufferIndentEscapeString(const virBufferPtr buf, int indent, + const char *format, const char *str); + +# define virBufferIndentAddLit(buf_, indent_, literal_string_) \ + virBufferIndentAdd(buf_, indent_, "" literal_string_ "", \ + sizeof literal_string_ - 1) #endif /* __VIR_BUFFER_H__ */ diff --git a/tests/testutils.c b/tests/testutils.c index d9582af..b107d3c 100644 --- a/tests/testutils.c +++ b/tests/testutils.c @@ -359,7 +359,7 @@ int virtTestDifference(FILE *stream, } /* Show the trimmed differences */ - fprintf(stream, "\nExpect ["); + fprintf(stream, "\nOffset %d\nExpect [", (int) (expectStart - expect)); if ((expectEnd - expectStart + 1) && fwrite(expectStart, (expectEnd-expectStart+1), 1, stream) != 1) return -1; diff --git a/tests/virbuftest.c b/tests/virbuftest.c index 01db313..0a99e78 100644 --- a/tests/virbuftest.c +++ b/tests/virbuftest.c @@ -20,7 +20,7 @@ struct testInfo { int doEscape; }; -static int testBufInfiniteLoop(const void *data ATTRIBUTE_UNUSED) +static int testBufInfiniteLoop(const void *data) { virBuffer bufinit = VIR_BUFFER_INITIALIZER; virBufferPtr buf = &bufinit; @@ -63,6 +63,35 @@ out: return ret; } +static int testBufIndentation(const void *data ATTRIBUTE_UNUSED) +{ + virBuffer bufinit = VIR_BUFFER_INITIALIZER; + virBufferPtr buf = &bufinit; + const char expected[] = "ab c d &"; + char *result = NULL; + int ret = 0; + + virBufferIndentAdd(buf, 0, "a", -1); + virBufferIndentAdd(buf, 0, "", -1); + virBufferIndentAdd(buf, 0, "", 0); + virBufferIndentAddLit(buf, 0, ""); + virBufferIndentAddLit(buf, 0, "b"); + virBufferIndentAdd(buf, 3, NULL, -1); + virBufferIndentAdd(buf, 2, "c", -1); + virBufferIndentAddLit(buf, 1, ""); + virBufferIndentEscapeString(buf, 1, "%s", "d"); + virBufferIndentEscapeString(buf, 3, "%s", NULL); + virBufferIndentEscapeString(buf, 2, "%s", "&"); + + result = virBufferContentAndReset(buf); + if (!result || STRNEQ(result, expected)) { + TEST_ERROR("Built buffer was wrong: %s", NULLSTR(result)); + ret = -1; + } + VIR_FREE(result); + return ret; +} + static int mymain(void) { @@ -78,6 +107,7 @@ mymain(void) DO_TEST("EscapeString infinite loop", testBufInfiniteLoop, 1); DO_TEST("VSprintf infinite loop", testBufInfiniteLoop, 0); + DO_TEST("Indentation", testBufIndentation, 0); return(ret==0 ? EXIT_SUCCESS : EXIT_FAILURE); } -- 1.7.4.4
On Thu, Sep 22, 2011 at 02:34:55PM -0600, Eric Blake wrote:
Future patches can take advantage of this to generate nicer XML output with parameterizable indentation.
On the side, I had some temporary test failures as I was using these functions in later patches, with output that looked like:
Expected [<] Actual [ <]
which is pretty hard to figure out. Adding an Offset designation made it much easier to find which particular '<' was at the wrong indentation, to fix the right part of the code.
* src/util/buf.h (virBufferIndentAdd, virBufferIndentAddLit) (virBufferIndentEscapeString): New prototypes and macro. * src/libvirt_private.syms (buf.h): Export new functions. * src/util/buf.c (virBufferAdd): Move body... (virBufferIndentAdd): ...to new function. (virBufferIndentEscapeString): New function. * tests/virbuftest.c (testBufIndentation): Test it. * tests/testutils.c (virtTestDifference): Make it easier to diagnose test failures. --- src/libvirt_private.syms | 2 + src/util/buf.c | 70 ++++++++++++++++++++++++++++++++++++--------- src/util/buf.h | 19 ++++++++++-- tests/testutils.c | 2 +- tests/virbuftest.c | 32 ++++++++++++++++++++- 5 files changed, 105 insertions(+), 20 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 8235ea1..1523289 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -28,6 +28,8 @@ virBufferError; virBufferEscapeSexpr; virBufferEscapeString; virBufferFreeAndReset; +virBufferIndentAdd; +virBufferIndentEscapeString; virBufferStrcat; virBufferURIEncodeString; virBufferUse; diff --git a/src/util/buf.c b/src/util/buf.c index 5002486..061d83b 100644 --- a/src/util/buf.c +++ b/src/util/buf.c @@ -78,21 +78,23 @@ virBufferGrow(virBufferPtr buf, unsigned int len) }
/** - * virBufferAdd: - * @buf: the buffer to add to - * @str: the string - * @len: the number of bytes to add + * virBufferIndentAdd: + * @buf: the buffer to add to + * @indent: amount of indentation + * @str: the string, or NULL to skip indentation + * @len: the number of bytes to add * - * Add a string range to an XML buffer. if len == -1, the length of - * str is recomputed to the full string. + * Add indentation, then a string range to an XML buffer. if len == -1, the + * length of str is recomputed to the full string. * */ void -virBufferAdd(const virBufferPtr buf, const char *str, int len) +virBufferIndentAdd(const virBufferPtr buf, int indent, + const char *str, int len) { unsigned int needSize;
- if ((str == NULL) || (buf == NULL) || (len == 0)) + if (!str || !buf || (len == 0 && indent == 0)) return;
if (buf->error) @@ -101,17 +103,34 @@ virBufferAdd(const virBufferPtr buf, const char *str, int len) if (len < 0) len = strlen(str);
- needSize = buf->use + len + 2; + needSize = buf->use + indent + len + 2; if (needSize > buf->size && virBufferGrow(buf, needSize - buf->use) < 0) return;
- memcpy (&buf->content[buf->use], str, len); - buf->use += len; + memset (&buf->content[buf->use], ' ', indent); + memcpy (&buf->content[buf->use + indent], str, len); + buf->use += indent + len; buf->content[buf->use] = '\0'; }
/** + * virBufferAdd: + * @buf: the buffer to add to + * @str: the string + * @len: the number of bytes to add + * + * Add a string range to an XML buffer. if len == -1, the length of + * str is recomputed to the full string. + * + */ +void +virBufferAdd(const virBufferPtr buf, const char *str, int len) +{ + virBufferIndentAdd(buf, 0, str, len); +} + +/** * virBufferAddChar: * @buf: the buffer to add to * @c: the character to add @@ -120,7 +139,7 @@ virBufferAdd(const virBufferPtr buf, const char *str, int len) * */ void -virBufferAddChar (virBufferPtr buf, char c) +virBufferAddChar(virBufferPtr buf, char c) { unsigned int needSize;
@@ -290,10 +309,12 @@ virBufferVasprintf(const virBufferPtr buf, const char *format, va_list argptr) * @str: the string argument which need to be escaped * * Do a formatted print with a single string to an XML buffer. The string - * is escaped to avoid generating a not well-formed XML instance. + * is escaped to avoid generating a not well-formed XML instance. If + * @str is NULL, nothing is added (not even the rest of @format). */ void -virBufferEscapeString(const virBufferPtr buf, const char *format, const char *str) +virBufferEscapeString(const virBufferPtr buf, const char *format, + const char *str) { int len; char *escaped, *out; @@ -369,6 +390,27 @@ virBufferEscapeString(const virBufferPtr buf, const char *format, const char *st }
/** + * virBufferIndentEscapeString: + * @buf: the buffer to dump + * @indent: amount of indentation + * @format: a printf like format string but with only one %s parameter + * @str: the string argument which need to be escaped, or NULL for no action + * + * Do a formatted print with a single string to an XML buffer, with leading + * indentation. The single %s string is escaped to avoid generating a not + * well-formed XML instance. + */ +void +virBufferIndentEscapeString(const virBufferPtr buf, int indent, + const char *format, const char *str) +{ + if (str) { + virBufferIndentAdd(buf, indent, "", 0); + virBufferEscapeString(buf, format, str); + } +} + +/** * virBufferEscapeSexpr: * @buf: the buffer to dump * @format: a printf like format string but with only one %s parameter diff --git a/src/util/buf.h b/src/util/buf.h index 06d01ba..c5e2874 100644 --- a/src/util/buf.h +++ b/src/util/buf.h @@ -48,11 +48,22 @@ void virBufferVasprintf(const virBufferPtr buf, const char *format, va_list ap) ATTRIBUTE_FMT_PRINTF(2, 0); void virBufferStrcat(const virBufferPtr buf, ...) ATTRIBUTE_SENTINEL; -void virBufferEscapeString(const virBufferPtr buf, const char *format, const char *str); -void virBufferEscapeSexpr(const virBufferPtr buf, const char *format, const char *str); -void virBufferURIEncodeString (const virBufferPtr buf, const char *str); +void virBufferEscapeString(const virBufferPtr buf, const char *format, + const char *str); +void virBufferEscapeSexpr(const virBufferPtr buf, const char *format, + const char *str); +void virBufferURIEncodeString(const virBufferPtr buf, const char *str);
# define virBufferAddLit(buf_, literal_string_) \ - virBufferAdd (buf_, "" literal_string_ "", sizeof literal_string_ - 1) + virBufferAdd(buf_, "" literal_string_ "", sizeof literal_string_ - 1) + +void virBufferIndentAdd(const virBufferPtr buf, int indent, + const char *str, int len); +void virBufferIndentEscapeString(const virBufferPtr buf, int indent, + const char *format, const char *str); + +# define virBufferIndentAddLit(buf_, indent_, literal_string_) \ + virBufferIndentAdd(buf_, indent_, "" literal_string_ "", \ + sizeof literal_string_ - 1)
#endif /* __VIR_BUFFER_H__ */ diff --git a/tests/testutils.c b/tests/testutils.c index d9582af..b107d3c 100644 --- a/tests/testutils.c +++ b/tests/testutils.c @@ -359,7 +359,7 @@ int virtTestDifference(FILE *stream, }
/* Show the trimmed differences */ - fprintf(stream, "\nExpect ["); + fprintf(stream, "\nOffset %d\nExpect [", (int) (expectStart - expect)); if ((expectEnd - expectStart + 1) && fwrite(expectStart, (expectEnd-expectStart+1), 1, stream) != 1) return -1; diff --git a/tests/virbuftest.c b/tests/virbuftest.c index 01db313..0a99e78 100644 --- a/tests/virbuftest.c +++ b/tests/virbuftest.c @@ -20,7 +20,7 @@ struct testInfo { int doEscape; };
-static int testBufInfiniteLoop(const void *data ATTRIBUTE_UNUSED) +static int testBufInfiniteLoop(const void *data) { virBuffer bufinit = VIR_BUFFER_INITIALIZER; virBufferPtr buf = &bufinit; @@ -63,6 +63,35 @@ out: return ret; }
+static int testBufIndentation(const void *data ATTRIBUTE_UNUSED) +{ + virBuffer bufinit = VIR_BUFFER_INITIALIZER; + virBufferPtr buf = &bufinit; + const char expected[] = "ab c d &"; + char *result = NULL; + int ret = 0; + + virBufferIndentAdd(buf, 0, "a", -1); + virBufferIndentAdd(buf, 0, "", -1); + virBufferIndentAdd(buf, 0, "", 0); + virBufferIndentAddLit(buf, 0, ""); + virBufferIndentAddLit(buf, 0, "b"); + virBufferIndentAdd(buf, 3, NULL, -1); + virBufferIndentAdd(buf, 2, "c", -1); + virBufferIndentAddLit(buf, 1, ""); + virBufferIndentEscapeString(buf, 1, "%s", "d"); + virBufferIndentEscapeString(buf, 3, "%s", NULL); + virBufferIndentEscapeString(buf, 2, "%s", "&"); + + result = virBufferContentAndReset(buf); + if (!result || STRNEQ(result, expected)) { + TEST_ERROR("Built buffer was wrong: %s", NULLSTR(result)); + ret = -1; + } + VIR_FREE(result); + return ret; +} + static int mymain(void) { @@ -78,6 +107,7 @@ mymain(void)
DO_TEST("EscapeString infinite loop", testBufInfiniteLoop, 1); DO_TEST("VSprintf infinite loop", testBufInfiniteLoop, 0); + DO_TEST("Indentation", testBufIndentation, 0);
return(ret==0 ? EXIT_SUCCESS : EXIT_FAILURE); }
Looks fine to me, ACK Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
On Thu, Sep 22, 2011 at 02:34:55PM -0600, Eric Blake wrote:
Future patches can take advantage of this to generate nicer XML output with parameterizable indentation.
Hmm, is there any way we can work with virBuffer so that we don't need to pass an indent level to every single API call ? eg, can we have something which lets us do virBufferSetIndent(buf, " "); and then it will automagically prepend that indent every time a API call is made following a newline. If we could do this, then pretty much all of your patches would disappear Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 09/27/2011 03:03 AM, Daniel P. Berrange wrote:
On Thu, Sep 22, 2011 at 02:34:55PM -0600, Eric Blake wrote:
Future patches can take advantage of this to generate nicer XML output with parameterizable indentation.
Hmm, is there any way we can work with virBuffer so that we don't need to pass an indent level to every single API call ?
eg, can we have something which lets us do
virBufferSetIndent(buf, " ");
and then it will automagically prepend that indent every time a API call is made following a newline. If we could do this, then pretty much all of your patches would disappear
I think there's still some other aspects in the other patches that may mean that things won't disappear entirely, but I like the idea. I see several potential usage styles: virBufferIndent...(indent, "<parent>\n"); virBufferIndent...(indent + 2, "<child/>\n"); virBufferIndent...(indent, "</parent>\n"); or virBufferAddIndent(indent); virBuffer...("<parent>\n"); virBuffer...(" <child/>\n"); virBuffer...("</parent>\n"); virBufferAddIndent(-indent); or virBufferAddIndent(indent); virBuffer...("<parent>\n"); virBufferAddIndent(2); virBuffer...("<child/>\n"); virBufferAddIndent(-2); virBuffer...("</parent>\n"); virBufferAddIndent(-indent); and so on. And for all three styles above, there's code modifications, whether to pass the indentation, or to prep and cleanup indentation before the unchanged original text. But I definitely like your idea, since there are fewer points to prep and cleanup instead of changing every single format line. Having virBuffer track an indent to automagically add on any new API call if the previous API left a trailing newline makes sense, so I'll roll out a v2 with that incorporated. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On Tue, Sep 27, 2011 at 06:50:23AM -0600, Eric Blake wrote:
On 09/27/2011 03:03 AM, Daniel P. Berrange wrote:
On Thu, Sep 22, 2011 at 02:34:55PM -0600, Eric Blake wrote:
Future patches can take advantage of this to generate nicer XML output with parameterizable indentation.
Hmm, is there any way we can work with virBuffer so that we don't need to pass an indent level to every single API call ?
eg, can we have something which lets us do
virBufferSetIndent(buf, " ");
and then it will automagically prepend that indent every time a API call is made following a newline. If we could do this, then pretty much all of your patches would disappear
I think there's still some other aspects in the other patches that may mean that things won't disappear entirely, but I like the idea. I see several potential usage styles:
virBufferIndent...(indent, "<parent>\n"); virBufferIndent...(indent + 2, "<child/>\n"); virBufferIndent...(indent, "</parent>\n");
or
virBufferAddIndent(indent); virBuffer...("<parent>\n"); virBuffer...(" <child/>\n"); virBuffer...("</parent>\n"); virBufferAddIndent(-indent);
or
virBufferAddIndent(indent); virBuffer...("<parent>\n"); virBufferAddIndent(2); virBuffer...("<child/>\n"); virBufferAddIndent(-2); virBuffer...("</parent>\n"); virBufferAddIndent(-indent);
I was thinking of the middle option here. I think it is visually helpful to still see the indentation in our API calls thus: virBuffer...("<parent>\n"); virBuffer...(" <child/>\n"); virBuffer...("</parent>\n"); IMHO we should just focus on addressing the use case of embedding one XML tree inside another independantly generated XML tree. There are a couple of places where we need todo this: - Generating the state XML for /var/run/libvirt/qemu/$GUEST.xml - Embedding storage encryption data - Embedding domain XML in snapshot XML If we really want to be able to simplify indentation for all our XML building internal APIs, then arguably we should stop using virBuffer completely, and create a simple XML DOM like API for constructing documents. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
<domainsnapshot> is the first public instance of <domain> being used as a sub-element, although we have two other private uses (runtime state, and migration cookie). Although indentation has no effect on XML parsing, using it makes the output more consistent. The overall process of adding indentation will be rather mechanical, but breaking it into steps will help review. This step adds the framework to request the indentation. * src/conf/domain_conf.h (virDomainDefFormatInternal): New prototype. * src/conf/domain_conf.c (virDomainDefFormatInternal): Add parameter, and export. (virDomainDefFormat, virDomainObjFormat): Update callers. * src/libvirt_private.syms (domain_conf.h): Add new export. * src/qemu/qemu_migration.c (qemuMigrationCookieXMLFormat): Use new function. (qemuMigrationCookieXMLFormatStr): Update caller. --- src/conf/domain_conf.c | 24 +++++++++++++++--------- src/conf/domain_conf.h | 4 ++++ src/libvirt_private.syms | 1 + src/qemu/qemu_migration.c | 23 ++++++++++++----------- 4 files changed, 32 insertions(+), 20 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 7463d7c..3e3be3c 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10450,14 +10450,14 @@ verify(((VIR_DOMAIN_XML_INTERNAL_STATUS | /* This internal version can accept VIR_DOMAIN_XML_INTERNAL_*, * whereas the public version cannot. Also, it appends to an existing - * buffer, rather than flattening to string. Return -1 on failure. */ -static int + * buffer, rather than flattening to string, as well as allowing + * additional indentation. Return -1 on failure. */ +int virDomainDefFormatInternal(virDomainDefPtr def, + int indent, unsigned int flags, virBufferPtr buf) { - /* XXX Also need to take an indentation parameter - either int or - * string prefix, so that snapshot xml gets uniform indentation. */ unsigned char *uuid; char uuidstr[VIR_UUID_STRING_BUFLEN]; const char *type = NULL; @@ -10477,13 +10477,16 @@ virDomainDefFormatInternal(virDomainDefPtr def, if (def->id == -1) flags |= VIR_DOMAIN_XML_INACTIVE; - virBufferAsprintf(buf, "<domain type='%s'", type); + virBufferAsprintf(buf, "%*s<domain type='%s'", indent, "", type); if (!(flags & VIR_DOMAIN_XML_INACTIVE)) virBufferAsprintf(buf, " id='%d'", def->id); if (def->namespaceData && def->ns.href) virBufferAsprintf(buf, " %s", (def->ns.href)()); virBufferAddLit(buf, ">\n"); + indent += 2; + /* XXX Fix indentation of the body */ + virBufferEscapeString(buf, " <name>%s</name>\n", def->name); uuid = def->uuid; @@ -10895,7 +10898,10 @@ virDomainDefFormatInternal(virDomainDefPtr def, goto cleanup; } - virBufferAddLit(buf, "</domain>\n"); + /* XXX Fix indentation of body prior to here */ + indent -= 2; + + virBufferIndentAddLit(buf, indent, "</domain>\n"); if (virBufferError(buf)) goto no_memory; @@ -10915,7 +10921,7 @@ virDomainDefFormat(virDomainDefPtr def, unsigned int flags) virBuffer buf = VIR_BUFFER_INITIALIZER; virCheckFlags(DUMPXML_FLAGS, NULL); - if (virDomainDefFormatInternal(def, flags, &buf) < 0) + if (virDomainDefFormatInternal(def, 0, flags, &buf) < 0) return NULL; return virBufferContentAndReset(&buf); @@ -10947,7 +10953,7 @@ static char *virDomainObjFormat(virCapsPtr caps, ((caps->privateDataXMLFormat)(&buf, obj->privateData)) < 0) goto error; - if (virDomainDefFormatInternal(obj->def, flags, &buf) < 0) + if (virDomainDefFormatInternal(obj->def, 2, flags, &buf) < 0) goto error; virBufferAddLit(&buf, "</domstatus>\n"); @@ -11963,7 +11969,7 @@ char *virDomainSnapshotDefFormat(char *domain_uuid, virBufferAddLit(&buf, " </disks>\n"); } if (def->dom) { - virDomainDefFormatInternal(def->dom, flags, &buf); + virDomainDefFormatInternal(def->dom, 2, flags, &buf); } else { virBufferAddLit(&buf, " <domain>\n"); virBufferAsprintf(&buf, " <uuid>%s</uuid>\n", domain_uuid); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 371f270..7e3c06c 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1643,6 +1643,10 @@ int virDomainDefAddImplicitControllers(virDomainDefPtr def); char *virDomainDefFormat(virDomainDefPtr def, unsigned int flags); +int virDomainDefFormatInternal(virDomainDefPtr def, + int indent, + unsigned int flags, + virBufferPtr buf); int virDomainCpuSetParse(const char **str, char sep, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 1523289..f4064c0 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -265,6 +265,7 @@ virDomainDefCheckABIStability; virDomainDefClearDeviceAliases; virDomainDefClearPCIAddresses; virDomainDefFormat; +virDomainDefFormatInternal; virDomainDefFree; virDomainDefParseFile; virDomainDefParseNode; diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 0a5a13d..a131c5c 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -378,12 +378,12 @@ static void qemuMigrationCookieGraphicsXMLFormat(virBufferPtr buf, } -static void qemuMigrationCookieXMLFormat(virBufferPtr buf, - qemuMigrationCookiePtr mig) +static int +qemuMigrationCookieXMLFormat(virBufferPtr buf, + qemuMigrationCookiePtr mig) { char uuidstr[VIR_UUID_STRING_BUFLEN]; char hostuuidstr[VIR_UUID_STRING_BUFLEN]; - char *domXML; int i; virUUIDFormat(mig->uuid, uuidstr); @@ -415,15 +415,15 @@ static void qemuMigrationCookieXMLFormat(virBufferPtr buf, } if ((mig->flags & QEMU_MIGRATION_COOKIE_PERSISTENT) && - mig->persistent) { - domXML = virDomainDefFormat(mig->persistent, - VIR_DOMAIN_XML_INACTIVE | - VIR_DOMAIN_XML_SECURE); - virBufferAdd(buf, domXML, -1); - VIR_FREE(domXML); - } + mig->persistent && + virDomainDefFormatInternal(mig->persistent, 2, + VIR_DOMAIN_XML_INACTIVE | + VIR_DOMAIN_XML_SECURE, + buf) < 0) + return -1; virBufferAddLit(buf, "</qemu-migration>\n"); + return 0; } @@ -431,7 +431,8 @@ static char *qemuMigrationCookieXMLFormatStr(qemuMigrationCookiePtr mig) { virBuffer buf = VIR_BUFFER_INITIALIZER; - qemuMigrationCookieXMLFormat(&buf, mig); + if (qemuMigrationCookieXMLFormat(&buf, mig) < 0) + return NULL; if (virBufferError(&buf)) { virReportOOMError(); -- 1.7.4.4
On Thu, Sep 22, 2011 at 02:34:56PM -0600, Eric Blake wrote:
<domainsnapshot> is the first public instance of <domain> being used as a sub-element, although we have two other private uses (runtime state, and migration cookie). Although indentation has no effect on XML parsing, using it makes the output more consistent.
The overall process of adding indentation will be rather mechanical, but breaking it into steps will help review. This step adds the framework to request the indentation.
* src/conf/domain_conf.h (virDomainDefFormatInternal): New prototype. * src/conf/domain_conf.c (virDomainDefFormatInternal): Add parameter, and export. (virDomainDefFormat, virDomainObjFormat): Update callers. * src/libvirt_private.syms (domain_conf.h): Add new export. * src/qemu/qemu_migration.c (qemuMigrationCookieXMLFormat): Use new function. (qemuMigrationCookieXMLFormatStr): Update caller. --- src/conf/domain_conf.c | 24 +++++++++++++++--------- src/conf/domain_conf.h | 4 ++++ src/libvirt_private.syms | 1 + src/qemu/qemu_migration.c | 23 ++++++++++++----------- 4 files changed, 32 insertions(+), 20 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 7463d7c..3e3be3c 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10450,14 +10450,14 @@ verify(((VIR_DOMAIN_XML_INTERNAL_STATUS |
/* This internal version can accept VIR_DOMAIN_XML_INTERNAL_*, * whereas the public version cannot. Also, it appends to an existing - * buffer, rather than flattening to string. Return -1 on failure. */ -static int + * buffer, rather than flattening to string, as well as allowing + * additional indentation. Return -1 on failure. */ +int virDomainDefFormatInternal(virDomainDefPtr def, + int indent, unsigned int flags, virBufferPtr buf) { - /* XXX Also need to take an indentation parameter - either int or - * string prefix, so that snapshot xml gets uniform indentation. */ unsigned char *uuid; char uuidstr[VIR_UUID_STRING_BUFLEN]; const char *type = NULL; @@ -10477,13 +10477,16 @@ virDomainDefFormatInternal(virDomainDefPtr def, if (def->id == -1) flags |= VIR_DOMAIN_XML_INACTIVE;
- virBufferAsprintf(buf, "<domain type='%s'", type); + virBufferAsprintf(buf, "%*s<domain type='%s'", indent, "", type);
Hum I have never seen that formatting command used before. I would rather add a virBufferIndentAsprintf() instead to be honnest and avoid this, that's clearer from my POV.
if (!(flags & VIR_DOMAIN_XML_INACTIVE)) virBufferAsprintf(buf, " id='%d'", def->id); if (def->namespaceData && def->ns.href) virBufferAsprintf(buf, " %s", (def->ns.href)()); virBufferAddLit(buf, ">\n");
+ indent += 2; + /* XXX Fix indentation of the body */ + virBufferEscapeString(buf, " <name>%s</name>\n", def->name);
uuid = def->uuid; @@ -10895,7 +10898,10 @@ virDomainDefFormatInternal(virDomainDefPtr def, goto cleanup; }
- virBufferAddLit(buf, "</domain>\n"); + /* XXX Fix indentation of body prior to here */ + indent -= 2; + + virBufferIndentAddLit(buf, indent, "</domain>\n");
if (virBufferError(buf)) goto no_memory; @@ -10915,7 +10921,7 @@ virDomainDefFormat(virDomainDefPtr def, unsigned int flags) virBuffer buf = VIR_BUFFER_INITIALIZER;
virCheckFlags(DUMPXML_FLAGS, NULL); - if (virDomainDefFormatInternal(def, flags, &buf) < 0) + if (virDomainDefFormatInternal(def, 0, flags, &buf) < 0) return NULL;
return virBufferContentAndReset(&buf); @@ -10947,7 +10953,7 @@ static char *virDomainObjFormat(virCapsPtr caps, ((caps->privateDataXMLFormat)(&buf, obj->privateData)) < 0) goto error;
- if (virDomainDefFormatInternal(obj->def, flags, &buf) < 0) + if (virDomainDefFormatInternal(obj->def, 2, flags, &buf) < 0) goto error;
virBufferAddLit(&buf, "</domstatus>\n"); @@ -11963,7 +11969,7 @@ char *virDomainSnapshotDefFormat(char *domain_uuid, virBufferAddLit(&buf, " </disks>\n"); } if (def->dom) { - virDomainDefFormatInternal(def->dom, flags, &buf); + virDomainDefFormatInternal(def->dom, 2, flags, &buf); } else { virBufferAddLit(&buf, " <domain>\n"); virBufferAsprintf(&buf, " <uuid>%s</uuid>\n", domain_uuid); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 371f270..7e3c06c 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1643,6 +1643,10 @@ int virDomainDefAddImplicitControllers(virDomainDefPtr def);
char *virDomainDefFormat(virDomainDefPtr def, unsigned int flags); +int virDomainDefFormatInternal(virDomainDefPtr def, + int indent, + unsigned int flags, + virBufferPtr buf);
int virDomainCpuSetParse(const char **str, char sep, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 1523289..f4064c0 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -265,6 +265,7 @@ virDomainDefCheckABIStability; virDomainDefClearDeviceAliases; virDomainDefClearPCIAddresses; virDomainDefFormat; +virDomainDefFormatInternal; virDomainDefFree; virDomainDefParseFile; virDomainDefParseNode; diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 0a5a13d..a131c5c 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -378,12 +378,12 @@ static void qemuMigrationCookieGraphicsXMLFormat(virBufferPtr buf, }
-static void qemuMigrationCookieXMLFormat(virBufferPtr buf, - qemuMigrationCookiePtr mig) +static int +qemuMigrationCookieXMLFormat(virBufferPtr buf, + qemuMigrationCookiePtr mig) { char uuidstr[VIR_UUID_STRING_BUFLEN]; char hostuuidstr[VIR_UUID_STRING_BUFLEN]; - char *domXML; int i;
virUUIDFormat(mig->uuid, uuidstr); @@ -415,15 +415,15 @@ static void qemuMigrationCookieXMLFormat(virBufferPtr buf, }
if ((mig->flags & QEMU_MIGRATION_COOKIE_PERSISTENT) && - mig->persistent) { - domXML = virDomainDefFormat(mig->persistent, - VIR_DOMAIN_XML_INACTIVE | - VIR_DOMAIN_XML_SECURE); - virBufferAdd(buf, domXML, -1); - VIR_FREE(domXML); - } + mig->persistent && + virDomainDefFormatInternal(mig->persistent, 2, + VIR_DOMAIN_XML_INACTIVE | + VIR_DOMAIN_XML_SECURE, + buf) < 0) + return -1;
virBufferAddLit(buf, "</qemu-migration>\n"); + return 0; }
@@ -431,7 +431,8 @@ static char *qemuMigrationCookieXMLFormatStr(qemuMigrationCookiePtr mig) { virBuffer buf = VIR_BUFFER_INITIALIZER;
- qemuMigrationCookieXMLFormat(&buf, mig); + if (qemuMigrationCookieXMLFormat(&buf, mig) < 0) + return NULL;
if (virBufferError(&buf)) { virReportOOMError();
That looks correct, I think pushing this would be fine but a separate patch implementing and using virBufferIndentAsprintf() would be a nice improvement, ACK, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
On 09/27/2011 01:20 AM, Daniel Veillard wrote:
@@ -10477,13 +10477,16 @@ virDomainDefFormatInternal(virDomainDefPtr def, if (def->id == -1) flags |= VIR_DOMAIN_XML_INACTIVE;
- virBufferAsprintf(buf, "<domain type='%s'", type); + virBufferAsprintf(buf, "%*s<domain type='%s'", indent, "", type);
Hum I have never seen that formatting command used before. I would rather add a virBufferIndentAsprintf() instead to be honnest and avoid this, that's clearer from my POV.
I had to add virBufferIndentEscapeString, since that could only take one %s in the format, but left virBufferAsprintf up to the user to exploit full formatting power. But now that you mention it, yes, virBufferIndentAsprintf would make things easier to read. I'll incorporate that into my v2.
That looks correct, I think pushing this would be fine but a separate patch implementing and using virBufferIndentAsprintf() would be a nice improvement,
Rather, I'll wait to push until I have a v2; Dan's suggestion for folding more indent smarts into virBuffer will also impact me. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On Tue, Sep 27, 2011 at 06:42:22AM -0600, Eric Blake wrote:
On 09/27/2011 01:20 AM, Daniel Veillard wrote:
@@ -10477,13 +10477,16 @@ virDomainDefFormatInternal(virDomainDefPtr def, if (def->id == -1) flags |= VIR_DOMAIN_XML_INACTIVE;
- virBufferAsprintf(buf, "<domain type='%s'", type); + virBufferAsprintf(buf, "%*s<domain type='%s'", indent, "", type);
Hum I have never seen that formatting command used before. I would rather add a virBufferIndentAsprintf() instead to be honnest and avoid this, that's clearer from my POV.
I had to add virBufferIndentEscapeString, since that could only take one %s in the format, but left virBufferAsprintf up to the user to exploit full formatting power. But now that you mention it, yes, virBufferIndentAsprintf would make things easier to read. I'll incorporate that into my v2.
Okay, thanks ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
Pretty mechanical. Also includes some long line breaks, as well as relying on the fact that virBufferIndentEscapeString(buf, indent, format, NULL) is a no-op. I've marked all the sub-elements that need indentation fixups in later patches. * src/conf/domain_conf.c (virDomainDefFormatInternal): Indent all parts of <domain> body generated here. --- src/conf/domain_conf.c | 259 ++++++++++++++++++++++++------------------------ 1 files changed, 130 insertions(+), 129 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 3e3be3c..64bc82d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10485,65 +10485,65 @@ virDomainDefFormatInternal(virDomainDefPtr def, virBufferAddLit(buf, ">\n"); indent += 2; - /* XXX Fix indentation of the body */ - virBufferEscapeString(buf, " <name>%s</name>\n", def->name); + virBufferIndentEscapeString(buf, indent, "<name>%s</name>\n", def->name); uuid = def->uuid; virUUIDFormat(uuid, uuidstr); - virBufferAsprintf(buf, " <uuid>%s</uuid>\n", uuidstr); + virBufferAsprintf(buf, "%*s<uuid>%s</uuid>\n", indent, "", uuidstr); - if (def->description) - virBufferEscapeString(buf, " <description>%s</description>\n", - def->description); + virBufferIndentEscapeString(buf, indent, + "<description>%s</description>\n", + def->description); - virBufferAsprintf(buf, " <memory>%lu</memory>\n", def->mem.max_balloon); - virBufferAsprintf(buf, " <currentMemory>%lu</currentMemory>\n", - def->mem.cur_balloon); + virBufferAsprintf(buf, "%*s<memory>%lu</memory>\n", + indent, "", def->mem.max_balloon); + virBufferAsprintf(buf, "%*s<currentMemory>%lu</currentMemory>\n", + indent, "", def->mem.cur_balloon); /* add blkiotune only if there are any */ if (def->blkio.weight) { - virBufferAsprintf(buf, " <blkiotune>\n"); - virBufferAsprintf(buf, " <weight>%u</weight>\n", - def->blkio.weight); - virBufferAsprintf(buf, " </blkiotune>\n"); + virBufferIndentAddLit(buf, indent, "<blkiotune>\n"); + virBufferAsprintf(buf, "%*s<weight>%u</weight>\n", + indent + 2, "", def->blkio.weight); + virBufferIndentAddLit(buf, indent, "</blkiotune>\n"); } /* add memtune only if there are any */ if (def->mem.hard_limit || def->mem.soft_limit || def->mem.min_guarantee || def->mem.swap_hard_limit) - virBufferAsprintf(buf, " <memtune>\n"); + virBufferIndentAddLit(buf, indent, "<memtune>\n"); if (def->mem.hard_limit) { - virBufferAsprintf(buf, " <hard_limit>%lu</hard_limit>\n", - def->mem.hard_limit); + virBufferAsprintf(buf, "%*s<hard_limit>%lu</hard_limit>\n", + indent + 2, "", def->mem.hard_limit); } if (def->mem.soft_limit) { - virBufferAsprintf(buf, " <soft_limit>%lu</soft_limit>\n", - def->mem.soft_limit); + virBufferAsprintf(buf, "%*s<soft_limit>%lu</soft_limit>\n", + indent + 2, "", def->mem.soft_limit); } if (def->mem.min_guarantee) { - virBufferAsprintf(buf, " <min_guarantee>%lu</min_guarantee>\n", - def->mem.min_guarantee); + virBufferAsprintf(buf, "%*s<min_guarantee>%lu</min_guarantee>\n", + indent + 2, "", def->mem.min_guarantee); } if (def->mem.swap_hard_limit) { - virBufferAsprintf(buf, " <swap_hard_limit>%lu</swap_hard_limit>\n", - def->mem.swap_hard_limit); + virBufferAsprintf(buf, "%*s<swap_hard_limit>%lu</swap_hard_limit>\n", + indent + 2, "", def->mem.swap_hard_limit); } if (def->mem.hard_limit || def->mem.soft_limit || def->mem.min_guarantee || def->mem.swap_hard_limit) - virBufferAsprintf(buf, " </memtune>\n"); + virBufferIndentAddLit(buf, indent, "</memtune>\n"); if (def->mem.hugepage_backed) { - virBufferAddLit(buf, " <memoryBacking>\n"); - virBufferAddLit(buf, " <hugepages/>\n"); - virBufferAddLit(buf, " </memoryBacking>\n"); + virBufferIndentAddLit(buf, indent, "<memoryBacking>\n"); + virBufferIndentAddLit(buf, indent, " <hugepages/>\n"); + virBufferIndentAddLit(buf, indent, "</memoryBacking>\n"); } for (n = 0 ; n < def->cpumasklen ; n++) if (def->cpumask[n] != 1) allones = 0; - virBufferAddLit(buf, " <vcpu"); + virBufferIndentAddLit(buf, indent, "<vcpu"); if (!allones) { char *cpumask = NULL; if ((cpumask = @@ -10558,22 +10558,22 @@ virDomainDefFormatInternal(virDomainDefPtr def, if (def->cputune.shares || def->cputune.vcpupin || def->cputune.period || def->cputune.quota) - virBufferAddLit(buf, " <cputune>\n"); + virBufferIndentAddLit(buf, indent, "<cputune>\n"); if (def->cputune.shares) - virBufferAsprintf(buf, " <shares>%lu</shares>\n", - def->cputune.shares); + virBufferAsprintf(buf, "%*s<shares>%lu</shares>\n", + indent + 2, "", def->cputune.shares); if (def->cputune.period) - virBufferAsprintf(buf, " <period>%llu</period>\n", - def->cputune.period); + virBufferAsprintf(buf, "%*s<period>%llu</period>\n", + indent + 2, "", def->cputune.period); if (def->cputune.quota) - virBufferAsprintf(buf, " <quota>%lld</quota>\n", - def->cputune.quota); + virBufferAsprintf(buf, "%*s<quota>%lld</quota>\n", + indent + 2, "", def->cputune.quota); if (def->cputune.vcpupin) { int i; for (i = 0; i < def->cputune.nvcpupin; i++) { - virBufferAsprintf(buf, " <vcpupin vcpu='%u' ", - def->cputune.vcpupin[i]->vcpuid); + virBufferAsprintf(buf, "%*s<vcpupin vcpu='%u' ", + indent + 2, "", def->cputune.vcpupin[i]->vcpuid); char *cpumask = NULL; cpumask = virDomainCpuSetFormat(def->cputune.vcpupin[i]->cpumask, @@ -10592,43 +10592,43 @@ virDomainDefFormatInternal(virDomainDefPtr def, if (def->cputune.shares || def->cputune.vcpupin || def->cputune.period || def->cputune.quota) - virBufferAddLit(buf, " </cputune>\n"); - - if (def->numatune.memory.nodemask) - virBufferAddLit(buf, " <numatune>\n"); + virBufferIndentAddLit(buf, indent, "</cputune>\n"); if (def->numatune.memory.nodemask) { char *nodemask = NULL; + + virBufferIndentAddLit(buf, indent, "<numatune>\n"); nodemask = virDomainCpuSetFormat(def->numatune.memory.nodemask, VIR_DOMAIN_CPUMASK_LEN); if (nodemask == NULL) { - virDomainReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("failed to format nodeset for NUMA memory tuning")); + virDomainReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to format nodeset for NUMA memory tuning")); goto cleanup; } - virBufferAsprintf(buf, " <memory mode='%s' nodeset='%s'/>\n", - virDomainNumatuneMemModeTypeToString(def->numatune.memory.mode), + virBufferAsprintf(buf, "%*s<memory mode='%s' nodeset='%s'/>\n", + indent + 2, "", + virDomainNumatuneMemModeTypeToString(def->numatune. + memory.mode), nodemask); VIR_FREE(nodemask); + virBufferIndentAddLit(buf, indent, "</numatune>\n"); } - if (def->numatune.memory.nodemask) - virBufferAddLit(buf, " </numatune>\n"); - if (def->sysinfo) - virDomainSysinfoDefFormat(buf, def->sysinfo); + virDomainSysinfoDefFormat(buf, def->sysinfo); /* XXX indent */ if (def->os.bootloader) { - virBufferEscapeString(buf, " <bootloader>%s</bootloader>\n", - def->os.bootloader); - if (def->os.bootloaderArgs) - virBufferEscapeString(buf, " <bootloader_args>%s</bootloader_args>\n", - def->os.bootloaderArgs); + virBufferIndentEscapeString(buf, indent, + "<bootloader>%s</bootloader>\n", + def->os.bootloader); + virBufferIndentEscapeString(buf, indent, + "<bootloader_args>%s</bootloader_args>\n", + def->os.bootloaderArgs); } - virBufferAddLit(buf, " <os>\n"); - virBufferAddLit(buf, " <type"); + virBufferIndentAddLit(buf, indent, "<os>\n"); + virBufferIndentAddLit(buf, indent + 2, "<type"); if (def->os.arch) virBufferAsprintf(buf, " arch='%s'", def->os.arch); if (def->os.machine) @@ -10644,24 +10644,18 @@ virDomainDefFormatInternal(virDomainDefPtr def, else virBufferAsprintf(buf, ">%s</type>\n", def->os.type); - if (def->os.init) - virBufferEscapeString(buf, " <init>%s</init>\n", - def->os.init); - if (def->os.loader) - virBufferEscapeString(buf, " <loader>%s</loader>\n", - def->os.loader); - if (def->os.kernel) - virBufferEscapeString(buf, " <kernel>%s</kernel>\n", - def->os.kernel); - if (def->os.initrd) - virBufferEscapeString(buf, " <initrd>%s</initrd>\n", - def->os.initrd); - if (def->os.cmdline) - virBufferEscapeString(buf, " <cmdline>%s</cmdline>\n", - def->os.cmdline); - if (def->os.root) - virBufferEscapeString(buf, " <root>%s</root>\n", - def->os.root); + virBufferIndentEscapeString(buf, indent + 2, "<init>%s</init>\n", + def->os.init); + virBufferIndentEscapeString(buf, indent + 2, "<loader>%s</loader>\n", + def->os.loader); + virBufferIndentEscapeString(buf, indent + 2, "<kernel>%s</kernel>\n", + def->os.kernel); + virBufferIndentEscapeString(buf, indent + 2, "<initrd>%s</initrd>\n", + def->os.initrd); + virBufferIndentEscapeString(buf, indent + 2, "<cmdline>%s</cmdline>\n", + def->os.cmdline); + virBufferIndentEscapeString(buf, indent + 2, "<root>%s</root>\n", + def->os.root); if (!def->os.bootloader) { for (n = 0 ; n < def->os.nBootDevs ; n++) { @@ -10673,21 +10667,24 @@ virDomainDefFormatInternal(virDomainDefPtr def, def->os.bootDevs[n]); goto cleanup; } - virBufferAsprintf(buf, " <boot dev='%s'/>\n", boottype); + virBufferAsprintf(buf, "%*s<boot dev='%s'/>\n", + indent + 2, "", boottype); } if (def->os.bootmenu != VIR_DOMAIN_BOOT_MENU_DEFAULT) { const char *enabled = (def->os.bootmenu == VIR_DOMAIN_BOOT_MENU_ENABLED ? "yes" : "no"); - virBufferAsprintf(buf, " <bootmenu enable='%s'/>\n", enabled); + virBufferAsprintf(buf, "%*s<bootmenu enable='%s'/>\n", + indent + 2, "", enabled); } if (def->os.bios.useserial) { const char *useserial = (def->os.bios.useserial == VIR_DOMAIN_BIOS_USESERIAL_YES ? "yes" : "no"); - virBufferAsprintf(buf, " <bios useserial='%s'/>\n", useserial); + virBufferAsprintf(buf, "%*s<bios useserial='%s'/>\n", + indent + 2, "", useserial); } } @@ -10700,14 +10697,15 @@ virDomainDefFormatInternal(virDomainDefPtr def, _("unexpected smbios mode %d"), def->os.smbios_mode); goto cleanup; } - virBufferAsprintf(buf, " <smbios mode='%s'/>\n", mode); + virBufferAsprintf(buf, "%*s<smbios mode='%s'/>\n", + indent + 2, "", mode); } - virBufferAddLit(buf, " </os>\n"); + virBufferIndentAddLit(buf, indent, "</os>\n"); if (def->features) { int i; - virBufferAddLit(buf, " <features>\n"); + virBufferIndentAddLit(buf, indent, "<features>\n"); for (i = 0 ; i < VIR_DOMAIN_FEATURE_LAST ; i++) { if (def->features & (1 << i)) { const char *name = virDomainFeatureTypeToString(i); @@ -10716,20 +10714,21 @@ virDomainDefFormatInternal(virDomainDefPtr def, _("unexpected feature %d"), i); goto cleanup; } - virBufferAsprintf(buf, " <%s/>\n", name); + virBufferAsprintf(buf, "%*s<%s/>\n", indent + 2, "", name); } } - virBufferAddLit(buf, " </features>\n"); + virBufferIndentAddLit(buf, indent, "</features>\n"); } - if (virCPUDefFormatBuf(buf, def->cpu, " ", 0) < 0) + if (virCPUDefFormatBuf(buf, def->cpu, " ", 0) < 0) /* XXX indent */ goto cleanup; - virBufferAsprintf(buf, " <clock offset='%s'", + virBufferAsprintf(buf, "%*s<clock offset='%s'", indent, "", virDomainClockOffsetTypeToString(def->clock.offset)); switch (def->clock.offset) { case VIR_DOMAIN_CLOCK_OFFSET_VARIABLE: - virBufferAsprintf(buf, " adjustment='%lld'", def->clock.data.adjustment); + virBufferAsprintf(buf, " adjustment='%lld'", + def->clock.data.adjustment); break; case VIR_DOMAIN_CLOCK_OFFSET_TIMEZONE: virBufferEscapeString(buf, " timezone='%s'", def->clock.data.timezone); @@ -10740,67 +10739,67 @@ virDomainDefFormatInternal(virDomainDefPtr def, } else { virBufferAddLit(buf, ">\n"); for (n = 0; n < def->clock.ntimers; n++) { - if (virDomainTimerDefFormat(buf, def->clock.timers[n]) < 0) + if (virDomainTimerDefFormat(buf, def->clock.timers[n]) < 0) /* XXX indent */ goto cleanup; } - virBufferAddLit(buf, " </clock>\n"); + virBufferIndentAddLit(buf, indent, "</clock>\n"); } if (virDomainLifecycleDefFormat(buf, def->onPoweroff, "on_poweroff", - virDomainLifecycleTypeToString) < 0) + virDomainLifecycleTypeToString) < 0) /* XXX indent */ goto cleanup; if (virDomainLifecycleDefFormat(buf, def->onReboot, "on_reboot", - virDomainLifecycleTypeToString) < 0) + virDomainLifecycleTypeToString) < 0) /* XXX indent */ goto cleanup; if (virDomainLifecycleDefFormat(buf, def->onCrash, "on_crash", - virDomainLifecycleCrashTypeToString) < 0) + virDomainLifecycleCrashTypeToString) < 0) /* XXX indent */ goto cleanup; - virBufferAddLit(buf, " <devices>\n"); + virBufferIndentAddLit(buf, indent, "<devices>\n"); + indent += 2; - if (def->emulator) - virBufferEscapeString(buf, " <emulator>%s</emulator>\n", - def->emulator); + virBufferIndentEscapeString(buf, indent, "<emulator>%s</emulator>\n", + def->emulator); for (n = 0 ; n < def->ndisks ; n++) - if (virDomainDiskDefFormat(buf, def->disks[n], flags) < 0) + if (virDomainDiskDefFormat(buf, def->disks[n], flags) < 0) /* XXX indent */ goto cleanup; for (n = 0 ; n < def->ncontrollers ; n++) - if (virDomainControllerDefFormat(buf, def->controllers[n], flags) < 0) + if (virDomainControllerDefFormat(buf, def->controllers[n], flags) < 0) /* XXX indent */ goto cleanup; for (n = 0 ; n < def->nleases ; n++) - if (virDomainLeaseDefFormat(buf, def->leases[n]) < 0) + if (virDomainLeaseDefFormat(buf, def->leases[n]) < 0) /* XXX indent */ goto cleanup; for (n = 0 ; n < def->nfss ; n++) - if (virDomainFSDefFormat(buf, def->fss[n], flags) < 0) + if (virDomainFSDefFormat(buf, def->fss[n], flags) < 0) /* XXX indent */ goto cleanup; for (n = 0 ; n < def->nnets ; n++) - if (virDomainNetDefFormat(buf, def->nets[n], flags) < 0) + if (virDomainNetDefFormat(buf, def->nets[n], flags) < 0) /* XXX indent */ goto cleanup; for (n = 0 ; n < def->nsmartcards ; n++) - if (virDomainSmartcardDefFormat(buf, def->smartcards[n], flags) < 0) + if (virDomainSmartcardDefFormat(buf, def->smartcards[n], flags) < 0) /* XXX indent */ goto cleanup; for (n = 0 ; n < def->nserials ; n++) - if (virDomainChrDefFormat(buf, def->serials[n], flags) < 0) + if (virDomainChrDefFormat(buf, def->serials[n], flags) < 0) /* XXX indent */ goto cleanup; for (n = 0 ; n < def->nparallels ; n++) - if (virDomainChrDefFormat(buf, def->parallels[n], flags) < 0) + if (virDomainChrDefFormat(buf, def->parallels[n], flags) < 0) /* XXX indent */ goto cleanup; /* If there's a PV console that's preferred.. */ if (def->console) { - if (virDomainChrDefFormat(buf, def->console, flags) < 0) + if (virDomainChrDefFormat(buf, def->console, flags) < 0) /* XXX indent */ goto cleanup; } else if (def->nserials != 0) { /* ..else for legacy compat duplicate the first serial device as a @@ -10808,17 +10807,17 @@ virDomainDefFormatInternal(virDomainDefPtr def, virDomainChrDef console; memcpy(&console, def->serials[0], sizeof(console)); console.deviceType = VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE; - if (virDomainChrDefFormat(buf, &console, flags) < 0) + if (virDomainChrDefFormat(buf, &console, flags) < 0) /* XXX indent */ goto cleanup; } for (n = 0 ; n < def->nchannels ; n++) - if (virDomainChrDefFormat(buf, def->channels[n], flags) < 0) + if (virDomainChrDefFormat(buf, def->channels[n], flags) < 0) /* XXX indent */ goto cleanup; for (n = 0 ; n < def->ninputs ; n++) if (def->inputs[n]->bus == VIR_DOMAIN_INPUT_BUS_USB && - virDomainInputDefFormat(buf, def->inputs[n], flags) < 0) + virDomainInputDefFormat(buf, def->inputs[n], flags) < 0) /* XXX indent */ goto cleanup; if (def->ngraphics > 0) { @@ -10830,41 +10829,42 @@ virDomainDefFormatInternal(virDomainDefPtr def, { .alias = NULL }, }; - if (virDomainInputDefFormat(buf, &autoInput, flags) < 0) + if (virDomainInputDefFormat(buf, &autoInput, flags) < 0) /* XXX indent */ goto cleanup; for (n = 0 ; n < def->ngraphics ; n++) - if (virDomainGraphicsDefFormat(buf, def->graphics[n], flags) < 0) + if (virDomainGraphicsDefFormat(buf, def->graphics[n], flags) < 0) /* XXX indent */ goto cleanup; } for (n = 0 ; n < def->nsounds ; n++) - if (virDomainSoundDefFormat(buf, def->sounds[n], flags) < 0) + if (virDomainSoundDefFormat(buf, def->sounds[n], flags) < 0) /* XXX indent */ goto cleanup; for (n = 0 ; n < def->nvideos ; n++) - if (virDomainVideoDefFormat(buf, def->videos[n], flags) < 0) + if (virDomainVideoDefFormat(buf, def->videos[n], flags) < 0) /* XXX indent */ goto cleanup; for (n = 0 ; n < def->nhostdevs ; n++) - if (virDomainHostdevDefFormat(buf, def->hostdevs[n], flags) < 0) + if (virDomainHostdevDefFormat(buf, def->hostdevs[n], flags) < 0) /* XXX indent */ goto cleanup; for (n = 0 ; n < def->nredirdevs ; n++) - if (virDomainRedirdevDefFormat(buf, def->redirdevs[n], flags) < 0) + if (virDomainRedirdevDefFormat(buf, def->redirdevs[n], flags) < 0) /* XXX indent */ goto cleanup; for (n = 0 ; n < def->nhubs ; n++) - if (virDomainHubDefFormat(buf, def->hubs[n], flags) < 0) + if (virDomainHubDefFormat(buf, def->hubs[n], flags) < 0) /* XXX indent */ goto cleanup; if (def->watchdog) - virDomainWatchdogDefFormat (buf, def->watchdog, flags); + virDomainWatchdogDefFormat (buf, def->watchdog, flags); /* XXX indent */ if (def->memballoon) - virDomainMemballoonDefFormat (buf, def->memballoon, flags); + virDomainMemballoonDefFormat (buf, def->memballoon, flags); /* XXX indent */ - virBufferAddLit(buf, " </devices>\n"); + indent -= 2; + virBufferIndentAddLit(buf, indent, "</devices>\n"); if (def->seclabel.model) { const char *sectype = virDomainSeclabelTypeToString(def->seclabel.type); @@ -10876,29 +10876,30 @@ virDomainDefFormatInternal(virDomainDefPtr def, (flags & VIR_DOMAIN_XML_INACTIVE)) { /* This is the default for inactive xml, so nothing to output. */ } else { - virBufferAsprintf(buf, " <seclabel type='%s' model='%s' relabel='%s'>\n", + virBufferAsprintf(buf, "%*s<seclabel type='%s' model='%s' " + "relabel='%s'>\n", indent, "", sectype, def->seclabel.model, def->seclabel.norelabel ? "no" : "yes"); - if (def->seclabel.label) - virBufferEscapeString(buf, " <label>%s</label>\n", - def->seclabel.label); - if (!def->seclabel.norelabel && def->seclabel.imagelabel) - virBufferEscapeString(buf, " <imagelabel>%s</imagelabel>\n", - def->seclabel.imagelabel); - if (def->seclabel.baselabel && - (def->seclabel.type == VIR_DOMAIN_SECLABEL_DYNAMIC)) - virBufferEscapeString(buf, " <baselabel>%s</baselabel>\n", - def->seclabel.baselabel); - virBufferAddLit(buf, " </seclabel>\n"); + virBufferIndentEscapeString(buf, indent + 2, + "<label>%s</label>\n", + def->seclabel.label); + if (!def->seclabel.norelabel) + virBufferIndentEscapeString(buf, indent + 2, + "<imagelabel>%s</imagelabel>\n", + def->seclabel.imagelabel); + if (def->seclabel.type == VIR_DOMAIN_SECLABEL_DYNAMIC) + virBufferIndentEscapeString(buf, indent + 2, + "<baselabel>%s</baselabel>\n", + def->seclabel.baselabel); + virBufferIndentAddLit(buf, indent, "</seclabel>\n"); } } if (def->namespaceData && def->ns.format) { - if ((def->ns.format)(buf, def->namespaceData) < 0) + if ((def->ns.format)(buf, def->namespaceData) < 0) /* XXX indent */ goto cleanup; } - /* XXX Fix indentation of body prior to here */ indent -= 2; virBufferIndentAddLit(buf, indent, "</domain>\n"); -- 1.7.4.4
On Thu, Sep 22, 2011 at 02:34:57PM -0600, Eric Blake wrote:
Pretty mechanical. Also includes some long line breaks, as well as relying on the fact that virBufferIndentEscapeString(buf, indent, format, NULL) is a no-op. I've marked all the sub-elements that need indentation fixups in later patches.
* src/conf/domain_conf.c (virDomainDefFormatInternal): Indent all parts of <domain> body generated here. --- src/conf/domain_conf.c | 259 ++++++++++++++++++++++++------------------------ 1 files changed, 130 insertions(+), 129 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 3e3be3c..64bc82d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10485,65 +10485,65 @@ virDomainDefFormatInternal(virDomainDefPtr def, virBufferAddLit(buf, ">\n");
indent += 2; - /* XXX Fix indentation of the body */
- virBufferEscapeString(buf, " <name>%s</name>\n", def->name); + virBufferIndentEscapeString(buf, indent, "<name>%s</name>\n", def->name);
uuid = def->uuid; virUUIDFormat(uuid, uuidstr); - virBufferAsprintf(buf, " <uuid>%s</uuid>\n", uuidstr); + virBufferAsprintf(buf, "%*s<uuid>%s</uuid>\n", indent, "", uuidstr);
- if (def->description) - virBufferEscapeString(buf, " <description>%s</description>\n", - def->description); + virBufferIndentEscapeString(buf, indent, + "<description>%s</description>\n", + def->description);
- virBufferAsprintf(buf, " <memory>%lu</memory>\n", def->mem.max_balloon); - virBufferAsprintf(buf, " <currentMemory>%lu</currentMemory>\n", - def->mem.cur_balloon); + virBufferAsprintf(buf, "%*s<memory>%lu</memory>\n", + indent, "", def->mem.max_balloon); + virBufferAsprintf(buf, "%*s<currentMemory>%lu</currentMemory>\n", + indent, "", def->mem.cur_balloon);
yeah what I was afraid of when looking at 2/14 I think virBufferIndentAsprintf is better. One only has to concentrate on reviewing the XML content, not any of the formatting. I assume the indent string "" here is used only to make this a no-op untill all fits in place, I would rather see virBufferIndentAsprintf(buf, 0, "<currentMemory>%lu</currentMemory>\n", def->mem.cur_balloon); than the above.
/* add blkiotune only if there are any */ if (def->blkio.weight) { - virBufferAsprintf(buf, " <blkiotune>\n"); - virBufferAsprintf(buf, " <weight>%u</weight>\n", - def->blkio.weight); - virBufferAsprintf(buf, " </blkiotune>\n"); + virBufferIndentAddLit(buf, indent, "<blkiotune>\n"); + virBufferAsprintf(buf, "%*s<weight>%u</weight>\n", + indent + 2, "", def->blkio.weight); + virBufferIndentAddLit(buf, indent, "</blkiotune>\n"); }
/* add memtune only if there are any */ if (def->mem.hard_limit || def->mem.soft_limit || def->mem.min_guarantee || def->mem.swap_hard_limit) - virBufferAsprintf(buf, " <memtune>\n"); + virBufferIndentAddLit(buf, indent, "<memtune>\n"); if (def->mem.hard_limit) { - virBufferAsprintf(buf, " <hard_limit>%lu</hard_limit>\n", - def->mem.hard_limit); + virBufferAsprintf(buf, "%*s<hard_limit>%lu</hard_limit>\n", + indent + 2, "", def->mem.hard_limit); } if (def->mem.soft_limit) { - virBufferAsprintf(buf, " <soft_limit>%lu</soft_limit>\n", - def->mem.soft_limit); + virBufferAsprintf(buf, "%*s<soft_limit>%lu</soft_limit>\n", + indent + 2, "", def->mem.soft_limit); } if (def->mem.min_guarantee) { - virBufferAsprintf(buf, " <min_guarantee>%lu</min_guarantee>\n", - def->mem.min_guarantee); + virBufferAsprintf(buf, "%*s<min_guarantee>%lu</min_guarantee>\n", + indent + 2, "", def->mem.min_guarantee); } if (def->mem.swap_hard_limit) { - virBufferAsprintf(buf, " <swap_hard_limit>%lu</swap_hard_limit>\n", - def->mem.swap_hard_limit); + virBufferAsprintf(buf, "%*s<swap_hard_limit>%lu</swap_hard_limit>\n", + indent + 2, "", def->mem.swap_hard_limit); } if (def->mem.hard_limit || def->mem.soft_limit || def->mem.min_guarantee || def->mem.swap_hard_limit) - virBufferAsprintf(buf, " </memtune>\n"); + virBufferIndentAddLit(buf, indent, "</memtune>\n");
if (def->mem.hugepage_backed) { - virBufferAddLit(buf, " <memoryBacking>\n"); - virBufferAddLit(buf, " <hugepages/>\n"); - virBufferAddLit(buf, " </memoryBacking>\n"); + virBufferIndentAddLit(buf, indent, "<memoryBacking>\n"); + virBufferIndentAddLit(buf, indent, " <hugepages/>\n"); + virBufferIndentAddLit(buf, indent, "</memoryBacking>\n"); }
for (n = 0 ; n < def->cpumasklen ; n++) if (def->cpumask[n] != 1) allones = 0;
- virBufferAddLit(buf, " <vcpu"); + virBufferIndentAddLit(buf, indent, "<vcpu"); if (!allones) { char *cpumask = NULL; if ((cpumask = @@ -10558,22 +10558,22 @@ virDomainDefFormatInternal(virDomainDefPtr def,
if (def->cputune.shares || def->cputune.vcpupin || def->cputune.period || def->cputune.quota) - virBufferAddLit(buf, " <cputune>\n"); + virBufferIndentAddLit(buf, indent, "<cputune>\n");
if (def->cputune.shares) - virBufferAsprintf(buf, " <shares>%lu</shares>\n", - def->cputune.shares); + virBufferAsprintf(buf, "%*s<shares>%lu</shares>\n", + indent + 2, "", def->cputune.shares); if (def->cputune.period) - virBufferAsprintf(buf, " <period>%llu</period>\n", - def->cputune.period); + virBufferAsprintf(buf, "%*s<period>%llu</period>\n", + indent + 2, "", def->cputune.period); if (def->cputune.quota) - virBufferAsprintf(buf, " <quota>%lld</quota>\n", - def->cputune.quota); + virBufferAsprintf(buf, "%*s<quota>%lld</quota>\n", + indent + 2, "", def->cputune.quota); if (def->cputune.vcpupin) { int i; for (i = 0; i < def->cputune.nvcpupin; i++) { - virBufferAsprintf(buf, " <vcpupin vcpu='%u' ", - def->cputune.vcpupin[i]->vcpuid); + virBufferAsprintf(buf, "%*s<vcpupin vcpu='%u' ", + indent + 2, "", def->cputune.vcpupin[i]->vcpuid);
char *cpumask = NULL; cpumask = virDomainCpuSetFormat(def->cputune.vcpupin[i]->cpumask, @@ -10592,43 +10592,43 @@ virDomainDefFormatInternal(virDomainDefPtr def,
if (def->cputune.shares || def->cputune.vcpupin || def->cputune.period || def->cputune.quota) - virBufferAddLit(buf, " </cputune>\n"); - - if (def->numatune.memory.nodemask) - virBufferAddLit(buf, " <numatune>\n"); + virBufferIndentAddLit(buf, indent, "</cputune>\n");
if (def->numatune.memory.nodemask) { char *nodemask = NULL; + + virBufferIndentAddLit(buf, indent, "<numatune>\n"); nodemask = virDomainCpuSetFormat(def->numatune.memory.nodemask, VIR_DOMAIN_CPUMASK_LEN); if (nodemask == NULL) { - virDomainReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("failed to format nodeset for NUMA memory tuning")); + virDomainReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to format nodeset for NUMA memory tuning")); goto cleanup; }
- virBufferAsprintf(buf, " <memory mode='%s' nodeset='%s'/>\n", - virDomainNumatuneMemModeTypeToString(def->numatune.memory.mode), + virBufferAsprintf(buf, "%*s<memory mode='%s' nodeset='%s'/>\n", + indent + 2, "", + virDomainNumatuneMemModeTypeToString(def->numatune. + memory.mode), nodemask); VIR_FREE(nodemask); + virBufferIndentAddLit(buf, indent, "</numatune>\n"); }
- if (def->numatune.memory.nodemask) - virBufferAddLit(buf, " </numatune>\n"); - if (def->sysinfo) - virDomainSysinfoDefFormat(buf, def->sysinfo); + virDomainSysinfoDefFormat(buf, def->sysinfo); /* XXX indent */
if (def->os.bootloader) { - virBufferEscapeString(buf, " <bootloader>%s</bootloader>\n", - def->os.bootloader); - if (def->os.bootloaderArgs) - virBufferEscapeString(buf, " <bootloader_args>%s</bootloader_args>\n", - def->os.bootloaderArgs); + virBufferIndentEscapeString(buf, indent, + "<bootloader>%s</bootloader>\n", + def->os.bootloader); + virBufferIndentEscapeString(buf, indent, + "<bootloader_args>%s</bootloader_args>\n", + def->os.bootloaderArgs); } - virBufferAddLit(buf, " <os>\n");
- virBufferAddLit(buf, " <type"); + virBufferIndentAddLit(buf, indent, "<os>\n"); + virBufferIndentAddLit(buf, indent + 2, "<type"); if (def->os.arch) virBufferAsprintf(buf, " arch='%s'", def->os.arch); if (def->os.machine) @@ -10644,24 +10644,18 @@ virDomainDefFormatInternal(virDomainDefPtr def, else virBufferAsprintf(buf, ">%s</type>\n", def->os.type);
- if (def->os.init) - virBufferEscapeString(buf, " <init>%s</init>\n", - def->os.init); - if (def->os.loader) - virBufferEscapeString(buf, " <loader>%s</loader>\n", - def->os.loader); - if (def->os.kernel) - virBufferEscapeString(buf, " <kernel>%s</kernel>\n", - def->os.kernel); - if (def->os.initrd) - virBufferEscapeString(buf, " <initrd>%s</initrd>\n", - def->os.initrd); - if (def->os.cmdline) - virBufferEscapeString(buf, " <cmdline>%s</cmdline>\n", - def->os.cmdline); - if (def->os.root) - virBufferEscapeString(buf, " <root>%s</root>\n", - def->os.root); + virBufferIndentEscapeString(buf, indent + 2, "<init>%s</init>\n", + def->os.init); + virBufferIndentEscapeString(buf, indent + 2, "<loader>%s</loader>\n", + def->os.loader); + virBufferIndentEscapeString(buf, indent + 2, "<kernel>%s</kernel>\n", + def->os.kernel); + virBufferIndentEscapeString(buf, indent + 2, "<initrd>%s</initrd>\n", + def->os.initrd); + virBufferIndentEscapeString(buf, indent + 2, "<cmdline>%s</cmdline>\n", + def->os.cmdline); + virBufferIndentEscapeString(buf, indent + 2, "<root>%s</root>\n", + def->os.root);
if (!def->os.bootloader) { for (n = 0 ; n < def->os.nBootDevs ; n++) { @@ -10673,21 +10667,24 @@ virDomainDefFormatInternal(virDomainDefPtr def, def->os.bootDevs[n]); goto cleanup; } - virBufferAsprintf(buf, " <boot dev='%s'/>\n", boottype); + virBufferAsprintf(buf, "%*s<boot dev='%s'/>\n", + indent + 2, "", boottype); }
if (def->os.bootmenu != VIR_DOMAIN_BOOT_MENU_DEFAULT) { const char *enabled = (def->os.bootmenu == VIR_DOMAIN_BOOT_MENU_ENABLED ? "yes" : "no"); - virBufferAsprintf(buf, " <bootmenu enable='%s'/>\n", enabled); + virBufferAsprintf(buf, "%*s<bootmenu enable='%s'/>\n", + indent + 2, "", enabled); }
if (def->os.bios.useserial) { const char *useserial = (def->os.bios.useserial == VIR_DOMAIN_BIOS_USESERIAL_YES ? "yes" : "no"); - virBufferAsprintf(buf, " <bios useserial='%s'/>\n", useserial); + virBufferAsprintf(buf, "%*s<bios useserial='%s'/>\n", + indent + 2, "", useserial); } }
@@ -10700,14 +10697,15 @@ virDomainDefFormatInternal(virDomainDefPtr def, _("unexpected smbios mode %d"), def->os.smbios_mode); goto cleanup; } - virBufferAsprintf(buf, " <smbios mode='%s'/>\n", mode); + virBufferAsprintf(buf, "%*s<smbios mode='%s'/>\n", + indent + 2, "", mode); }
- virBufferAddLit(buf, " </os>\n"); + virBufferIndentAddLit(buf, indent, "</os>\n");
if (def->features) { int i; - virBufferAddLit(buf, " <features>\n"); + virBufferIndentAddLit(buf, indent, "<features>\n"); for (i = 0 ; i < VIR_DOMAIN_FEATURE_LAST ; i++) { if (def->features & (1 << i)) { const char *name = virDomainFeatureTypeToString(i); @@ -10716,20 +10714,21 @@ virDomainDefFormatInternal(virDomainDefPtr def, _("unexpected feature %d"), i); goto cleanup; } - virBufferAsprintf(buf, " <%s/>\n", name); + virBufferAsprintf(buf, "%*s<%s/>\n", indent + 2, "", name); } } - virBufferAddLit(buf, " </features>\n"); + virBufferIndentAddLit(buf, indent, "</features>\n"); }
- if (virCPUDefFormatBuf(buf, def->cpu, " ", 0) < 0) + if (virCPUDefFormatBuf(buf, def->cpu, " ", 0) < 0) /* XXX indent */ goto cleanup;
- virBufferAsprintf(buf, " <clock offset='%s'", + virBufferAsprintf(buf, "%*s<clock offset='%s'", indent, "", virDomainClockOffsetTypeToString(def->clock.offset)); switch (def->clock.offset) { case VIR_DOMAIN_CLOCK_OFFSET_VARIABLE: - virBufferAsprintf(buf, " adjustment='%lld'", def->clock.data.adjustment); + virBufferAsprintf(buf, " adjustment='%lld'", + def->clock.data.adjustment); break; case VIR_DOMAIN_CLOCK_OFFSET_TIMEZONE: virBufferEscapeString(buf, " timezone='%s'", def->clock.data.timezone); @@ -10740,67 +10739,67 @@ virDomainDefFormatInternal(virDomainDefPtr def, } else { virBufferAddLit(buf, ">\n"); for (n = 0; n < def->clock.ntimers; n++) { - if (virDomainTimerDefFormat(buf, def->clock.timers[n]) < 0) + if (virDomainTimerDefFormat(buf, def->clock.timers[n]) < 0) /* XXX indent */ goto cleanup; } - virBufferAddLit(buf, " </clock>\n"); + virBufferIndentAddLit(buf, indent, "</clock>\n"); }
if (virDomainLifecycleDefFormat(buf, def->onPoweroff, "on_poweroff", - virDomainLifecycleTypeToString) < 0) + virDomainLifecycleTypeToString) < 0) /* XXX indent */ goto cleanup; if (virDomainLifecycleDefFormat(buf, def->onReboot, "on_reboot", - virDomainLifecycleTypeToString) < 0) + virDomainLifecycleTypeToString) < 0) /* XXX indent */ goto cleanup; if (virDomainLifecycleDefFormat(buf, def->onCrash, "on_crash", - virDomainLifecycleCrashTypeToString) < 0) + virDomainLifecycleCrashTypeToString) < 0) /* XXX indent */ goto cleanup;
- virBufferAddLit(buf, " <devices>\n"); + virBufferIndentAddLit(buf, indent, "<devices>\n"); + indent += 2;
- if (def->emulator) - virBufferEscapeString(buf, " <emulator>%s</emulator>\n", - def->emulator); + virBufferIndentEscapeString(buf, indent, "<emulator>%s</emulator>\n", + def->emulator);
for (n = 0 ; n < def->ndisks ; n++) - if (virDomainDiskDefFormat(buf, def->disks[n], flags) < 0) + if (virDomainDiskDefFormat(buf, def->disks[n], flags) < 0) /* XXX indent */ goto cleanup;
for (n = 0 ; n < def->ncontrollers ; n++) - if (virDomainControllerDefFormat(buf, def->controllers[n], flags) < 0) + if (virDomainControllerDefFormat(buf, def->controllers[n], flags) < 0) /* XXX indent */ goto cleanup;
for (n = 0 ; n < def->nleases ; n++) - if (virDomainLeaseDefFormat(buf, def->leases[n]) < 0) + if (virDomainLeaseDefFormat(buf, def->leases[n]) < 0) /* XXX indent */ goto cleanup;
for (n = 0 ; n < def->nfss ; n++) - if (virDomainFSDefFormat(buf, def->fss[n], flags) < 0) + if (virDomainFSDefFormat(buf, def->fss[n], flags) < 0) /* XXX indent */ goto cleanup;
for (n = 0 ; n < def->nnets ; n++) - if (virDomainNetDefFormat(buf, def->nets[n], flags) < 0) + if (virDomainNetDefFormat(buf, def->nets[n], flags) < 0) /* XXX indent */ goto cleanup;
for (n = 0 ; n < def->nsmartcards ; n++) - if (virDomainSmartcardDefFormat(buf, def->smartcards[n], flags) < 0) + if (virDomainSmartcardDefFormat(buf, def->smartcards[n], flags) < 0) /* XXX indent */ goto cleanup;
for (n = 0 ; n < def->nserials ; n++) - if (virDomainChrDefFormat(buf, def->serials[n], flags) < 0) + if (virDomainChrDefFormat(buf, def->serials[n], flags) < 0) /* XXX indent */ goto cleanup;
for (n = 0 ; n < def->nparallels ; n++) - if (virDomainChrDefFormat(buf, def->parallels[n], flags) < 0) + if (virDomainChrDefFormat(buf, def->parallels[n], flags) < 0) /* XXX indent */ goto cleanup;
/* If there's a PV console that's preferred.. */ if (def->console) { - if (virDomainChrDefFormat(buf, def->console, flags) < 0) + if (virDomainChrDefFormat(buf, def->console, flags) < 0) /* XXX indent */ goto cleanup; } else if (def->nserials != 0) { /* ..else for legacy compat duplicate the first serial device as a @@ -10808,17 +10807,17 @@ virDomainDefFormatInternal(virDomainDefPtr def, virDomainChrDef console; memcpy(&console, def->serials[0], sizeof(console)); console.deviceType = VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE; - if (virDomainChrDefFormat(buf, &console, flags) < 0) + if (virDomainChrDefFormat(buf, &console, flags) < 0) /* XXX indent */ goto cleanup; }
for (n = 0 ; n < def->nchannels ; n++) - if (virDomainChrDefFormat(buf, def->channels[n], flags) < 0) + if (virDomainChrDefFormat(buf, def->channels[n], flags) < 0) /* XXX indent */ goto cleanup;
for (n = 0 ; n < def->ninputs ; n++) if (def->inputs[n]->bus == VIR_DOMAIN_INPUT_BUS_USB && - virDomainInputDefFormat(buf, def->inputs[n], flags) < 0) + virDomainInputDefFormat(buf, def->inputs[n], flags) < 0) /* XXX indent */ goto cleanup;
if (def->ngraphics > 0) { @@ -10830,41 +10829,42 @@ virDomainDefFormatInternal(virDomainDefPtr def, { .alias = NULL }, };
- if (virDomainInputDefFormat(buf, &autoInput, flags) < 0) + if (virDomainInputDefFormat(buf, &autoInput, flags) < 0) /* XXX indent */ goto cleanup;
for (n = 0 ; n < def->ngraphics ; n++) - if (virDomainGraphicsDefFormat(buf, def->graphics[n], flags) < 0) + if (virDomainGraphicsDefFormat(buf, def->graphics[n], flags) < 0) /* XXX indent */ goto cleanup; }
for (n = 0 ; n < def->nsounds ; n++) - if (virDomainSoundDefFormat(buf, def->sounds[n], flags) < 0) + if (virDomainSoundDefFormat(buf, def->sounds[n], flags) < 0) /* XXX indent */ goto cleanup;
for (n = 0 ; n < def->nvideos ; n++) - if (virDomainVideoDefFormat(buf, def->videos[n], flags) < 0) + if (virDomainVideoDefFormat(buf, def->videos[n], flags) < 0) /* XXX indent */ goto cleanup;
for (n = 0 ; n < def->nhostdevs ; n++) - if (virDomainHostdevDefFormat(buf, def->hostdevs[n], flags) < 0) + if (virDomainHostdevDefFormat(buf, def->hostdevs[n], flags) < 0) /* XXX indent */ goto cleanup;
for (n = 0 ; n < def->nredirdevs ; n++) - if (virDomainRedirdevDefFormat(buf, def->redirdevs[n], flags) < 0) + if (virDomainRedirdevDefFormat(buf, def->redirdevs[n], flags) < 0) /* XXX indent */ goto cleanup;
for (n = 0 ; n < def->nhubs ; n++) - if (virDomainHubDefFormat(buf, def->hubs[n], flags) < 0) + if (virDomainHubDefFormat(buf, def->hubs[n], flags) < 0) /* XXX indent */ goto cleanup;
if (def->watchdog) - virDomainWatchdogDefFormat (buf, def->watchdog, flags); + virDomainWatchdogDefFormat (buf, def->watchdog, flags); /* XXX indent */
if (def->memballoon) - virDomainMemballoonDefFormat (buf, def->memballoon, flags); + virDomainMemballoonDefFormat (buf, def->memballoon, flags); /* XXX indent */
- virBufferAddLit(buf, " </devices>\n"); + indent -= 2; + virBufferIndentAddLit(buf, indent, "</devices>\n");
if (def->seclabel.model) { const char *sectype = virDomainSeclabelTypeToString(def->seclabel.type); @@ -10876,29 +10876,30 @@ virDomainDefFormatInternal(virDomainDefPtr def, (flags & VIR_DOMAIN_XML_INACTIVE)) { /* This is the default for inactive xml, so nothing to output. */ } else { - virBufferAsprintf(buf, " <seclabel type='%s' model='%s' relabel='%s'>\n", + virBufferAsprintf(buf, "%*s<seclabel type='%s' model='%s' " + "relabel='%s'>\n", indent, "", sectype, def->seclabel.model, def->seclabel.norelabel ? "no" : "yes"); - if (def->seclabel.label) - virBufferEscapeString(buf, " <label>%s</label>\n", - def->seclabel.label); - if (!def->seclabel.norelabel && def->seclabel.imagelabel) - virBufferEscapeString(buf, " <imagelabel>%s</imagelabel>\n", - def->seclabel.imagelabel); - if (def->seclabel.baselabel && - (def->seclabel.type == VIR_DOMAIN_SECLABEL_DYNAMIC)) - virBufferEscapeString(buf, " <baselabel>%s</baselabel>\n", - def->seclabel.baselabel); - virBufferAddLit(buf, " </seclabel>\n"); + virBufferIndentEscapeString(buf, indent + 2, + "<label>%s</label>\n", + def->seclabel.label); + if (!def->seclabel.norelabel) + virBufferIndentEscapeString(buf, indent + 2, + "<imagelabel>%s</imagelabel>\n", + def->seclabel.imagelabel); + if (def->seclabel.type == VIR_DOMAIN_SECLABEL_DYNAMIC) + virBufferIndentEscapeString(buf, indent + 2, + "<baselabel>%s</baselabel>\n", + def->seclabel.baselabel); + virBufferIndentAddLit(buf, indent, "</seclabel>\n"); } }
if (def->namespaceData && def->ns.format) { - if ((def->ns.format)(buf, def->namespaceData) < 0) + if ((def->ns.format)(buf, def->namespaceData) < 0) /* XXX indent */ goto cleanup; }
- /* XXX Fix indentation of body prior to here */ indent -= 2;
virBufferIndentAddLit(buf, indent, "</domain>\n"); -- 1.7.4.4
virBufferAsprintf(buf, "%*s ... can probably be changed rather mecanically What do you think of introducing that new function instead ? Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
Focusing just on sysinfo output. The new virBufferIndent* functions allow us to shave off quite a few lines of code. * src/util/sysinfo.h (virSysinfoFormat): Alter signature. * src/util/sysinfo.c (virSysinfoFormat, virSysinfoBIOSFormat) (virSysinfoSystemFormat, virSysinfoProcessorFormat) (virSysinfoMemoryFormat): Change indentation parameter. * src/conf/domain_conf.c (virDomainSysinfoDefFormat) (virDomainDefFormatInternal): Adjust callers. * src/qemu/qemu_driver.c (qemuGetSysinfo): Likewise. --- src/conf/domain_conf.c | 7 +- src/qemu/qemu_driver.c | 2 +- src/util/sysinfo.c | 407 ++++++++++++++++++------------------------------ src/util/sysinfo.h | 4 +- 4 files changed, 155 insertions(+), 265 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 64bc82d..9b6b9ca 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9874,9 +9874,10 @@ virDomainMemballoonDefFormat(virBufferPtr buf, static int virDomainSysinfoDefFormat(virBufferPtr buf, - virSysinfoDefPtr def) + virSysinfoDefPtr def, + int indent) { - char *format = virSysinfoFormat(def, " "); + char *format = virSysinfoFormat(def, indent); if (!format) return -1; @@ -10616,7 +10617,7 @@ virDomainDefFormatInternal(virDomainDefPtr def, } if (def->sysinfo) - virDomainSysinfoDefFormat(buf, def->sysinfo); /* XXX indent */ + virDomainSysinfoDefFormat(buf, def->sysinfo, indent); if (def->os.bootloader) { virBufferIndentEscapeString(buf, indent, diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 0d0bea2..08310b4 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -972,7 +972,7 @@ qemuGetSysinfo(virConnectPtr conn, unsigned int flags) return NULL; } - return virSysinfoFormat(driver->hostsysinfo, ""); + return virSysinfoFormat(driver->hostsysinfo, 0); } static int qemudGetMaxVCPUs(virConnectPtr conn ATTRIBUTE_UNUSED, const char *type) { diff --git a/src/util/sysinfo.c b/src/util/sysinfo.c index 6625cae..203a936 100644 --- a/src/util/sysinfo.c +++ b/src/util/sysinfo.c @@ -515,298 +515,187 @@ no_memory: #endif /* !WIN32 && x86 */ static void -virSysinfoBIOSFormat(virSysinfoDefPtr def, const char *prefix, - virBufferPtr buf) +virSysinfoBIOSFormat(virSysinfoDefPtr def, int indent, virBufferPtr buf) { - int len = strlen(prefix); - - if ((def->bios_vendor != NULL) || (def->bios_version != NULL) || - (def->bios_date != NULL) || (def->bios_release != NULL)) { - virBufferAsprintf(buf, "%s <bios>\n", prefix); - if (def->bios_vendor != NULL) { - virBufferAdd(buf, prefix, len); - virBufferEscapeString(buf, - " <entry name='vendor'>%s</entry>\n", - def->bios_vendor); - } - if (def->bios_version != NULL) { - virBufferAdd(buf, prefix, len); - virBufferEscapeString(buf, - " <entry name='version'>%s</entry>\n", - def->bios_version); - } - if (def->bios_date != NULL) { - virBufferAdd(buf, prefix, len); - virBufferEscapeString(buf, - " <entry name='date'>%s</entry>\n", - def->bios_date); - } - if (def->bios_release != NULL) { - virBufferAdd(buf, prefix, len); - virBufferEscapeString(buf, - " <entry name='release'>%s</entry>\n", - def->bios_release); - } - virBufferAsprintf(buf, "%s </bios>\n", prefix); - } + if (!def->bios_vendor && !def->bios_version && + !def->bios_date && !def->bios_release) + return; - return; + virBufferIndentAddLit(buf, indent, "<bios>\n"); + virBufferIndentEscapeString(buf, indent + 2, + "<entry name='vendor'>%s</entry>\n", + def->bios_vendor); + virBufferIndentEscapeString(buf, indent + 2, + "<entry name='version'>%s</entry>\n", + def->bios_version); + virBufferIndentEscapeString(buf, indent + 2, + "<entry name='date'>%s</entry>\n", + def->bios_date); + virBufferIndentEscapeString(buf, indent + 2, + "<entry name='release'>%s</entry>\n", + def->bios_release); + virBufferIndentAddLit(buf, indent, "</bios>\n"); } static void -virSysinfoSystemFormat(virSysinfoDefPtr def, const char *prefix, - virBufferPtr buf) +virSysinfoSystemFormat(virSysinfoDefPtr def, int indent, virBufferPtr buf) { - int len = strlen(prefix); - - if ((def->system_manufacturer != NULL) || (def->system_product != NULL) || - (def->system_version != NULL) || (def->system_serial != NULL) || - (def->system_uuid != NULL) || (def->system_sku != NULL) || - (def->system_family != NULL)) { - virBufferAsprintf(buf, "%s <system>\n", prefix); - if (def->system_manufacturer != NULL) { - virBufferAdd(buf, prefix, len); - virBufferEscapeString(buf, - " <entry name='manufacturer'>%s</entry>\n", - def->system_manufacturer); - } - if (def->system_product != NULL) { - virBufferAdd(buf, prefix, len); - virBufferEscapeString(buf, - " <entry name='product'>%s</entry>\n", - def->system_product); - } - if (def->system_version != NULL) { - virBufferAdd(buf, prefix, len); - virBufferEscapeString(buf, - " <entry name='version'>%s</entry>\n", - def->system_version); - } - if (def->system_serial != NULL) { - virBufferAdd(buf, prefix, len); - virBufferEscapeString(buf, - " <entry name='serial'>%s</entry>\n", - def->system_serial); - } - if (def->system_uuid != NULL) { - virBufferAdd(buf, prefix, len); - virBufferEscapeString(buf, - " <entry name='uuid'>%s</entry>\n", - def->system_uuid); - } - if (def->system_sku != NULL) { - virBufferAdd(buf, prefix, len); - virBufferEscapeString(buf, - " <entry name='sku'>%s</entry>\n", - def->system_sku); - } - if (def->system_family != NULL) { - virBufferAdd(buf, prefix, len); - virBufferEscapeString(buf, - " <entry name='family'>%s</entry>\n", - def->system_family); - } - virBufferAsprintf(buf, "%s </system>\n", prefix); - } + if (!def->system_manufacturer && !def->system_product && + !def->system_version && !def->system_serial && + !def->system_uuid && !def->system_sku && !def->system_family) + return; - return; + virBufferIndentAddLit(buf, indent, "<system>\n"); + virBufferIndentEscapeString(buf, indent + 2, + "<entry name='manufacturer'>%s</entry>\n", + def->system_manufacturer); + virBufferIndentEscapeString(buf, indent + 2, + "<entry name='product'>%s</entry>\n", + def->system_product); + virBufferIndentEscapeString(buf, indent + 2, + "<entry name='version'>%s</entry>\n", + def->system_version); + virBufferIndentEscapeString(buf, indent + 2, + "<entry name='serial'>%s</entry>\n", + def->system_serial); + virBufferIndentEscapeString(buf, indent + 2, + "<entry name='uuid'>%s</entry>\n", + def->system_uuid); + virBufferIndentEscapeString(buf, indent + 2, + "<entry name='sku'>%s</entry>\n", + def->system_sku); + virBufferIndentEscapeString(buf, indent + 2, + "<entry name='family'>%s</entry>\n", + def->system_family); + virBufferIndentAddLit(buf, indent, "</system>\n"); } static void -virSysinfoProcessorFormat(virSysinfoDefPtr def, const char *prefix, - virBufferPtr buf) +virSysinfoProcessorFormat(virSysinfoDefPtr def, int indent, virBufferPtr buf) { int i; - int len = strlen(prefix); virSysinfoProcessorDefPtr processor; for (i = 0; i < def->nprocessor; i++) { processor = &def->processor[i]; - if ((processor->processor_socket_destination != NULL) || - (processor->processor_type != NULL) || - (processor->processor_family != NULL) || - (processor->processor_manufacturer != NULL) || - (processor->processor_signature != NULL) || - (processor->processor_version != NULL) || - (processor->processor_external_clock != NULL) || - (processor->processor_max_speed != NULL) || - (processor->processor_status != NULL) || - (processor->processor_serial_number != NULL) || - (processor->processor_part_number != NULL)) { - virBufferAsprintf(buf, "%s <processor>\n", prefix); - if (processor->processor_socket_destination != NULL) { - virBufferAdd(buf, prefix, len); - virBufferEscapeString(buf, - " <entry name='socket_destination'>%s</entry>\n", - processor->processor_socket_destination); - } - if (processor->processor_type != NULL) { - virBufferAdd(buf, prefix, len); - virBufferEscapeString(buf, - " <entry name='type'>%s</entry>\n", - processor->processor_type); - } - if (processor->processor_family != NULL) { - virBufferAdd(buf, prefix, len); - virBufferEscapeString(buf, - " <entry name='family'>%s</entry>\n", - processor->processor_family); - } - if (processor->processor_manufacturer != NULL) { - virBufferAdd(buf, prefix, len); - virBufferEscapeString(buf, - " <entry name='manufacturer'>%s</entry>\n", - processor->processor_manufacturer); - } - if (processor->processor_signature != NULL) { - virBufferAdd(buf, prefix, len); - virBufferEscapeString(buf, - " <entry name='signature'>%s</entry>\n", - processor->processor_signature); - } - if (processor->processor_version != NULL) { - virBufferAdd(buf, prefix, len); - virBufferEscapeString(buf, - " <entry name='version'>%s</entry>\n", - processor->processor_version); - } - if (processor->processor_external_clock != NULL) { - virBufferAdd(buf, prefix, len); - virBufferEscapeString(buf, - " <entry name='external_clock'>%s</entry>\n", - processor->processor_external_clock); - } - if (processor->processor_max_speed != NULL) { - virBufferAdd(buf, prefix, len); - virBufferEscapeString(buf, - " <entry name='max_speed'>%s</entry>\n", - processor->processor_max_speed); - } - if (processor->processor_status != NULL) { - virBufferAdd(buf, prefix, len); - virBufferEscapeString(buf, - " <entry name='status'>%s</entry>\n", - processor->processor_status); - } - if (processor->processor_serial_number != NULL) { - virBufferAdd(buf, prefix, len); - virBufferEscapeString(buf, - " <entry name='serial_number'>%s</entry>\n", - processor->processor_serial_number); - } - if (processor->processor_part_number != NULL) { - virBufferAdd(buf, prefix, len); - virBufferEscapeString(buf, - " <entry name='part_number'>%s</entry>\n", - processor->processor_part_number); - } - virBufferAsprintf(buf, "%s </processor>\n", prefix); - } + if (!processor->processor_socket_destination && + !processor->processor_type && + !processor->processor_family && + !processor->processor_manufacturer && + !processor->processor_signature && + !processor->processor_version && + !processor->processor_external_clock && + !processor->processor_max_speed && + !processor->processor_status && + !processor->processor_serial_number && + !processor->processor_part_number) + continue; + + virBufferIndentAddLit(buf, indent, "<processor>\n"); + virBufferIndentEscapeString(buf, indent + 2, + "<entry name='socket_destination'>%s</entry>\n", + processor->processor_socket_destination); + virBufferIndentEscapeString(buf, indent + 2, + "<entry name='type'>%s</entry>\n", + processor->processor_type); + virBufferIndentEscapeString(buf, indent + 2, + "<entry name='family'>%s</entry>\n", + processor->processor_family); + virBufferIndentEscapeString(buf, indent + 2, + "<entry name='manufacturer'>%s</entry>\n", + processor->processor_manufacturer); + virBufferIndentEscapeString(buf, indent + 2, + "<entry name='signature'>%s</entry>\n", + processor->processor_signature); + virBufferIndentEscapeString(buf, indent + 2, + "<entry name='version'>%s</entry>\n", + processor->processor_version); + virBufferIndentEscapeString(buf, indent + 2, + "<entry name='external_clock'>%s</entry>\n", + processor->processor_external_clock); + virBufferIndentEscapeString(buf, indent + 2, + "<entry name='max_speed'>%s</entry>\n", + processor->processor_max_speed); + virBufferIndentEscapeString(buf, indent + 2, + "<entry name='status'>%s</entry>\n", + processor->processor_status); + virBufferIndentEscapeString(buf, indent + 2, + "<entry name='serial_number'>%s</entry>\n", + processor->processor_serial_number); + virBufferIndentEscapeString(buf, indent + 2, + "<entry name='part_number'>%s</entry>\n", + processor->processor_part_number); + virBufferIndentAddLit(buf, indent, "</processor>\n"); } - - return; } static void -virSysinfoMemoryFormat(virSysinfoDefPtr def, const char *prefix, - virBufferPtr buf) +virSysinfoMemoryFormat(virSysinfoDefPtr def, int indent, virBufferPtr buf) { int i; - int len = strlen(prefix); virSysinfoMemoryDefPtr memory; for (i = 0; i < def->nmemory; i++) { memory = &def->memory[i]; - if ((memory->memory_size != NULL) || - (memory->memory_form_factor != NULL) || - (memory->memory_locator != NULL) || - (memory->memory_bank_locator != NULL) || - (memory->memory_type != NULL) || - (memory->memory_type_detail != NULL) || - (memory->memory_speed != NULL) || - (memory->memory_manufacturer != NULL) || - (memory->memory_serial_number != NULL) || - (memory->memory_part_number != NULL)) { - virBufferAsprintf(buf, "%s <memory_device>\n", prefix); - if (memory->memory_size != NULL) { - virBufferAdd(buf, prefix, len); - virBufferEscapeString(buf, - " <entry name='size'>%s</entry>\n", - memory->memory_size); - } - if (memory->memory_form_factor != NULL) { - virBufferAdd(buf, prefix, len); - virBufferEscapeString(buf, - " <entry name='form_factor'>%s</entry>\n", - memory->memory_form_factor); - } - if (memory->memory_locator != NULL) { - virBufferAdd(buf, prefix, len); - virBufferEscapeString(buf, - " <entry name='locator'>%s</entry>\n", - memory->memory_locator); - } - if (memory->memory_bank_locator != NULL) { - virBufferAdd(buf, prefix, len); - virBufferEscapeString(buf, - " <entry name='bank_locator'>%s</entry>\n", - memory->memory_bank_locator); - } - if (memory->memory_type != NULL) { - virBufferAdd(buf, prefix, len); - virBufferEscapeString(buf, - " <entry name='type'>%s</entry>\n", - memory->memory_type); - } - if (memory->memory_type_detail != NULL) { - virBufferAdd(buf, prefix, len); - virBufferEscapeString(buf, - " <entry name='type_detail'>%s</entry>\n", - memory->memory_type_detail); - } - if (memory->memory_speed != NULL) { - virBufferAdd(buf, prefix, len); - virBufferEscapeString(buf, - " <entry name='speed'>%s</entry>\n", - memory->memory_speed); - } - if (memory->memory_manufacturer != NULL) { - virBufferAdd(buf, prefix, len); - virBufferEscapeString(buf, - " <entry name='manufacturer'>%s</entry>\n", - memory->memory_manufacturer); - } - if (memory->memory_serial_number != NULL) { - virBufferAdd(buf, prefix, len); - virBufferEscapeString(buf, - " <entry name='serial_number'>%s</entry>\n", - memory->memory_serial_number); - } - if (memory->memory_part_number != NULL) { - virBufferAdd(buf, prefix, len); - virBufferEscapeString(buf, - " <entry name='part_number'>%s</entry>\n", - memory->memory_part_number); - } - virBufferAsprintf(buf, "%s </memory_device>\n", prefix); - } + if (!memory->memory_size && + !memory->memory_form_factor && + !memory->memory_locator && + !memory->memory_bank_locator && + !memory->memory_type && + !memory->memory_type_detail && + !memory->memory_speed && + !memory->memory_manufacturer && + !memory->memory_serial_number && + !memory->memory_part_number) + continue; + + virBufferIndentAddLit(buf, indent, "<memory_device>\n"); + virBufferIndentEscapeString(buf, indent + 2, + "<entry name='size'>%s</entry>\n", + memory->memory_size); + virBufferIndentEscapeString(buf, indent + 2, + "<entry name='form_factor'>%s</entry>\n", + memory->memory_form_factor); + virBufferIndentEscapeString(buf, indent + 2, + "<entry name='locator'>%s</entry>\n", + memory->memory_locator); + virBufferIndentEscapeString(buf, indent + 2, + "<entry name='bank_locator'>%s</entry>\n", + memory->memory_bank_locator); + virBufferIndentEscapeString(buf, indent + 2, + "<entry name='type'>%s</entry>\n", + memory->memory_type); + virBufferIndentEscapeString(buf, indent + 2, + "<entry name='type_detail'>%s</entry>\n", + memory->memory_type_detail); + virBufferIndentEscapeString(buf, indent + 2, + "<entry name='speed'>%s</entry>\n", + memory->memory_speed); + virBufferIndentEscapeString(buf, indent + 2, + "<entry name='manufacturer'>%s</entry>\n", + memory->memory_manufacturer); + virBufferIndentEscapeString(buf, indent + 2, + "<entry name='serial_number'>%s</entry>\n", + memory->memory_serial_number); + virBufferIndentEscapeString(buf, indent + 2, + "<entry name='part_number'>%s</entry>\n", + memory->memory_part_number); + virBufferIndentAddLit(buf, indent, "</memory_device>\n"); } - - return; } /** * virSysinfoFormat: * @def: structure to convert to xml string - * @prefix: string to prefix before each line of xml + * @indent: number of spaces to output before each line of xml * * This returns the XML description of the sysinfo, or NULL after * generating an error message. */ char * -virSysinfoFormat(virSysinfoDefPtr def, const char *prefix) +virSysinfoFormat(virSysinfoDefPtr def, int indent) { const char *type = virSysinfoTypeToString(def->type); virBuffer buf = VIR_BUFFER_INITIALIZER; @@ -818,14 +707,14 @@ virSysinfoFormat(virSysinfoDefPtr def, const char *prefix) return NULL; } - virBufferAsprintf(&buf, "%s<sysinfo type='%s'>\n", prefix, type); + virBufferAsprintf(&buf, "%*s<sysinfo type='%s'>\n", indent, "", type); - virSysinfoBIOSFormat(def, prefix, &buf); - virSysinfoSystemFormat(def, prefix, &buf); - virSysinfoProcessorFormat(def, prefix, &buf); - virSysinfoMemoryFormat(def, prefix, &buf); + virSysinfoBIOSFormat(def, indent + 2, &buf); + virSysinfoSystemFormat(def, indent + 2, &buf); + virSysinfoProcessorFormat(def, indent + 2, &buf); + virSysinfoMemoryFormat(def, indent + 2, &buf); - virBufferAsprintf(&buf, "%s</sysinfo>\n", prefix); + virBufferIndentAddLit(&buf, indent, "</sysinfo>\n"); if (virBufferError(&buf)) { virReportOOMError(); diff --git a/src/util/sysinfo.h b/src/util/sysinfo.h index 86fd20f..3f17152 100644 --- a/src/util/sysinfo.h +++ b/src/util/sysinfo.h @@ -93,8 +93,8 @@ virSysinfoDefPtr virSysinfoRead(void); void virSysinfoDefFree(virSysinfoDefPtr def); -char *virSysinfoFormat(virSysinfoDefPtr def, const char *prefix) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); +char *virSysinfoFormat(virSysinfoDefPtr def, int indent) + ATTRIBUTE_NONNULL(1); bool virSysinfoIsEqual(virSysinfoDefPtr src, virSysinfoDefPtr dst); -- 1.7.4.4
This round focuses on CPU features. * src/conf/cpu_conf.h (virCPUFormatFlags): Fix typo. (virCPUDefFormat): Drop unused argument. (virCPUDefFormatBuf): Alter signature. * src/conf/cpu_conf.c (virCPUDefFormat, virCPUDefFormatBuf): Alter 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 | 6 +++--- src/conf/cpu_conf.c | 43 ++++++++++++++++++++----------------------- src/conf/cpu_conf.h | 7 +++---- src/conf/domain_conf.c | 2 +- src/cpu/cpu.c | 2 +- tests/cputest.c | 2 +- 6 files changed, 29 insertions(+), 33 deletions(-) diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c index 2f243ae..7e074fb 100644 --- a/src/conf/capabilities.c +++ b/src/conf/capabilities.c @@ -1,7 +1,7 @@ /* * capabilities.c: hypervisor capabilities * - * Copyright (C) 2006-2008, 2010 Red Hat, Inc. + * Copyright (C) 2006-2008, 2010-2011 Red Hat, Inc. * Copyright (C) 2006-2008 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -681,8 +681,8 @@ virCapabilitiesFormatXML(virCapsPtr caps) virBufferAddLit(&xml, " </features>\n"); } - virCPUDefFormatBuf(&xml, caps->host.cpu, " ", - VIR_CPU_FORMAT_EMBEDED); + virCPUDefFormatBuf(&xml, caps->host.cpu, 4, + VIR_CPU_FORMAT_EMBEDDED); virBufferAddLit(&xml, " </cpu>\n"); diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c index 5cecda2..ce67e78 100644 --- a/src/conf/cpu_conf.c +++ b/src/conf/cpu_conf.c @@ -306,12 +306,11 @@ error: char * virCPUDefFormat(virCPUDefPtr def, - const char *indent, unsigned int flags) { virBuffer buf = VIR_BUFFER_INITIALIZER; - if (virCPUDefFormatBuf(&buf, def, indent, flags) < 0) + if (virCPUDefFormatBuf(&buf, def, 0, flags) < 0) goto cleanup; if (virBufferError(&buf)) @@ -330,7 +329,7 @@ cleanup: int virCPUDefFormatBuf(virBufferPtr buf, virCPUDefPtr def, - const char *indent, + int indent, unsigned int flags) { unsigned int i; @@ -338,16 +337,13 @@ virCPUDefFormatBuf(virBufferPtr buf, if (!def) return 0; - if (indent == NULL) - indent = ""; - if (!def->model && def->nfeatures) { virCPUReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Non-empty feature list specified without CPU model")); return -1; } - if (!(flags & VIR_CPU_FORMAT_EMBEDED)) { + if (!(flags & VIR_CPU_FORMAT_EMBEDDED)) { if (def->type == VIR_CPU_TYPE_GUEST && def->model) { const char *match; if (!(match = virCPUMatchTypeToString(def->match))) { @@ -356,25 +352,27 @@ virCPUDefFormatBuf(virBufferPtr buf, return -1; } - virBufferAsprintf(buf, "%s<cpu match='%s'>\n", indent, match); + virBufferAsprintf(buf, "%*s<cpu match='%s'>\n", indent, "", match); + } else { + virBufferIndentAddLit(buf, indent, "<cpu>\n"); } - else - virBufferAsprintf(buf, "%s<cpu>\n", indent); if (def->arch) - virBufferAsprintf(buf, "%s <arch>%s</arch>\n", indent, def->arch); + virBufferAsprintf(buf, "%*s<arch>%s</arch>\n", indent + 2, "", + def->arch); } if (def->model) - virBufferAsprintf(buf, "%s <model>%s</model>\n", indent, def->model); + virBufferAsprintf(buf, "%*s<model>%s</model>\n", indent + 2, "", + def->model); if (def->vendor) { - virBufferAsprintf(buf, "%s <vendor>%s</vendor>\n", - indent, def->vendor); + virBufferAsprintf(buf, "%*s<vendor>%s</vendor>\n", indent + 2, "", + def->vendor); } if (def->sockets && def->cores && def->threads) { - virBufferAsprintf(buf, "%s <topology", indent); + virBufferIndentAddLit(buf, indent + 2, "<topology"); virBufferAsprintf(buf, " sockets='%u'", def->sockets); virBufferAsprintf(buf, " cores='%u'", def->cores); virBufferAsprintf(buf, " threads='%u'", def->threads); @@ -399,17 +397,16 @@ virCPUDefFormatBuf(virBufferPtr buf, _("Unexpected CPU feature policy %d"), feature->policy); return -1; } - virBufferAsprintf(buf, "%s <feature policy='%s' name='%s'/>\n", - indent, policy, feature->name); - } - else { - virBufferAsprintf(buf, "%s <feature name='%s'/>\n", - indent, feature->name); + virBufferAsprintf(buf, "%*s<feature policy='%s' name='%s'/>\n", + indent + 2, "", policy, feature->name); + } else { + virBufferAsprintf(buf, "%*s<feature name='%s'/>\n", + indent + 2, "", feature->name); } } - if (!(flags & VIR_CPU_FORMAT_EMBEDED)) - virBufferAsprintf(buf, "%s</cpu>\n", indent); + if (!(flags & VIR_CPU_FORMAT_EMBEDDED)) + virBufferIndentAddLit(buf, indent, "</cpu>\n"); return 0; } diff --git a/src/conf/cpu_conf.h b/src/conf/cpu_conf.h index 57b85e1..60cfd44 100644 --- a/src/conf/cpu_conf.h +++ b/src/conf/cpu_conf.h @@ -96,8 +96,8 @@ virCPUDefParseXML(const xmlNodePtr node, enum virCPUType mode); enum virCPUFormatFlags { - VIR_CPU_FORMAT_EMBEDED = (1 << 0) /* embed into existing <cpu/> element - * in host capabilities */ + VIR_CPU_FORMAT_EMBEDDED = (1 << 0) /* embed into existing <cpu/> element + * in host capabilities */ }; bool @@ -106,13 +106,12 @@ virCPUDefIsEqual(virCPUDefPtr src, char * virCPUDefFormat(virCPUDefPtr def, - const char *indent, unsigned int flags); int virCPUDefFormatBuf(virBufferPtr buf, virCPUDefPtr def, - const char *indent, + int indent, unsigned int flags); int diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 9b6b9ca..4b825fb 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10721,7 +10721,7 @@ virDomainDefFormatInternal(virDomainDefPtr def, virBufferIndentAddLit(buf, indent, "</features>\n"); } - if (virCPUDefFormatBuf(buf, def->cpu, " ", 0) < 0) /* XXX indent */ + if (virCPUDefFormatBuf(buf, def->cpu, indent, 0) < 0) goto cleanup; virBufferAsprintf(buf, "%*s<clock offset='%s'", indent, "", diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c index 2906be9..72169b4 100644 --- a/src/cpu/cpu.c +++ b/src/cpu/cpu.c @@ -320,7 +320,7 @@ cpuBaselineXML(const char **xmlCPUs, if (!(cpu = cpuBaseline(cpus, ncpus, models, nmodels))) goto error; - cpustr = virCPUDefFormat(cpu, "", 0); + cpustr = virCPUDefFormat(cpu, 0); cleanup: if (cpus) { diff --git a/tests/cputest.c b/tests/cputest.c index b5272c1..e9f5cbb 100644 --- a/tests/cputest.c +++ b/tests/cputest.c @@ -176,7 +176,7 @@ cpuTestCompareXML(const char *arch, if (virtTestLoadFile(xml, &expected) < 0) goto cleanup; - if (!(actual = virCPUDefFormat(cpu, NULL, 0))) + if (!(actual = virCPUDefFormat(cpu, 0))) goto cleanup; if (STRNEQ(expected, actual)) { -- 1.7.4.4
Last indentation prior to <devices>. * src/conf/domain_conf.c (virDomainTimerDefFormat) (virDomainLifecycleDefFormat): Add parameter. (virDomainDefFormatInternal): Adjust caller. --- src/conf/domain_conf.c | 30 ++++++++++++++++++------------ 1 files changed, 18 insertions(+), 12 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 4b825fb..f6df921 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9043,6 +9043,7 @@ static int virDomainLifecycleDefFormat(virBufferPtr buf, int type, const char *name, + int indent, virLifecycleToStringFunc convFunc) { const char *typeStr = convFunc(type); @@ -9052,7 +9053,8 @@ virDomainLifecycleDefFormat(virBufferPtr buf, return -1; } - virBufferAsprintf(buf, " <%s>%s</%s>\n", name, typeStr, name); + virBufferAsprintf(buf, "%*s<%s>%s</%s>\n", + indent, "", name, typeStr, name); return 0; } @@ -10008,7 +10010,8 @@ virDomainInputDefFormat(virBufferPtr buf, static int virDomainTimerDefFormat(virBufferPtr buf, - virDomainTimerDefPtr def) + virDomainTimerDefPtr def, + int indent) { const char *name = virDomainTimerNameTypeToString(def->name); @@ -10017,7 +10020,7 @@ virDomainTimerDefFormat(virBufferPtr buf, _("unexpected timer name %d"), def->name); return -1; } - virBufferAsprintf(buf, " <timer name='%s'", name); + virBufferAsprintf(buf, "%*s<timer name='%s'", indent, "", name); if (def->present == 0) { virBufferAddLit(buf, " present='no'"); @@ -10075,7 +10078,8 @@ virDomainTimerDefFormat(virBufferPtr buf, && (def->catchup.limit == 0)) { virBufferAddLit(buf, "/>\n"); } else { - virBufferAddLit(buf, ">\n <catchup "); + virBufferAddLit(buf, ">\n"); + virBufferIndentAddLit(buf, indent + 2, "<catchup "); if (def->catchup.threshold > 0) { virBufferAsprintf(buf, " threshold='%lu'", def->catchup.threshold); } @@ -10085,7 +10089,8 @@ virDomainTimerDefFormat(virBufferPtr buf, if (def->catchup.limit > 0) { virBufferAsprintf(buf, " limit='%lu'", def->catchup.limit); } - virBufferAddLit(buf, "/>\n </timer>\n"); + virBufferAddLit(buf, "/>\n"); + virBufferIndentAddLit(buf, indent, "</timer>\n"); } return 0; @@ -10740,23 +10745,24 @@ virDomainDefFormatInternal(virDomainDefPtr def, } else { virBufferAddLit(buf, ">\n"); for (n = 0; n < def->clock.ntimers; n++) { - if (virDomainTimerDefFormat(buf, def->clock.timers[n]) < 0) /* XXX indent */ + if (virDomainTimerDefFormat(buf, def->clock.timers[n], + indent + 2) < 0) goto cleanup; } virBufferIndentAddLit(buf, indent, "</clock>\n"); } if (virDomainLifecycleDefFormat(buf, def->onPoweroff, - "on_poweroff", - virDomainLifecycleTypeToString) < 0) /* XXX indent */ + "on_poweroff", indent, + virDomainLifecycleTypeToString) < 0) goto cleanup; if (virDomainLifecycleDefFormat(buf, def->onReboot, - "on_reboot", - virDomainLifecycleTypeToString) < 0) /* XXX indent */ + "on_reboot", indent, + virDomainLifecycleTypeToString) < 0) goto cleanup; if (virDomainLifecycleDefFormat(buf, def->onCrash, - "on_crash", - virDomainLifecycleCrashTypeToString) < 0) /* XXX indent */ + "on_crash", indent, + virDomainLifecycleCrashTypeToString) < 0) goto cleanup; virBufferIndentAddLit(buf, indent, "<devices>\n"); -- 1.7.4.4
Tackle the domain-specific callbacks. * src/conf/capabilities.h (virDomainDefNamespaceXMLFormat) (_virCaps): Add indent argument. * src/conf/domain_conf.c (virDomainObjFormat) (virDomainDefFormatInternal): Adjust callers. * src/qemu/qemu_domain.c (qemuDomainObjPrivateXMLFormat) (qemuDomainDefNamespaceFormatXML): Adjust callbacks. --- src/conf/capabilities.h | 6 +++--- src/conf/domain_conf.c | 4 ++-- src/qemu/qemu_domain.c | 39 +++++++++++++++++++++++---------------- 3 files changed, 28 insertions(+), 21 deletions(-) diff --git a/src/conf/capabilities.h b/src/conf/capabilities.h index e2fa1d6..2e2a93d 100644 --- a/src/conf/capabilities.h +++ b/src/conf/capabilities.h @@ -1,7 +1,7 @@ /* * capabilities.h: hypervisor capabilities * - * Copyright (C) 2006-2008, 2010 Red Hat, Inc. + * Copyright (C) 2006-2008, 2010-2011 Red Hat, Inc. * Copyright (C) 2006-2008 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -121,7 +121,7 @@ struct _virCapsHost { typedef int (*virDomainDefNamespaceParse)(xmlDocPtr, xmlNodePtr, xmlXPathContextPtr, void **); typedef void (*virDomainDefNamespaceFree)(void *); -typedef int (*virDomainDefNamespaceXMLFormat)(virBufferPtr, void *); +typedef int (*virDomainDefNamespaceXMLFormat)(virBufferPtr, int, void *); typedef const char *(*virDomainDefNamespaceHref)(void); typedef struct _virDomainXMLNamespace virDomainXMLNamespace; @@ -147,7 +147,7 @@ struct _virCaps { int defaultConsoleTargetType; void *(*privateDataAllocFunc)(void); void (*privateDataFreeFunc)(void *); - int (*privateDataXMLFormat)(virBufferPtr, void *); + int (*privateDataXMLFormat)(virBufferPtr, int, void *); int (*privateDataXMLParse)(xmlXPathContextPtr, void *); bool hasWideScsiBus; const char *defaultInitPath; diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index f6df921..0a5e50b 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10903,7 +10903,7 @@ virDomainDefFormatInternal(virDomainDefPtr def, } if (def->namespaceData && def->ns.format) { - if ((def->ns.format)(buf, def->namespaceData) < 0) /* XXX indent */ + if ((def->ns.format)(buf, indent, def->namespaceData) < 0) goto cleanup; } @@ -10958,7 +10958,7 @@ static char *virDomainObjFormat(virCapsPtr caps, } if (caps->privateDataXMLFormat && - ((caps->privateDataXMLFormat)(&buf, obj->privateData)) < 0) + ((caps->privateDataXMLFormat)(&buf, 2, obj->privateData)) < 0) goto error; if (virDomainDefFormatInternal(obj->def, 2, flags, &buf) < 0) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 4023648..d913cc1 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -241,7 +241,8 @@ static void qemuDomainObjPrivateFree(void *data) } -static int qemuDomainObjPrivateXMLFormat(virBufferPtr buf, void *data) +static int +qemuDomainObjPrivateXMLFormat(virBufferPtr buf, int indent, void *data) { qemuDomainObjPrivatePtr priv = data; const char *monitorpath; @@ -258,7 +259,8 @@ static int qemuDomainObjPrivateXMLFormat(virBufferPtr buf, void *data) break; } - virBufferEscapeString(buf, " <monitor path='%s'", monitorpath); + virBufferIndentEscapeString(buf, indent, "<monitor path='%s'", + monitorpath); if (priv->monJSON) virBufferAddLit(buf, " json='1'"); virBufferAsprintf(buf, " type='%s'/>\n", @@ -268,30 +270,33 @@ static int qemuDomainObjPrivateXMLFormat(virBufferPtr buf, void *data) if (priv->nvcpupids) { int i; - virBufferAddLit(buf, " <vcpus>\n"); + virBufferIndentAddLit(buf, indent, "<vcpus>\n"); for (i = 0 ; i < priv->nvcpupids ; i++) { - virBufferAsprintf(buf, " <vcpu pid='%d'/>\n", priv->vcpupids[i]); + virBufferAsprintf(buf, "%*s<vcpu pid='%d'/>\n", indent + 2, "", + priv->vcpupids[i]); } - virBufferAddLit(buf, " </vcpus>\n"); + virBufferIndentAddLit(buf, indent, "</vcpus>\n"); } if (priv->qemuCaps) { int i; - virBufferAddLit(buf, " <qemuCaps>\n"); + virBufferIndentAddLit(buf, indent, "<qemuCaps>\n"); for (i = 0 ; i < QEMU_CAPS_LAST ; i++) { if (qemuCapsGet(priv->qemuCaps, i)) { - virBufferAsprintf(buf, " <flag name='%s'/>\n", + virBufferAsprintf(buf, "%*s<flag name='%s'/>\n", + indent + 2, "", qemuCapsTypeToString(i)); } } - virBufferAddLit(buf, " </qemuCaps>\n"); + virBufferIndentAddLit(buf, indent, "</qemuCaps>\n"); } if (priv->lockState) - virBufferAsprintf(buf, " <lockstate>%s</lockstate>\n", priv->lockState); + virBufferAsprintf(buf, "%*s<lockstate>%s</lockstate>\n", indent, "", + priv->lockState); if (priv->job.active || priv->job.asyncJob) { - virBufferAsprintf(buf, " <job type='%s' async='%s'", + virBufferAsprintf(buf, "%*s<job type='%s' async='%s'", indent, "", qemuDomainJobTypeToString(priv->job.active), qemuDomainAsyncJobTypeToString(priv->job.asyncJob)); if (priv->job.phase) { @@ -590,7 +595,7 @@ error: } static int -qemuDomainDefNamespaceFormatXML(virBufferPtr buf, +qemuDomainDefNamespaceFormatXML(virBufferPtr buf, int indent, void *nsdata) { qemuDomainCmdlineDefPtr cmd = nsdata; @@ -599,17 +604,19 @@ qemuDomainDefNamespaceFormatXML(virBufferPtr buf, if (!cmd->num_args && !cmd->num_env) return 0; - virBufferAddLit(buf, " <qemu:commandline>\n"); + virBufferIndentAddLit(buf, indent, "<qemu:commandline>\n"); for (i = 0; i < cmd->num_args; i++) - virBufferEscapeString(buf, " <qemu:arg value='%s'/>\n", - cmd->args[i]); + virBufferIndentEscapeString(buf, indent + 2, + "<qemu:arg value='%s'/>\n", + cmd->args[i]); for (i = 0; i < cmd->num_env; i++) { - virBufferAsprintf(buf, " <qemu:env name='%s'", cmd->env_name[i]); + virBufferAsprintf(buf, "%*s<qemu:env name='%s'", indent + 2, "", + cmd->env_name[i]); if (cmd->env_value[i]) virBufferEscapeString(buf, " value='%s'", cmd->env_value[i]); virBufferAddLit(buf, "/>\n"); } - virBufferAddLit(buf, " </qemu:commandline>\n"); + virBufferIndentAddLit(buf, indent, "</qemu:commandline>\n"); return 0; } -- 1.7.4.4
Callers are still hard-coded, for now. * src/conf/domain_conf.c (virDomainDeviceInfoFormat): Add parameter. Adjust callers. --- src/conf/domain_conf.c | 40 +++++++++++++++++++++------------------- 1 files changed, 21 insertions(+), 19 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 0a5e50b..4d2e6c3 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1619,28 +1619,30 @@ void virDomainDefClearDeviceAliases(virDomainDefPtr def) static int ATTRIBUTE_NONNULL(2) virDomainDeviceInfoFormat(virBufferPtr buf, virDomainDeviceInfoPtr info, + int indent, unsigned int flags) { if (info->alias && !(flags & VIR_DOMAIN_XML_INACTIVE)) { - virBufferAsprintf(buf, " <alias name='%s'/>\n", info->alias); + virBufferAsprintf(buf, "%*s<alias name='%s'/>\n", indent, "", + info->alias); } if (info->mastertype == VIR_DOMAIN_CONTROLLER_MASTER_USB) { - virBufferAsprintf(buf, " <master startport='%d'/>\n", + virBufferAsprintf(buf, "%*s<master startport='%d'/>\n", indent, "", info->master.usb.startport); } if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) return 0; - /* We'll be in domain/devices/[device type]/ so 3 level indent */ - virBufferAsprintf(buf, " <address type='%s'", + virBufferAsprintf(buf, "%*s<address type='%s'", indent, "", virDomainDeviceAddressTypeToString(info->type)); switch (info->type) { case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI: - virBufferAsprintf(buf, " domain='0x%.4x' bus='0x%.2x' slot='0x%.2x' function='0x%.1x'", + virBufferAsprintf(buf, " domain='0x%.4x' bus='0x%.2x' slot='0x%.2x'" + " function='0x%.1x'", info->addr.pci.domain, info->addr.pci.bus, info->addr.pci.slot, @@ -9206,7 +9208,7 @@ virDomainDiskDefFormat(virBufferPtr buf, virStorageEncryptionFormat(buf, def->encryption, 6) < 0) return -1; - if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) + if (virDomainDeviceInfoFormat(buf, &def->info, 6, flags) < 0) return -1; virBufferAddLit(buf, " </disk>\n"); @@ -9276,7 +9278,7 @@ virDomainControllerDefFormat(virBufferPtr buf, if (virDomainDeviceInfoIsSet(&def->info, flags)) { virBufferAddLit(buf, ">\n"); - if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) + if (virDomainDeviceInfoFormat(buf, &def->info, 6, flags) < 0) return -1; virBufferAddLit(buf, " </controller>\n"); } else { @@ -9340,7 +9342,7 @@ virDomainFSDefFormat(virBufferPtr buf, if (def->readonly) virBufferAddLit(buf, " <readonly/>\n"); - if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) + if (virDomainDeviceInfoFormat(buf, &def->info, 6, flags) < 0) return -1; virBufferAddLit(buf, " </filesystem>\n"); @@ -9564,7 +9566,7 @@ virDomainNetDefFormat(virBufferPtr buf, if (virBandwidthDefFormat(buf, def->bandwidth, " ") < 0) return -1; - if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) + if (virDomainDeviceInfoFormat(buf, &def->info, 6, flags) < 0) return -1; virBufferAddLit(buf, " </interface>\n"); @@ -9756,7 +9758,7 @@ virDomainChrDefFormat(virBufferPtr buf, } if (virDomainDeviceInfoIsSet(&def->info, flags)) { - if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) + if (virDomainDeviceInfoFormat(buf, &def->info, 6, flags) < 0) return -1; } @@ -9811,7 +9813,7 @@ virDomainSmartcardDefFormat(virBufferPtr buf, _("unexpected smartcard type %d"), def->type); return -1; } - if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) + if (virDomainDeviceInfoFormat(buf, &def->info, 6, flags) < 0) return -1; virBufferAddLit(buf, " </smartcard>\n"); return 0; @@ -9835,7 +9837,7 @@ virDomainSoundDefFormat(virBufferPtr buf, if (virDomainDeviceInfoIsSet(&def->info, flags)) { virBufferAddLit(buf, ">\n"); - if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) + if (virDomainDeviceInfoFormat(buf, &def->info, 6, flags) < 0) return -1; virBufferAddLit(buf, " </sound>\n"); } else { @@ -9864,7 +9866,7 @@ virDomainMemballoonDefFormat(virBufferPtr buf, if (virDomainDeviceInfoIsSet(&def->info, flags)) { virBufferAddLit(buf, ">\n"); - if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) + if (virDomainDeviceInfoFormat(buf, &def->info, 6, flags) < 0) return -1; virBufferAddLit(buf, " </memballoon>\n"); } else { @@ -9914,7 +9916,7 @@ virDomainWatchdogDefFormat(virBufferPtr buf, if (virDomainDeviceInfoIsSet(&def->info, flags)) { virBufferAddLit(buf, ">\n"); - if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) + if (virDomainDeviceInfoFormat(buf, &def->info, 6, flags) < 0) return -1; virBufferAddLit(buf, " </watchdog>\n"); } else { @@ -9965,7 +9967,7 @@ virDomainVideoDefFormat(virBufferPtr buf, virBufferAddLit(buf, "/>\n"); } - if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) + if (virDomainDeviceInfoFormat(buf, &def->info, 6, flags) < 0) return -1; virBufferAddLit(buf, " </video>\n"); @@ -9997,7 +9999,7 @@ virDomainInputDefFormat(virBufferPtr buf, if (virDomainDeviceInfoIsSet(&def->info, flags)) { virBufferAddLit(buf, ">\n"); - if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) + if (virDomainDeviceInfoFormat(buf, &def->info, 6, flags) < 0) return -1; virBufferAddLit(buf, " </input>\n"); } else { @@ -10390,7 +10392,7 @@ virDomainHostdevDefFormat(virBufferPtr buf, if (def->bootIndex) virBufferAsprintf(buf, " <boot order='%d'/>\n", def->bootIndex); - if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) + if (virDomainDeviceInfoFormat(buf, &def->info, 6, flags) < 0) return -1; virBufferAddLit(buf, " </hostdev>\n"); @@ -10410,7 +10412,7 @@ virDomainRedirdevDefFormat(virBufferPtr buf, virBufferAsprintf(buf, " <redirdev bus='%s'", bus); if (virDomainChrSourceDefFormat(buf, &def->source.chr, false, flags) < 0) return -1; - if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) + if (virDomainDeviceInfoFormat(buf, &def->info, 6, flags) < 0) return -1; virBufferAddLit(buf, " </redirdev>\n"); @@ -10434,7 +10436,7 @@ virDomainHubDefFormat(virBufferPtr buf, if (virDomainDeviceInfoIsSet(&def->info, flags)) { virBufferAddLit(buf, ">\n"); - if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) + if (virDomainDeviceInfoFormat(buf, &def->info, 6, flags) < 0) return -1; virBufferAddLit(buf, " </hub>\n"); } else { -- 1.7.4.4
This round fixes shared network formatting to use int rather than char * for specifying the indent. * src/util/network.h (virVirtualPortProfileFormat) (virBandwidthDefFormat): Alter signature. * src/util/network.c (virVirtualPortProfileFormat) (virBandwidthDefFormat): Alter indentation. (virBandwidthChildDefFormat): Add parameter. * src/conf/domain_conf.c (virDomainActualNetDefFormat) (virDomainNetDefFormat): Adjust callers. * src/conf/network_conf.c (virPortGroupDefFormat) (virNetworkDefFormat): Likewise. --- src/conf/domain_conf.c | 13 ++++------- src/conf/network_conf.c | 8 +++--- src/util/network.c | 49 +++++++++++++++++++--------------------------- src/util/network.h | 9 ++++--- 4 files changed, 34 insertions(+), 45 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 4d2e6c3..be32598 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9398,14 +9398,13 @@ virDomainActualNetDefFormat(virBufferPtr buf, return ret; } virBufferAsprintf(buf, " mode='%s'/>\n", mode); - virVirtualPortProfileFormat(buf, def->data.direct.virtPortProfile, - " "); + virVirtualPortProfileFormat(buf, def->data.direct.virtPortProfile, 8); break; default: break; } - if (virBandwidthDefFormat(buf, def->bandwidth, " ") < 0) + if (virBandwidthDefFormat(buf, def->bandwidth, 6) < 0) goto error; virBufferAddLit(buf, " </actual>\n"); @@ -9445,8 +9444,7 @@ virDomainNetDefFormat(virBufferPtr buf, def->data.network.portgroup); } virBufferAddLit(buf, "/>\n"); - virVirtualPortProfileFormat(buf, def->data.network.virtPortProfile, - " "); + virVirtualPortProfileFormat(buf, def->data.network.virtPortProfile, 6); if ((flags & VIR_DOMAIN_XML_INTERNAL_ACTUAL_NET) && (virDomainActualNetDefFormat(buf, def->data.network.actual) < 0)) return -1; @@ -9497,8 +9495,7 @@ virDomainNetDefFormat(virBufferPtr buf, virBufferAsprintf(buf, " mode='%s'", virMacvtapModeTypeToString(def->data.direct.mode)); virBufferAddLit(buf, "/>\n"); - virVirtualPortProfileFormat(buf, def->data.direct.virtPortProfile, - " "); + virVirtualPortProfileFormat(buf, def->data.direct.virtPortProfile, 6); break; case VIR_DOMAIN_NET_TYPE_USER: @@ -9563,7 +9560,7 @@ virDomainNetDefFormat(virBufferPtr buf, virBufferAsprintf(buf, " <link state='%s'/>\n", virDomainNetInterfaceLinkStateTypeToString(def->linkstate)); - if (virBandwidthDefFormat(buf, def->bandwidth, " ") < 0) + if (virBandwidthDefFormat(buf, def->bandwidth, 6) < 0) return -1; if (virDomainDeviceInfoFormat(buf, &def->info, 6, flags) < 0) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index b98ffad..f5f712f 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -1267,8 +1267,8 @@ virPortGroupDefFormat(virBufferPtr buf, virBufferAddLit(buf, " default='yes'"); } virBufferAddLit(buf, ">\n"); - virVirtualPortProfileFormat(buf, def->virtPortProfile, " "); - virBandwidthDefFormat(buf, def->bandwidth, " "); + virVirtualPortProfileFormat(buf, def->virtPortProfile, 4); + virBandwidthDefFormat(buf, def->bandwidth, 4); virBufferAddLit(buf, " </portgroup>\n"); } @@ -1341,7 +1341,7 @@ char *virNetworkDefFormat(const virNetworkDefPtr def) if (virNetworkDNSDefFormat(&buf, def->dns) < 0) goto error; - if (virBandwidthDefFormat(&buf, def->bandwidth, " ") < 0) + if (virBandwidthDefFormat(&buf, def->bandwidth, 2) < 0) goto error; for (ii = 0; ii < def->nips; ii++) { @@ -1349,7 +1349,7 @@ char *virNetworkDefFormat(const virNetworkDefPtr def) goto error; } - virVirtualPortProfileFormat(&buf, def->virtPortProfile, " "); + virVirtualPortProfileFormat(&buf, def->virtPortProfile, 2); for (ii = 0; ii < def->nPortGroups; ii++) virPortGroupDefFormat(&buf, &def->portGroups[ii]); diff --git a/src/util/network.c b/src/util/network.c index ee69557..9969e80 100644 --- a/src/util/network.c +++ b/src/util/network.c @@ -882,15 +882,15 @@ virVirtualPortProfileEqual(virVirtualPortProfileParamsPtr a, virVirtualPortProfi void virVirtualPortProfileFormat(virBufferPtr buf, virVirtualPortProfileParamsPtr virtPort, - const char *indent) + int indent) { char uuidstr[VIR_UUID_STRING_BUFLEN]; if (!virtPort || virtPort->virtPortType == VIR_VIRTUALPORT_NONE) return; - virBufferAsprintf(buf, "%s<virtualport type='%s'>\n", - indent, + virBufferAsprintf(buf, "%*s<virtualport type='%s'>\n", + indent, "", virVirtualPortTypeToString(virtPort->virtPortType)); switch (virtPort->virtPortType) { @@ -902,9 +902,9 @@ virVirtualPortProfileFormat(virBufferPtr buf, virUUIDFormat(virtPort->u.virtPort8021Qbg.instanceID, uuidstr); virBufferAsprintf(buf, - "%s <parameters managerid='%d' typeid='%d' " + "%*s<parameters managerid='%d' typeid='%d' " "typeidversion='%d' instanceid='%s'/>\n", - indent, + indent + 2, "", virtPort->u.virtPort8021Qbg.managerID, virtPort->u.virtPort8021Qbg.typeID, virtPort->u.virtPort8021Qbg.typeIDVersion, @@ -913,13 +913,13 @@ virVirtualPortProfileFormat(virBufferPtr buf, case VIR_VIRTUALPORT_8021QBH: virBufferAsprintf(buf, - "%s <parameters profileid='%s'/>\n", - indent, + "%*s<parameters profileid='%s'/>\n", + indent + 2, "", virtPort->u.virtPort8021Qbh.profileID); break; } - virBufferAsprintf(buf, "%s</virtualport>\n", indent); + virBufferIndentAddLit(buf, indent, "</virtualport>\n"); } static int @@ -1072,13 +1072,15 @@ virBandwidthDefFree(virBandwidthPtr def) static int virBandwidthChildDefFormat(virBufferPtr buf, virRatePtr def, - const char *elem_name) + const char *elem_name, + int indent) { if (!buf || !def || !elem_name) return -1; if (def->average) { - virBufferAsprintf(buf, "<%s average='%llu'", elem_name, def->average); + virBufferAsprintf(buf, "%*s<%s average='%llu'", indent, "", + elem_name, def->average); if (def->peak) virBufferAsprintf(buf, " peak='%llu'", def->peak); @@ -1095,17 +1097,16 @@ virBandwidthChildDefFormat(virBufferPtr buf, * virBandwidthDefFormat: * @buf: Buffer to print to * @def: Data source - * @indent: prepend all lines printed with this + * @indent: prepend all lines printed with this many spaces * * Formats bandwidth and prepend each line with @indent. - * Passing NULL to @indent is equivalent passing "". * * Returns 0 on success, else -1. */ int virBandwidthDefFormat(virBufferPtr buf, virBandwidthPtr def, - const char *indent) + int indent) { int ret = -1; @@ -1117,23 +1118,13 @@ virBandwidthDefFormat(virBufferPtr buf, goto cleanup; } - if (!indent) - indent = ""; - - virBufferAsprintf(buf, "%s<bandwidth>\n", indent); - if (def->in) { - virBufferAsprintf(buf, "%s ", indent); - if (virBandwidthChildDefFormat(buf, def->in, "inbound") < 0) - goto cleanup; - } - - if (def->out) { - virBufferAsprintf(buf, "%s ", indent); - if (virBandwidthChildDefFormat(buf, def->out, "outbound") < 0) - goto cleanup; - } + virBufferIndentAddLit(buf, indent, "<bandwidth>\n"); + if (virBandwidthChildDefFormat(buf, def->in, "inbound", indent + 2) < 0) + goto cleanup; + if (virBandwidthChildDefFormat(buf, def->out, "outbound", indent + 2) < 0) + goto cleanup; - virBufferAsprintf(buf, "%s</bandwidth>\n", indent); + virBufferIndentAddLit(buf, indent, "</bandwidth>\n"); ret = 0; diff --git a/src/util/network.h b/src/util/network.h index 4d195af..9fb9a19 100644 --- a/src/util/network.h +++ b/src/util/network.h @@ -1,7 +1,7 @@ /* * network.h: network helper APIs for libvirt * - * Copyright (C) 2009-2009 Red Hat, Inc. + * Copyright (C) 2009-2009, 2011 Red Hat, Inc. * * See COPYING.LIB for the License of this software * @@ -148,15 +148,16 @@ virVirtualPortProfileParseXML(xmlNodePtr node, void virVirtualPortProfileFormat(virBufferPtr buf, virVirtualPortProfileParamsPtr virtPort, - const char *indent); + int indent); -bool virVirtualPortProfileEqual(virVirtualPortProfileParamsPtr a, virVirtualPortProfileParamsPtr b); +bool virVirtualPortProfileEqual(virVirtualPortProfileParamsPtr a, + virVirtualPortProfileParamsPtr b); virBandwidthPtr virBandwidthDefParseNode(xmlNodePtr node); void virBandwidthDefFree(virBandwidthPtr def); int virBandwidthDefFormat(virBufferPtr buf, virBandwidthPtr def, - const char *indent); + int indent); int virBandwidthEnable(virBandwidthPtr bandwidth, const char *iface); int virBandwidthDisable(const char *iface, bool may_fail); -- 1.7.4.4
Focusing on guest disk devices. * src/conf/domain_conf.c (virDomainDiskDefFormat) (virDomainControllerDefFormat, virDomainLeaseDefFormat) (virDomainFSDefFormat): Add parameter. (virDomainDefFormatInternal): Adjust caller. * src/conf/storage_encryption_conf.c (virStorageEncryptionFormat): Simplify. --- src/conf/domain_conf.c | 126 ++++++++++++++++++++---------------- src/conf/storage_encryption_conf.c | 4 +- 2 files changed, 73 insertions(+), 57 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index be32598..a9d274b 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9064,16 +9064,19 @@ virDomainLifecycleDefFormat(virBufferPtr buf, static int virDomainLeaseDefFormat(virBufferPtr buf, - virDomainLeaseDefPtr def) + virDomainLeaseDefPtr def, + int indent) { - virBufferAddLit(buf, " <lease>\n"); - virBufferEscapeString(buf, " <lockspace>%s</lockspace>\n", def->lockspace); - virBufferEscapeString(buf, " <key>%s</key>\n", def->key); - virBufferEscapeString(buf, " <target path='%s'", def->path); + virBufferIndentAddLit(buf, indent, "<lease>\n"); + virBufferIndentEscapeString(buf, indent + 2, "<lockspace>%s</lockspace>\n", + def->lockspace); + virBufferIndentEscapeString(buf, indent + 2, "<key>%s</key>\n", def->key); + virBufferIndentEscapeString(buf, indent + 2, "<target path='%s'", + def->path); if (def->offset) virBufferAsprintf(buf, " offset='%llu'", def->offset); virBufferAddLit(buf, "/>\n"); - virBufferAddLit(buf, " </lease>\n"); + virBufferIndentAddLit(buf, indent, "</lease>\n"); return 0; } @@ -9081,6 +9084,7 @@ virDomainLeaseDefFormat(virBufferPtr buf, static int virDomainDiskDefFormat(virBufferPtr buf, virDomainDiskDefPtr def, + int indent, unsigned int flags) { const char *type = virDomainDiskTypeToString(def->type); @@ -9119,7 +9123,7 @@ virDomainDiskDefFormat(virBufferPtr buf, } virBufferAsprintf(buf, - " <disk type='%s' device='%s'", + "%*s<disk type='%s' device='%s'", indent, "", type, device); if (def->snapshot && !(def->snapshot == VIR_DOMAIN_DISK_SNAPSHOT_NO && def->readonly)) @@ -9129,7 +9133,7 @@ virDomainDiskDefFormat(virBufferPtr buf, if (def->driverName || def->driverType || def->cachemode || def->ioeventfd || def->event_idx) { - virBufferAsprintf(buf, " <driver"); + virBufferIndentAddLit(buf, indent + 2, "<driver"); if (def->driverName) virBufferAsprintf(buf, " name='%s'", def->driverName); if (def->driverType) @@ -9144,42 +9148,47 @@ virDomainDiskDefFormat(virBufferPtr buf, virBufferAsprintf(buf, " ioeventfd='%s'", ioeventfd); if (def->event_idx) virBufferAsprintf(buf, " event_idx='%s'", event_idx); - virBufferAsprintf(buf, "/>\n"); + virBufferAddLit(buf, "/>\n"); } if (def->src || def->nhosts > 0) { switch (def->type) { case VIR_DOMAIN_DISK_TYPE_FILE: - virBufferEscapeString(buf, " <source file='%s'/>\n", - def->src); + virBufferIndentEscapeString(buf, indent + 2, + "<source file='%s'/>\n", + def->src); break; case VIR_DOMAIN_DISK_TYPE_BLOCK: - virBufferEscapeString(buf, " <source dev='%s'/>\n", - def->src); + virBufferIndentEscapeString(buf, indent + 2, + "<source dev='%s'/>\n", + def->src); break; case VIR_DOMAIN_DISK_TYPE_DIR: - virBufferEscapeString(buf, " <source dir='%s'/>\n", - def->src); + virBufferIndentEscapeString(buf, indent + 2, + "<source dir='%s'/>\n", + def->src); break; case VIR_DOMAIN_DISK_TYPE_NETWORK: - virBufferAsprintf(buf, " <source protocol='%s'", + virBufferAsprintf(buf, "%*s<source protocol='%s'", + indent + 2, "", virDomainDiskProtocolTypeToString(def->protocol)); if (def->src) { virBufferEscapeString(buf, " name='%s'", def->src); } if (def->nhosts == 0) { - virBufferAsprintf(buf, "/>\n"); + virBufferAddLit(buf, "/>\n"); } else { int i; - virBufferAsprintf(buf, ">\n"); + virBufferAddLit(buf, ">\n"); for (i = 0; i < def->nhosts; i++) { - virBufferEscapeString(buf, " <host name='%s'", - def->hosts[i].name); + virBufferIndentEscapeString(buf, indent + 4, + "<host name='%s'", + def->hosts[i].name); virBufferEscapeString(buf, " port='%s'/>\n", def->hosts[i].port); } - virBufferAsprintf(buf, " </source>\n"); + virBufferIndentAddLit(buf, indent + 2, "</source>\n"); } break; default: @@ -9190,28 +9199,28 @@ virDomainDiskDefFormat(virBufferPtr buf, } } - virBufferAsprintf(buf, " <target dev='%s' bus='%s'/>\n", - def->dst, bus); + virBufferAsprintf(buf, "%*s<target dev='%s' bus='%s'/>\n", + indent + 2, "", def->dst, bus); if (def->bootIndex) - virBufferAsprintf(buf, " <boot order='%d'/>\n", def->bootIndex); + virBufferAsprintf(buf, "%*s<boot order='%d'/>\n", indent + 2, "", + def->bootIndex); if (def->readonly) - virBufferAddLit(buf, " <readonly/>\n"); + virBufferIndentAddLit(buf, indent + 2, "<readonly/>\n"); if (def->shared) - virBufferAddLit(buf, " <shareable/>\n"); + virBufferIndentAddLit(buf, indent + 2, "<shareable/>\n"); if (def->transient) - virBufferAddLit(buf, " <transient/>\n"); - if (def->serial) - virBufferEscapeString(buf, " <serial>%s</serial>\n", - def->serial); + virBufferIndentAddLit(buf, indent + 2, "<transient/>\n"); + virBufferIndentEscapeString(buf, indent + 2, "<serial>%s</serial>\n", + def->serial); if (def->encryption != NULL && - virStorageEncryptionFormat(buf, def->encryption, 6) < 0) + virStorageEncryptionFormat(buf, def->encryption, indent + 2) < 0) return -1; - if (virDomainDeviceInfoFormat(buf, &def->info, 6, flags) < 0) + if (virDomainDeviceInfoFormat(buf, &def->info, indent + 2, flags) < 0) return -1; - virBufferAddLit(buf, " </disk>\n"); + virBufferIndentAddLit(buf, indent, "</disk>\n"); return 0; } @@ -9231,6 +9240,7 @@ virDomainControllerModelTypeToString(virDomainControllerDefPtr def, static int virDomainControllerDefFormat(virBufferPtr buf, virDomainControllerDefPtr def, + int indent, unsigned int flags) { const char *type = virDomainControllerTypeToString(def->type); @@ -9253,7 +9263,7 @@ virDomainControllerDefFormat(virBufferPtr buf, } virBufferAsprintf(buf, - " <controller type='%s' index='%d'", + "%*s<controller type='%s' index='%d'", indent, "", type, def->idx); if (model) { @@ -9278,9 +9288,9 @@ virDomainControllerDefFormat(virBufferPtr buf, if (virDomainDeviceInfoIsSet(&def->info, flags)) { virBufferAddLit(buf, ">\n"); - if (virDomainDeviceInfoFormat(buf, &def->info, 6, flags) < 0) + if (virDomainDeviceInfoFormat(buf, &def->info, indent + 2, flags) < 0) return -1; - virBufferAddLit(buf, " </controller>\n"); + virBufferIndentAddLit(buf, indent, "</controller>\n"); } else { virBufferAddLit(buf, "/>\n"); } @@ -9291,6 +9301,7 @@ virDomainControllerDefFormat(virBufferPtr buf, static int virDomainFSDefFormat(virBufferPtr buf, virDomainFSDefPtr def, + int indent, unsigned int flags) { const char *type = virDomainFSTypeToString(def->type); @@ -9310,42 +9321,46 @@ virDomainFSDefFormat(virBufferPtr buf, virBufferAsprintf(buf, - " <filesystem type='%s' accessmode='%s'>\n", - type, accessmode); + "%*s<filesystem type='%s' accessmode='%s'>\n", + indent, "", type, accessmode); if (def->src) { switch (def->type) { case VIR_DOMAIN_FS_TYPE_MOUNT: - virBufferEscapeString(buf, " <source dir='%s'/>\n", - def->src); + virBufferIndentEscapeString(buf, indent + 2, + "<source dir='%s'/>\n", + def->src); break; case VIR_DOMAIN_FS_TYPE_BLOCK: - virBufferEscapeString(buf, " <source dev='%s'/>\n", - def->src); + virBufferIndentEscapeString(buf, indent + 2, + "<source dev='%s'/>\n", + def->src); break; case VIR_DOMAIN_FS_TYPE_FILE: - virBufferEscapeString(buf, " <source file='%s'/>\n", - def->src); + virBufferIndentEscapeString(buf, indent + 2, + "<source file='%s'/>\n", + def->src); break; case VIR_DOMAIN_FS_TYPE_TEMPLATE: - virBufferEscapeString(buf, " <source name='%s'/>\n", - def->src); + virBufferIndentEscapeString(buf, indent + 2, + "<source name='%s'/>\n", + def->src); } } - virBufferEscapeString(buf, " <target dir='%s'/>\n", - def->dst); + virBufferIndentEscapeString(buf, indent + 2, "<target dir='%s'/>\n", + def->dst); if (def->readonly) - virBufferAddLit(buf, " <readonly/>\n"); + virBufferIndentAddLit(buf, indent + 2, "<readonly/>\n"); - if (virDomainDeviceInfoFormat(buf, &def->info, 6, flags) < 0) + if (virDomainDeviceInfoFormat(buf, &def->info, indent + 2, flags) < 0) return -1; - virBufferAddLit(buf, " </filesystem>\n"); + virBufferIndentAddLit(buf, indent, "</filesystem>\n"); return 0; } @@ -10771,19 +10786,20 @@ virDomainDefFormatInternal(virDomainDefPtr def, def->emulator); for (n = 0 ; n < def->ndisks ; n++) - if (virDomainDiskDefFormat(buf, def->disks[n], flags) < 0) /* XXX indent */ + if (virDomainDiskDefFormat(buf, def->disks[n], indent, flags) < 0) goto cleanup; for (n = 0 ; n < def->ncontrollers ; n++) - if (virDomainControllerDefFormat(buf, def->controllers[n], flags) < 0) /* XXX indent */ + if (virDomainControllerDefFormat(buf, def->controllers[n], indent, + flags) < 0) goto cleanup; for (n = 0 ; n < def->nleases ; n++) - if (virDomainLeaseDefFormat(buf, def->leases[n]) < 0) /* XXX indent */ + if (virDomainLeaseDefFormat(buf, def->leases[n], indent) < 0) goto cleanup; for (n = 0 ; n < def->nfss ; n++) - if (virDomainFSDefFormat(buf, def->fss[n], flags) < 0) /* XXX indent */ + if (virDomainFSDefFormat(buf, def->fss[n], indent, flags) < 0) goto cleanup; diff --git a/src/conf/storage_encryption_conf.c b/src/conf/storage_encryption_conf.c index 73e16ed..7d77893 100644 --- a/src/conf/storage_encryption_conf.c +++ b/src/conf/storage_encryption_conf.c @@ -1,7 +1,7 @@ /* * storage_encryption_conf.c: volume encryption information * - * Copyright (C) 2009-2010 Red Hat, Inc. + * Copyright (C) 2009-2011 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -253,7 +253,7 @@ virStorageEncryptionFormat(virBufferPtr buf, return -1; } - virBufferAsprintf(buf, "%*s</encryption>\n", indent, ""); + virBufferIndentAddLit(buf, indent, "</encryption>\n"); return 0; } -- 1.7.4.4
Networking definitions. Fixing this involved some refactoring of common code out of domain_conf and nwfilter_conf into nwfilter_params. * src/conf/nwfilter_params.h (virNWFilterFormatParamAttributes): Adjust signature. * src/conf/nwfilter_params.c (_formatParameterAttrs) (virNWFilterFormatParamAttributes): Adjust indentation. * src/conf/domain_conf.c (virDomainNetDefFormat) (virDomainActualNetDefFormat): Add parameter. (virDomainDefFormatInternal): Adjust caller. * src/conf/nwfilter_conf.c (virNWFilterIncludeDefFormat): Likewise. --- src/conf/domain_conf.c | 127 +++++++++++++++++++++---------------------- src/conf/nwfilter_conf.c | 16 ++---- src/conf/nwfilter_params.c | 41 ++++++++------ src/conf/nwfilter_params.h | 8 ++- 4 files changed, 97 insertions(+), 95 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a9d274b..64bb337 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9367,7 +9367,8 @@ virDomainFSDefFormat(virBufferPtr buf, static int virDomainActualNetDefFormat(virBufferPtr buf, - virDomainActualNetDefPtr def) + virDomainActualNetDefPtr def, + int indent) { int ret = -1; const char *type; @@ -9389,18 +9390,17 @@ virDomainActualNetDefFormat(virBufferPtr buf, _("unexpected net type %s"), type); goto error; } - virBufferAsprintf(buf, " <actual type='%s'>\n", type); + virBufferAsprintf(buf, "%*s<actual type='%s'>\n", indent, "", type); switch (def->type) { case VIR_DOMAIN_NET_TYPE_BRIDGE: - if (def->data.bridge.brname) { - virBufferEscapeString(buf, " <source bridge='%s'/>\n", - def->data.bridge.brname); - } + virBufferIndentEscapeString(buf, indent + 2, + "<source bridge='%s'/>\n", + def->data.bridge.brname); break; case VIR_DOMAIN_NET_TYPE_DIRECT: - virBufferAddLit(buf, " <source"); + virBufferIndentAddLit(buf, indent + 2, "<source"); if (def->data.direct.linkdev) virBufferEscapeString(buf, " dev='%s'", def->data.direct.linkdev); @@ -9413,16 +9413,17 @@ virDomainActualNetDefFormat(virBufferPtr buf, return ret; } virBufferAsprintf(buf, " mode='%s'/>\n", mode); - virVirtualPortProfileFormat(buf, def->data.direct.virtPortProfile, 8); + virVirtualPortProfileFormat(buf, def->data.direct.virtPortProfile, + indent + 2); break; default: break; } - if (virBandwidthDefFormat(buf, def->bandwidth, 6) < 0) + if (virBandwidthDefFormat(buf, def->bandwidth, indent + 2) < 0) goto error; - virBufferAddLit(buf, " </actual>\n"); + virBufferIndentAddLit(buf, indent, "</actual>\n"); ret = 0; error: @@ -9432,10 +9433,10 @@ error: static int virDomainNetDefFormat(virBufferPtr buf, virDomainNetDefPtr def, + int indent, unsigned int flags) { const char *type = virDomainNetTypeToString(def->type); - char *attrs; if (!type) { virDomainReportError(VIR_ERR_INTERNAL_ERROR, @@ -9443,74 +9444,74 @@ virDomainNetDefFormat(virBufferPtr buf, return -1; } - virBufferAsprintf(buf, " <interface type='%s'>\n", type); + virBufferAsprintf(buf, "%*s<interface type='%s'>\n", indent, "", type); virBufferAsprintf(buf, - " <mac address='%02x:%02x:%02x:%02x:%02x:%02x'/>\n", + "%*s<mac address='%02x:%02x:%02x:%02x:%02x:%02x'/>\n", + indent + 2, "", def->mac[0], def->mac[1], def->mac[2], def->mac[3], def->mac[4], def->mac[5]); switch (def->type) { case VIR_DOMAIN_NET_TYPE_NETWORK: - virBufferEscapeString(buf, " <source network='%s'", - def->data.network.name); - if (def->data.network.portgroup) { - virBufferEscapeString(buf, " portgroup='%s'", - def->data.network.portgroup); - } + virBufferIndentEscapeString(buf, indent + 2, "<source network='%s'", + def->data.network.name); + virBufferEscapeString(buf, " portgroup='%s'", + def->data.network.portgroup); virBufferAddLit(buf, "/>\n"); - virVirtualPortProfileFormat(buf, def->data.network.virtPortProfile, 6); + virVirtualPortProfileFormat(buf, def->data.network.virtPortProfile, + indent + 2); if ((flags & VIR_DOMAIN_XML_INTERNAL_ACTUAL_NET) && - (virDomainActualNetDefFormat(buf, def->data.network.actual) < 0)) + (virDomainActualNetDefFormat(buf, def->data.network.actual, + indent + 2) < 0)) return -1; break; case VIR_DOMAIN_NET_TYPE_ETHERNET: - if (def->data.ethernet.dev) - virBufferEscapeString(buf, " <source dev='%s'/>\n", - def->data.ethernet.dev); + virBufferIndentEscapeString(buf, indent + 2, "<source dev='%s'/>\n", + def->data.ethernet.dev); if (def->data.ethernet.ipaddr) - virBufferAsprintf(buf, " <ip address='%s'/>\n", + virBufferAsprintf(buf, "%*s<ip address='%s'/>\n", indent + 2, "", def->data.ethernet.ipaddr); - if (def->data.ethernet.script) - virBufferEscapeString(buf, " <script path='%s'/>\n", - def->data.ethernet.script); + virBufferIndentEscapeString(buf, indent + 2, "<script path='%s'/>\n", + def->data.ethernet.script); break; case VIR_DOMAIN_NET_TYPE_BRIDGE: - virBufferEscapeString(buf, " <source bridge='%s'/>\n", - def->data.bridge.brname); + virBufferIndentEscapeString(buf, indent + 2, "<source bridge='%s'/>\n", + def->data.bridge.brname); if (def->data.bridge.ipaddr) - virBufferAsprintf(buf, " <ip address='%s'/>\n", + virBufferAsprintf(buf, "%*s<ip address='%s'/>\n", indent + 2, "", def->data.bridge.ipaddr); - if (def->data.bridge.script) - virBufferEscapeString(buf, " <script path='%s'/>\n", - def->data.bridge.script); + virBufferIndentEscapeString(buf, indent + 2, "<script path='%s'/>\n", + def->data.bridge.script); break; case VIR_DOMAIN_NET_TYPE_SERVER: case VIR_DOMAIN_NET_TYPE_CLIENT: case VIR_DOMAIN_NET_TYPE_MCAST: if (def->data.socket.address) - virBufferAsprintf(buf, " <source address='%s' port='%d'/>\n", + virBufferAsprintf(buf, "%*s<source address='%s' port='%d'/>\n", + indent + 2, "", def->data.socket.address, def->data.socket.port); else - virBufferAsprintf(buf, " <source port='%d'/>\n", + virBufferAsprintf(buf, "%*s<source port='%d'/>\n", indent + 2, "", def->data.socket.port); break; case VIR_DOMAIN_NET_TYPE_INTERNAL: - virBufferEscapeString(buf, " <source name='%s'/>\n", - def->data.internal.name); + virBufferIndentEscapeString(buf, indent + 2, "<source name='%s'/>\n", + def->data.internal.name); break; case VIR_DOMAIN_NET_TYPE_DIRECT: - virBufferEscapeString(buf, " <source dev='%s'", - def->data.direct.linkdev); + virBufferIndentEscapeString(buf, indent + 2, "<source dev='%s'", + def->data.direct.linkdev); virBufferAsprintf(buf, " mode='%s'", - virMacvtapModeTypeToString(def->data.direct.mode)); + virMacvtapModeTypeToString(def->data.direct.mode)); virBufferAddLit(buf, "/>\n"); - virVirtualPortProfileFormat(buf, def->data.direct.virtPortProfile, 6); + virVirtualPortProfileFormat(buf, def->data.direct.virtPortProfile, + indent + 2); break; case VIR_DOMAIN_NET_TYPE_USER: @@ -9523,15 +9524,15 @@ virDomainNetDefFormat(virBufferPtr buf, !((flags & VIR_DOMAIN_XML_INACTIVE) && (STRPREFIX(def->ifname, VIR_NET_GENERATED_PREFIX)))) { /* Skip auto-generated target names for inactive config. */ - virBufferEscapeString(buf, " <target dev='%s'/>\n", - def->ifname); + virBufferIndentEscapeString(buf, indent + 2, "<target dev='%s'/>\n", + def->ifname); } if (def->model) { - virBufferEscapeString(buf, " <model type='%s'/>\n", - def->model); + virBufferIndentEscapeString(buf, indent + 2, "<model type='%s'/>\n", + def->model); if (STREQ(def->model, "virtio") && (def->driver.virtio.name || def->driver.virtio.txmode)) { - virBufferAddLit(buf, " <driver"); + virBufferIndentAddLit(buf, indent + 2, "<driver"); if (def->driver.virtio.name) { virBufferAsprintf(buf, " name='%s'", virDomainNetBackendTypeToString(def->driver.virtio.name)); @@ -9552,36 +9553,32 @@ virDomainNetDefFormat(virBufferPtr buf, } } if (def->filter) { - virBufferEscapeString(buf, " <filterref filter='%s'", - def->filter); - attrs = virNWFilterFormatParamAttributes(def->filterparams, - " "); - if (!attrs || strlen(attrs) <= 1) - virBufferAddLit(buf, "/>\n"); - else - virBufferAsprintf(buf, ">\n%s </filterref>\n", attrs); - VIR_FREE(attrs); + if (virNWFilterFormatParamAttributes(buf, def->filterparams, + def->filter, indent + 4) < 0) + return -1; } if (def->bootIndex) - virBufferAsprintf(buf, " <boot order='%d'/>\n", def->bootIndex); + virBufferAsprintf(buf, "%*s<boot order='%d'/>\n", indent + 2, "", + def->bootIndex); if (def->tune.sndbuf_specified) { - virBufferAddLit(buf, " <tune>\n"); - virBufferAsprintf(buf, " <sndbuf>%lu</sndbuf>\n", def->tune.sndbuf); - virBufferAddLit(buf, " </tune>\n"); + virBufferIndentAddLit(buf, indent + 2, "<tune>\n"); + virBufferAsprintf(buf, "%*s<sndbuf>%lu</sndbuf>\n", indent + 4, "", + def->tune.sndbuf); + virBufferIndentAddLit(buf, indent + 2, "</tune>\n"); } if (def->linkstate) - virBufferAsprintf(buf, " <link state='%s'/>\n", + virBufferAsprintf(buf, "%*s<link state='%s'/>\n", indent + 2, "", virDomainNetInterfaceLinkStateTypeToString(def->linkstate)); - if (virBandwidthDefFormat(buf, def->bandwidth, 6) < 0) + if (virBandwidthDefFormat(buf, def->bandwidth, indent + 2) < 0) return -1; - if (virDomainDeviceInfoFormat(buf, &def->info, 6, flags) < 0) + if (virDomainDeviceInfoFormat(buf, &def->info, indent + 2, flags) < 0) return -1; - virBufferAddLit(buf, " </interface>\n"); + virBufferIndentAddLit(buf, indent, "</interface>\n"); return 0; } @@ -10804,7 +10801,7 @@ virDomainDefFormatInternal(virDomainDefPtr def, for (n = 0 ; n < def->nnets ; n++) - if (virDomainNetDefFormat(buf, def->nets[n], flags) < 0) /* XXX indent */ + if (virDomainNetDefFormat(buf, def->nets[n], indent, flags) < 0) goto cleanup; for (n = 0 ; n < def->nsmartcards ; n++) diff --git a/src/conf/nwfilter_conf.c b/src/conf/nwfilter_conf.c index 08ede48..20f97f2 100644 --- a/src/conf/nwfilter_conf.c +++ b/src/conf/nwfilter_conf.c @@ -2849,19 +2849,13 @@ no_memory: static char * virNWFilterIncludeDefFormat(virNWFilterIncludeDefPtr inc) { - char *attrs; virBuffer buf = VIR_BUFFER_INITIALIZER; - virBufferAsprintf(&buf," <filterref filter='%s'", - inc->filterref); - - attrs = virNWFilterFormatParamAttributes(inc->params, " "); - - if (!attrs || strlen(attrs) <= 1) - virBufferAddLit(&buf, "/>\n"); - else - virBufferAsprintf(&buf, ">\n%s </filterref>\n", attrs); - + if (virNWFilterFormatParamAttributes(&buf, inc->params, inc->filterref, + 2) < 0) { + virBufferFreeAndReset(&buf); + return NULL; + } if (virBufferError(&buf)) { virReportOOMError(); virBufferFreeAndReset(&buf); diff --git a/src/conf/nwfilter_params.c b/src/conf/nwfilter_params.c index ee10b21..601a61c 100644 --- a/src/conf/nwfilter_params.c +++ b/src/conf/nwfilter_params.c @@ -260,7 +260,7 @@ skip_entry: struct formatterParam { virBufferPtr buf; - const char *indent; + int indent; }; @@ -269,30 +269,37 @@ _formatParameterAttrs(void *payload, const void *name, void *data) { struct formatterParam *fp = (struct formatterParam *)data; - virBufferAsprintf(fp->buf, "%s<parameter name='%s' value='%s'/>\n", - fp->indent, + virBufferAsprintf(fp->buf, "%*s<parameter name='%s' value='%s'/>\n", + fp->indent, "", (const char *)name, (char *)payload); } -char * -virNWFilterFormatParamAttributes(virNWFilterHashTablePtr table, - const char *indent) +int +virNWFilterFormatParamAttributes(virBufferPtr buf, + virNWFilterHashTablePtr table, + const char *filterref, + int indent) { - virBuffer buf = VIR_BUFFER_INITIALIZER; struct formatterParam fp = { - .buf = &buf, - .indent = indent, + .buf = buf, + .indent = indent + 2, }; + int count = virHashSize(table->hashTable); - virHashForEach(table->hashTable, _formatParameterAttrs, &fp); - - if (virBufferError(&buf)) { - virReportOOMError(); - virBufferFreeAndReset(&buf); - return NULL; + if (count < 0) { + virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("missing filter parameter table")); + return -1; } - - return virBufferContentAndReset(&buf); + virBufferAsprintf(buf, "%*s<filterref filter='%s'", indent, "", filterref); + if (count) { + virBufferAddLit(buf, ">\n"); + virHashForEach(table->hashTable, _formatParameterAttrs, &fp); + virBufferIndentAddLit(buf, indent, "</filterref>\n"); + } else { + virBufferAddLit(buf, "/>\n"); + } + return 0; } diff --git a/src/conf/nwfilter_params.h b/src/conf/nwfilter_params.h index 012d6a1..889c351 100644 --- a/src/conf/nwfilter_params.h +++ b/src/conf/nwfilter_params.h @@ -1,6 +1,7 @@ /* * nwfilter_params.h: parsing and data maintenance of filter parameters * + * Copyright (C) 2011 Red Hat, Inc. * Copyright (C) 2010 IBM Corporation * * This library is free software; you can redistribute it and/or @@ -23,6 +24,7 @@ # define NWFILTER_PARAMS_H # include "hash.h" +# include "buf.h" typedef struct _virNWFilterHashTable virNWFilterHashTable; typedef virNWFilterHashTable *virNWFilterHashTablePtr; @@ -35,8 +37,10 @@ struct _virNWFilterHashTable { virNWFilterHashTablePtr virNWFilterParseParamAttributes(xmlNodePtr cur); -char * virNWFilterFormatParamAttributes(virNWFilterHashTablePtr table, - const char *indent); +int virNWFilterFormatParamAttributes(virBufferPtr buf, + virNWFilterHashTablePtr table, + const char *filterref, + int indent); virNWFilterHashTablePtr virNWFilterHashTableCreate(int n); void virNWFilterHashTableFree(virNWFilterHashTablePtr table); -- 1.7.4.4
All character-like devices. * src/conf/domain_conf.c (virDomainSmartcardDefFormat) (virDomainChrSourceDefFormat, virDomainChrDefFormat) (virDomainRedirdevDefFormat): Alter signature. (virDomainDefFormatInternal): Adjust caller. --- src/conf/domain_conf.c | 104 +++++++++++++++++++++++++++--------------------- 1 files changed, 58 insertions(+), 46 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 64bb337..16feb27 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9590,6 +9590,7 @@ static int virDomainChrSourceDefFormat(virBufferPtr buf, virDomainChrSourceDefPtr def, bool tty_compat, + int indent, unsigned int flags) { const char *type = virDomainChrTypeToString(def->type); @@ -9600,7 +9601,7 @@ virDomainChrSourceDefFormat(virBufferPtr buf, return -1; } - /* Compat with legacy <console tty='/dev/pts/5'/> syntax */ + /* Compat with legacy <console tty='/dev/pts/5'/> syntax */ virBufferAsprintf(buf, " type='%s'", type); if (tty_compat) { virBufferEscapeString(buf, " tty='%s'", @@ -9623,8 +9624,9 @@ virDomainChrSourceDefFormat(virBufferPtr buf, if (def->type != VIR_DOMAIN_CHR_TYPE_PTY || (def->data.file.path && !(flags & VIR_DOMAIN_XML_INACTIVE))) { - virBufferEscapeString(buf, " <source path='%s'/>\n", - def->data.file.path); + virBufferIndentEscapeString(buf, indent + 2, + "<source path='%s'/>\n", + def->data.file.path); } break; @@ -9632,48 +9634,49 @@ virDomainChrSourceDefFormat(virBufferPtr buf, if (def->data.udp.bindService && def->data.udp.bindHost) { virBufferAsprintf(buf, - " <source mode='bind' host='%s' " - "service='%s'/>\n", + "%*s<source mode='bind' host='%s' " + "service='%s'/>\n", indent + 2, "", def->data.udp.bindHost, def->data.udp.bindService); } else if (def->data.udp.bindHost) { - virBufferAsprintf(buf, " <source mode='bind' host='%s'/>\n", - def->data.udp.bindHost); + virBufferAsprintf(buf, "%*s<source mode='bind' host='%s'/>\n", + indent + 2, "", def->data.udp.bindHost); } else if (def->data.udp.bindService) { - virBufferAsprintf(buf, " <source mode='bind' service='%s'/>\n", - def->data.udp.bindService); + virBufferAsprintf(buf, "%*s<source mode='bind' service='%s'/>\n", + indent + 2, "", def->data.udp.bindService); } if (def->data.udp.connectService && def->data.udp.connectHost) { virBufferAsprintf(buf, - " <source mode='connect' host='%s' " - "service='%s'/>\n", + "%*s<source mode='connect' host='%s' " + "service='%s'/>\n", indent + 2, "", def->data.udp.connectHost, def->data.udp.connectService); } else if (def->data.udp.connectHost) { - virBufferAsprintf(buf, " <source mode='connect' host='%s'/>\n", - def->data.udp.connectHost); + virBufferAsprintf(buf, "%*s<source mode='connect' host='%s'/>\n", + indent + 2, "", def->data.udp.connectHost); } else if (def->data.udp.connectService) { virBufferAsprintf(buf, - " <source mode='connect' service='%s'/>\n", - def->data.udp.connectService); + "%*s<source mode='connect' service='%s'/>\n", + indent + 2, "", def->data.udp.connectService); } break; case VIR_DOMAIN_CHR_TYPE_TCP: virBufferAsprintf(buf, - " <source mode='%s' host='%s' service='%s'/>\n", + "%*s<source mode='%s' host='%s' service='%s'/>\n", + indent + 2, "", def->data.tcp.listen ? "bind" : "connect", def->data.tcp.host, def->data.tcp.service); - virBufferAsprintf(buf, " <protocol type='%s'/>\n", + virBufferAsprintf(buf, "%*s<protocol type='%s'/>\n", indent + 2, "", virDomainChrTcpProtocolTypeToString( def->data.tcp.protocol)); break; case VIR_DOMAIN_CHR_TYPE_UNIX: - virBufferAsprintf(buf, " <source mode='%s'", + virBufferAsprintf(buf, "%*s<source mode='%s'", indent + 2, "", def->data.nix.listen ? "bind" : "connect"); virBufferEscapeString(buf, " path='%s'/>\n", def->data.nix.path); @@ -9686,6 +9689,7 @@ virDomainChrSourceDefFormat(virBufferPtr buf, static int virDomainChrDefFormat(virBufferPtr buf, virDomainChrDefPtr def, + int indent, unsigned int flags) { const char *elementName = virDomainChrDeviceTypeToString(def->deviceType); @@ -9702,13 +9706,14 @@ virDomainChrDefFormat(virBufferPtr buf, return -1; } - virBufferAsprintf(buf, " <%s", elementName); + virBufferAsprintf(buf, "%*s<%s", indent, "", elementName); tty_compat = (def->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE && def->target.port == 0 && def->source.type == VIR_DOMAIN_CHR_TYPE_PTY && !(flags & VIR_DOMAIN_XML_INACTIVE) && def->source.data.file.path); - if (virDomainChrSourceDefFormat(buf, &def->source, tty_compat, flags) < 0) + if (virDomainChrSourceDefFormat(buf, &def->source, tty_compat, indent, + flags) < 0) return -1; /* Format <target> block */ @@ -9719,7 +9724,8 @@ virDomainChrDefFormat(virBufferPtr buf, _("Could not format channel target type")); return -1; } - virBufferAsprintf(buf, " <target type='%s'", targetType); + virBufferAsprintf(buf, "%*s<target type='%s'", indent + 2, "", + targetType); switch (def->targetType) { case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_GUESTFWD: { @@ -9754,25 +9760,25 @@ virDomainChrDefFormat(virBufferPtr buf, case VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE: virBufferAsprintf(buf, - " <target type='%s' port='%d'/>\n", + "%*s<target type='%s' port='%d'/>\n", + indent + 2, "", virDomainChrTargetTypeToString(def->deviceType, def->targetType), def->target.port); break; default: - virBufferAsprintf(buf, " <target port='%d'/>\n", + virBufferAsprintf(buf, "%*s<target port='%d'/>\n", indent + 2, "", def->target.port); break; } if (virDomainDeviceInfoIsSet(&def->info, flags)) { - if (virDomainDeviceInfoFormat(buf, &def->info, 6, flags) < 0) + if (virDomainDeviceInfoFormat(buf, &def->info, indent + 2, flags) < 0) return -1; } - virBufferAsprintf(buf, " </%s>\n", - elementName); + virBufferAsprintf(buf, "%*s</%s>\n", indent, "", elementName); return ret; } @@ -9780,6 +9786,7 @@ virDomainChrDefFormat(virBufferPtr buf, static int virDomainSmartcardDefFormat(virBufferPtr buf, virDomainSmartcardDefPtr def, + int indent, unsigned int flags) { const char *mode = virDomainSmartcardTypeToString(def->type); @@ -9791,7 +9798,7 @@ virDomainSmartcardDefFormat(virBufferPtr buf, return -1; } - virBufferAsprintf(buf, " <smartcard mode='%s'", mode); + virBufferAsprintf(buf, "%*s<smartcard mode='%s'", indent, "", mode); switch (def->type) { case VIR_DOMAIN_SMARTCARD_TYPE_HOST: if (!virDomainDeviceInfoIsSet(&def->info, flags)) { @@ -9804,16 +9811,17 @@ virDomainSmartcardDefFormat(virBufferPtr buf, case VIR_DOMAIN_SMARTCARD_TYPE_HOST_CERTIFICATES: virBufferAddLit(buf, ">\n"); for (i = 0; i < VIR_DOMAIN_SMARTCARD_NUM_CERTIFICATES; i++) - virBufferEscapeString(buf, " <certificate>%s</certificate>\n", - def->data.cert.file[i]); - if (def->data.cert.database) - virBufferEscapeString(buf, " <database>%s</database>\n", - def->data.cert.database); + virBufferIndentEscapeString(buf, indent + 2, + "<certificate>%s</certificate>\n", + def->data.cert.file[i]); + virBufferIndentEscapeString(buf, indent + 2, + "<database>%s</database>\n", + def->data.cert.database); break; case VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH: if (virDomainChrSourceDefFormat(buf, &def->data.passthru, false, - flags) < 0) + indent, flags) < 0) return -1; break; @@ -9822,9 +9830,9 @@ virDomainSmartcardDefFormat(virBufferPtr buf, _("unexpected smartcard type %d"), def->type); return -1; } - if (virDomainDeviceInfoFormat(buf, &def->info, 6, flags) < 0) + if (virDomainDeviceInfoFormat(buf, &def->info, indent + 2, flags) < 0) return -1; - virBufferAddLit(buf, " </smartcard>\n"); + virBufferIndentAddLit(buf, indent, "</smartcard>\n"); return 0; } @@ -10412,18 +10420,20 @@ virDomainHostdevDefFormat(virBufferPtr buf, static int virDomainRedirdevDefFormat(virBufferPtr buf, virDomainRedirdevDefPtr def, + int indent, unsigned int flags) { const char *bus; bus = virDomainRedirdevBusTypeToString(def->bus); - virBufferAsprintf(buf, " <redirdev bus='%s'", bus); - if (virDomainChrSourceDefFormat(buf, &def->source.chr, false, flags) < 0) + virBufferAsprintf(buf, "%*s<redirdev bus='%s'", indent, "", bus); + if (virDomainChrSourceDefFormat(buf, &def->source.chr, false, indent, + flags) < 0) return -1; - if (virDomainDeviceInfoFormat(buf, &def->info, 6, flags) < 0) + if (virDomainDeviceInfoFormat(buf, &def->info, indent + 2, flags) < 0) return -1; - virBufferAddLit(buf, " </redirdev>\n"); + virBufferIndentAddLit(buf, indent, "</redirdev>\n"); return 0; } @@ -10805,20 +10815,21 @@ virDomainDefFormatInternal(virDomainDefPtr def, goto cleanup; for (n = 0 ; n < def->nsmartcards ; n++) - if (virDomainSmartcardDefFormat(buf, def->smartcards[n], flags) < 0) /* XXX indent */ + if (virDomainSmartcardDefFormat(buf, def->smartcards[n], indent, + flags) < 0) goto cleanup; for (n = 0 ; n < def->nserials ; n++) - if (virDomainChrDefFormat(buf, def->serials[n], flags) < 0) /* XXX indent */ + if (virDomainChrDefFormat(buf, def->serials[n], indent, flags) < 0) goto cleanup; for (n = 0 ; n < def->nparallels ; n++) - if (virDomainChrDefFormat(buf, def->parallels[n], flags) < 0) /* XXX indent */ + if (virDomainChrDefFormat(buf, def->parallels[n], indent, flags) < 0) goto cleanup; /* If there's a PV console that's preferred.. */ if (def->console) { - if (virDomainChrDefFormat(buf, def->console, flags) < 0) /* XXX indent */ + if (virDomainChrDefFormat(buf, def->console, indent, flags) < 0) goto cleanup; } else if (def->nserials != 0) { /* ..else for legacy compat duplicate the first serial device as a @@ -10826,12 +10837,12 @@ virDomainDefFormatInternal(virDomainDefPtr def, virDomainChrDef console; memcpy(&console, def->serials[0], sizeof(console)); console.deviceType = VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE; - if (virDomainChrDefFormat(buf, &console, flags) < 0) /* XXX indent */ + if (virDomainChrDefFormat(buf, &console, indent, flags) < 0) goto cleanup; } for (n = 0 ; n < def->nchannels ; n++) - if (virDomainChrDefFormat(buf, def->channels[n], flags) < 0) /* XXX indent */ + if (virDomainChrDefFormat(buf, def->channels[n], indent, flags) < 0) goto cleanup; for (n = 0 ; n < def->ninputs ; n++) @@ -10869,7 +10880,8 @@ virDomainDefFormatInternal(virDomainDefPtr def, goto cleanup; for (n = 0 ; n < def->nredirdevs ; n++) - if (virDomainRedirdevDefFormat(buf, def->redirdevs[n], flags) < 0) /* XXX indent */ + if (virDomainRedirdevDefFormat(buf, def->redirdevs[n], indent, + flags) < 0) goto cleanup; for (n = 0 ; n < def->nhubs ; n++) -- 1.7.4.4
All remaining aspects of <domain> indentation. * src/conf/domain_conf.c (virDomainInputDefFormat) (virDomainGraphicsDefFormat, virDomainGraphicsListenDefFormat) (virDomainSoundDefFormat, virDomainVideoDefFormat) (virDomainVideoAccelDefFormat, virDomainHostdevDefFormat) (virDomainHubDefFormat, virDomainWatchdogDefFormat): Alter signature. (virDomainDefFormatInternal): Adjust caller. --- src/conf/domain_conf.c | 140 ++++++++++++++++++++++++++++------------------- 1 files changed, 83 insertions(+), 57 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 16feb27..eb5a33e 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9839,6 +9839,7 @@ virDomainSmartcardDefFormat(virBufferPtr buf, static int virDomainSoundDefFormat(virBufferPtr buf, virDomainSoundDefPtr def, + int indent, unsigned int flags) { const char *model = virDomainSoundModelTypeToString(def->model); @@ -9849,14 +9850,13 @@ virDomainSoundDefFormat(virBufferPtr buf, return -1; } - virBufferAsprintf(buf, " <sound model='%s'", - model); + virBufferAsprintf(buf, "%*s<sound model='%s'", indent, "", model); if (virDomainDeviceInfoIsSet(&def->info, flags)) { virBufferAddLit(buf, ">\n"); - if (virDomainDeviceInfoFormat(buf, &def->info, 6, flags) < 0) + if (virDomainDeviceInfoFormat(buf, &def->info, indent + 2, flags) < 0) return -1; - virBufferAddLit(buf, " </sound>\n"); + virBufferIndentAddLit(buf, indent, "</sound>\n"); } else { virBufferAddLit(buf, "/>\n"); } @@ -9868,6 +9868,7 @@ virDomainSoundDefFormat(virBufferPtr buf, static int virDomainMemballoonDefFormat(virBufferPtr buf, virDomainMemballoonDefPtr def, + int indent, unsigned int flags) { const char *model = virDomainMemballoonModelTypeToString(def->model); @@ -9878,14 +9879,14 @@ virDomainMemballoonDefFormat(virBufferPtr buf, return -1; } - virBufferAsprintf(buf, " <memballoon model='%s'", + virBufferAsprintf(buf, "%*s<memballoon model='%s'", indent, "", model); if (virDomainDeviceInfoIsSet(&def->info, flags)) { virBufferAddLit(buf, ">\n"); - if (virDomainDeviceInfoFormat(buf, &def->info, 6, flags) < 0) + if (virDomainDeviceInfoFormat(buf, &def->info, indent + 2, flags) < 0) return -1; - virBufferAddLit(buf, " </memballoon>\n"); + virBufferIndentAddLit(buf, indent, "</memballoon>\n"); } else { virBufferAddLit(buf, "/>\n"); } @@ -9911,6 +9912,7 @@ virDomainSysinfoDefFormat(virBufferPtr buf, static int virDomainWatchdogDefFormat(virBufferPtr buf, virDomainWatchdogDefPtr def, + int indent, unsigned int flags) { const char *model = virDomainWatchdogModelTypeToString (def->model); @@ -9928,14 +9930,14 @@ virDomainWatchdogDefFormat(virBufferPtr buf, return -1; } - virBufferAsprintf(buf, " <watchdog model='%s' action='%s'", - model, action); + virBufferAsprintf(buf, "%*s<watchdog model='%s' action='%s'", + indent, "", model, action); if (virDomainDeviceInfoIsSet(&def->info, flags)) { virBufferAddLit(buf, ">\n"); - if (virDomainDeviceInfoFormat(buf, &def->info, 6, flags) < 0) + if (virDomainDeviceInfoFormat(buf, &def->info, indent + 2, flags) < 0) return -1; - virBufferAddLit(buf, " </watchdog>\n"); + virBufferIndentAddLit(buf, indent, "</watchdog>\n"); } else { virBufferAddLit(buf, "/>\n"); } @@ -9946,9 +9948,10 @@ virDomainWatchdogDefFormat(virBufferPtr buf, static void virDomainVideoAccelDefFormat(virBufferPtr buf, - virDomainVideoAccelDefPtr def) + virDomainVideoAccelDefPtr def, + int indent) { - virBufferAsprintf(buf, " <acceleration accel3d='%s'", + virBufferAsprintf(buf, "%*s<acceleration accel3d='%s'", indent, "", def->support3d ? "yes" : "no"); virBufferAsprintf(buf, " accel2d='%s'", def->support2d ? "yes" : "no"); @@ -9959,6 +9962,7 @@ virDomainVideoAccelDefFormat(virBufferPtr buf, static int virDomainVideoDefFormat(virBufferPtr buf, virDomainVideoDefPtr def, + int indent, unsigned int flags) { const char *model = virDomainVideoTypeToString(def->type); @@ -9969,8 +9973,8 @@ virDomainVideoDefFormat(virBufferPtr buf, return -1; } - virBufferAddLit(buf, " <video>\n"); - virBufferAsprintf(buf, " <model type='%s'", + virBufferIndentAddLit(buf, indent, "<video>\n"); + virBufferAsprintf(buf, "%*s<model type='%s'", indent + 2, "", model); if (def->vram) virBufferAsprintf(buf, " vram='%u'", def->vram); @@ -9978,16 +9982,16 @@ virDomainVideoDefFormat(virBufferPtr buf, virBufferAsprintf(buf, " heads='%u'", def->heads); if (def->accel) { virBufferAddLit(buf, ">\n"); - virDomainVideoAccelDefFormat(buf, def->accel); - virBufferAddLit(buf, " </model>\n"); + virDomainVideoAccelDefFormat(buf, def->accel, indent + 4); + virBufferIndentAddLit(buf, indent + 2, "</model>\n"); } else { virBufferAddLit(buf, "/>\n"); } - if (virDomainDeviceInfoFormat(buf, &def->info, 6, flags) < 0) + if (virDomainDeviceInfoFormat(buf, &def->info, indent + 2, flags) < 0) return -1; - virBufferAddLit(buf, " </video>\n"); + virBufferIndentAddLit(buf, indent, "</video>\n"); return 0; } @@ -9995,6 +9999,7 @@ virDomainVideoDefFormat(virBufferPtr buf, static int virDomainInputDefFormat(virBufferPtr buf, virDomainInputDefPtr def, + int indent, unsigned int flags) { const char *type = virDomainInputTypeToString(def->type); @@ -10011,14 +10016,14 @@ virDomainInputDefFormat(virBufferPtr buf, return -1; } - virBufferAsprintf(buf, " <input type='%s' bus='%s'", + virBufferAsprintf(buf, "%*s<input type='%s' bus='%s'", indent, "", type, bus); if (virDomainDeviceInfoIsSet(&def->info, flags)) { virBufferAddLit(buf, ">\n"); - if (virDomainDeviceInfoFormat(buf, &def->info, 6, flags) < 0) + if (virDomainDeviceInfoFormat(buf, &def->info, indent + 2, flags) < 0) return -1; - virBufferAddLit(buf, " </input>\n"); + virBufferIndentAddLit(buf, indent, "</input>\n"); } else { virBufferAddLit(buf, "/>\n"); } @@ -10144,9 +10149,10 @@ virDomainGraphicsAuthDefFormatAttr(virBufferPtr buf, static void virDomainGraphicsListenDefFormat(virBufferPtr buf, virDomainGraphicsListenDefPtr def, + int indent, unsigned int flags) { - virBufferAddLit(buf, " <listen"); + virBufferIndentAddLit(buf, indent, "<listen"); if (def->type) { virBufferAsprintf(buf, " type='%s'", @@ -10174,6 +10180,7 @@ virDomainGraphicsListenDefFormat(virBufferPtr buf, static int virDomainGraphicsDefFormat(virBufferPtr buf, virDomainGraphicsDefPtr def, + int indent, unsigned int flags) { const char *type = virDomainGraphicsTypeToString(def->type); @@ -10198,7 +10205,7 @@ virDomainGraphicsDefFormat(virBufferPtr buf, } } - virBufferAsprintf(buf, " <graphics type='%s'", type); + virBufferAsprintf(buf, "%*s<graphics type='%s'", indent, "", type); switch (def->type) { case VIR_DOMAIN_GRAPHICS_TYPE_VNC: @@ -10304,7 +10311,8 @@ virDomainGraphicsDefFormat(virBufferPtr buf, virBufferAddLit(buf, ">\n"); children = 1; } - virDomainGraphicsListenDefFormat(buf, &def->listens[i], flags); + virDomainGraphicsListenDefFormat(buf, &def->listens[i], indent + 2, + flags); } if (def->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { @@ -10318,7 +10326,8 @@ virDomainGraphicsDefFormat(virBufferPtr buf, children = 1; } - virBufferAsprintf(buf, " <channel name='%s' mode='%s'/>\n", + virBufferAsprintf(buf, "%*s<channel name='%s' mode='%s'/>\n", + indent + 2, "", virDomainGraphicsSpiceChannelNameTypeToString(i), virDomainGraphicsSpiceChannelModeTypeToString(mode)); } @@ -10329,27 +10338,33 @@ virDomainGraphicsDefFormat(virBufferPtr buf, children = 1; } if (def->data.spice.image) - virBufferAsprintf(buf, " <image compression='%s'/>\n", + virBufferAsprintf(buf, "%*s<image compression='%s'/>\n", + indent + 2, "", virDomainGraphicsSpiceImageCompressionTypeToString(def->data.spice.image)); if (def->data.spice.jpeg) - virBufferAsprintf(buf, " <jpeg compression='%s'/>\n", + virBufferAsprintf(buf, "%*s<jpeg compression='%s'/>\n", + indent + 2, "", virDomainGraphicsSpiceJpegCompressionTypeToString(def->data.spice.jpeg)); if (def->data.spice.zlib) - virBufferAsprintf(buf, " <zlib compression='%s'/>\n", + virBufferAsprintf(buf, "%*s<zlib compression='%s'/>\n", + indent + 2, "", virDomainGraphicsSpiceZlibCompressionTypeToString(def->data.spice.zlib)); if (def->data.spice.playback) - virBufferAsprintf(buf, " <playback compression='%s'/>\n", + virBufferAsprintf(buf, "%*s<playback compression='%s'/>\n", + indent + 2, "", virDomainGraphicsSpicePlaybackCompressionTypeToString(def->data.spice.playback)); if (def->data.spice.streaming) - virBufferAsprintf(buf, " <streaming mode='%s'/>\n", + virBufferAsprintf(buf, "%*s<streaming mode='%s'/>\n", + indent + 2, "", virDomainGraphicsSpiceStreamingModeTypeToString(def->data.spice.streaming)); if (def->data.spice.copypaste) - virBufferAsprintf(buf, " <clipboard copypaste='%s'/>\n", + virBufferAsprintf(buf, "%*s<clipboard copypaste='%s'/>\n", + indent + 2, "", virDomainGraphicsSpiceClipboardCopypasteTypeToString(def->data.spice.copypaste)); } if (children) { - virBufferAddLit(buf, " </graphics>\n"); + virBufferIndentAddLit(buf, indent, "</graphics>\n"); } else { virBufferAddLit(buf, "/>\n"); } @@ -10361,6 +10376,7 @@ virDomainGraphicsDefFormat(virBufferPtr buf, static int virDomainHostdevDefFormat(virBufferPtr buf, virDomainHostdevDefPtr def, + int indent, unsigned int flags) { const char *mode = virDomainHostdevModeTypeToString(def->mode); @@ -10380,39 +10396,46 @@ virDomainHostdevDefFormat(virBufferPtr buf, return -1; } - virBufferAsprintf(buf, " <hostdev mode='%s' type='%s' managed='%s'>\n", + virBufferAsprintf(buf, "%*s<hostdev mode='%s' type='%s' managed='%s'>\n", + indent, "", mode, type, def->managed ? "yes" : "no"); - virBufferAddLit(buf, " <source>\n"); + virBufferIndentAddLit(buf, indent + 2, "<source>\n"); if (def->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB) { if (def->source.subsys.u.usb.vendor) { - virBufferAsprintf(buf, " <vendor id='0x%.4x'/>\n", + virBufferAsprintf(buf, "%*s<vendor id='0x%.4x'/>\n", + indent + 4, "", def->source.subsys.u.usb.vendor); - virBufferAsprintf(buf, " <product id='0x%.4x'/>\n", + virBufferAsprintf(buf, "%*s<product id='0x%.4x'/>\n", + indent + 4, "", def->source.subsys.u.usb.product); } if (def->source.subsys.u.usb.bus || def->source.subsys.u.usb.device) - virBufferAsprintf(buf, " <address bus='%d' device='%d'/>\n", + virBufferAsprintf(buf, "%*s<address bus='%d' device='%d'/>\n", + indent + 4, "", def->source.subsys.u.usb.bus, def->source.subsys.u.usb.device); } else if (def->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) { - virBufferAsprintf(buf, " <address domain='0x%.4x' bus='0x%.2x' slot='0x%.2x' function='0x%.1x'/>\n", + virBufferAsprintf(buf, "%*s<address domain='0x%.4x' bus='0x%.2x' " + "slot='0x%.2x' function='0x%.1x'/>\n", + indent + 4, "", def->source.subsys.u.pci.domain, def->source.subsys.u.pci.bus, def->source.subsys.u.pci.slot, def->source.subsys.u.pci.function); } - virBufferAddLit(buf, " </source>\n"); + virBufferIndentAddLit(buf, indent + 2, "</source>\n"); if (def->bootIndex) - virBufferAsprintf(buf, " <boot order='%d'/>\n", def->bootIndex); + virBufferAsprintf(buf, "%*s<boot order='%d'/>\n", indent + 2, "", + def->bootIndex); - if (virDomainDeviceInfoFormat(buf, &def->info, 6, flags) < 0) + if (virDomainDeviceInfoFormat(buf, &def->info, indent + 2, flags) < 0) return -1; - virBufferAddLit(buf, " </hostdev>\n"); + virBufferIndentAddLit(buf, indent, "</hostdev>\n"); return 0; } @@ -10440,8 +10463,9 @@ virDomainRedirdevDefFormat(virBufferPtr buf, static int virDomainHubDefFormat(virBufferPtr buf, - virDomainHubDefPtr def, - unsigned int flags) + virDomainHubDefPtr def, + int indent, + unsigned int flags) { const char *type = virDomainHubTypeToString(def->type); @@ -10451,13 +10475,13 @@ virDomainHubDefFormat(virBufferPtr buf, return -1; } - virBufferAsprintf(buf, " <hub type='%s'", type); + virBufferAsprintf(buf, "%*s<hub type='%s'", indent, "", type); if (virDomainDeviceInfoIsSet(&def->info, flags)) { virBufferAddLit(buf, ">\n"); - if (virDomainDeviceInfoFormat(buf, &def->info, 6, flags) < 0) + if (virDomainDeviceInfoFormat(buf, &def->info, indent + 2, flags) < 0) return -1; - virBufferAddLit(buf, " </hub>\n"); + virBufferIndentAddLit(buf, indent, "</hub>\n"); } else { virBufferAddLit(buf, "/>\n"); } @@ -10847,7 +10871,7 @@ virDomainDefFormatInternal(virDomainDefPtr def, for (n = 0 ; n < def->ninputs ; n++) if (def->inputs[n]->bus == VIR_DOMAIN_INPUT_BUS_USB && - virDomainInputDefFormat(buf, def->inputs[n], flags) < 0) /* XXX indent */ + virDomainInputDefFormat(buf, def->inputs[n], indent, flags) < 0) goto cleanup; if (def->ngraphics > 0) { @@ -10859,24 +10883,26 @@ virDomainDefFormatInternal(virDomainDefPtr def, { .alias = NULL }, }; - if (virDomainInputDefFormat(buf, &autoInput, flags) < 0) /* XXX indent */ + if (virDomainInputDefFormat(buf, &autoInput, indent, flags) < 0) goto cleanup; for (n = 0 ; n < def->ngraphics ; n++) - if (virDomainGraphicsDefFormat(buf, def->graphics[n], flags) < 0) /* XXX indent */ + if (virDomainGraphicsDefFormat(buf, def->graphics[n], indent, + flags) < 0) goto cleanup; } for (n = 0 ; n < def->nsounds ; n++) - if (virDomainSoundDefFormat(buf, def->sounds[n], flags) < 0) /* XXX indent */ + if (virDomainSoundDefFormat(buf, def->sounds[n], indent, flags) < 0) goto cleanup; for (n = 0 ; n < def->nvideos ; n++) - if (virDomainVideoDefFormat(buf, def->videos[n], flags) < 0) /* XXX indent */ + if (virDomainVideoDefFormat(buf, def->videos[n], indent, flags) < 0) goto cleanup; for (n = 0 ; n < def->nhostdevs ; n++) - if (virDomainHostdevDefFormat(buf, def->hostdevs[n], flags) < 0) /* XXX indent */ + if (virDomainHostdevDefFormat(buf, def->hostdevs[n], indent, + flags) < 0) goto cleanup; for (n = 0 ; n < def->nredirdevs ; n++) @@ -10885,14 +10911,14 @@ virDomainDefFormatInternal(virDomainDefPtr def, goto cleanup; for (n = 0 ; n < def->nhubs ; n++) - if (virDomainHubDefFormat(buf, def->hubs[n], flags) < 0) /* XXX indent */ + if (virDomainHubDefFormat(buf, def->hubs[n], indent, flags) < 0) goto cleanup; if (def->watchdog) - virDomainWatchdogDefFormat (buf, def->watchdog, flags); /* XXX indent */ + virDomainWatchdogDefFormat(buf, def->watchdog, indent, flags); if (def->memballoon) - virDomainMemballoonDefFormat (buf, def->memballoon, flags); /* XXX indent */ + virDomainMemballoonDefFormat(buf, def->memballoon, indent, flags); indent -= 2; virBufferIndentAddLit(buf, indent, "</devices>\n"); -- 1.7.4.4
Add a test for all my indentation changes, and fix the fallout. * tests/domainsnapshotxml2xmltest.c: New test. * tests/Makefile.am (domainsnapshotxml2xmltest_SOURCES): Build it. * src/conf/domain_conf.c (virDomainSnapshotDefFormat): Avoid NULL deref, match documented order. * src/conf/domain_conf.h (virDomainSnapshotDefFormat): Add const. * tests/domainsnapshotxml2xmlout/all_parameters.xml: Tweak output. * tests/domainsnapshotxml2xmlout/disk_snapshot.xml: Likewise. * tests/domainsnapshotxml2xmlout/full_domain.xml: Likewise. * .gitignore: Exempt new binary. --- .gitignore | 1 + src/conf/domain_conf.c | 10 +- src/conf/domain_conf.h | 2 +- tests/Makefile.am | 14 ++- tests/domainsnapshotxml2xmlout/all_parameters.xml | 2 +- tests/domainsnapshotxml2xmlout/disk_snapshot.xml | 102 ++++++++-------- tests/domainsnapshotxml2xmlout/full_domain.xml | 52 ++++---- tests/domainsnapshotxml2xmltest.c | 128 +++++++++++++++++++++ 8 files changed, 224 insertions(+), 87 deletions(-) create mode 100644 tests/domainsnapshotxml2xmltest.c diff --git a/.gitignore b/.gitignore index 41fa50f..bfe8e87 100644 --- a/.gitignore +++ b/.gitignore @@ -69,6 +69,7 @@ /src/util/virkeymaps.h /tests/*.log /tests/cputest +/tests/domainsnapshotxml2xmltest /tests/hashtest /tests/jsontest /tests/networkxml2argvtest diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index eb5a33e..1bbf22d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -11972,7 +11972,7 @@ cleanup: return ret; } -char *virDomainSnapshotDefFormat(char *domain_uuid, +char *virDomainSnapshotDefFormat(const char *domain_uuid, virDomainSnapshotDefPtr def, unsigned int flags, int internal) @@ -12013,12 +12013,12 @@ char *virDomainSnapshotDefFormat(char *domain_uuid, virDomainDiskSnapshotTypeToString(disk->snapshot)); if (disk->file || disk->driverType) { virBufferAddLit(&buf, ">\n"); - if (disk->file) - virBufferEscapeString(&buf, " <source file='%s'/>\n", - disk->file); if (disk->driverType) virBufferEscapeString(&buf, " <driver type='%s'/>\n", disk->driverType); + if (disk->file) + virBufferEscapeString(&buf, " <source file='%s'/>\n", + disk->file); virBufferAddLit(&buf, " </disk>\n"); } else { virBufferAddLit(&buf, "/>\n"); @@ -12028,7 +12028,7 @@ char *virDomainSnapshotDefFormat(char *domain_uuid, } if (def->dom) { virDomainDefFormatInternal(def->dom, 2, flags, &buf); - } else { + } else if (domain_uuid) { virBufferAddLit(&buf, " <domain>\n"); virBufferAsprintf(&buf, " <uuid>%s</uuid>\n", domain_uuid); virBufferAddLit(&buf, " </domain>\n"); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 7e3c06c..87fa03a 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1462,7 +1462,7 @@ virDomainSnapshotDefPtr virDomainSnapshotDefParseString(const char *xmlStr, unsigned int expectedVirtTypes, unsigned int flags); void virDomainSnapshotDefFree(virDomainSnapshotDefPtr def); -char *virDomainSnapshotDefFormat(char *domain_uuid, +char *virDomainSnapshotDefFormat(const char *domain_uuid, virDomainSnapshotDefPtr def, unsigned int flags, int internal); diff --git a/tests/Makefile.am b/tests/Makefile.am index cbbbc6f..4769fa5 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -102,7 +102,8 @@ check_PROGRAMS += xml2sexprtest sexpr2xmltest \ xmconfigtest xencapstest statstest reconnect endif if WITH_QEMU -check_PROGRAMS += qemuxml2argvtest qemuxml2xmltest qemuargv2xmltest qemuhelptest +check_PROGRAMS += qemuxml2argvtest qemuxml2xmltest qemuargv2xmltest \ + qemuhelptest domainsnapshotxml2xmltest endif if WITH_OPENVZ @@ -228,7 +229,7 @@ endif if WITH_QEMU TESTS += qemuxml2argvtest qemuxml2xmltest qemuargv2xmltest qemuhelptest -TESTS += nwfilterxml2xmltest +TESTS += domainsnapshotxml2xmltest nwfilterxml2xmltest endif if WITH_OPENVZ @@ -350,8 +351,15 @@ qemuargv2xmltest_LDADD = $(qemu_LDADDS) $(LDADDS) qemuhelptest_SOURCES = qemuhelptest.c testutils.c testutils.h qemuhelptest_LDADD = $(qemu_LDADDS) $(LDADDS) + +domainsnapshotxml2xmltest_SOURCES = \ + domainsnapshotxml2xmltest.c testutilsqemu.c testutilsqemu.h \ + testutils.c testutils.h +domainsnapshotxml2xmltest_LDADD = $(qemu_LDADDS) $(LDADDS) else -EXTRA_DIST += qemuxml2argvtest.c qemuxml2xmltest.c qemuargv2xmltest.c qemuhelptest.c testutilsqemu.c testutilsqemu.h +EXTRA_DIST += qemuxml2argvtest.c qemuxml2xmltest.c qemuargv2xmltest.c \ + qemuhelptest.c domainsnapshotxml2xmltest.c \ + testutilsqemu.c testutilsqemu.h endif if WITH_OPENVZ diff --git a/tests/domainsnapshotxml2xmlout/all_parameters.xml b/tests/domainsnapshotxml2xmlout/all_parameters.xml index ed4a600..eb2ee85 100644 --- a/tests/domainsnapshotxml2xmlout/all_parameters.xml +++ b/tests/domainsnapshotxml2xmlout/all_parameters.xml @@ -1,10 +1,10 @@ <domainsnapshot> <name>my snap name</name> <description>!@#$%^</description> + <state>running</state> <parent> <name>earlier_snap</name> </parent> - <state>running</state> <creationTime>1272917631</creationTime> <domain> <uuid>9d37b878-a7cc-9f9a-b78f-49b3abad25a8</uuid> diff --git a/tests/domainsnapshotxml2xmlout/disk_snapshot.xml b/tests/domainsnapshotxml2xmlout/disk_snapshot.xml index e0414a1..a0a0965 100644 --- a/tests/domainsnapshotxml2xmlout/disk_snapshot.xml +++ b/tests/domainsnapshotxml2xmlout/disk_snapshot.xml @@ -1,10 +1,10 @@ <domainsnapshot> <name>my snap name</name> <description>!@#$%^</description> + <state>disk-snapshot</state> <parent> <name>earlier_snap</name> </parent> - <state>disk-snapshot</state> <creationTime>1272917631</creationTime> <disks> <disk name='hda' snapshot='no'/> @@ -23,55 +23,55 @@ <source file='/path/to/generated5'/> </disk> </disks> -<domain type='qemu'> - <name>QEMUGuest1</name> - <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> - <memory>219100</memory> - <currentMemory>219100</currentMemory> - <vcpu cpuset='1-4,8-20,525'>1</vcpu> - <os> - <type arch='i686' machine='pc'>hvm</type> - <boot dev='hd'/> - </os> - <clock offset='utc'/> - <on_poweroff>destroy</on_poweroff> - <on_reboot>restart</on_reboot> - <on_crash>destroy</on_crash> - <devices> - <emulator>/usr/bin/qemu</emulator> - <disk type='block' device='disk'> - <source dev='/dev/HostVG/QEMUGuest1'/> - <target dev='hda' bus='ide'/> - <address type='drive' controller='0' bus='0' unit='0'/> - </disk> - <disk type='block' device='disk'> - <source dev='/dev/HostVG/QEMUGuest2'/> - <target dev='hdb' bus='ide'/> - <address type='drive' controller='0' bus='1' unit='0'/> - </disk> - <disk type='block' device='disk'> - <source dev='/dev/HostVG/QEMUGuest3'/> - <target dev='hdc' bus='ide'/> - <address type='drive' controller='0' bus='2' unit='0'/> - </disk> - <disk type='block' device='disk'> - <source dev='/dev/HostVG/QEMUGuest4'/> - <target dev='hdd' bus='ide'/> - <address type='drive' controller='0' bus='3' unit='0'/> - </disk> - <disk type='block' device='disk'> - <source dev='/dev/HostVG/QEMUGuest5'/> - <target dev='hde' bus='ide'/> - <address type='drive' controller='0' bus='4' unit='0'/> - </disk> - <disk type='block' device='disk'> - <source dev='/dev/HostVG/QEMUGuest6'/> - <target dev='hdf' bus='ide'/> - <address type='drive' controller='0' bus='5' unit='0'/> - </disk> - <controller type='ide' index='0'/> - <memballoon model='virtio'/> - </devices> -</domain> + <domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory>219100</memory> + <currentMemory>219100</currentMemory> + <vcpu cpuset='1-4,8-20,525'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' unit='0'/> + </disk> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest2'/> + <target dev='hdb' bus='ide'/> + <address type='drive' controller='0' bus='1' unit='0'/> + </disk> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest3'/> + <target dev='hdc' bus='ide'/> + <address type='drive' controller='0' bus='2' unit='0'/> + </disk> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest4'/> + <target dev='hdd' bus='ide'/> + <address type='drive' controller='0' bus='3' unit='0'/> + </disk> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest5'/> + <target dev='hde' bus='ide'/> + <address type='drive' controller='0' bus='4' unit='0'/> + </disk> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest6'/> + <target dev='hdf' bus='ide'/> + <address type='drive' controller='0' bus='5' unit='0'/> + </disk> + <controller type='ide' index='0'/> + <memballoon model='virtio'/> + </devices> + </domain> <active>1</active> </domainsnapshot> diff --git a/tests/domainsnapshotxml2xmlout/full_domain.xml b/tests/domainsnapshotxml2xmlout/full_domain.xml index 942bd7f..76f17e1 100644 --- a/tests/domainsnapshotxml2xmlout/full_domain.xml +++ b/tests/domainsnapshotxml2xmlout/full_domain.xml @@ -1,35 +1,35 @@ <domainsnapshot> <name>my snap name</name> <description>!@#$%^</description> + <state>running</state> <parent> <name>earlier_snap</name> </parent> - <state>running</state> <creationTime>1272917631</creationTime> -<domain type='qemu'> - <name>QEMUGuest1</name> - <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> - <memory>219100</memory> - <currentMemory>219100</currentMemory> - <vcpu cpuset='1-4,8-20,525'>1</vcpu> - <os> - <type arch='i686' machine='pc'>hvm</type> - <boot dev='hd'/> - </os> - <clock offset='utc'/> - <on_poweroff>destroy</on_poweroff> - <on_reboot>restart</on_reboot> - <on_crash>destroy</on_crash> - <devices> - <emulator>/usr/bin/qemu</emulator> - <disk type='block' device='disk'> - <source dev='/dev/HostVG/QEMUGuest1'/> - <target dev='hda' bus='ide'/> - <address type='drive' controller='0' bus='0' unit='0'/> - </disk> - <controller type='ide' index='0'/> - <memballoon model='virtio'/> - </devices> -</domain> + <domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory>219100</memory> + <currentMemory>219100</currentMemory> + <vcpu cpuset='1-4,8-20,525'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' unit='0'/> + </disk> + <controller type='ide' index='0'/> + <memballoon model='virtio'/> + </devices> + </domain> <active>1</active> </domainsnapshot> diff --git a/tests/domainsnapshotxml2xmltest.c b/tests/domainsnapshotxml2xmltest.c new file mode 100644 index 0000000..c2266f4 --- /dev/null +++ b/tests/domainsnapshotxml2xmltest.c @@ -0,0 +1,128 @@ +#include <config.h> + +#include <stdio.h> +#include <stdlib.h> +#include <unistd.h> +#include <string.h> + +#include <sys/types.h> +#include <fcntl.h> + +#ifdef WITH_QEMU + +# include "internal.h" +# include "testutils.h" +# include "qemu/qemu_conf.h" +# include "qemu/qemu_domain.h" +# include "testutilsqemu.h" + +static struct qemud_driver driver; + +static int +testCompareXMLToXMLFiles(const char *inxml, const char *uuid, int internal) +{ + char *inXmlData = NULL; + char *actual = NULL; + int ret = -1; + virDomainSnapshotDefPtr def = NULL; + unsigned int flags = (VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE | + VIR_DOMAIN_SNAPSHOT_PARSE_DISKS); + + if (virtTestLoadFile(inxml, &inXmlData) < 0) + goto fail; + + if (internal) + flags |= VIR_DOMAIN_SNAPSHOT_PARSE_INTERNAL; + if (!(def = virDomainSnapshotDefParseString(inXmlData, driver.caps, + QEMU_EXPECTED_VIRT_TYPES, + flags))) + goto fail; + + if (!(actual = virDomainSnapshotDefFormat(uuid, def, + VIR_DOMAIN_XML_SECURE, + internal))) + goto fail; + + + if (STRNEQ(inXmlData, actual)) { + virtTestDifference(stderr, inXmlData, actual); + goto fail; + } + + ret = 0; + fail: + free(inXmlData); + free(actual); + virDomainSnapshotDefFree(def); + return ret; +} + +struct testInfo { + const char *name; + const char *uuid; + int internal; +}; + +static int +testCompareXMLToXMLHelper(const void *data) +{ + const struct testInfo *info = data; + char *xml_in = NULL; + int ret = -1; + + if (virAsprintf(&xml_in, "%s/domainsnapshotxml2xmlout/%s.xml", + abs_srcdir, info->name) < 0) + goto cleanup; + + ret = testCompareXMLToXMLFiles(xml_in, info->uuid, info->internal); + +cleanup: + free(xml_in); + return ret; +} + + +static int +mymain(void) +{ + int ret = 0; + + if ((driver.caps = testQemuCapsInit()) == NULL) + return (EXIT_FAILURE); + +# define DO_TEST(name, uuid, internal) \ + do { \ + const struct testInfo info = {name, uuid, internal}; \ + if (virtTestRun("SNAPSHOT XML-2-XML " name, \ + 1, testCompareXMLToXMLHelper, &info) < 0) \ + ret = -1; \ + } while (0) + + /* Unset or set all envvars here that are copied in qemudBuildCommandLine + * using ADD_ENV_COPY, otherwise these tests may fail due to unexpected + * values for these envvars */ + setenv("PATH", "/bin", 1); + + DO_TEST("all_parameters", "9d37b878-a7cc-9f9a-b78f-49b3abad25a8", 1); + DO_TEST("disk_snapshot", "c7a5fdbd-edaf-9455-926a-d65c16db1809", 1); + DO_TEST("full_domain", "c7a5fdbd-edaf-9455-926a-d65c16db1809", 1); + DO_TEST("noparent_nodescription_noactive", NULL, 0); + DO_TEST("noparent_nodescription", NULL, 1); + DO_TEST("noparent", "9d37b878-a7cc-9f9a-b78f-49b3abad25a8", 0); + + virCapabilitiesFree(driver.caps); + + return (ret==0 ? EXIT_SUCCESS : EXIT_FAILURE); +} + +VIRT_TEST_MAIN(mymain) + +#else + +int +main(void) +{ + return EXIT_AM_SKIP; +} + +#endif /* WITH_QEMU */ -- 1.7.4.4
On 09/23/2011 04:34 AM, Eric Blake wrote:
This series fixes 'virsh snapshot-dumpxml' to use nicer formatting. Patch 1 adds some nice helper routines, patches 2-13 are mostly mechanical conversions to use the helpers and pass indentation levels through the entire call chain, and patch 14 adds a test which uncovered a couple minor issues in how formatting was done.
Hi, I applied these patches and tested it a little bit. I created a snapshot of a running vm, before applying these patches, snapshot-dumpxml the snapshot output: <domainsnapshot> <name>firstshot</name> <state>running</state> <creationTime>1316766961</creationTime> <domain type='kvm'> <name>lihaidong-redhat0001</name> <uuid>060673b4-81ab-985d-5203-b76e36946eb4</uuid> <memory>2097152</memory> <currentMemory>2097152</currentMemory> <vcpu>4</vcpu> <os> <type arch='x86_64' machine='rhel6.1.0'>hvm</type> <boot dev='hd'/> </os> <features> <acpi/> <apic/> <pae/> </features> <clock offset='utc'/> <on_poweroff>destroy</on_poweroff> <on_reboot>restart</on_reboot> <on_crash>restart</on_crash> <devices> <emulator>/usr/libexec/qemu-kvm</emulator> <disk type='block' device='disk'> <driver name='qemu' type='qcow2' cache='none'/> <source dev='/var/lib/libvirt/images/lihaidong-redhat0001.qcow2'/> <target dev='vda' bus='virtio'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> </disk> <interface type='bridge'> <mac address='00:ff:fe:00:00:fb'/> <source bridge='br0'/> <target dev='redhat0001'/> <model type='virtio'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> </interface> <serial type='pty'> <target port='0'/> </serial> <console type='pty'> <target type='serial' port='0'/> </console> <input type='mouse' bus='ps2'/> <graphics type='vnc' port='-1' autoport='yes' keymap='en-us'/> <video> <model type='cirrus' vram='9216' heads='1'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> </video> <memballoon model='virtio'> <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/> </memballoon> </devices> </domain> </domainsnapshot> It can be seen that <domain></domain> didn't incident well, it's in the same level with the domainsnapshot. After applying these patches making, restarting the libvirtd, then virsh snapshotdumpxml the snapshot output: <domainsnapshot> <name>firstshot</name> <state>running</state> <creationTime>1316766961</creationTime> <domain type='kvm'> <name>lihaidong-redhat0001</name> <uuid>060673b4-81ab-985d-5203-b76e36946eb4</uuid> <memory>2097152</memory> <currentMemory>2097152</currentMemory> <vcpu>4</vcpu> <os> <type arch='x86_64' machine='rhel6.1.0'>hvm</type> <boot dev='hd'/> </os> <features> <acpi/> <apic/> <pae/> </features> <clock offset='utc'/> <on_poweroff>destroy</on_poweroff> <on_reboot>restart</on_reboot> <on_crash>restart</on_crash> <devices> <emulator>/usr/libexec/qemu-kvm</emulator> <disk type='block' device='disk'> <driver name='qemu' type='qcow2' cache='none'/> <source dev='/var/lib/libvirt/images/lihaidong-redhat0001.qcow2'/> <target dev='vda' bus='virtio'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> </disk> <interface type='bridge'> <mac address='00:ff:fe:00:00:fb'/> <source bridge='br0'/> <target dev='redhat0001'/> <model type='virtio'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> </interface> <serial type='pty'> <target port='0'/> </serial> <console type='pty'> <target type='serial' port='0'/> </console> <input type='mouse' bus='ps2'/> <graphics type='vnc' port='-1' autoport='yes' keymap='en-us'/> <video> <model type='cirrus' vram='9216' heads='1'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> </video> <memballoon model='virtio'> <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/> </memballoon> </devices> </domain> </domainsnapshot> It can be seen that the <domain></domain> indent correctly. I typed 'make check' in the tests directory. The domainsnapshotxml2xmltest part output: TEST: domainsnapshotxml2xmltest ...... 6 OK PASS: domainsnapshotxml2xmltest I ran the domainsnapshotxml2xmltest script, it output: TEST: domainsnapshotxml2xmltest ...... 6 OK That's what I've done. Thanks.
On 09/26/2011 05:03 PM, Hai Dong Li wrote:
On 09/23/2011 04:34 AM, Eric Blake wrote:
This series fixes 'virsh snapshot-dumpxml' to use nicer formatting. Patch 1 adds some nice helper routines, patches 2-13 are mostly mechanical conversions to use the helpers and pass indentation levels through the entire call chain, and patch 14 adds a test which uncovered a couple minor issues in how formatting was done.
Hi, I applied these patches and tested it a little bit. I created a snapshot of a running vm, before applying these patches, snapshot-dumpxml the snapshot output: <domainsnapshot> <name>firstshot</name> <state>running</state> <creationTime>1316766961</creationTime> <domain type='kvm'> <name>lihaidong-redhat0001</name> <uuid>060673b4-81ab-985d-5203-b76e36946eb4</uuid> <memory>2097152</memory> <currentMemory>2097152</currentMemory> <vcpu>4</vcpu> <os> <type arch='x86_64' machine='rhel6.1.0'>hvm</type> <boot dev='hd'/> </os> <features> <acpi/> <apic/> <pae/> </features> <clock offset='utc'/> <on_poweroff>destroy</on_poweroff> <on_reboot>restart</on_reboot> <on_crash>restart</on_crash> <devices> <emulator>/usr/libexec/qemu-kvm</emulator> <disk type='block' device='disk'> <driver name='qemu' type='qcow2' cache='none'/> <source dev='/var/lib/libvirt/images/lihaidong-redhat0001.qcow2'/> <target dev='vda' bus='virtio'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> </disk> <interface type='bridge'> <mac address='00:ff:fe:00:00:fb'/> <source bridge='br0'/> <target dev='redhat0001'/> <model type='virtio'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> </interface> <serial type='pty'> <target port='0'/> </serial> <console type='pty'> <target type='serial' port='0'/> </console> <input type='mouse' bus='ps2'/> <graphics type='vnc' port='-1' autoport='yes' keymap='en-us'/> <video> <model type='cirrus' vram='9216' heads='1'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> </video> <memballoon model='virtio'> <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/> </memballoon> </devices> </domain> </domainsnapshot>
It can be seen that <domain></domain> didn't incident well, it's in the same level with the domainsnapshot. After applying these patches making, restarting the libvirtd, then virsh snapshotdumpxml the snapshot output: <domainsnapshot> <name>firstshot</name> <state>running</state> <creationTime>1316766961</creationTime> <domain type='kvm'> <name>lihaidong-redhat0001</name> <uuid>060673b4-81ab-985d-5203-b76e36946eb4</uuid> <memory>2097152</memory> <currentMemory>2097152</currentMemory> <vcpu>4</vcpu> <os> <type arch='x86_64' machine='rhel6.1.0'>hvm</type> <boot dev='hd'/> </os> <features> <acpi/> <apic/> <pae/> </features> <clock offset='utc'/> <on_poweroff>destroy</on_poweroff> <on_reboot>restart</on_reboot> <on_crash>restart</on_crash> <devices> <emulator>/usr/libexec/qemu-kvm</emulator> <disk type='block' device='disk'> <driver name='qemu' type='qcow2' cache='none'/> <source dev='/var/lib/libvirt/images/lihaidong-redhat0001.qcow2'/> <target dev='vda' bus='virtio'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> </disk> <interface type='bridge'> <mac address='00:ff:fe:00:00:fb'/> <source bridge='br0'/> <target dev='redhat0001'/> <model type='virtio'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> </interface> <serial type='pty'> <target port='0'/> </serial> <console type='pty'> <target type='serial' port='0'/> </console> <input type='mouse' bus='ps2'/> <graphics type='vnc' port='-1' autoport='yes' keymap='en-us'/> <video> <model type='cirrus' vram='9216' heads='1'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> </video> <memballoon model='virtio'> <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/> </memballoon> </devices> </domain> </domainsnapshot>
It can be seen that the <domain></domain> indent correctly.
I typed 'make check' in the tests directory. The domainsnapshotxml2xmltest part output: TEST: domainsnapshotxml2xmltest ...... 6 OK PASS: domainsnapshotxml2xmltest
I ran the domainsnapshotxml2xmltest script, it output: TEST: domainsnapshotxml2xmltest ...... 6 OK
That's what I've done. Thanks.
Seems the email mess up the indentation.The attached file dumpxml_old_new_diff shows the differences.
On 09/26/2011 03:21 AM, Hai Dong Li wrote:
On 09/26/2011 05:03 PM, Hai Dong Li wrote:
On 09/23/2011 04:34 AM, Eric Blake wrote:
This series fixes 'virsh snapshot-dumpxml' to use nicer formatting. Patch 1 adds some nice helper routines, patches 2-13 are mostly mechanical conversions to use the helpers and pass indentation levels through the entire call chain, and patch 14 adds a test which uncovered a couple minor issues in how formatting was done.
Hi, I applied these patches and tested it a little bit.
Thanks for doing that.
It can be seen that the <domain></domain> indent correctly.
I typed 'make check' in the tests directory. The domainsnapshotxml2xmltest part output: TEST: domainsnapshotxml2xmltest ...... 6 OK PASS: domainsnapshotxml2xmltest
I'll take that as a series ack, and push the 14 patches then.
I ran the domainsnapshotxml2xmltest script, it output: TEST: domainsnapshotxml2xmltest ...... 6 OK
That's what I've done. Thanks.
Seems the email mess up the indentation.The attached file dumpxml_old_new_diff shows the differences.
Yeah, whitespace munging on email can make indentation patches and results harder than you want :) -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On 09/26/2011 03:45 PM, Eric Blake wrote:
It can be seen that the <domain></domain> indent correctly.
I typed 'make check' in the tests directory. The domainsnapshotxml2xmltest part output: TEST: domainsnapshotxml2xmltest ...... 6 OK PASS: domainsnapshotxml2xmltest
I'll take that as a series ack, and push the 14 patches then.
Actually, I'd feel more comfortable also getting a code review, and not just a compile- and runtime-check test, since I am modifying some internal APIs, and since I may have overlooked some lines not covered by the particular <domain> that you tested. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On 09/27/2011 06:39 AM, Eric Blake wrote:
On 09/26/2011 03:45 PM, Eric Blake wrote:
It can be seen that the <domain></domain> indent correctly.
I typed 'make check' in the tests directory. The domainsnapshotxml2xmltest part output: TEST: domainsnapshotxml2xmltest ...... 6 OK PASS: domainsnapshotxml2xmltest
I'll take that as a series ack, and push the 14 patches then.
Actually, I'd feel more comfortable also getting a code review, and not just a compile- and runtime-check test, since I am modifying some internal APIs, and since I may have overlooked some lines not covered by the particular <domain> that you tested.
Yeah, that's the second step I was going to do. I'll tell you once I had done that. And maybe some questions to you if I encountered some difficulties. Thanks.
participants (4)
-
Daniel P. Berrange -
Daniel Veillard -
Eric Blake -
Hai Dong Li