We parse the seclabels and use them internally so omitting them when formatting would be misleading. Additionally our schema actually allows them. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/conf/domain_conf.c | 3 +-- tests/qemuxml2argvdata/disk-backing-chains.xml | 6 +++++- tests/qemuxml2xmloutdata/disk-backing-chains-active.xml | 6 +++++- tests/qemuxml2xmloutdata/disk-backing-chains-inactive.xml | 6 +++++- 4 files changed, 16 insertions(+), 5 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 9d8eabb42a..87db233d9f 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -23876,8 +23876,7 @@ virDomainDiskBackingStoreFormat(virBufferPtr buf, virBufferAdjustIndent(buf, 2); virBufferAsprintf(buf, "<format type='%s'/>\n", format); - /* We currently don't output seclabels for backing chain element */ - if (virDomainDiskSourceFormatInternal(buf, backingStore, 0, flags, false, + if (virDomainDiskSourceFormatInternal(buf, backingStore, 0, flags, true, false, xmlopt) < 0 || virDomainDiskBackingStoreFormat(buf, backingStore->backingStore, xmlopt, flags) < 0) diff --git a/tests/qemuxml2argvdata/disk-backing-chains.xml b/tests/qemuxml2argvdata/disk-backing-chains.xml index 703c60c439..2eaafb34e7 100644 --- a/tests/qemuxml2argvdata/disk-backing-chains.xml +++ b/tests/qemuxml2argvdata/disk-backing-chains.xml @@ -38,7 +38,11 @@ <source file='/tmp/image2.qcow'/> <backingStore type='file' index='3'> <format type='qcow2'/> - <source file='/tmp/image3.qcow'/> + <source file='/tmp/image3.qcow'> + <seclabel model='selinux' relabel='yes'> + <label>system_u:system_r:public_content_t:s0</label> + </seclabel> + </source> <backingStore type='file' index='4'> <format type='qcow2'/> <source file='/tmp/image4.qcow'/> diff --git a/tests/qemuxml2xmloutdata/disk-backing-chains-active.xml b/tests/qemuxml2xmloutdata/disk-backing-chains-active.xml index d1fd2442c3..e24956f106 100644 --- a/tests/qemuxml2xmloutdata/disk-backing-chains-active.xml +++ b/tests/qemuxml2xmloutdata/disk-backing-chains-active.xml @@ -39,7 +39,11 @@ <source file='/tmp/image2.qcow'/> <backingStore type='file' index='3'> <format type='qcow2'/> - <source file='/tmp/image3.qcow'/> + <source file='/tmp/image3.qcow'> + <seclabel model='selinux' relabel='yes'> + <label>system_u:system_r:public_content_t:s0</label> + </seclabel> + </source> <backingStore type='file' index='4'> <format type='qcow2'/> <source file='/tmp/image4.qcow'/> diff --git a/tests/qemuxml2xmloutdata/disk-backing-chains-inactive.xml b/tests/qemuxml2xmloutdata/disk-backing-chains-inactive.xml index c1af58ff6f..e39e218873 100644 --- a/tests/qemuxml2xmloutdata/disk-backing-chains-inactive.xml +++ b/tests/qemuxml2xmloutdata/disk-backing-chains-inactive.xml @@ -39,7 +39,11 @@ <source file='/tmp/image2.qcow'/> <backingStore type='file'> <format type='qcow2'/> - <source file='/tmp/image3.qcow'/> + <source file='/tmp/image3.qcow'> + <seclabel model='selinux' relabel='yes'> + <label>system_u:system_r:public_content_t:s0</label> + </seclabel> + </source> <backingStore type='file'> <format type='qcow2'/> <source file='/tmp/image4.qcow'/> -- 2.20.1

All callers including transitive callers through virDomainDiskSourceFormatInternal always pass true. Remove the argument. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/conf/domain_conf.c | 15 ++++++--------- src/conf/domain_conf.h | 3 +-- src/qemu/qemu_domain.c | 2 +- 3 files changed, 8 insertions(+), 12 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 87db233d9f..69fd4a7c57 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -23720,8 +23720,7 @@ int virDomainStorageSourceFormat(virBufferPtr attrBuf, virBufferPtr childBuf, virStorageSourcePtr src, - unsigned int flags, - bool seclabels) + unsigned int flags) { switch ((virStorageType)src->type) { case VIR_STORAGE_TYPE_FILE: @@ -23761,7 +23760,7 @@ virDomainStorageSourceFormat(virBufferPtr attrBuf, return -1; } - if (seclabels && src->type != VIR_STORAGE_TYPE_NETWORK) + if (src->type != VIR_STORAGE_TYPE_NETWORK) virDomainSourceDefFormatSeclabel(childBuf, src->nseclabels, src->seclabels, flags); @@ -23793,7 +23792,6 @@ virDomainDiskSourceFormatInternal(virBufferPtr buf, virStorageSourcePtr src, int policy, unsigned int flags, - bool seclabels, bool attrIndex, virDomainXMLOptionPtr xmlopt) { @@ -23803,8 +23801,7 @@ virDomainDiskSourceFormatInternal(virBufferPtr buf, virBufferSetChildIndent(&childBuf, buf); - if (virDomainStorageSourceFormat(&attrBuf, &childBuf, src, flags, - seclabels) < 0) + if (virDomainStorageSourceFormat(&attrBuf, &childBuf, src, flags) < 0) goto cleanup; if (policy && src->type != VIR_STORAGE_TYPE_NETWORK) @@ -23834,7 +23831,7 @@ virDomainDiskSourceFormat(virBufferPtr buf, unsigned int flags, virDomainXMLOptionPtr xmlopt) { - return virDomainDiskSourceFormatInternal(buf, src, policy, flags, true, + return virDomainDiskSourceFormatInternal(buf, src, policy, flags, false, xmlopt); } @@ -23876,7 +23873,7 @@ virDomainDiskBackingStoreFormat(virBufferPtr buf, virBufferAdjustIndent(buf, 2); virBufferAsprintf(buf, "<format type='%s'/>\n", format); - if (virDomainDiskSourceFormatInternal(buf, backingStore, 0, flags, true, + if (virDomainDiskSourceFormatInternal(buf, backingStore, 0, flags, false, xmlopt) < 0 || virDomainDiskBackingStoreFormat(buf, backingStore->backingStore, xmlopt, flags) < 0) @@ -24135,7 +24132,7 @@ virDomainDiskDefFormat(virBufferPtr buf, virStorageAuthDefFormat(buf, def->src->auth); if (virDomainDiskSourceFormatInternal(buf, def->src, def->startupPolicy, - flags, true, true, xmlopt) < 0) + flags, true, xmlopt) < 0) return -1; /* Don't format backingStore to inactive XMLs until the code for diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 4a25480662..25c10a9af3 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3459,8 +3459,7 @@ int virDomainDiskDefCheckDuplicateInfo(const virDomainDiskDef *a, int virDomainStorageSourceFormat(virBufferPtr attrBuf, virBufferPtr childBuf, virStorageSourcePtr src, - unsigned int flags, - bool seclabels) + unsigned int flags) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); int virDomainStorageSourceParse(xmlNodePtr node, diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 9ca4ca33e5..a1b39220d1 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2358,7 +2358,7 @@ qemuDomainObjPrivateXMLFormatNBDMigrationSource(virBufferPtr buf, virStorageFileFormatTypeToString(src->format)); if (virDomainStorageSourceFormat(&attrBuf, &childBuf, src, - VIR_DOMAIN_DEF_FORMAT_STATUS, true) < 0) + VIR_DOMAIN_DEF_FORMAT_STATUS) < 0) goto cleanup; if (qemuStorageSourcePrivateDataFormat(src, &privateDataBuf) < 0) -- 2.20.1

Remove the wrapper and fix callers. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/conf/domain_conf.c | 35 +++++++++++------------------------ src/conf/domain_conf.h | 1 + src/conf/snapshot_conf.c | 2 +- tests/qemublocktest.c | 2 +- tests/virstoragetest.c | 2 +- 5 files changed, 15 insertions(+), 27 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 69fd4a7c57..adb20e33f2 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -23787,13 +23787,13 @@ virDomainStorageSourceFormat(virBufferPtr attrBuf, } -static int -virDomainDiskSourceFormatInternal(virBufferPtr buf, - virStorageSourcePtr src, - int policy, - unsigned int flags, - bool attrIndex, - virDomainXMLOptionPtr xmlopt) +int +virDomainDiskSourceFormat(virBufferPtr buf, + virStorageSourcePtr src, + int policy, + bool attrIndex, + unsigned int flags, + virDomainXMLOptionPtr xmlopt) { VIR_AUTOCLEAN(virBuffer) attrBuf = VIR_BUFFER_INITIALIZER; VIR_AUTOCLEAN(virBuffer) childBuf = VIR_BUFFER_INITIALIZER; @@ -23824,18 +23824,6 @@ virDomainDiskSourceFormatInternal(virBufferPtr buf, } -int -virDomainDiskSourceFormat(virBufferPtr buf, - virStorageSourcePtr src, - int policy, - unsigned int flags, - virDomainXMLOptionPtr xmlopt) -{ - return virDomainDiskSourceFormatInternal(buf, src, policy, flags, - false, xmlopt); -} - - static int virDomainDiskBackingStoreFormat(virBufferPtr buf, virStorageSourcePtr backingStore, @@ -23873,8 +23861,7 @@ virDomainDiskBackingStoreFormat(virBufferPtr buf, virBufferAdjustIndent(buf, 2); virBufferAsprintf(buf, "<format type='%s'/>\n", format); - if (virDomainDiskSourceFormatInternal(buf, backingStore, 0, flags, - false, xmlopt) < 0 || + if (virDomainDiskSourceFormat(buf, backingStore, 0, false, flags, xmlopt) < 0 || virDomainDiskBackingStoreFormat(buf, backingStore->backingStore, xmlopt, flags) < 0) return -1; @@ -24035,7 +24022,7 @@ virDomainDiskDefFormatMirror(virBufferPtr buf, virBufferAddLit(buf, ">\n"); virBufferAdjustIndent(buf, 2); virBufferEscapeString(buf, "<format type='%s'/>\n", formatStr); - if (virDomainDiskSourceFormat(buf, disk->mirror, 0, 0, xmlopt) < 0) + if (virDomainDiskSourceFormat(buf, disk->mirror, 0, false, 0, xmlopt) < 0) return -1; virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "</mirror>\n"); @@ -24131,8 +24118,8 @@ virDomainDiskDefFormat(virBufferPtr buf, if (def->src->auth && !def->src->authInherited) virStorageAuthDefFormat(buf, def->src->auth); - if (virDomainDiskSourceFormatInternal(buf, def->src, def->startupPolicy, - flags, true, xmlopt) < 0) + if (virDomainDiskSourceFormat(buf, def->src, def->startupPolicy, true, + flags, xmlopt) < 0) return -1; /* Don't format backingStore to inactive XMLs until the code for diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 25c10a9af3..f0c2a51212 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3014,6 +3014,7 @@ int virDomainDefFormatInternal(virDomainDefPtr def, int virDomainDiskSourceFormat(virBufferPtr buf, virStorageSourcePtr src, int policy, + bool attrIndex, unsigned int flags, virDomainXMLOptionPtr xmlopt); diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c index 52abafab0f..58d2cfee99 100644 --- a/src/conf/snapshot_conf.c +++ b/src/conf/snapshot_conf.c @@ -776,7 +776,7 @@ virDomainSnapshotDiskDefFormat(virBufferPtr buf, if (disk->src->format > 0) virBufferEscapeString(buf, "<driver type='%s'/>\n", virStorageFileFormatTypeToString(disk->src->format)); - if (virDomainDiskSourceFormat(buf, disk->src, 0, 0, xmlopt) < 0) + if (virDomainDiskSourceFormat(buf, disk->src, 0, false, 0, xmlopt) < 0) return -1; virBufferAdjustIndent(buf, -2); diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c index becd31a0ad..46e75c2df5 100644 --- a/tests/qemublocktest.c +++ b/tests/qemublocktest.c @@ -86,7 +86,7 @@ testBackingXMLjsonXML(const void *args) return -1; } - if (virDomainDiskSourceFormat(&buf, jsonsrc, 0, 0, NULL) < 0 || + if (virDomainDiskSourceFormat(&buf, jsonsrc, 0, false, 0, NULL) < 0 || !(actualxml = virBufferContentAndReset(&buf))) { fprintf(stderr, "failed to format disk source xml\n"); return -1; diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index fb98903f02..d29181dea9 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -657,7 +657,7 @@ testBackingParse(const void *args) goto cleanup; } - if (virDomainDiskSourceFormat(&buf, src, 0, 0, NULL) < 0 || + if (virDomainDiskSourceFormat(&buf, src, 0, false, 0, NULL) < 0 || !(xml = virBufferContentAndReset(&buf))) { fprintf(stderr, "failed to format disk source xml\n"); goto cleanup; -- 2.20.1

Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/libvirt_private.syms | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index bf5625fbf4..c26c525d37 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -354,6 +354,7 @@ virDomainDiskSetDriver; virDomainDiskSetFormat; virDomainDiskSetSource; virDomainDiskSetType; +virDomainDiskSourceFormat; virDomainDiskTranslateSourcePool; virDomainFeatureTypeFromString; virDomainFeatureTypeToString; -- 2.20.1

Block job related data will be stored in a has table and formatted into the status XML. Use the mock to guarantee stable tests. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- tests/qemuxml2xmltest.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 83a0d1cf7b..408d20a92a 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -1305,7 +1305,8 @@ mymain(void) VIR_TEST_MAIN_PRELOAD(mymain, abs_builddir "/.libs/virpcimock.so", - abs_builddir "/.libs/virrandommock.so") + abs_builddir "/.libs/virrandommock.so", + abs_builddir "/.libs/virdeterministichashmock.so") #else -- 2.20.1

Upcomming change will modify some aspects. To allow testing upgrade path add another disk. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- .../migration-out-nbd-tls-in.xml | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tests/qemustatusxml2xmldata/migration-out-nbd-tls-in.xml b/tests/qemustatusxml2xmldata/migration-out-nbd-tls-in.xml index be1dc7347e..f3bbc752b6 100644 --- a/tests/qemustatusxml2xmldata/migration-out-nbd-tls-in.xml +++ b/tests/qemustatusxml2xmldata/migration-out-nbd-tls-in.xml @@ -247,6 +247,17 @@ </privateData> </migrationSource> </disk> + <disk dev='vdc' migrating='yes'> + <migrationSource type='network' format='raw' protocol='nbd' name='drive-virtio-disk2' tlsFromConfig='0'> + <host name='andariel.usersys.redhat.com' port='49152'/> + <privateData> + <nodenames> + <nodename type='storage' name='migration-vdc-storage'/> + <nodename type='format' name='migration-vdc-format'/> + </nodenames> + </privateData> + </migrationSource> + </disk> <disk dev='hda' migrating='no'/> </job> <devices> @@ -334,6 +345,14 @@ <alias name='virtio-disk1'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x0b' function='0x0'/> </disk> + <disk type='file' device='disk'> + <driver name='qemu' type='qcow2'/> + <source file='/var/lib/libvirt/images/b.qcow2'/> + <backingStore/> + <target dev='vdc' bus='virtio'/> + <alias name='virtio-disk2'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x0c' function='0x0'/> + </disk> <disk type='file' device='cdrom'> <driver name='qemu' type='raw'/> <source file='/var/lib/libvirt/images/systemrescuecd-x86-4.9.5.iso'/> -- 2.20.1

Upcomming change will modify some aspects. To allow testing upgrade path add a separate output file so that we can see the conversion from old to new config. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- .../migration-out-nbd-tls-out.xml | 484 +++++++++++++++++- 1 file changed, 483 insertions(+), 1 deletion(-) mode change 120000 => 100644 tests/qemustatusxml2xmldata/migration-out-nbd-tls-out.xml diff --git a/tests/qemustatusxml2xmldata/migration-out-nbd-tls-out.xml b/tests/qemustatusxml2xmldata/migration-out-nbd-tls-out.xml deleted file mode 120000 index e8cdbdb55e..0000000000 --- a/tests/qemustatusxml2xmldata/migration-out-nbd-tls-out.xml +++ /dev/null @@ -1 +0,0 @@ -migration-out-nbd-tls-in.xml \ No newline at end of file diff --git a/tests/qemustatusxml2xmldata/migration-out-nbd-tls-out.xml b/tests/qemustatusxml2xmldata/migration-out-nbd-tls-out.xml new file mode 100644 index 0000000000..f3bbc752b6 --- /dev/null +++ b/tests/qemustatusxml2xmldata/migration-out-nbd-tls-out.xml @@ -0,0 +1,483 @@ +<domstatus state='running' reason='booted' pid='68472'> + <taint flag='high-privileges'/> + <monitor path='/var/lib/libvirt/qemu/domain-3-upstream/monitor.sock' json='1' type='unix'/> + <namespaces> + <mount/> + </namespaces> + <vcpus> + <vcpu id='0' pid='68477'/> + <vcpu id='1' pid='68478'/> + </vcpus> + <qemuCaps> + <flag name='kvm'/> + <flag name='mem-path'/> + <flag name='drive-serial'/> + <flag name='monitor-json'/> + <flag name='sdl'/> + <flag name='netdev'/> + <flag name='rtc'/> + <flag name='vhost-net'/> + <flag name='no-hpet'/> + <flag name='no-kvm-pit'/> + <flag name='nodefconfig'/> + <flag name='boot-menu'/> + <flag name='fsdev'/> + <flag name='name-process'/> + <flag name='smbios-type'/> + <flag name='spice'/> + <flag name='vga-none'/> + <flag name='boot-index'/> + <flag name='hda-duplex'/> + <flag name='drive-aio'/> + <flag name='ccid-emulated'/> + <flag name='ccid-passthru'/> + <flag name='chardev-spicevmc'/> + <flag name='virtio-tx-alg'/> + <flag name='pci-multifunction'/> + <flag name='virtio-blk-pci.ioeventfd'/> + <flag name='sga'/> + <flag name='virtio-blk-pci.event_idx'/> + <flag name='virtio-net-pci.event_idx'/> + <flag name='cache-directsync'/> + <flag name='piix3-usb-uhci'/> + <flag name='piix4-usb-uhci'/> + <flag name='usb-ehci'/> + <flag name='ich9-usb-ehci1'/> + <flag name='vt82c686b-usb-uhci'/> + <flag name='pci-ohci'/> + <flag name='usb-redir'/> + <flag name='usb-hub'/> + <flag name='no-shutdown'/> + <flag name='cache-unsafe'/> + <flag name='ich9-ahci'/> + <flag name='no-acpi'/> + <flag name='fsdev-readonly'/> + <flag name='virtio-blk-pci.scsi'/> + <flag name='drive-copy-on-read'/> + <flag name='fsdev-writeout'/> + <flag name='drive-iotune'/> + <flag name='system_wakeup'/> + <flag name='scsi-disk.channel'/> + <flag name='scsi-block'/> + <flag name='transaction'/> + <flag name='block-job-async'/> + <flag name='scsi-cd'/> + <flag name='ide-cd'/> + <flag name='no-user-config'/> + <flag name='hda-micro'/> + <flag name='dump-guest-memory'/> + <flag name='nec-usb-xhci'/> + <flag name='balloon-event'/> + <flag name='bridge'/> + <flag name='lsi'/> + <flag name='virtio-scsi-pci'/> + <flag name='blockio'/> + <flag name='disable-s3'/> + <flag name='disable-s4'/> + <flag name='usb-redir.filter'/> + <flag name='ide-drive.wwn'/> + <flag name='scsi-disk.wwn'/> + <flag name='seccomp-sandbox'/> + <flag name='reboot-timeout'/> + <flag name='dump-guest-core'/> + <flag name='seamless-migration'/> + <flag name='block-commit'/> + <flag name='vnc'/> + <flag name='drive-mirror'/> + <flag name='usb-redir.bootindex'/> + <flag name='usb-host.bootindex'/> + <flag name='blockdev-snapshot-sync'/> + <flag name='qxl'/> + <flag name='VGA'/> + <flag name='cirrus-vga'/> + <flag name='vmware-svga'/> + <flag name='device-video-primary'/> + <flag name='usb-serial'/> + <flag name='usb-net'/> + <flag name='add-fd'/> + <flag name='nbd-server'/> + <flag name='virtio-rng'/> + <flag name='rng-random'/> + <flag name='rng-egd'/> + <flag name='dtb'/> + <flag name='megasas'/> + <flag name='ipv6-migration'/> + <flag name='machine-opt'/> + <flag name='machine-usb-opt'/> + <flag name='tpm-passthrough'/> + <flag name='tpm-tis'/> + <flag name='pci-bridge'/> + <flag name='vfio-pci'/> + <flag name='vfio-pci.bootindex'/> + <flag name='scsi-generic'/> + <flag name='scsi-generic.bootindex'/> + <flag name='mem-merge'/> + <flag name='vnc-websocket'/> + <flag name='drive-discard'/> + <flag name='mlock'/> + <flag name='vnc-share-policy'/> + <flag name='device-del-event'/> + <flag name='dmi-to-pci-bridge'/> + <flag name='i440fx-pci-hole64-size'/> + <flag name='q35-pci-hole64-size'/> + <flag name='usb-storage'/> + <flag name='usb-storage.removable'/> + <flag name='virtio-mmio'/> + <flag name='ich9-intel-hda'/> + <flag name='kvm-pit-lost-tick-policy'/> + <flag name='boot-strict'/> + <flag name='pvpanic'/> + <flag name='spice-file-xfer-disable'/> + <flag name='spiceport'/> + <flag name='usb-kbd'/> + <flag name='host-pci-multidomain'/> + <flag name='msg-timestamp'/> + <flag name='active-commit'/> + <flag name='change-backing-file'/> + <flag name='memory-backend-ram'/> + <flag name='numa'/> + <flag name='memory-backend-file'/> + <flag name='usb-audio'/> + <flag name='rtc-reset-reinjection'/> + <flag name='splash-timeout'/> + <flag name='iothread'/> + <flag name='migrate-rdma'/> + <flag name='ivshmem'/> + <flag name='drive-iotune-max'/> + <flag name='VGA.vgamem_mb'/> + <flag name='vmware-svga.vgamem_mb'/> + <flag name='qxl.vgamem_mb'/> + <flag name='pc-dimm'/> + <flag name='machine-vmport-opt'/> + <flag name='aes-key-wrap'/> + <flag name='dea-key-wrap'/> + <flag name='pci-serial'/> + <flag name='vhost-user-multiqueue'/> + <flag name='migration-event'/> + <flag name='ioh3420'/> + <flag name='x3130-upstream'/> + <flag name='xio3130-downstream'/> + <flag name='rtl8139'/> + <flag name='e1000'/> + <flag name='virtio-net'/> + <flag name='gic-version'/> + <flag name='incoming-defer'/> + <flag name='virtio-gpu'/> + <flag name='virtio-gpu.virgl'/> + <flag name='virtio-keyboard'/> + <flag name='virtio-mouse'/> + <flag name='virtio-tablet'/> + <flag name='virtio-input-host'/> + <flag name='chardev-file-append'/> + <flag name='ich9-disable-s3'/> + <flag name='ich9-disable-s4'/> + <flag name='vserport-change-event'/> + <flag name='virtio-balloon-pci.deflate-on-oom'/> + <flag name='mptsas1068'/> + <flag name='qxl.vram64_size_mb'/> + <flag name='chardev-logfile'/> + <flag name='debug-threads'/> + <flag name='secret'/> + <flag name='pxb'/> + <flag name='pxb-pcie'/> + <flag name='device-tray-moved-event'/> + <flag name='nec-usb-xhci-ports'/> + <flag name='virtio-scsi-pci.iothread'/> + <flag name='name-guest'/> + <flag name='qxl.max_outputs'/> + <flag name='spice-unix'/> + <flag name='drive-detect-zeroes'/> + <flag name='tls-creds-x509'/> + <flag name='display'/> + <flag name='intel-iommu'/> + <flag name='smm'/> + <flag name='virtio-pci-disable-legacy'/> + <flag name='query-hotpluggable-cpus'/> + <flag name='virtio-net.rx_queue_size'/> + <flag name='virtio-vga'/> + <flag name='drive-iotune-max-length'/> + <flag name='ivshmem-plain'/> + <flag name='ivshmem-doorbell'/> + <flag name='query-qmp-schema'/> + <flag name='gluster.debug_level'/> + <flag name='vhost-scsi'/> + <flag name='drive-iotune-group'/> + <flag name='query-cpu-model-expansion'/> + <flag name='virtio-net.host_mtu'/> + <flag name='nvdimm'/> + <flag name='pcie-root-port'/> + <flag name='query-cpu-definitions'/> + <flag name='block-write-threshold'/> + <flag name='query-named-block-nodes'/> + <flag name='cpu-cache'/> + <flag name='qemu-xhci'/> + <flag name='kernel-irqchip'/> + <flag name='kernel-irqchip.split'/> + <flag name='intel-iommu.intremap'/> + <flag name='intel-iommu.caching-mode'/> + <flag name='intel-iommu.eim'/> + <flag name='intel-iommu.device-iotlb'/> + <flag name='virtio.iommu_platform'/> + <flag name='virtio.ats'/> + <flag name='loadparm'/> + <flag name='vnc-multi-servers'/> + <flag name='virtio-net.tx_queue_size'/> + <flag name='chardev-reconnect'/> + <flag name='virtio-gpu.max_outputs'/> + <flag name='vxhs'/> + <flag name='virtio-blk.num-queues'/> + <flag name='vmcoreinfo'/> + <flag name='numa.dist'/> + <flag name='disk-share-rw'/> + <flag name='iscsi.password-secret'/> + <flag name='isa-serial'/> + <flag name='dump-completed'/> + <flag name='nbd-tls'/> + <flag name='blockdev-del'/> + </qemuCaps> + <job type='none' async='migration out' phase='perform3' flags='0x0'> + <disk dev='vdb' migrating='yes'> + <migrationSource type='network' format='raw' protocol='nbd' name='drive-virtio-disk1' tlsFromConfig='0'> + <host name='andariel.usersys.redhat.com' port='49152'/> + <privateData> + <nodenames> + <nodename type='storage' name='migration-vdb-storage'/> + <nodename type='format' name='migration-vdb-format'/> + </nodenames> + </privateData> + </migrationSource> + </disk> + <disk dev='vdc' migrating='yes'> + <migrationSource type='network' format='raw' protocol='nbd' name='drive-virtio-disk2' tlsFromConfig='0'> + <host name='andariel.usersys.redhat.com' port='49152'/> + <privateData> + <nodenames> + <nodename type='storage' name='migration-vdc-storage'/> + <nodename type='format' name='migration-vdc-format'/> + </nodenames> + </privateData> + </migrationSource> + </disk> + <disk dev='hda' migrating='no'/> + </job> + <devices> + <device alias='rng0'/> + <device alias='sound0-codec0'/> + <device alias='virtio-disk1'/> + <device alias='virtio-serial0'/> + <device alias='video0'/> + <device alias='serial0'/> + <device alias='sound0'/> + <device alias='balloon0'/> + <device alias='channel1'/> + <device alias='channel0'/> + <device alias='net0'/> + <device alias='input0'/> + <device alias='redir0'/> + <device alias='redir1'/> + <device alias='scsi0'/> + <device alias='usb'/> + <device alias='ide0-0-0'/> + </devices> + <numad nodeset='0' cpuset='0-7'/> + <libDir path='/var/lib/libvirt/qemu/domain-3-upstream'/> + <channelTargetDir path='/var/lib/libvirt/qemu/channel/target/domain-3-upstream'/> + <chardevStdioLogd/> + <allowReboot value='yes'/> + <blockjobs active='no'/> + <domain type='kvm' id='3'> + <name>upstream</name> + <uuid>dcf47dbd-46d1-4d5b-b442-262a806a333a</uuid> + <memory unit='KiB'>1024000</memory> + <currentMemory unit='KiB'>1024000</currentMemory> + <memoryBacking> + <access mode='shared'/> + </memoryBacking> + <vcpu placement='auto' current='2'>8</vcpu> + <numatune> + <memory mode='strict' placement='auto'/> + </numatune> + <resource> + <partition>/machine</partition> + </resource> + <os> + <type arch='x86_64' machine='pc-i440fx-2.9'>hvm</type> + <bootmenu enable='yes'/> + </os> + <features> + <acpi/> + <apic/> + <vmport state='off'/> + </features> + <cpu> + <numa> + <cell id='0' cpus='0,2,4,6' memory='512000' unit='KiB'/> + <cell id='1' cpus='1,3,5,7' memory='512000' unit='KiB'/> + </numa> + </cpu> + <clock offset='utc'> + <timer name='rtc' tickpolicy='catchup'/> + <timer name='pit' tickpolicy='delay'/> + <timer name='hpet' present='no'/> + </clock> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>restart</on_crash> + <pm> + <suspend-to-mem enabled='no'/> + <suspend-to-disk enabled='no'/> + </pm> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <disk type='file' device='disk'> + <driver name='qemu' type='qcow2' discard='unmap' detect_zeroes='on'/> + <source file='/var/lib/libvirt/images/a.qcow2'/> + <backingStore type='file' index='1'> + <format type='qcow2'/> + <source file='/var/lib/libvirt/images/base.qcow2'> + <privateData> + <relPath>base.qcow2</relPath> + </privateData> + </source> + <backingStore/> + </backingStore> + <target dev='vdb' bus='virtio'/> + <alias name='virtio-disk1'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x0b' function='0x0'/> + </disk> + <disk type='file' device='disk'> + <driver name='qemu' type='qcow2'/> + <source file='/var/lib/libvirt/images/b.qcow2'/> + <backingStore/> + <target dev='vdc' bus='virtio'/> + <alias name='virtio-disk2'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x0c' function='0x0'/> + </disk> + <disk type='file' device='cdrom'> + <driver name='qemu' type='raw'/> + <source file='/var/lib/libvirt/images/systemrescuecd-x86-4.9.5.iso'/> + <backingStore/> + <target dev='hda' bus='ide'/> + <readonly/> + <boot order='1'/> + <alias name='ide0-0-0'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='usb' index='0' model='ich9-ehci1'> + <alias name='usb'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x7'/> + </controller> + <controller type='usb' index='0' model='ich9-uhci1'> + <alias name='usb'/> + <master startport='0'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0' multifunction='on'/> + </controller> + <controller type='usb' index='0' model='ich9-uhci2'> + <alias name='usb'/> + <master startport='2'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x1'/> + </controller> + <controller type='usb' index='0' model='ich9-uhci3'> + <alias name='usb'/> + <master startport='4'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x2'/> + </controller> + <controller type='pci' index='0' model='pci-root'> + <alias name='pci.0'/> + </controller> + <controller type='virtio-serial' index='0'> + <alias name='virtio-serial0'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/> + </controller> + <controller type='ide' index='0'> + <alias name='ide'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> + </controller> + <controller type='scsi' index='0' model='lsilogic'> + <alias name='scsi0'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x09' function='0x0'/> + </controller> + <controller type='fdc' index='0'> + <alias name='fdc0'/> + </controller> + <interface type='network'> + <mac address='52:54:00:36:bd:3b'/> + <source network='default'/> + <actual type='network'> + <source bridge='virbr0'/> + </actual> + <target dev='vnet0'/> + <model type='virtio'/> + <alias name='net0'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </interface> + <serial type='pty'> + <source path='/dev/pts/67'/> + <target type='isa-serial' port='0'> + <model name='isa-serial'/> + </target> + <alias name='serial0'/> + </serial> + <console type='pty' tty='/dev/pts/67'> + <source path='/dev/pts/67'/> + <target type='serial' port='0'/> + <alias name='serial0'/> + </console> + <channel type='unix'> + <source mode='bind' path='/var/lib/libvirt/qemu/channel/target/domain-3-upstream/org.qemu.guest_agent.0'/> + <target type='virtio' name='org.qemu.guest_agent.0' state='disconnected'/> + <alias name='channel0'/> + <address type='virtio-serial' controller='0' bus='0' port='1'/> + </channel> + <channel type='spicevmc'> + <target type='virtio' name='com.redhat.spice.0' state='disconnected'/> + <alias name='channel1'/> + <address type='virtio-serial' controller='0' bus='0' port='2'/> + </channel> + <input type='tablet' bus='usb'> + <alias name='input0'/> + <address type='usb' bus='0' port='1'/> + </input> + <input type='mouse' bus='ps2'> + <alias name='input1'/> + </input> + <input type='keyboard' bus='ps2'> + <alias name='input2'/> + </input> + <graphics type='spice' port='5900' autoport='yes' listen='127.0.0.1'> + <listen type='address' address='127.0.0.1' fromConfig='1' autoGenerated='no'/> + <image compression='off'/> + </graphics> + <sound model='ich6'> + <alias name='sound0'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> + </sound> + <video> + <model type='qxl' ram='65536' vram='65536' vgamem='16384' heads='1' primary='yes'/> + <alias name='video0'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </video> + <redirdev bus='usb' type='spicevmc'> + <alias name='redir0'/> + <address type='usb' bus='0' port='2'/> + </redirdev> + <redirdev bus='usb' type='spicevmc'> + <alias name='redir1'/> + <address type='usb' bus='0' port='3'/> + </redirdev> + <memballoon model='virtio'> + <alias name='balloon0'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x08' function='0x0'/> + </memballoon> + <rng model='virtio'> + <backend model='random'>/dev/random</backend> + <alias name='rng0'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x07' function='0x0'/> + </rng> + </devices> + <seclabel type='dynamic' model='dac' relabel='yes'> + <label>+0:+0</label> + <imagelabel>+0:+0</imagelabel> + </seclabel> + </domain> +</domstatus> -- 2.20.1

When adding <migrationSource> I've used a slightly unusual approach. To allow using the disk source XML parser and formatter convert <migrationSource> to look like <disk>. This means that <source> will be added as a subelement of <migrationSource> rather than being formatted inline. Conversion from the old format in the parser is very simple as it involves only moving the XPath context current node slightly if the new format is found. The status XML to XML test shows that the upgrade is done correctly. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_domain.c | 23 ++++++------ .../migration-out-nbd-tls-in.xml | 18 +++++----- .../migration-out-nbd-tls-out.xml | 36 ++++++++++--------- 3 files changed, 41 insertions(+), 36 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index a1b39220d1..3b0887a194 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2343,28 +2343,21 @@ qemuDomainObjPrivateXMLFormatPR(virBufferPtr buf, static int qemuDomainObjPrivateXMLFormatNBDMigrationSource(virBufferPtr buf, - virStorageSourcePtr src) + virStorageSourcePtr src, + virDomainXMLOptionPtr xmlopt) { VIR_AUTOCLEAN(virBuffer) attrBuf = VIR_BUFFER_INITIALIZER; VIR_AUTOCLEAN(virBuffer) childBuf = VIR_BUFFER_INITIALIZER; - VIR_AUTOCLEAN(virBuffer) privateDataBuf = VIR_BUFFER_INITIALIZER; int ret = -1; virBufferSetChildIndent(&childBuf, buf); - virBufferSetChildIndent(&privateDataBuf, &childBuf); virBufferAsprintf(&attrBuf, " type='%s' format='%s'", virStorageTypeToString(src->type), virStorageFileFormatTypeToString(src->format)); - if (virDomainStorageSourceFormat(&attrBuf, &childBuf, src, - VIR_DOMAIN_DEF_FORMAT_STATUS) < 0) - goto cleanup; - - if (qemuStorageSourcePrivateDataFormat(src, &privateDataBuf) < 0) - goto cleanup; - - if (virXMLFormatElement(&childBuf, "privateData", NULL, &privateDataBuf) < 0) + if (virDomainDiskSourceFormat(&childBuf, src, 0, false, + VIR_DOMAIN_DEF_FORMAT_STATUS, xmlopt) < 0) goto cleanup; if (virXMLFormatElement(buf, "migrationSource", &attrBuf, &childBuf) < 0) @@ -2381,6 +2374,7 @@ static int qemuDomainObjPrivateXMLFormatNBDMigration(virBufferPtr buf, virDomainObjPtr vm) { + qemuDomainObjPrivatePtr priv = vm->privateData; VIR_AUTOCLEAN(virBuffer) attrBuf = VIR_BUFFER_INITIALIZER; VIR_AUTOCLEAN(virBuffer) childBuf = VIR_BUFFER_INITIALIZER; size_t i; @@ -2399,7 +2393,8 @@ qemuDomainObjPrivateXMLFormatNBDMigration(virBufferPtr buf, if (diskPriv->migrSource && qemuDomainObjPrivateXMLFormatNBDMigrationSource(&childBuf, - diskPriv->migrSource) < 0) + diskPriv->migrSource, + priv->driver->xmlopt) < 0) goto cleanup; if (virXMLFormatElement(buf, "disk", &attrBuf, &childBuf) < 0) @@ -2724,6 +2719,7 @@ qemuDomainObjPrivateXMLParseJobNBDSource(xmlNodePtr node, char *type = NULL; int ret = -1; VIR_AUTOUNREF(virStorageSourcePtr) migrSource = NULL; + xmlNodePtr sourceNode; ctxt->node = node; @@ -2759,6 +2755,9 @@ qemuDomainObjPrivateXMLParseJobNBDSource(xmlNodePtr node, goto cleanup; } + if ((sourceNode = virXPathNode("./source", ctxt))) + ctxt->node = sourceNode; + if (virDomainStorageSourceParse(ctxt->node, ctxt, migrSource, VIR_DOMAIN_DEF_PARSE_STATUS, NULL) < 0) goto cleanup; diff --git a/tests/qemustatusxml2xmldata/migration-out-nbd-tls-in.xml b/tests/qemustatusxml2xmldata/migration-out-nbd-tls-in.xml index f3bbc752b6..2cd6c9a5e9 100644 --- a/tests/qemustatusxml2xmldata/migration-out-nbd-tls-in.xml +++ b/tests/qemustatusxml2xmldata/migration-out-nbd-tls-in.xml @@ -237,14 +237,16 @@ </qemuCaps> <job type='none' async='migration out' phase='perform3' flags='0x0'> <disk dev='vdb' migrating='yes'> - <migrationSource type='network' format='raw' protocol='nbd' name='drive-virtio-disk1' tlsFromConfig='0'> - <host name='andariel.usersys.redhat.com' port='49152'/> - <privateData> - <nodenames> - <nodename type='storage' name='migration-vdb-storage'/> - <nodename type='format' name='migration-vdb-format'/> - </nodenames> - </privateData> + <migrationSource type='network' format='raw'> + <source protocol='nbd' name='drive-virtio-disk1' tlsFromConfig='0'> + <host name='andariel.usersys.redhat.com' port='49152'/> + <privateData> + <nodenames> + <nodename type='storage' name='migration-vdb-storage'/> + <nodename type='format' name='migration-vdb-format'/> + </nodenames> + </privateData> + </source> </migrationSource> </disk> <disk dev='vdc' migrating='yes'> diff --git a/tests/qemustatusxml2xmldata/migration-out-nbd-tls-out.xml b/tests/qemustatusxml2xmldata/migration-out-nbd-tls-out.xml index f3bbc752b6..869f37d488 100644 --- a/tests/qemustatusxml2xmldata/migration-out-nbd-tls-out.xml +++ b/tests/qemustatusxml2xmldata/migration-out-nbd-tls-out.xml @@ -237,25 +237,29 @@ </qemuCaps> <job type='none' async='migration out' phase='perform3' flags='0x0'> <disk dev='vdb' migrating='yes'> - <migrationSource type='network' format='raw' protocol='nbd' name='drive-virtio-disk1' tlsFromConfig='0'> - <host name='andariel.usersys.redhat.com' port='49152'/> - <privateData> - <nodenames> - <nodename type='storage' name='migration-vdb-storage'/> - <nodename type='format' name='migration-vdb-format'/> - </nodenames> - </privateData> + <migrationSource type='network' format='raw'> + <source protocol='nbd' name='drive-virtio-disk1' tlsFromConfig='0'> + <host name='andariel.usersys.redhat.com' port='49152'/> + <privateData> + <nodenames> + <nodename type='storage' name='migration-vdb-storage'/> + <nodename type='format' name='migration-vdb-format'/> + </nodenames> + </privateData> + </source> </migrationSource> </disk> <disk dev='vdc' migrating='yes'> - <migrationSource type='network' format='raw' protocol='nbd' name='drive-virtio-disk2' tlsFromConfig='0'> - <host name='andariel.usersys.redhat.com' port='49152'/> - <privateData> - <nodenames> - <nodename type='storage' name='migration-vdc-storage'/> - <nodename type='format' name='migration-vdc-format'/> - </nodenames> - </privateData> + <migrationSource type='network' format='raw'> + <source protocol='nbd' name='drive-virtio-disk2' tlsFromConfig='0'> + <host name='andariel.usersys.redhat.com' port='49152'/> + <privateData> + <nodenames> + <nodename type='storage' name='migration-vdc-storage'/> + <nodename type='format' name='migration-vdc-format'/> + </nodenames> + </privateData> + </source> </migrationSource> </disk> <disk dev='hda' migrating='no'/> -- 2.20.1

It's not used outside of src/conf/domain_conf.c Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/conf/domain_conf.c | 2 +- src/conf/domain_conf.h | 5 ----- src/libvirt_private.syms | 1 - 3 files changed, 1 insertion(+), 7 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index adb20e33f2..ec81d380d5 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -23716,7 +23716,7 @@ virDomainDiskSourceFormatPrivateData(virBufferPtr buf, } -int +static int virDomainStorageSourceFormat(virBufferPtr attrBuf, virBufferPtr childBuf, virStorageSourcePtr src, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index f0c2a51212..cca81be013 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3457,11 +3457,6 @@ int virDomainDiskDefCheckDuplicateInfo(const virDomainDiskDef *a, const virDomainDiskDef *b) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); -int virDomainStorageSourceFormat(virBufferPtr attrBuf, - virBufferPtr childBuf, - virStorageSourcePtr src, - unsigned int flags) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); int virDomainStorageSourceParse(xmlNodePtr node, xmlXPathContextPtr ctxt, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index c26c525d37..681ff911b6 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -564,7 +564,6 @@ virDomainStateReasonToString; virDomainStateTypeFromString; virDomainStateTypeToString; virDomainStorageNetworkParseHost; -virDomainStorageSourceFormat; virDomainStorageSourceParse; virDomainTaintTypeFromString; virDomainTaintTypeToString; -- 2.20.1

Now that the cleanup is handled automatically it can be removed. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/conf/domain_conf.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ec81d380d5..fa4454a6ca 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -23797,12 +23797,11 @@ virDomainDiskSourceFormat(virBufferPtr buf, { VIR_AUTOCLEAN(virBuffer) attrBuf = VIR_BUFFER_INITIALIZER; VIR_AUTOCLEAN(virBuffer) childBuf = VIR_BUFFER_INITIALIZER; - int ret = -1; virBufferSetChildIndent(&childBuf, buf); if (virDomainStorageSourceFormat(&attrBuf, &childBuf, src, flags) < 0) - goto cleanup; + return -1; if (policy && src->type != VIR_STORAGE_TYPE_NETWORK) virBufferEscapeString(&attrBuf, " startupPolicy='%s'", @@ -23812,15 +23811,12 @@ virDomainDiskSourceFormat(virBufferPtr buf, virBufferAsprintf(&attrBuf, " index='%u'", src->id); if (virDomainDiskSourceFormatPrivateData(&childBuf, src, flags, xmlopt) < 0) - goto cleanup; + return -1; if (virXMLFormatElement(buf, "source", &attrBuf, &childBuf) < 0) - goto cleanup; - - ret = 0; + return -1; - cleanup: - return ret; + return 0; } -- 2.20.1

Modify the check that the format is in range to be standalone and use the convertor function directly. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/conf/domain_conf.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index fa4454a6ca..5adca7c29c 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -23826,7 +23826,6 @@ virDomainDiskBackingStoreFormat(virBufferPtr buf, virDomainXMLOptionPtr xmlopt, unsigned int flags) { - const char *format; bool inactive = flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE; if (!backingStore) @@ -23841,8 +23840,7 @@ virDomainDiskBackingStoreFormat(virBufferPtr buf, return 0; } - if (backingStore->format <= 0 || - !(format = virStorageFileFormatTypeToString(backingStore->format))) { + if (backingStore->format <= 0 || backingStore->format >= VIR_STORAGE_FILE_LAST) { virReportError(VIR_ERR_INTERNAL_ERROR, _("unexpected disk backing store format %d"), backingStore->format); @@ -23856,7 +23854,8 @@ virDomainDiskBackingStoreFormat(virBufferPtr buf, virBufferAddLit(buf, ">\n"); virBufferAdjustIndent(buf, 2); - virBufferAsprintf(buf, "<format type='%s'/>\n", format); + virBufferAsprintf(buf, "<format type='%s'/>\n", + virStorageFileFormatTypeToString(backingStore->format)); if (virDomainDiskSourceFormat(buf, backingStore, 0, false, flags, xmlopt) < 0 || virDomainDiskBackingStoreFormat(buf, backingStore->backingStore, xmlopt, flags) < 0) -- 2.20.1

Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/conf/domain_conf.c | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 5adca7c29c..89e2900df2 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -23826,8 +23826,12 @@ virDomainDiskBackingStoreFormat(virBufferPtr buf, virDomainXMLOptionPtr xmlopt, unsigned int flags) { + VIR_AUTOCLEAN(virBuffer) attrBuf = VIR_BUFFER_INITIALIZER; + VIR_AUTOCLEAN(virBuffer) childBuf = VIR_BUFFER_INITIALIZER; bool inactive = flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE; + virBufferSetChildIndent(&childBuf, buf); + if (!backingStore) return 0; @@ -23847,22 +23851,23 @@ virDomainDiskBackingStoreFormat(virBufferPtr buf, return -1; } - virBufferAsprintf(buf, "<backingStore type='%s'", + virBufferAsprintf(&attrBuf, " type='%s'", virStorageTypeToString(backingStore->type)); if (backingStore->id != 0) - virBufferAsprintf(buf, " index='%u'", backingStore->id); - virBufferAddLit(buf, ">\n"); - virBufferAdjustIndent(buf, 2); + virBufferAsprintf(&attrBuf, " index='%u'", backingStore->id); - virBufferAsprintf(buf, "<format type='%s'/>\n", + virBufferAsprintf(&childBuf, "<format type='%s'/>\n", virStorageFileFormatTypeToString(backingStore->format)); - if (virDomainDiskSourceFormat(buf, backingStore, 0, false, flags, xmlopt) < 0 || - virDomainDiskBackingStoreFormat(buf, backingStore->backingStore, + if (virDomainDiskSourceFormat(&childBuf, backingStore, 0, false, flags, xmlopt) < 0) + return -1; + + if (virDomainDiskBackingStoreFormat(&childBuf, backingStore->backingStore, xmlopt, flags) < 0) return -1; - virBufferAdjustIndent(buf, -2); - virBufferAddLit(buf, "</backingStore>\n"); + if (virXMLFormatElement(buf, "backingStore", &attrBuf, &childBuf) < 0) + return -1; + return 0; } -- 2.20.1

There was only one caller, remove the unnecessary wrapper. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/conf/domain_conf.c | 60 ++++++++++++++++-------------------------- 1 file changed, 23 insertions(+), 37 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 89e2900df2..db02005f5b 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -23716,38 +23716,45 @@ virDomainDiskSourceFormatPrivateData(virBufferPtr buf, } -static int -virDomainStorageSourceFormat(virBufferPtr attrBuf, - virBufferPtr childBuf, - virStorageSourcePtr src, - unsigned int flags) +int +virDomainDiskSourceFormat(virBufferPtr buf, + virStorageSourcePtr src, + int policy, + bool attrIndex, + unsigned int flags, + virDomainXMLOptionPtr xmlopt) { + VIR_AUTOCLEAN(virBuffer) attrBuf = VIR_BUFFER_INITIALIZER; + VIR_AUTOCLEAN(virBuffer) childBuf = VIR_BUFFER_INITIALIZER; + + virBufferSetChildIndent(&childBuf, buf); + switch ((virStorageType)src->type) { case VIR_STORAGE_TYPE_FILE: - virBufferEscapeString(attrBuf, " file='%s'", src->path); + virBufferEscapeString(&attrBuf, " file='%s'", src->path); break; case VIR_STORAGE_TYPE_BLOCK: - virBufferEscapeString(attrBuf, " dev='%s'", src->path); + virBufferEscapeString(&attrBuf, " dev='%s'", src->path); break; case VIR_STORAGE_TYPE_DIR: - virBufferEscapeString(attrBuf, " dir='%s'", src->path); + virBufferEscapeString(&attrBuf, " dir='%s'", src->path); break; case VIR_STORAGE_TYPE_NETWORK: - if (virDomainDiskSourceFormatNetwork(attrBuf, childBuf, + if (virDomainDiskSourceFormatNetwork(&attrBuf, &childBuf, src, flags) < 0) return -1; break; case VIR_STORAGE_TYPE_VOLUME: if (src->srcpool) { - virBufferEscapeString(attrBuf, " pool='%s'", src->srcpool->pool); - virBufferEscapeString(attrBuf, " volume='%s'", + virBufferEscapeString(&attrBuf, " pool='%s'", src->srcpool->pool); + virBufferEscapeString(&attrBuf, " volume='%s'", src->srcpool->volume); if (src->srcpool->mode) - virBufferAsprintf(attrBuf, " mode='%s'", + virBufferAsprintf(&attrBuf, " mode='%s'", virStorageSourcePoolModeTypeToString(src->srcpool->mode)); } @@ -23761,7 +23768,7 @@ virDomainStorageSourceFormat(virBufferPtr attrBuf, } if (src->type != VIR_STORAGE_TYPE_NETWORK) - virDomainSourceDefFormatSeclabel(childBuf, src->nseclabels, + virDomainSourceDefFormatSeclabel(&childBuf, src->nseclabels, src->seclabels, flags); /* Storage Source formatting will not carry through the blunder @@ -23771,38 +23778,17 @@ virDomainStorageSourceFormat(virBufferPtr attrBuf, * So avoid formatting it for volumes. */ if (src->auth && src->authInherited && src->type != VIR_STORAGE_TYPE_VOLUME) - virStorageAuthDefFormat(childBuf, src->auth); + virStorageAuthDefFormat(&childBuf, src->auth); /* If we found encryption as a child of <source>, then format it * as we found it. */ if (src->encryption && src->encryptionInherited && - virStorageEncryptionFormat(childBuf, src->encryption) < 0) + virStorageEncryptionFormat(&childBuf, src->encryption) < 0) return -1; if (src->pr) - virStoragePRDefFormat(childBuf, src->pr, + virStoragePRDefFormat(&childBuf, src->pr, flags & VIR_DOMAIN_DEF_FORMAT_MIGRATABLE); - - return 0; -} - - -int -virDomainDiskSourceFormat(virBufferPtr buf, - virStorageSourcePtr src, - int policy, - bool attrIndex, - unsigned int flags, - virDomainXMLOptionPtr xmlopt) -{ - VIR_AUTOCLEAN(virBuffer) attrBuf = VIR_BUFFER_INITIALIZER; - VIR_AUTOCLEAN(virBuffer) childBuf = VIR_BUFFER_INITIALIZER; - - virBufferSetChildIndent(&childBuf, buf); - - if (virDomainStorageSourceFormat(&attrBuf, &childBuf, src, flags) < 0) - return -1; - if (policy && src->type != VIR_STORAGE_TYPE_NETWORK) virBufferEscapeString(&attrBuf, " startupPolicy='%s'", virDomainStartupPolicyTypeToString(policy)); -- 2.20.1

On Fri, Mar 22, 2019 at 07:00:51PM +0100, Peter Krempa wrote: Hmm, so we have virDomainStorageSourceParse and a corresponding virDomainDiskSourceFormat? Maybe the previous commit dropped the wrong function. Reviewed-by: Ján Tomko <jtomko(a)redhat.com&gt; Jano

On Tue, Apr 02, 2019 at 02:09:24PM +0200, Peter Krempa wrote: Please do. Jano

On Fri, Mar 22, 2019 at 07:00:56PM +0100, Peter Krempa wrote: This feels odd to me and there is only one caller using this at the end of the series. Most of the callers already have a node handy and there's the migrationSource vs. source where the 'source' node is preferred and optional. I'd rather open-code the lookup in DiskDefMirrorParse Jano

On Fri, Mar 22, 2019 at 07:00:59PM +0100, Peter Krempa wrote: Joining the error messages here is not necessary to use virDomainStorageSourceParseBase and therefore does not belong in this commit. Moreover, hereunder, if the error messages do not matter in practice I suggest leaving them untouched to save the translators some work. Wit this hunk left alone: Reviewed-by: Ján Tomko <jtomko(a)redhat.com&gt; Jano

On Fri, Mar 22, 2019 at 07:01:03PM +0100, Peter Krempa wrote: Would be nicer to read with the virDomainStorageSourceParseBase trick and the virDomainStorageSourceParse trick being separate. I'd move this here. Reviewed-by: Ján Tomko <jtomko(a)redhat.com&gt; Jano