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(-)
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;
}
--
2.13.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
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(-) > Reviewed-by: John Ferlan <jferlan@redhat.com> John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
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;
> }
>
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Wed, Aug 02, 2017 at 01:12:15PM -0400, Cole Robinson wrote: >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, I have sent a fix: https://www.redhat.com/archives/libvir-list/2017-August/msg00134.html Jan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 08/03/2017 08:57 AM, Ján Tomko wrote: > On Wed, Aug 02, 2017 at 01:12:15PM -0400, Cole Robinson wrote: >> 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, I have sent a fix: > https://www.redhat.com/archives/libvir-list/2017-August/msg00134.html > John sent patches too: https://www.redhat.com/archives/libvir-list/2017-August/msg00101.html - Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2026 Red Hat, Inc.