[libvirt] [PATCH 0/1] Fix encryption XML indentation

The following patch fixes the storage XML indentation problem caused by my earlier patch to fix domain XML indentation problems. I liked the style of Laine's suggestion of passing in the number of spaces to indent, so I adopted that approach. I also added a regression test for the domain XML. Dave David Allan (1): Fix indentation for storage conf XML src/conf/domain_conf.c | 2 +- src/conf/storage_conf.c | 2 +- src/conf/storage_encryption_conf.c | 16 +++++++---- src/conf/storage_encryption_conf.h | 3 +- .../qemuxml2argv-encrypted-disk.args | 1 + .../qemuxml2argv-encrypted-disk.xml | 27 ++++++++++++++++++++ tests/qemuxml2xmltest.c | 2 + 7 files changed, 44 insertions(+), 9 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-encrypted-disk.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-encrypted-disk.xml

* virStorageEncryptionFormat is called from both virDomainDiskDefFormat and virStorageVolTargetDefFormat. The proper indentation in the generated XML depends on the caller. My earlier patch to fix the incorrect indentation for the domain XML broke the indentation for the storage XML. This patch adopts Laine's suggestion of requring the caller of virStorageEncryptionFormat to provide an unsigned int with the number of spaces the output should be indented. The patch modifies both callers to provide the additional argument. * Add a regression test for the domain XML --- src/conf/domain_conf.c | 2 +- src/conf/storage_conf.c | 2 +- src/conf/storage_encryption_conf.c | 16 +++++++---- src/conf/storage_encryption_conf.h | 3 +- .../qemuxml2argv-encrypted-disk.args | 1 + .../qemuxml2argv-encrypted-disk.xml | 27 ++++++++++++++++++++ tests/qemuxml2xmltest.c | 2 + 7 files changed, 44 insertions(+), 9 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-encrypted-disk.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-encrypted-disk.xml diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 2de838b..23444fa 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4957,7 +4957,7 @@ virDomainDiskDefFormat(virBufferPtr buf, virBufferEscapeString(buf, " <serial>%s</serial>\n", def->serial); if (def->encryption != NULL && - virStorageEncryptionFormat(buf, def->encryption) < 0) + virStorageEncryptionFormat(buf, def->encryption, 6) < 0) return -1; if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 6467c73..6218e02 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -1169,7 +1169,7 @@ virStorageVolTargetDefFormat(virStorageVolOptionsPtr options, virBufferAddLit(buf," </permissions>\n"); if (def->encryption != NULL && - virStorageEncryptionFormat(buf, def->encryption) < 0) + virStorageEncryptionFormat(buf, def->encryption, 4) < 0) return -1; virBufferVSprintf(buf, " </%s>\n", type); diff --git a/src/conf/storage_encryption_conf.c b/src/conf/storage_encryption_conf.c index 7f68d67..7a64050 100644 --- a/src/conf/storage_encryption_conf.c +++ b/src/conf/storage_encryption_conf.c @@ -215,7 +215,8 @@ virStorageEncryptionParseNode(xmlDocPtr xml, xmlNodePtr root) static int virStorageEncryptionSecretFormat(virBufferPtr buf, - virStorageEncryptionSecretPtr secret) + virStorageEncryptionSecretPtr secret, + unsigned int indent) { const char *type; char uuidstr[VIR_UUID_STRING_BUFLEN]; @@ -228,13 +229,15 @@ virStorageEncryptionSecretFormat(virBufferPtr buf, } virUUIDFormat(secret->uuid, uuidstr); - virBufferVSprintf(buf, " <secret type='%s' uuid='%s'/>\n", type, uuidstr); + virBufferVSprintf(buf, "%*s<secret type='%s' uuid='%s'/>\n", + indent, "", type, uuidstr); return 0; } int virStorageEncryptionFormat(virBufferPtr buf, - virStorageEncryptionPtr enc) + virStorageEncryptionPtr enc, + unsigned int indent) { const char *format; size_t i; @@ -245,14 +248,15 @@ virStorageEncryptionFormat(virBufferPtr buf, "%s", _("unexpected encryption format")); return -1; } - virBufferVSprintf(buf, " <encryption format='%s'>\n", format); + virBufferVSprintf(buf, "%*s<encryption format='%s'>\n", + indent, "", format); for (i = 0; i < enc->nsecrets; i++) { - if (virStorageEncryptionSecretFormat(buf, enc->secrets[i]) < 0) + if (virStorageEncryptionSecretFormat(buf, enc->secrets[i], indent + 2) < 0) return -1; } - virBufferAddLit(buf, " </encryption>\n"); + virBufferVSprintf(buf, "%*s</encryption>\n", indent, ""); return 0; } diff --git a/src/conf/storage_encryption_conf.h b/src/conf/storage_encryption_conf.h index fd435fc..8309255 100644 --- a/src/conf/storage_encryption_conf.h +++ b/src/conf/storage_encryption_conf.h @@ -67,7 +67,8 @@ void virStorageEncryptionFree(virStorageEncryptionPtr enc); virStorageEncryptionPtr virStorageEncryptionParseNode(xmlDocPtr xml, xmlNodePtr root); int virStorageEncryptionFormat(virBufferPtr buf, - virStorageEncryptionPtr enc); + virStorageEncryptionPtr enc, + unsigned int indent); /* A helper for VIR_STORAGE_ENCRYPTION_FORMAT_QCOW */ enum { diff --git a/tests/qemuxml2argvdata/qemuxml2argv-encrypted-disk.args b/tests/qemuxml2argvdata/qemuxml2argv-encrypted-disk.args new file mode 100644 index 0000000..d8c1053 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-encrypted-disk.args @@ -0,0 +1 @@ +LC_ALL=C PATH=/sbin:/usr/sbin:/bin:/usr/bin HOME=/root USER=root LOGNAME=root /usr/bin/qemu -S -M fedora-13 -m 1024 -smp 1,sockets=1,cores=1,threads=1 -name encryptdisk -uuid 496898a6-e6ff-f7c8-5dc2-3cf410945ee9 -nographic -nodefaults -chardev socket,id=monitor,path=//var/lib/libvirt/qemu/encryptdisk.monitor,server,nowait -mon chardev=monitor,mode=readline -rtc base=utc -no-acpi -boot c -drive file=/storage/guest_disks/encryptdisk,if=none,id=drive-virtio-disk0,boot=on,format=qcow2 -device virtio-blk-pci,bus=pci.0,addr=0x4,drive=drive-virtio-disk0,id=virtio-disk0 -usb -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-encrypted-disk.xml b/tests/qemuxml2argvdata/qemuxml2argv-encrypted-disk.xml new file mode 100644 index 0000000..cb7b06d --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-encrypted-disk.xml @@ -0,0 +1,27 @@ +<domain type='kvm'> + <name>encryptdisk</name> + <uuid>496898a6-e6ff-f7c8-5dc2-3cf410945ee9</uuid> + <memory>1048576</memory> + <currentMemory>524288</currentMemory> + <vcpu>1</vcpu> + <os> + <type arch='i686' machine='fedora-13'>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</emulator> + <disk type='file' device='disk'> + <driver name='qemu' type='qcow2'/> + <source file='/storage/guest_disks/encryptdisk'/> + <target dev='vda' bus='virtio'/> + <encryption format='qcow'> + <secret type='passphrase' uuid='0a81f5b2-8403-7b23-c8d6-21ccc2f80d6f'/> + </encryption> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> + </disk> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 1ac6edc..69829b1 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -138,6 +138,8 @@ mymain(int argc, char **argv) DO_TEST("hostdev-usb-address"); DO_TEST("hostdev-pci-address"); + DO_TEST("encrypted-disk"); + virCapabilitiesFree(driver.caps); return (ret==0 ? EXIT_SUCCESS : EXIT_FAILURE); -- 1.7.0.1

On Fri, Apr 23, 2010 at 11:09:01PM -0400, David Allan wrote:
* virStorageEncryptionFormat is called from both virDomainDiskDefFormat and virStorageVolTargetDefFormat. The proper indentation in the generated XML depends on the caller. My earlier patch to fix the incorrect indentation for the domain XML broke the indentation for the storage XML. This patch adopts Laine's suggestion of requring the caller of virStorageEncryptionFormat to provide an unsigned int with the number of spaces the output should be indented. The patch modifies both callers to provide the additional argument.
* Add a regression test for the domain XML --- src/conf/domain_conf.c | 2 +- src/conf/storage_conf.c | 2 +- src/conf/storage_encryption_conf.c | 16 +++++++---- src/conf/storage_encryption_conf.h | 3 +- .../qemuxml2argv-encrypted-disk.args | 1 + .../qemuxml2argv-encrypted-disk.xml | 27 ++++++++++++++++++++ tests/qemuxml2xmltest.c | 2 +
ACK, looks fine to me ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

On Mon, Apr 26, 2010 at 06:06:31PM +0200, Daniel Veillard wrote:
On Fri, Apr 23, 2010 at 11:09:01PM -0400, David Allan wrote:
* virStorageEncryptionFormat is called from both virDomainDiskDefFormat and virStorageVolTargetDefFormat. The proper indentation in the generated XML depends on the caller. My earlier patch to fix the incorrect indentation for the domain XML broke the indentation for the storage XML. This patch adopts Laine's suggestion of requring the caller of virStorageEncryptionFormat to provide an unsigned int with the number of spaces the output should be indented. The patch modifies both callers to provide the additional argument.
* Add a regression test for the domain XML --- src/conf/domain_conf.c | 2 +- src/conf/storage_conf.c | 2 +- src/conf/storage_encryption_conf.c | 16 +++++++---- src/conf/storage_encryption_conf.h | 3 +- .../qemuxml2argv-encrypted-disk.args | 1 + .../qemuxml2argv-encrypted-disk.xml | 27 ++++++++++++++++++++ tests/qemuxml2xmltest.c | 2 +
ACK, looks fine to me !
Okay I pushed it ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
participants (2)
-
Daniel Veillard
-
David Allan