From e02ff020cac1aa2b35c7eed4e0446be9a49496ac Mon Sep 17 00:00:00 2001 From: Pavel Hrdina Date: Thu, 24 Aug 2017 16:46:58 +0200 Subject: [PATCH] conf: don't close the source element inside different function While formatting disk or chardev element they both uses virDomainDiskSourceDefFormatSeclabel() function which also closes the source element. This is not extendable. Use the new virXMLFormatElement() to properly format the source element with possible child elements. As a side effect it fixes a bug in disk source formatting. Reviewed-by: John Ferlan Signed-off-by: Pavel Hrdina --- src/conf/domain_conf.c | 99 +++++++++++-------- ...uxml2xmlout-seclabel-dynamic-labelskip.xml | 3 +- 2 files changed, 59 insertions(+), 43 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 51a7d003dc..5bad3976cf 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -21500,14 +21500,8 @@ virDomainDiskSourceDefFormatSeclabel(virBufferPtr buf, size_t n; if (nseclabels && !skipSeclables) { - virBufferAddLit(buf, ">\n"); - virBufferAdjustIndent(buf, 2); for (n = 0; n < nseclabels; n++) virSecurityDeviceLabelDefFormat(buf, seclabels[n], flags); - virBufferAdjustIndent(buf, -2); - virBufferAddLit(buf, "\n"); - } else { - virBufferAddLit(buf, "/>\n"); } } @@ -21580,6 +21574,10 @@ virDomainDiskSourceFormatInternal(virBufferPtr buf, bool skipSeclabels) { const char *startupPolicy = NULL; + virBuffer attrBuf = VIR_BUFFER_INITIALIZER; + virBuffer childBuf = VIR_BUFFER_INITIALIZER; + + virBufferSetChildIndent(&childBuf, buf); if (policy) startupPolicy = virDomainStartupPolicyTypeToString(policy); @@ -21587,51 +21585,45 @@ virDomainDiskSourceFormatInternal(virBufferPtr buf, if (src->path || src->nhosts > 0 || src->srcpool || startupPolicy) { switch ((virStorageType)src->type) { case VIR_STORAGE_TYPE_FILE: - virBufferAddLit(buf, "path); - virBufferEscapeString(buf, " startupPolicy='%s'", startupPolicy); + virBufferEscapeString(&attrBuf, " file='%s'", src->path); + virBufferEscapeString(&attrBuf, " startupPolicy='%s'", startupPolicy); - virDomainDiskSourceDefFormatSeclabel(buf, src->nseclabels, + virDomainDiskSourceDefFormatSeclabel(&childBuf, src->nseclabels, src->seclabels, flags, skipSeclabels); break; case VIR_STORAGE_TYPE_BLOCK: - virBufferAddLit(buf, "path); - virBufferEscapeString(buf, " startupPolicy='%s'", startupPolicy); + virBufferEscapeString(&attrBuf, " dev='%s'", src->path); + virBufferEscapeString(&attrBuf, " startupPolicy='%s'", startupPolicy); - virDomainDiskSourceDefFormatSeclabel(buf, src->nseclabels, + virDomainDiskSourceDefFormatSeclabel(&childBuf, src->nseclabels, src->seclabels, flags, skipSeclabels); break; case VIR_STORAGE_TYPE_DIR: - virBufferAddLit(buf, "path); - virBufferEscapeString(buf, " startupPolicy='%s'", startupPolicy); - virBufferAddLit(buf, "/>\n"); + virBufferEscapeString(&attrBuf, " dir='%s'", src->path); + virBufferEscapeString(&attrBuf, " startupPolicy='%s'", startupPolicy); break; case VIR_STORAGE_TYPE_NETWORK: if (virDomainDiskSourceFormatNetwork(buf, src) < 0) - return -1; + goto error; break; case VIR_STORAGE_TYPE_VOLUME: - virBufferAddLit(buf, "srcpool) { - virBufferEscapeString(buf, " pool='%s'", src->srcpool->pool); - virBufferEscapeString(buf, " volume='%s'", + virBufferEscapeString(&attrBuf, " pool='%s'", src->srcpool->pool); + virBufferEscapeString(&attrBuf, " volume='%s'", src->srcpool->volume); if (src->srcpool->mode) - virBufferAsprintf(buf, " mode='%s'", + virBufferAsprintf(&attrBuf, " mode='%s'", virStorageSourcePoolModeTypeToString(src->srcpool->mode)); } - virBufferEscapeString(buf, " startupPolicy='%s'", startupPolicy); + virBufferEscapeString(&attrBuf, " startupPolicy='%s'", startupPolicy); - virDomainDiskSourceDefFormatSeclabel(buf, src->nseclabels, + virDomainDiskSourceDefFormatSeclabel(&childBuf, src->nseclabels, src->seclabels, flags, skipSeclabels); break; @@ -21640,11 +21632,19 @@ virDomainDiskSourceFormatInternal(virBufferPtr buf, case VIR_STORAGE_TYPE_LAST: virReportError(VIR_ERR_INTERNAL_ERROR, _("unexpected disk type %d"), src->type); - return -1; + goto error; } + + if (virXMLFormatElement(buf, "source", &attrBuf, &childBuf) < 0) + goto error; } return 0; + + error: + virBufferFreeAndReset(&attrBuf); + virBufferFreeAndReset(&childBuf); + return -1; } @@ -23070,11 +23070,15 @@ virDomainChrAttrsDefFormat(virBufferPtr buf, return 0; } -static void +static int virDomainChrSourceDefFormat(virBufferPtr buf, virDomainChrSourceDefPtr def, unsigned int flags) { + virBuffer attrBuf = VIR_BUFFER_INITIALIZER; + virBuffer childBuf = VIR_BUFFER_INITIALIZER; + + virBufferSetChildIndent(&childBuf, buf); switch ((virDomainChrType)def->type) { case VIR_DOMAIN_CHR_TYPE_NULL: @@ -23092,14 +23096,17 @@ virDomainChrSourceDefFormat(virBufferPtr buf, if (def->type != VIR_DOMAIN_CHR_TYPE_PTY || (def->data.file.path && !(flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE))) { - virBufferEscapeString(buf, "data.file.path); if (def->type == VIR_DOMAIN_CHR_TYPE_FILE && def->data.file.append != VIR_TRISTATE_SWITCH_ABSENT) - virBufferAsprintf(buf, " append='%s'", + virBufferAsprintf(&attrBuf, " append='%s'", virTristateSwitchTypeToString(def->data.file.append)); - virDomainSourceDefFormatSeclabel(buf, def->nseclabels, + virDomainSourceDefFormatSeclabel(&childBuf, def->nseclabels, def->seclabels, flags); + + if (virXMLFormatElement(buf, "source", &attrBuf, &childBuf) < 0) + goto error; } break; @@ -23130,19 +23137,21 @@ virDomainChrSourceDefFormat(virBufferPtr buf, break; case VIR_DOMAIN_CHR_TYPE_TCP: - virBufferAsprintf(buf, "data.tcp.listen ? "bind" : "connect"); - virBufferEscapeString(buf, "host='%s' ", def->data.tcp.host); - virBufferEscapeString(buf, "service='%s'", def->data.tcp.service); + virBufferEscapeString(&attrBuf, "host='%s' ", def->data.tcp.host); + virBufferEscapeString(&attrBuf, "service='%s'", def->data.tcp.service); if (def->data.tcp.haveTLS != VIR_TRISTATE_BOOL_ABSENT && !(flags & VIR_DOMAIN_DEF_FORMAT_MIGRATABLE && def->data.tcp.tlsFromConfig)) - virBufferAsprintf(buf, " tls='%s'", + virBufferAsprintf(&attrBuf, " tls='%s'", virTristateBoolTypeToString(def->data.tcp.haveTLS)); if (flags & VIR_DOMAIN_DEF_FORMAT_STATUS) - virBufferAsprintf(buf, " tlsFromConfig='%d'", + virBufferAsprintf(&attrBuf, " tlsFromConfig='%d'", def->data.tcp.tlsFromConfig); - virBufferAddLit(buf, "/>\n"); + + if (virXMLFormatElement(buf, "source", &attrBuf, &childBuf) < 0) + goto error; virBufferAsprintf(buf, "\n", virDomainChrTcpProtocolTypeToString( @@ -23151,11 +23160,14 @@ virDomainChrSourceDefFormat(virBufferPtr buf, case VIR_DOMAIN_CHR_TYPE_UNIX: if (def->data.nix.path) { - virBufferAsprintf(buf, "data.nix.listen ? "bind" : "connect"); - virBufferEscapeString(buf, " path='%s'", def->data.nix.path); - virDomainSourceDefFormatSeclabel(buf, def->nseclabels, + virBufferEscapeString(&attrBuf, " path='%s'", def->data.nix.path); + virDomainSourceDefFormatSeclabel(&childBuf, def->nseclabels, def->seclabels, flags); + + if (virXMLFormatElement(buf, "source", &attrBuf, &childBuf) < 0) + goto error; } break; @@ -23175,7 +23187,12 @@ virDomainChrSourceDefFormat(virBufferPtr buf, virBufferAddLit(buf, "/>\n"); } - return; + return 0; + + error: + virBufferFreeAndReset(&attrBuf); + virBufferFreeAndReset(&childBuf); + return -1; } static int diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-seclabel-dynamic-labelskip.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-seclabel-dynamic-labelskip.xml index 6688d98eb6..3f4ff0aadf 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-seclabel-dynamic-labelskip.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-seclabel-dynamic-labelskip.xml @@ -15,8 +15,7 @@ /usr/bin/qemu-system-i686 - - +