[libvirt] [PATCH 0/2] Fix smartcard formatting regression

*** BLURB HERE *** Ján Tomko (2): tests: add smartcard test cases to qemuxml2xmltest conf: fix formatting of smartcard devices src/conf/domain_conf.c | 43 ++++++++++++++-------- .../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, 194 insertions(+), 15 deletions(-) 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 -- 2.13.0

Generated from the tests files used by qemuxml2argvtest with the formatting function from before my commit 0c1d8632 broke smartcard XML formatting. --- .../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 ++++ 6 files changed, 166 insertions(+) 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/tests/qemuxml2xmloutdata/qemuxml2xmlout-smartcard-controller.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-smartcard-controller.xml new file mode 100644 index 000000000..dc7365b0b --- /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 000000000..e0835f6c0 --- /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 000000000..abb2c4b6e --- /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 000000000..38755e2d7 --- /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 000000000..2232daa35 --- /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 bf4d507f6..0d549adec 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.13.0

My commit 0c1d863 broke formatting of passthrough smartcard devices: <smartcard mode='passthrough' type='spicevmc'/> resulted in invalid XML: <smartcard mode='passthrough'> type='spicevmc'> <address type='ccid' controller='0' slot='0'/> </smartcard> Split out chardev source formatting function into two - one formatting the attributes and other formatting the subelements. Reported-by: Cole Robinson <crobinso@redhat.com> --- src/conf/domain_conf.c | 43 ++++++++++++++++++++++++++++--------------- 1 file changed, 28 insertions(+), 15 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index eb7052303..640f29d3e 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) +virDomainChrAttrsDefFormat(virBufferPtr buf, + virDomainChrSourceDefPtr def, + bool tty_compat) { const char *type = virDomainChrTypeToString(def->type); @@ -22823,7 +22822,14 @@ virDomainChrSourceDefFormat(virBufferPtr buf, virBufferEscapeString(buf, " tty='%s'", def->data.file.path); } - 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: @@ -22924,7 +22930,7 @@ virDomainChrSourceDefFormat(virBufferPtr buf, virBufferAddLit(buf, "/>\n"); } - return 0; + return; } static int @@ -22953,8 +22959,10 @@ 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 (virDomainChrAttrsDefFormat(buf, def->source, tty_compat) < 0) return -1; + virBufferAddLit(buf, ">\n"); + virDomainChrSourceDefFormat(buf, def->source, flags); /* Format <target> block */ switch (def->deviceType) { @@ -23067,9 +23075,7 @@ virDomainSmartcardDefFormat(virBufferPtr buf, break; case VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH: - if (virDomainChrSourceDefFormat(&childBuf, def->data.passthru, false, - flags) < 0) - return -1; + virDomainChrSourceDefFormat(&childBuf, def->data.passthru, flags); break; default: @@ -23083,6 +23089,10 @@ virDomainSmartcardDefFormat(virBufferPtr buf, return -1; virBufferAsprintf(buf, "<smartcard mode='%s'", mode); + if (def->type == VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH && + virDomainChrAttrsDefFormat(buf, def->data.passthru, false) < 0) + return -1; + if (virBufferUse(&childBuf)) { virBufferAddLit(buf, ">\n"); virBufferAddBuffer(buf, &childBuf); @@ -23390,10 +23400,11 @@ virDomainRNGDefFormat(virBufferPtr buf, break; case VIR_DOMAIN_RNG_BACKEND_EGD: - virBufferAdjustIndent(buf, 2); - if (virDomainChrSourceDefFormat(buf, def->source.chardev, - false, flags) < 0) + if (virDomainChrAttrsDefFormat(buf, def->source.chardev, false) < 0) return -1; + virBufferAddLit(buf, ">\n"); + virBufferAdjustIndent(buf, 2); + virDomainChrSourceDefFormat(buf, def->source.chardev, flags); virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "</backend>\n"); @@ -24234,9 +24245,11 @@ 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 (virDomainChrAttrsDefFormat(buf, def->source, false) < 0) return -1; + virBufferAddLit(buf, ">\n"); + virBufferAdjustIndent(buf, 2); + virDomainChrSourceDefFormat(buf, def->source, flags); virDomainDeviceInfoFormat(buf, &def->info, flags | VIR_DOMAIN_DEF_FORMAT_ALLOW_BOOT); virBufferAdjustIndent(buf, -2); -- 2.13.0

On 08/03/2017 08:55 AM, Ján Tomko wrote:
*** BLURB HERE ***
Ján Tomko (2): tests: add smartcard test cases to qemuxml2xmltest conf: fix formatting of smartcard devices
src/conf/domain_conf.c | 43 ++++++++++++++-------- .../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, 194 insertions(+), 15 deletions(-) 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
I'm fine with this solution - instead of what I posted: https://www.redhat.com/archives/libvir-list/2017-August/msg00101.html The only differences between the two are: 1. Where the virBufferAddLit(buf, ">\n"); is placed for all callers 2. How the smartcard passthrough is processed I posted mainly as a means to have "something" available - I can drop my series... Reviewed-by: John Ferlan <jferlan@redhat.com> Although I assume you're going to merge them together so as to not break git bisect, right? John
participants (2)
-
John Ferlan
-
Ján Tomko