On 07/26/2017 09:29 AM, Ján Tomko wrote:
Convert virDomainSmartcardDefFormat to use a separate buffer for possible subelements, to avoid the need for duplicated formatting logic in virDomainDeviceInfoNeedsFormat. --- src/conf/domain_conf.c | 39 +++++++++++++++++++++++---------------- 1 file changed, 23 insertions(+), 16 deletions(-)
Looks like this patch causes a regression, currently breaking the virt-manager test suite that danpb pointed out to me. Edit an existing VM and add <smartcard mode='passthrough' type='spicevmc'/> The returned XML is invalid: <smartcard mode='passthrough'> type='spicevmc'> <address type='ccid' controller='0' slot='0'/> </smartcard> Unfortunately there aren't any xml2xml tests for smartcard bits that would have caught this... Thanks, Cole
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6c958bcf6..ead94976d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -23047,37 +23047,32 @@ virDomainSmartcardDefFormat(virBufferPtr buf, unsigned int flags) { const char *mode = virDomainSmartcardTypeToString(def->type); + virBuffer childBuf = VIR_BUFFER_INITIALIZER; size_t i;
+ virBufferAdjustIndent(&childBuf, virBufferGetIndent(buf, false) + 2); + if (!mode) { virReportError(VIR_ERR_INTERNAL_ERROR, _("unexpected smartcard type %d"), def->type); return -1; }
- virBufferAsprintf(buf, "<smartcard mode='%s'", mode); - virBufferAdjustIndent(buf, 2); switch (def->type) { case VIR_DOMAIN_SMARTCARD_TYPE_HOST: - if (!virDomainDeviceInfoNeedsFormat(&def->info, flags)) { - virBufferAdjustIndent(buf, -2); - virBufferAddLit(buf, "/>\n"); - return 0; - } - virBufferAddLit(buf, ">\n"); break;
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", + for (i = 0; i < VIR_DOMAIN_SMARTCARD_NUM_CERTIFICATES; i++) { + virBufferEscapeString(&childBuf, "<certificate>%s</certificate>\n", def->data.cert.file[i]); - virBufferEscapeString(buf, "<database>%s</database>\n", + } + virBufferEscapeString(&childBuf, "<database>%s</database>\n", def->data.cert.database); break;
case VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH: - if (virDomainChrSourceDefFormat(buf, def->data.passthru, false, + if (virDomainChrSourceDefFormat(&childBuf, def->data.passthru, false, flags) < 0) return -1; break; @@ -23087,10 +23082,22 @@ virDomainSmartcardDefFormat(virBufferPtr buf, _("unexpected smartcard type %d"), def->type); return -1; } - if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) + if (virDomainDeviceInfoFormat(&childBuf, &def->info, flags) < 0) { + virBufferFreeAndReset(&childBuf); return -1; - virBufferAdjustIndent(buf, -2); - virBufferAddLit(buf, "</smartcard>\n"); + } + + if (virBufferCheckError(&childBuf) < 0) + return -1; + + virBufferAsprintf(buf, "<smartcard mode='%s'", mode); + if (virBufferUse(&childBuf)) { + virBufferAddLit(buf, ">\n"); + virBufferAddBuffer(buf, &childBuf); + virBufferAddLit(buf, "</smartcard>\n"); + } else { + virBufferAddLit(buf, "/>\n"); + } return 0; }