[libvirt] [PATCH] conf: Fix printing of 'type' and 'tty_compat' for Chr devices

John Ferlan posted 1 patch 6 years, 8 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20170802195134.28814-1-jferlan@redhat.com
src/conf/domain_conf.c | 39 ++++++++++++++++++++++++---------------
1 file changed, 24 insertions(+), 15 deletions(-)
[libvirt] [PATCH] conf: Fix printing of 'type' and 'tty_compat' for Chr devices
Posted by John Ferlan 6 years, 8 months ago
Commit id '0c1d8632' caused a regression in the virt-manager
test suite when formatting the <smartcard mode='passthrough'
type='spicevmc'/>.

Adust the code to print the type in it's own new helper called
virDomainChrTypeFormat and have the virDomainChrSourceDefFormat
manage just formatting the source and change to a void type since
only 0 could be returned. Adjust the callers to handle properly.

Signed-off-by: John Ferlan <jferlan@redhat.com>
---

 Although technically a CI build breaker since virt-manager test is
 failing, I figured I'd let this one go through the formal review just
 in case someone has agita over new function name or would like to see
 things done in a different manner. 


 src/conf/domain_conf.c | 39 ++++++++++++++++++++++++---------------
 1 file changed, 24 insertions(+), 15 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index eb70523..878c15d 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -22804,10 +22804,9 @@ virDomainNetDefFormat(virBufferPtr buf,
 /* Assumes that "<device" has already been generated, and starts
  * output at " type='type'>". */
 static int
-virDomainChrSourceDefFormat(virBufferPtr buf,
-                            virDomainChrSourceDefPtr def,
-                            bool tty_compat,
-                            unsigned int flags)
+virDomainChrTypeFormat(virBufferPtr buf,
+                       virDomainChrSourceDefPtr def,
+                       bool tty_compat)
 {
     const char *type = virDomainChrTypeToString(def->type);
 
@@ -22825,6 +22824,15 @@ virDomainChrSourceDefFormat(virBufferPtr buf,
     }
     virBufferAddLit(buf, ">\n");
 
+    return 0;
+}
+
+
+static void
+virDomainChrSourceDefFormat(virBufferPtr buf,
+                            virDomainChrSourceDefPtr def,
+                            unsigned int flags)
+{
     switch ((virDomainChrType)def->type) {
     case VIR_DOMAIN_CHR_TYPE_NULL:
     case VIR_DOMAIN_CHR_TYPE_VC:
@@ -22923,8 +22931,6 @@ virDomainChrSourceDefFormat(virBufferPtr buf,
         }
         virBufferAddLit(buf, "/>\n");
     }
-
-    return 0;
 }
 
 static int
@@ -22953,8 +22959,9 @@ virDomainChrDefFormat(virBufferPtr buf,
                   def->source->type == VIR_DOMAIN_CHR_TYPE_PTY &&
                   !(flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE) &&
                   def->source->data.file.path);
-    if (virDomainChrSourceDefFormat(buf, def->source, tty_compat, flags) < 0)
+    if (virDomainChrTypeFormat(buf, def->source, tty_compat) < 0)
         return -1;
+    virDomainChrSourceDefFormat(buf, def->source, flags);
 
     /* Format <target> block */
     switch (def->deviceType) {
@@ -23053,6 +23060,8 @@ virDomainSmartcardDefFormat(virBufferPtr buf,
         return -1;
     }
 
+    virBufferAsprintf(buf, "<smartcard mode='%s'", mode);
+
     switch (def->type) {
     case VIR_DOMAIN_SMARTCARD_TYPE_HOST:
         break;
@@ -23067,9 +23076,9 @@ virDomainSmartcardDefFormat(virBufferPtr buf,
         break;
 
     case VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH:
-        if (virDomainChrSourceDefFormat(&childBuf, def->data.passthru, false,
-                                        flags) < 0)
+        if (virDomainChrTypeFormat(buf, def->data.passthru, false) < 0)
             return -1;
+        virDomainChrSourceDefFormat(&childBuf, def->data.passthru, flags);
         break;
 
     default:
@@ -23082,7 +23091,6 @@ virDomainSmartcardDefFormat(virBufferPtr buf,
     if (virBufferCheckError(&childBuf) < 0)
         return -1;
 
-    virBufferAsprintf(buf, "<smartcard mode='%s'", mode);
     if (virBufferUse(&childBuf)) {
         virBufferAddLit(buf, ">\n");
         virBufferAddBuffer(buf, &childBuf);
@@ -23390,10 +23398,10 @@ virDomainRNGDefFormat(virBufferPtr buf,
         break;
 
     case VIR_DOMAIN_RNG_BACKEND_EGD:
-        virBufferAdjustIndent(buf, 2);
-        if (virDomainChrSourceDefFormat(buf, def->source.chardev,
-                                        false, flags) < 0)
+        if (virDomainChrTypeFormat(buf, def->source.chardev, false) < 0)
             return -1;
+        virBufferAdjustIndent(buf, 2);
+        virDomainChrSourceDefFormat(buf, def->source.chardev, flags);
         virBufferAdjustIndent(buf, -2);
         virBufferAddLit(buf, "</backend>\n");
 
@@ -24234,9 +24242,10 @@ virDomainRedirdevDefFormat(virBufferPtr buf,
     bus = virDomainRedirdevBusTypeToString(def->bus);
 
     virBufferAsprintf(buf, "<redirdev bus='%s'", bus);
-    virBufferAdjustIndent(buf, 2);
-    if (virDomainChrSourceDefFormat(buf, def->source, false, flags) < 0)
+    if (virDomainChrTypeFormat(buf, def->source, false) < 0)
         return -1;
+    virBufferAdjustIndent(buf, 2);
+    virDomainChrSourceDefFormat(buf, def->source, flags);
     virDomainDeviceInfoFormat(buf, &def->info,
                               flags | VIR_DOMAIN_DEF_FORMAT_ALLOW_BOOT);
     virBufferAdjustIndent(buf, -2);
-- 
2.9.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] conf: Fix printing of 'type' and 'tty_compat' for Chr devices
Posted by Daniel P. Berrange 6 years, 8 months ago
On Wed, Aug 02, 2017 at 03:51:34PM -0400, John Ferlan wrote:
> Commit id '0c1d8632' caused a regression in the virt-manager
> test suite when formatting the <smartcard mode='passthrough'
> type='spicevmc'/>.
> 
> Adust the code to print the type in it's own new helper called
> virDomainChrTypeFormat and have the virDomainChrSourceDefFormat
> manage just formatting the source and change to a void type since
> only 0 could be returned. Adjust the callers to handle properly.
> 
> Signed-off-by: John Ferlan <jferlan@redhat.com>
> ---
> 
>  Although technically a CI build breaker since virt-manager test is
>  failing, I figured I'd let this one go through the formal review just
>  in case someone has agita over new function name or would like to see
>  things done in a different manner.
> 
> 
>  src/conf/domain_conf.c | 39 ++++++++++++++++++++++++---------------
>  1 file changed, 24 insertions(+), 15 deletions(-)

Can you add a test case for that, since the lack of tests is why
we have this regression in tree, and I'd feel comfortable that it
actually fixes the problem if tests show it.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/1] tests: Add xml2xml tests for smartcard processing
Posted by John Ferlan 6 years, 8 months ago
Merge into previous...  With the explanation to add smartcard xml2xml
tests.

As shown below other tests already exists.

This includes the aha moment where an extra > was being printed for
smartcards using type= as a result of virDomainChrTypeFormat doing the
printing.  Needed to print the > for the other two types explicitly.

Existing tests (far more exist for channel, console, serial, and parallel):

tests/qemuxml2xmloutdata/qemuxml2xmlout-channel-guestfwd.xml:
...
    <channel type='pipe'>
      <source path='/tmp/guestfwd'/>
...

tests/qemuxml2xmloutdata/qemuxml2xmlout-console-virtio.xml:
...
    <console type='pty'>
      <target type='virtio' port='0'/>
    </console>
...

tests/qemuxml2xmloutdata/qemuxml2xmlout-serial-pty.xml:
...
    <serial type='pty'>
      <target port='0'/>
    </serial>
...

tests/qemuxml2xmloutdata/qemuxml2xmlout-parallel-tcp.xml:
...
    <parallel type='tcp'>
      <source mode='bind' host='127.0.0.1' service='9999'/>
      <protocol type='raw'/>
...

qemuxml2xmlout-virtio-rng-egd.xml:
...
    <rng model='virtio'>
      <backend model='egd' type='tcp'>
        <source mode='connect' host='1.2.3.4' service='1234'/>
        <protocol type='raw'/>
      </backend>
...

qemuxml2xmlout-usb-redir-filter-version.xml:
...
    <redirdev bus='usb' type='spicevmc'>
      <address type='usb' bus='0' port='1'/>
    </redirdev>
...

Signed-off-by: John Ferlan <jferlan@redhat.com>
---
 src/conf/domain_conf.c                             |  3 +-
 .../qemuxml2xmlout-smartcard-controller.xml        | 31 ++++++++++++++++++++
 .../qemuxml2xmlout-smartcard-host-certificates.xml | 34 ++++++++++++++++++++++
 .../qemuxml2xmlout-smartcard-host.xml              | 31 ++++++++++++++++++++
 ...muxml2xmlout-smartcard-passthrough-spicevmc.xml | 31 ++++++++++++++++++++
 .../qemuxml2xmlout-smartcard-passthrough-tcp.xml   | 33 +++++++++++++++++++++
 tests/qemuxml2xmltest.c                            |  6 ++++
 7 files changed, 168 insertions(+), 1 deletion(-)
 create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-smartcard-controller.xml
 create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-smartcard-host-certificates.xml
 create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-smartcard-host.xml
 create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-smartcard-passthrough-spicevmc.xml
 create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-smartcard-passthrough-tcp.xml

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 878c15d..f758210 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -23064,9 +23064,11 @@ virDomainSmartcardDefFormat(virBufferPtr buf,
 
     switch (def->type) {
     case VIR_DOMAIN_SMARTCARD_TYPE_HOST:
+        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(&childBuf, "<certificate>%s</certificate>\n",
                                   def->data.cert.file[i]);
@@ -23092,7 +23094,6 @@ virDomainSmartcardDefFormat(virBufferPtr buf,
         return -1;
 
     if (virBufferUse(&childBuf)) {
-        virBufferAddLit(buf, ">\n");
         virBufferAddBuffer(buf, &childBuf);
         virBufferAddLit(buf, "</smartcard>\n");
     } else {
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-smartcard-controller.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-smartcard-controller.xml
new file mode 100644
index 0000000..dc7365b
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-smartcard-controller.xml
@@ -0,0 +1,31 @@
+<domain type='qemu'>
+  <name>QEMUGuest1</name>
+  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+  <memory unit='KiB'>219136</memory>
+  <currentMemory unit='KiB'>219136</currentMemory>
+  <vcpu placement='static'>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-system-i686</emulator>
+    <controller type='ccid' index='0'/>
+    <controller type='usb' index='0'>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/>
+    </controller>
+    <controller type='pci' index='0' model='pci-root'/>
+    <smartcard mode='host'>
+      <address type='ccid' controller='0' slot='0'/>
+    </smartcard>
+    <input type='mouse' bus='ps2'/>
+    <input type='keyboard' bus='ps2'/>
+    <memballoon model='virtio'>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
+    </memballoon>
+  </devices>
+</domain>
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-smartcard-host-certificates.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-smartcard-host-certificates.xml
new file mode 100644
index 0000000..e0835f6
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-smartcard-host-certificates.xml
@@ -0,0 +1,34 @@
+<domain type='qemu'>
+  <name>QEMUGuest1</name>
+  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+  <memory unit='KiB'>219136</memory>
+  <currentMemory unit='KiB'>219136</currentMemory>
+  <vcpu placement='static'>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-system-i686</emulator>
+    <controller type='usb' index='0'>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/>
+    </controller>
+    <controller type='pci' index='0' model='pci-root'/>
+    <controller type='ccid' index='0'/>
+    <smartcard mode='host-certificates'>
+      <certificate>cert1</certificate>
+      <certificate>cert2</certificate>
+      <certificate>cert3</certificate>
+      <address type='ccid' controller='0' slot='0'/>
+    </smartcard>
+    <input type='mouse' bus='ps2'/>
+    <input type='keyboard' bus='ps2'/>
+    <memballoon model='virtio'>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
+    </memballoon>
+  </devices>
+</domain>
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-smartcard-host.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-smartcard-host.xml
new file mode 100644
index 0000000..abb2c4b
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-smartcard-host.xml
@@ -0,0 +1,31 @@
+<domain type='qemu'>
+  <name>QEMUGuest1</name>
+  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+  <memory unit='KiB'>219136</memory>
+  <currentMemory unit='KiB'>219136</currentMemory>
+  <vcpu placement='static'>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-system-i686</emulator>
+    <controller type='usb' index='0'>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/>
+    </controller>
+    <controller type='pci' index='0' model='pci-root'/>
+    <controller type='ccid' index='0'/>
+    <smartcard mode='host'>
+      <address type='ccid' controller='0' slot='0'/>
+    </smartcard>
+    <input type='mouse' bus='ps2'/>
+    <input type='keyboard' bus='ps2'/>
+    <memballoon model='virtio'>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
+    </memballoon>
+  </devices>
+</domain>
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-smartcard-passthrough-spicevmc.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-smartcard-passthrough-spicevmc.xml
new file mode 100644
index 0000000..38755e2
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-smartcard-passthrough-spicevmc.xml
@@ -0,0 +1,31 @@
+<domain type='qemu'>
+  <name>QEMUGuest1</name>
+  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+  <memory unit='KiB'>219136</memory>
+  <currentMemory unit='KiB'>219136</currentMemory>
+  <vcpu placement='static'>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-system-i686</emulator>
+    <controller type='usb' index='0'>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/>
+    </controller>
+    <controller type='pci' index='0' model='pci-root'/>
+    <controller type='ccid' index='0'/>
+    <smartcard mode='passthrough' type='spicevmc'>
+      <address type='ccid' controller='0' slot='0'/>
+    </smartcard>
+    <input type='mouse' bus='ps2'/>
+    <input type='keyboard' bus='ps2'/>
+    <memballoon model='virtio'>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
+    </memballoon>
+  </devices>
+</domain>
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-smartcard-passthrough-tcp.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-smartcard-passthrough-tcp.xml
new file mode 100644
index 0000000..2232daa
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-smartcard-passthrough-tcp.xml
@@ -0,0 +1,33 @@
+<domain type='qemu'>
+  <name>QEMUGuest1</name>
+  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+  <memory unit='KiB'>219136</memory>
+  <currentMemory unit='KiB'>219136</currentMemory>
+  <vcpu placement='static'>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-system-i686</emulator>
+    <controller type='usb' index='0'>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/>
+    </controller>
+    <controller type='pci' index='0' model='pci-root'/>
+    <controller type='ccid' index='0'/>
+    <smartcard mode='passthrough' type='tcp'>
+      <source mode='bind' host='127.0.0.1' service='2001'/>
+      <protocol type='raw'/>
+      <address type='ccid' controller='0' slot='0'/>
+    </smartcard>
+    <input type='mouse' bus='ps2'/>
+    <input type='keyboard' bus='ps2'/>
+    <memballoon model='virtio'>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
+    </memballoon>
+  </devices>
+</domain>
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index bf4d507..0d549ad 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -1199,6 +1199,12 @@ mymain(void)
     DO_TEST("cpu-check-default-partial", NONE);
     DO_TEST("cpu-check-default-partial2", NONE);
 
+    DO_TEST("smartcard-host", NONE);
+    DO_TEST("smartcard-host-certificates", NONE);
+    DO_TEST("smartcard-passthrough-tcp", NONE);
+    DO_TEST("smartcard-passthrough-spicevmc", NONE);
+    DO_TEST("smartcard-controller", NONE);
+
     if (getenv("LIBVIRT_SKIP_CLEANUP") == NULL)
         virFileDeleteTree(fakerootdir);
 
-- 
2.9.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/1] tests: Add xml2xml tests for smartcard processing
Posted by Ján Tomko 6 years, 8 months ago
Thanks for volunteering to fix this after me, I did not expect that,
here is my attempt:
https://www.redhat.com/archives/libvir-list/2017-August/msg00134.html

On Thu, Aug 03, 2017 at 07:30:43AM -0400, John Ferlan wrote:
>Merge into previous...  With the explanation to add smartcard xml2xml

A v2 would have been easier to read

>tests.
>
>As shown below other tests already exists.
>
>This includes the aha moment where an extra > was being printed for
>smartcards using type= as a result of virDomainChrTypeFormat doing the
>printing.  Needed to print the > for the other two types explicitly.
>
>Existing tests (far more exist for channel, console, serial, and parallel):
>
>tests/qemuxml2xmloutdata/qemuxml2xmlout-channel-guestfwd.xml:
>...
>    <channel type='pipe'>
>      <source path='/tmp/guestfwd'/>
>...
>
>tests/qemuxml2xmloutdata/qemuxml2xmlout-console-virtio.xml:
>...
>    <console type='pty'>
>      <target type='virtio' port='0'/>
>    </console>
>...
>
>tests/qemuxml2xmloutdata/qemuxml2xmlout-serial-pty.xml:
>...
>    <serial type='pty'>
>      <target port='0'/>
>    </serial>
>...
>
>tests/qemuxml2xmloutdata/qemuxml2xmlout-parallel-tcp.xml:
>...
>    <parallel type='tcp'>
>      <source mode='bind' host='127.0.0.1' service='9999'/>
>      <protocol type='raw'/>
>...
>
>qemuxml2xmlout-virtio-rng-egd.xml:
>...
>    <rng model='virtio'>
>      <backend model='egd' type='tcp'>
>        <source mode='connect' host='1.2.3.4' service='1234'/>
>        <protocol type='raw'/>
>      </backend>
>...
>
>qemuxml2xmlout-usb-redir-filter-version.xml:
>...
>    <redirdev bus='usb' type='spicevmc'>
>      <address type='usb' bus='0' port='1'/>
>    </redirdev>
>...
>
>Signed-off-by: John Ferlan <jferlan@redhat.com>
>---
> src/conf/domain_conf.c                             |  3 +-
> .../qemuxml2xmlout-smartcard-controller.xml        | 31 ++++++++++++++++++++
> .../qemuxml2xmlout-smartcard-host-certificates.xml | 34 ++++++++++++++++++++++
> .../qemuxml2xmlout-smartcard-host.xml              | 31 ++++++++++++++++++++
> ...muxml2xmlout-smartcard-passthrough-spicevmc.xml | 31 ++++++++++++++++++++
> .../qemuxml2xmlout-smartcard-passthrough-tcp.xml   | 33 +++++++++++++++++++++
> tests/qemuxml2xmltest.c                            |  6 ++++
> 7 files changed, 168 insertions(+), 1 deletion(-)
> create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-smartcard-controller.xml
> create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-smartcard-host-certificates.xml
> create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-smartcard-host.xml
> create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-smartcard-passthrough-spicevmc.xml
> create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-smartcard-passthrough-tcp.xml
>
>diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>index 878c15d..f758210 100644
>--- a/src/conf/domain_conf.c
>+++ b/src/conf/domain_conf.c
>@@ -23064,9 +23064,11 @@ virDomainSmartcardDefFormat(virBufferPtr buf,
>
>     switch (def->type) {
>     case VIR_DOMAIN_SMARTCARD_TYPE_HOST:
>+        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(&childBuf, "<certificate>%s</certificate>\n",
>                                   def->data.cert.file[i]);
>@@ -23092,7 +23094,6 @@ virDomainSmartcardDefFormat(virBufferPtr buf,
>         return -1;
>
>     if (virBufferUse(&childBuf)) {
>-        virBufferAddLit(buf, ">\n");

This change makes the else branch pointless, since it assumes the element
will be a pair one.

I split the '>' addition out of the Chr.*Format functions completely,
to let the callers decide.

>         virBufferAddBuffer(buf, &childBuf);
>         virBufferAddLit(buf, "</smartcard>\n");
>     } else {

[...]

>diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
>index bf4d507..0d549ad 100644
>--- a/tests/qemuxml2xmltest.c
>+++ b/tests/qemuxml2xmltest.c
>@@ -1199,6 +1199,12 @@ mymain(void)
>     DO_TEST("cpu-check-default-partial", NONE);
>     DO_TEST("cpu-check-default-partial2", NONE);
>
>+    DO_TEST("smartcard-host", NONE);
>+    DO_TEST("smartcard-host-certificates", NONE);
>+    DO_TEST("smartcard-passthrough-tcp", NONE);
>+    DO_TEST("smartcard-passthrough-spicevmc", NONE);
>+    DO_TEST("smartcard-controller", NONE);
>+

But your test cases match mine exactly :D

Jan

>     if (getenv("LIBVIRT_SKIP_CLEANUP") == NULL)
>         virFileDeleteTree(fakerootdir);
>
>-- 
>2.9.4
>
>--
>libvir-list mailing list
>libvir-list@redhat.com
>https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list