[libvirt] [PATCH 3/8] Use a separate buffer for <smartcard> subelements

[Cole Robinson]

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;
>  }
>  
>