[libvirt/f20] Escape XML characters in volume XML (bz #1074528)
Cole Robinson
crobinso at fedoraproject.org
Mon Mar 10 13:26:08 UTC 2014
commit 2b7e43f1356d046760f8ba2da7778f5f199ddbdd
Author: Cole Robinson <crobinso at redhat.com>
Date: Mon Mar 10 09:25:58 2014 -0400
Escape XML characters in volume XML (bz #1074528)
0005-maint-fix-comma-style-issues-conf.patch | 342 ++++++++++++
...ge-use-valid-XML-for-awkward-volume-names.patch | 572 ++++++++++++++++++++
libvirt.spec | 11 +-
3 files changed, 924 insertions(+), 1 deletions(-)
---
diff --git a/0005-maint-fix-comma-style-issues-conf.patch b/0005-maint-fix-comma-style-issues-conf.patch
new file mode 100644
index 0000000..af143c1
--- /dev/null
+++ b/0005-maint-fix-comma-style-issues-conf.patch
@@ -0,0 +1,342 @@
+From d3e5327fed75feeb262f4571f280a68625561a82 Mon Sep 17 00:00:00 2001
+From: Eric Blake <eblake at redhat.com>
+Date: Tue, 19 Nov 2013 15:21:40 -0700
+Subject: [PATCH] maint: fix comma style issues: conf
+
+Most of our code base uses space after comma but not before;
+fix the remaining uses before adding a syntax check.
+
+* src/conf/capabilities.c: Consistently use commas.
+* src/conf/domain_conf.c: Likewise.
+* src/conf/network_conf.c: Likewise.
+* src/conf/storage_conf.c: Likewise.
+
+Signed-off-by: Eric Blake <eblake at redhat.com>
+(cherry picked from commit 6f4901e13b55b0a6adac303d7880740ac1bb5300)
+---
+ src/conf/capabilities.c | 2 +-
+ src/conf/domain_conf.c | 4 +-
+ src/conf/network_conf.c | 6 +--
+ src/conf/storage_conf.c | 97 +++++++++++++++++++++++++------------------------
+ 4 files changed, 55 insertions(+), 54 deletions(-)
+
+diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
+index 1acc936..ad6faa2 100644
+--- a/src/conf/capabilities.c
++++ b/src/conf/capabilities.c
+@@ -761,7 +761,7 @@ virCapabilitiesFormatXML(virCapsPtr caps)
+ virBufferAddLit(&xml, " <host>\n");
+ if (virUUIDIsValid(caps->host.host_uuid)) {
+ virUUIDFormat(caps->host.host_uuid, host_uuid);
+- virBufferAsprintf(&xml," <uuid>%s</uuid>\n", host_uuid);
++ virBufferAsprintf(&xml, " <uuid>%s</uuid>\n", host_uuid);
+ }
+ virBufferAddLit(&xml, " <cpu>\n");
+ if (caps->host.arch)
+diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
+index c812e71..af1909b 100644
+--- a/src/conf/domain_conf.c
++++ b/src/conf/domain_conf.c
+@@ -14243,7 +14243,7 @@ virDomainDiskBlockIoDefFormat(virBufferPtr buf,
+ {
+ if (def->blockio.logical_block_size > 0 ||
+ def->blockio.physical_block_size > 0) {
+- virBufferAddLit(buf," <blockio");
++ virBufferAddLit(buf, " <blockio");
+ if (def->blockio.logical_block_size > 0) {
+ virBufferAsprintf(buf,
+ " logical_block_size='%u'",
+@@ -14270,7 +14270,7 @@ virDomainDiskSourceDefFormat(virBufferPtr buf,
+ def->startupPolicy) {
+ switch (def->type) {
+ case VIR_DOMAIN_DISK_TYPE_FILE:
+- virBufferAddLit(buf," <source");
++ virBufferAddLit(buf, " <source");
+ if (def->src)
+ virBufferEscapeString(buf, " file='%s'", def->src);
+ if (def->startupPolicy)
+diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
+index c877a6d..fe29f82 100644
+--- a/src/conf/network_conf.c
++++ b/src/conf/network_conf.c
+@@ -2393,7 +2393,7 @@ virNetworkIpDefFormat(virBufferPtr buf,
+ VIR_FREE(addr);
+ }
+ if (def->prefix > 0) {
+- virBufferAsprintf(buf," prefix='%u'", def->prefix);
++ virBufferAsprintf(buf, " prefix='%u'", def->prefix);
+ }
+ virBufferAddLit(buf, ">\n");
+ virBufferAdjustIndent(buf, 2);
+@@ -2492,7 +2492,7 @@ virNetworkRouteDefFormat(virBufferPtr buf,
+ VIR_FREE(addr);
+ }
+ if (def->has_prefix) {
+- virBufferAsprintf(buf," prefix='%u'", def->prefix);
++ virBufferAsprintf(buf, " prefix='%u'", def->prefix);
+ }
+ if (VIR_SOCKET_ADDR_VALID(&def->gateway)) {
+ char *addr = virSocketAddrFormat(&def->gateway);
+@@ -2502,7 +2502,7 @@ virNetworkRouteDefFormat(virBufferPtr buf,
+ VIR_FREE(addr);
+ }
+ if (def->has_metric && def->metric > 0) {
+- virBufferAsprintf(buf," metric='%u'", def->metric);
++ virBufferAsprintf(buf, " metric='%u'", def->metric);
+ }
+ virBufferAddLit(buf, "/>\n");
+
+diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
+index 975e662..33e4caf 100644
+--- a/src/conf/storage_conf.c
++++ b/src/conf/storage_conf.c
+@@ -1053,7 +1053,7 @@ virStoragePoolSourceFormat(virBufferPtr buf,
+ size_t i, j;
+ char uuid[VIR_UUID_STRING_BUFLEN];
+
+- virBufferAddLit(buf," <source>\n");
++ virBufferAddLit(buf, " <source>\n");
+ if ((options->flags & VIR_STORAGE_POOL_SOURCE_HOST) && src->nhost) {
+ for (i = 0; i < src->nhost; i++) {
+ virBufferAsprintf(buf, " <host name='%s'", src->hosts[i].name);
+@@ -1067,14 +1067,14 @@ virStoragePoolSourceFormat(virBufferPtr buf,
+ src->ndevice) {
+ for (i = 0; i < src->ndevice; i++) {
+ if (src->devices[i].nfreeExtent) {
+- virBufferAsprintf(buf," <device path='%s'>\n",
++ virBufferAsprintf(buf, " <device path='%s'>\n",
+ src->devices[i].path);
+ for (j = 0; j < src->devices[i].nfreeExtent; j++) {
+ virBufferAsprintf(buf, " <freeExtent start='%llu' end='%llu'/>\n",
+ src->devices[i].freeExtents[j].start,
+ src->devices[i].freeExtents[j].end);
+ }
+- virBufferAddLit(buf," </device>\n");
++ virBufferAddLit(buf, " </device>\n");
+ } else {
+ virBufferAsprintf(buf, " <device path='%s'/>\n",
+ src->devices[i].path);
+@@ -1084,7 +1084,7 @@ virStoragePoolSourceFormat(virBufferPtr buf,
+
+ if ((options->flags & VIR_STORAGE_POOL_SOURCE_DIR) &&
+ src->dir)
+- virBufferAsprintf(buf," <dir path='%s'/>\n", src->dir);
++ virBufferAsprintf(buf, " <dir path='%s'/>\n", src->dir);
+
+ if ((options->flags & VIR_STORAGE_POOL_SOURCE_ADAPTER)) {
+ if (src->adapter.type == VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST ||
+@@ -1095,24 +1095,25 @@ virStoragePoolSourceFormat(virBufferPtr buf,
+ if (src->adapter.type == VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST) {
+ virBufferEscapeString(buf, " parent='%s'",
+ src->adapter.data.fchost.parent);
+- virBufferAsprintf(buf," wwnn='%s' wwpn='%s'/>\n",
++ virBufferAsprintf(buf, " wwnn='%s' wwpn='%s'/>\n",
+ src->adapter.data.fchost.wwnn,
+ src->adapter.data.fchost.wwpn);
+ } else if (src->adapter.type ==
+ VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_SCSI_HOST) {
+- virBufferAsprintf(buf," name='%s'/>\n", src->adapter.data.name);
++ virBufferAsprintf(buf, " name='%s'/>\n", src->adapter.data.name);
+ }
+ }
+
+ if ((options->flags & VIR_STORAGE_POOL_SOURCE_NAME) &&
+ src->name)
+- virBufferAsprintf(buf," <name>%s</name>\n", src->name);
++ virBufferAsprintf(buf, " <name>%s</name>\n", src->name);
+
+ if ((options->flags & VIR_STORAGE_POOL_SOURCE_INITIATOR_IQN) &&
+ src->initiator.iqn) {
+- virBufferAddLit(buf," <initiator>\n");
+- virBufferEscapeString(buf," <iqn name='%s'/>\n", src->initiator.iqn);
+- virBufferAddLit(buf," </initiator>\n");
++ virBufferAddLit(buf, " <initiator>\n");
++ virBufferEscapeString(buf, " <iqn name='%s'/>\n",
++ src->initiator.iqn);
++ virBufferAddLit(buf, " </initiator>\n");
+ }
+
+ if (options->formatToString) {
+@@ -1123,40 +1124,40 @@ virStoragePoolSourceFormat(virBufferPtr buf,
+ src->format);
+ return -1;
+ }
+- virBufferAsprintf(buf," <format type='%s'/>\n", format);
++ virBufferAsprintf(buf, " <format type='%s'/>\n", format);
+ }
+
+ if (src->authType == VIR_STORAGE_POOL_AUTH_CHAP ||
+ src->authType == VIR_STORAGE_POOL_AUTH_CEPHX) {
+- virBufferAsprintf(buf," <auth type='%s' username='%s'>\n",
++ virBufferAsprintf(buf, " <auth type='%s' username='%s'>\n",
+ virStoragePoolAuthTypeTypeToString(src->authType),
+ (src->authType == VIR_STORAGE_POOL_AUTH_CHAP ?
+ src->auth.chap.username :
+ src->auth.cephx.username));
+
+- virBufferAddLit(buf," <secret");
++ virBufferAddLit(buf, " <secret");
+ if (src->auth.cephx.secret.uuidUsable) {
+ virUUIDFormat(src->auth.cephx.secret.uuid, uuid);
+- virBufferAsprintf(buf," uuid='%s'", uuid);
++ virBufferAsprintf(buf, " uuid='%s'", uuid);
+ }
+
+ if (src->auth.cephx.secret.usage != NULL) {
+- virBufferAsprintf(buf," usage='%s'", src->auth.cephx.secret.usage);
++ virBufferAsprintf(buf, " usage='%s'", src->auth.cephx.secret.usage);
+ }
+- virBufferAddLit(buf,"/>\n");
++ virBufferAddLit(buf, "/>\n");
+
+- virBufferAddLit(buf," </auth>\n");
++ virBufferAddLit(buf, " </auth>\n");
+ }
+
+ if (src->vendor != NULL) {
+- virBufferEscapeString(buf," <vendor name='%s'/>\n", src->vendor);
++ virBufferEscapeString(buf, " <vendor name='%s'/>\n", src->vendor);
+ }
+
+ if (src->product != NULL) {
+- virBufferEscapeString(buf," <product name='%s'/>\n", src->product);
++ virBufferEscapeString(buf, " <product name='%s'/>\n", src->product);
+ }
+
+- virBufferAddLit(buf," </source>\n");
++ virBufferAddLit(buf, " </source>\n");
+
+ return 0;
+ }
+@@ -1181,16 +1182,16 @@ virStoragePoolDefFormat(virStoragePoolDefPtr def)
+ goto cleanup;
+ }
+ virBufferAsprintf(&buf, "<pool type='%s'>\n", type);
+- virBufferAsprintf(&buf," <name>%s</name>\n", def->name);
++ virBufferAsprintf(&buf, " <name>%s</name>\n", def->name);
+
+ virUUIDFormat(def->uuid, uuid);
+- virBufferAsprintf(&buf," <uuid>%s</uuid>\n", uuid);
++ virBufferAsprintf(&buf, " <uuid>%s</uuid>\n", uuid);
+
+- virBufferAsprintf(&buf," <capacity unit='bytes'>%llu</capacity>\n",
++ virBufferAsprintf(&buf, " <capacity unit='bytes'>%llu</capacity>\n",
+ def->capacity);
+- virBufferAsprintf(&buf," <allocation unit='bytes'>%llu</allocation>\n",
++ virBufferAsprintf(&buf, " <allocation unit='bytes'>%llu</allocation>\n",
+ def->allocation);
+- virBufferAsprintf(&buf," <available unit='bytes'>%llu</available>\n",
++ virBufferAsprintf(&buf, " <available unit='bytes'>%llu</available>\n",
+ def->available);
+
+ if (virStoragePoolSourceFormat(&buf, options, &def->source) < 0)
+@@ -1200,27 +1201,27 @@ virStoragePoolDefFormat(virStoragePoolDefPtr def)
+ * doesn't have a target */
+ if (def->type != VIR_STORAGE_POOL_RBD &&
+ def->type != VIR_STORAGE_POOL_SHEEPDOG) {
+- virBufferAddLit(&buf," <target>\n");
++ virBufferAddLit(&buf, " <target>\n");
+
+ if (def->target.path)
+- virBufferAsprintf(&buf," <path>%s</path>\n", def->target.path);
++ virBufferAsprintf(&buf, " <path>%s</path>\n", def->target.path);
+
+- virBufferAddLit(&buf," <permissions>\n");
+- virBufferAsprintf(&buf," <mode>0%o</mode>\n",
++ virBufferAddLit(&buf, " <permissions>\n");
++ virBufferAsprintf(&buf, " <mode>0%o</mode>\n",
+ def->target.perms.mode);
+- virBufferAsprintf(&buf," <owner>%d</owner>\n",
++ virBufferAsprintf(&buf, " <owner>%d</owner>\n",
+ (int) def->target.perms.uid);
+- virBufferAsprintf(&buf," <group>%d</group>\n",
++ virBufferAsprintf(&buf, " <group>%d</group>\n",
+ (int) def->target.perms.gid);
+
+ if (def->target.perms.label)
+- virBufferAsprintf(&buf," <label>%s</label>\n",
++ virBufferAsprintf(&buf, " <label>%s</label>\n",
+ def->target.perms.label);
+
+- virBufferAddLit(&buf," </permissions>\n");
+- virBufferAddLit(&buf," </target>\n");
++ virBufferAddLit(&buf, " </permissions>\n");
++ virBufferAddLit(&buf, " </target>\n");
+ }
+- virBufferAddLit(&buf,"</pool>\n");
++ virBufferAddLit(&buf, "</pool>\n");
+
+ if (virBufferError(&buf))
+ goto no_memory;
+@@ -1488,7 +1489,7 @@ virStorageVolTargetDefFormat(virStorageVolOptionsPtr options,
+ virBufferAsprintf(buf, " <%s>\n", type);
+
+ if (def->path)
+- virBufferAsprintf(buf," <path>%s</path>\n", def->path);
++ virBufferAsprintf(buf, " <path>%s</path>\n", def->path);
+
+ if (options->formatToString) {
+ const char *format = (options->formatToString)(def->format);
+@@ -1498,23 +1499,23 @@ virStorageVolTargetDefFormat(virStorageVolOptionsPtr options,
+ def->format);
+ return -1;
+ }
+- virBufferAsprintf(buf," <format type='%s'/>\n", format);
++ virBufferAsprintf(buf, " <format type='%s'/>\n", format);
+ }
+
+- virBufferAddLit(buf," <permissions>\n");
+- virBufferAsprintf(buf," <mode>0%o</mode>\n",
++ virBufferAddLit(buf, " <permissions>\n");
++ virBufferAsprintf(buf, " <mode>0%o</mode>\n",
+ def->perms.mode);
+- virBufferAsprintf(buf," <owner>%u</owner>\n",
++ virBufferAsprintf(buf, " <owner>%u</owner>\n",
+ (unsigned int) def->perms.uid);
+- virBufferAsprintf(buf," <group>%u</group>\n",
++ virBufferAsprintf(buf, " <group>%u</group>\n",
+ (unsigned int) def->perms.gid);
+
+
+ if (def->perms.label)
+- virBufferAsprintf(buf," <label>%s</label>\n",
++ virBufferAsprintf(buf, " <label>%s</label>\n",
+ def->perms.label);
+
+- virBufferAddLit(buf," </permissions>\n");
++ virBufferAddLit(buf, " </permissions>\n");
+
+ if (def->timestamps) {
+ virBufferAddLit(buf, " <timestamps>\n");
+@@ -1571,8 +1572,8 @@ virStorageVolDefFormat(virStoragePoolDefPtr pool,
+ return NULL;
+
+ virBufferAddLit(&buf, "<volume>\n");
+- virBufferAsprintf(&buf," <name>%s</name>\n", def->name);
+- virBufferAsprintf(&buf," <key>%s</key>\n", NULLSTR(def->key));
++ virBufferAsprintf(&buf, " <name>%s</name>\n", def->name);
++ virBufferAsprintf(&buf, " <key>%s</key>\n", NULLSTR(def->key));
+ virBufferAddLit(&buf, " <source>\n");
+
+ if (def->source.nextent) {
+@@ -1599,9 +1600,9 @@ virStorageVolDefFormat(virStoragePoolDefPtr pool,
+ }
+ virBufferAddLit(&buf, " </source>\n");
+
+- virBufferAsprintf(&buf," <capacity unit='bytes'>%llu</capacity>\n",
++ virBufferAsprintf(&buf, " <capacity unit='bytes'>%llu</capacity>\n",
+ def->capacity);
+- virBufferAsprintf(&buf," <allocation unit='bytes'>%llu</allocation>\n",
++ virBufferAsprintf(&buf, " <allocation unit='bytes'>%llu</allocation>\n",
+ def->allocation);
+
+ if (virStorageVolTargetDefFormat(options, &buf,
+@@ -1613,7 +1614,7 @@ virStorageVolDefFormat(virStoragePoolDefPtr pool,
+ &def->backingStore, "backingStore") < 0)
+ goto cleanup;
+
+- virBufferAddLit(&buf,"</volume>\n");
++ virBufferAddLit(&buf, "</volume>\n");
+
+ if (virBufferError(&buf))
+ goto no_memory;
diff --git a/0006-storage-use-valid-XML-for-awkward-volume-names.patch b/0006-storage-use-valid-XML-for-awkward-volume-names.patch
new file mode 100644
index 0000000..1274569
--- /dev/null
+++ b/0006-storage-use-valid-XML-for-awkward-volume-names.patch
@@ -0,0 +1,572 @@
+From a947da33c35e07bb68829c68b0c7c95a002a1407 Mon Sep 17 00:00:00 2001
+From: Eric Blake <eblake at redhat.com>
+Date: Wed, 20 Nov 2013 17:04:05 -0700
+Subject: [PATCH] storage: use valid XML for awkward volume names
+
+$ touch /var/lib/libvirt/images/'a<b>c'
+$ virsh pool-refresh default
+$ virsh vol-dumpxml 'a<b>c' default | head -n2
+<volume>
+ <name>a<b>c</name>
+
+Oops. That's not valid XML. And when we fix the XML
+generation, it fails RelaxNG validation.
+
+I'm also tired of seeing <key>(null)</key> in the example
+output for volume xml; while we used NULLSTR() to avoid
+a NULL deref rather than relying on glibc's printf
+extension behavior, it's even better if we avoid the issue
+in the first place. But this requires being careful that
+we don't invalidate any storage backends that were relying
+on key being unassigned during virStoragVolCreateXML[From].
+
+I would have split this into two patches (one for escaping,
+one for avoiding <key>(null)</key>), but since they both
+end up touching a lot of the same test files, I ended up
+merging it into one.
+
+Note that this patch allows pretty much any volume name
+that can appear in a directory (excluding . and .. because
+those are special), but does nothing to change the current
+(unenforced) RelaxNG claim that pool names will consist
+only of letters, numbers, _, -, and +. Tightening the C
+code to match RelaxNG patterns and/or relaxing the grammar
+to match the C code for pool names is a task for another
+day (but remember, we DID recently tighten C code for
+domain names to exclude a leading '.').
+
+* src/conf/storage_conf.c (virStoragePoolSourceFormat)
+(virStoragePoolDefFormat, virStorageVolTargetDefFormat)
+(virStorageVolDefFormat): Escape user-controlled strings.
+(virStorageVolDefParseXML): Parse key, for use in unit tests.
+* src/storage/storage_driver.c (storageVolCreateXML)
+(storageVolCreateXMLFrom): Ensure parsed key doesn't confuse
+volume creation.
+* docs/schemas/basictypes.rng (volName): Relax definition.
+* tests/storagepoolxml2xmltest.c (mymain): Test it.
+* tests/storagevolxml2xmltest.c (mymain): Likewise.
+* tests/storagepoolxml2xmlin/pool-dir-naming.xml: New file.
+* tests/storagepoolxml2xmlout/pool-dir-naming.xml: Likewise.
+* tests/storagevolxml2xmlin/vol-file-naming.xml: Likewise.
+* tests/storagevolxml2xmlout/vol-file-naming.xml: Likewise.
+* tests/storagevolxml2xmlout/vol-*.xml: Fix fallout.
+
+Signed-off-by: Eric Blake <eblake at redhat.com>
+(cherry picked from commit 6cc4d6a3fe82653c607c4f159901790298e80e1f)
+---
+ docs/schemas/basictypes.rng | 9 ++-
+ src/conf/storage_conf.c | 72 ++++++++++------------
+ src/storage/storage_driver.c | 8 ++-
+ tests/storagepoolxml2xmlin/pool-dir-naming.xml | 18 ++++++
+ tests/storagepoolxml2xmlout/pool-dir-naming.xml | 18 ++++++
+ tests/storagepoolxml2xmltest.c | 1 +
+ tests/storagevolxml2xmlin/vol-file-backing.xml | 1 +
+ tests/storagevolxml2xmlin/vol-file-naming.xml | 20 ++++++
+ tests/storagevolxml2xmlout/vol-file-backing.xml | 2 +-
+ tests/storagevolxml2xmlout/vol-file-naming.xml | 17 +++++
+ tests/storagevolxml2xmlout/vol-file.xml | 1 -
+ tests/storagevolxml2xmlout/vol-logical-backing.xml | 2 +-
+ tests/storagevolxml2xmlout/vol-logical.xml | 2 +-
+ tests/storagevolxml2xmlout/vol-partition.xml | 2 +-
+ tests/storagevolxml2xmlout/vol-qcow2-0.10-lazy.xml | 2 +-
+ tests/storagevolxml2xmlout/vol-qcow2-1.1.xml | 2 +-
+ tests/storagevolxml2xmlout/vol-qcow2-lazy.xml | 2 +-
+ tests/storagevolxml2xmlout/vol-qcow2-nobacking.xml | 2 +-
+ tests/storagevolxml2xmlout/vol-qcow2.xml | 2 +-
+ tests/storagevolxml2xmlout/vol-sheepdog.xml | 1 -
+ tests/storagevolxml2xmltest.c | 1 +
+ 21 files changed, 132 insertions(+), 53 deletions(-)
+ create mode 100644 tests/storagepoolxml2xmlin/pool-dir-naming.xml
+ create mode 100644 tests/storagepoolxml2xmlout/pool-dir-naming.xml
+ create mode 100644 tests/storagevolxml2xmlin/vol-file-naming.xml
+ create mode 100644 tests/storagevolxml2xmlout/vol-file-naming.xml
+
+diff --git a/docs/schemas/basictypes.rng b/docs/schemas/basictypes.rng
+index 34c2254..48806fc 100644
+--- a/docs/schemas/basictypes.rng
++++ b/docs/schemas/basictypes.rng
+@@ -291,8 +291,15 @@
+ </define>
+
+ <define name='volName'>
++ <!-- directory pools allow almost any file name as a volume name -->
+ <data type='string'>
+- <param name="pattern">[a-zA-Z0-9_\+\-\.]+</param>
++ <param name="pattern">[^/]+</param>
++ <except>
++ <choice>
++ <value>.</value>
++ <value>..</value>
++ </choice>
++ </except>
+ </data>
+ </define>
+
+diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
+index 33e4caf..8b378c2 100644
+--- a/src/conf/storage_conf.c
++++ b/src/conf/storage_conf.c
+@@ -1056,7 +1056,8 @@ virStoragePoolSourceFormat(virBufferPtr buf,
+ virBufferAddLit(buf, " <source>\n");
+ if ((options->flags & VIR_STORAGE_POOL_SOURCE_HOST) && src->nhost) {
+ for (i = 0; i < src->nhost; i++) {
+- virBufferAsprintf(buf, " <host name='%s'", src->hosts[i].name);
++ virBufferEscapeString(buf, " <host name='%s'",
++ src->hosts[i].name);
+ if (src->hosts[i].port)
+ virBufferAsprintf(buf, " port='%d'", src->hosts[i].port);
+ virBufferAddLit(buf, "/>\n");
+@@ -1067,8 +1068,8 @@ virStoragePoolSourceFormat(virBufferPtr buf,
+ src->ndevice) {
+ for (i = 0; i < src->ndevice; i++) {
+ if (src->devices[i].nfreeExtent) {
+- virBufferAsprintf(buf, " <device path='%s'>\n",
+- src->devices[i].path);
++ virBufferEscapeString(buf, " <device path='%s'>\n",
++ src->devices[i].path);
+ for (j = 0; j < src->devices[i].nfreeExtent; j++) {
+ virBufferAsprintf(buf, " <freeExtent start='%llu' end='%llu'/>\n",
+ src->devices[i].freeExtents[j].start,
+@@ -1076,15 +1077,14 @@ virStoragePoolSourceFormat(virBufferPtr buf,
+ }
+ virBufferAddLit(buf, " </device>\n");
+ } else {
+- virBufferAsprintf(buf, " <device path='%s'/>\n",
+- src->devices[i].path);
++ virBufferEscapeString(buf, " <device path='%s'/>\n",
++ src->devices[i].path);
+ }
+ }
+ }
+
+- if ((options->flags & VIR_STORAGE_POOL_SOURCE_DIR) &&
+- src->dir)
+- virBufferAsprintf(buf, " <dir path='%s'/>\n", src->dir);
++ if (options->flags & VIR_STORAGE_POOL_SOURCE_DIR)
++ virBufferEscapeString(buf, " <dir path='%s'/>\n", src->dir);
+
+ if ((options->flags & VIR_STORAGE_POOL_SOURCE_ADAPTER)) {
+ if (src->adapter.type == VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST ||
+@@ -1104,9 +1104,8 @@ virStoragePoolSourceFormat(virBufferPtr buf,
+ }
+ }
+
+- if ((options->flags & VIR_STORAGE_POOL_SOURCE_NAME) &&
+- src->name)
+- virBufferAsprintf(buf, " <name>%s</name>\n", src->name);
++ if (options->flags & VIR_STORAGE_POOL_SOURCE_NAME)
++ virBufferEscapeString(buf, " <name>%s</name>\n", src->name);
+
+ if ((options->flags & VIR_STORAGE_POOL_SOURCE_INITIATOR_IQN) &&
+ src->initiator.iqn) {
+@@ -1129,11 +1128,12 @@ virStoragePoolSourceFormat(virBufferPtr buf,
+
+ if (src->authType == VIR_STORAGE_POOL_AUTH_CHAP ||
+ src->authType == VIR_STORAGE_POOL_AUTH_CEPHX) {
+- virBufferAsprintf(buf, " <auth type='%s' username='%s'>\n",
+- virStoragePoolAuthTypeTypeToString(src->authType),
+- (src->authType == VIR_STORAGE_POOL_AUTH_CHAP ?
+- src->auth.chap.username :
+- src->auth.cephx.username));
++ virBufferAsprintf(buf, " <auth type='%s' ",
++ virStoragePoolAuthTypeTypeToString(src->authType));
++ virBufferEscapeString(buf, "username='%s'>\n",
++ (src->authType == VIR_STORAGE_POOL_AUTH_CHAP ?
++ src->auth.chap.username :
++ src->auth.cephx.username));
+
+ virBufferAddLit(buf, " <secret");
+ if (src->auth.cephx.secret.uuidUsable) {
+@@ -1149,13 +1149,8 @@ virStoragePoolSourceFormat(virBufferPtr buf,
+ virBufferAddLit(buf, " </auth>\n");
+ }
+
+- if (src->vendor != NULL) {
+- virBufferEscapeString(buf, " <vendor name='%s'/>\n", src->vendor);
+- }
+-
+- if (src->product != NULL) {
+- virBufferEscapeString(buf, " <product name='%s'/>\n", src->product);
+- }
++ virBufferEscapeString(buf, " <vendor name='%s'/>\n", src->vendor);
++ virBufferEscapeString(buf, " <product name='%s'/>\n", src->product);
+
+ virBufferAddLit(buf, " </source>\n");
+
+@@ -1182,7 +1177,7 @@ virStoragePoolDefFormat(virStoragePoolDefPtr def)
+ goto cleanup;
+ }
+ virBufferAsprintf(&buf, "<pool type='%s'>\n", type);
+- virBufferAsprintf(&buf, " <name>%s</name>\n", def->name);
++ virBufferEscapeString(&buf, " <name>%s</name>\n", def->name);
+
+ virUUIDFormat(def->uuid, uuid);
+ virBufferAsprintf(&buf, " <uuid>%s</uuid>\n", uuid);
+@@ -1203,8 +1198,7 @@ virStoragePoolDefFormat(virStoragePoolDefPtr def)
+ def->type != VIR_STORAGE_POOL_SHEEPDOG) {
+ virBufferAddLit(&buf, " <target>\n");
+
+- if (def->target.path)
+- virBufferAsprintf(&buf, " <path>%s</path>\n", def->target.path);
++ virBufferEscapeString(&buf, " <path>%s</path>\n", def->target.path);
+
+ virBufferAddLit(&buf, " <permissions>\n");
+ virBufferAsprintf(&buf, " <mode>0%o</mode>\n",
+@@ -1214,9 +1208,8 @@ virStoragePoolDefFormat(virStoragePoolDefPtr def)
+ virBufferAsprintf(&buf, " <group>%d</group>\n",
+ (int) def->target.perms.gid);
+
+- if (def->target.perms.label)
+- virBufferAsprintf(&buf, " <label>%s</label>\n",
+- def->target.perms.label);
++ virBufferEscapeString(&buf, " <label>%s</label>\n",
++ def->target.perms.label);
+
+ virBufferAddLit(&buf, " </permissions>\n");
+ virBufferAddLit(&buf, " </target>\n");
+@@ -1282,8 +1275,8 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool,
+ goto error;
+ }
+
+- /* Auto-generated so deliberately ignore */
+- /* ret->key = virXPathString("string(./key)", ctxt); */
++ /* Normally generated by pool refresh, but useful for unit tests */
++ ret->key = virXPathString("string(./key)", ctxt);
+
+ capacity = virXPathString("string(./capacity)", ctxt);
+ unit = virXPathString("string(./capacity/@unit)", ctxt);
+@@ -1485,11 +1478,11 @@ static int
+ virStorageVolTargetDefFormat(virStorageVolOptionsPtr options,
+ virBufferPtr buf,
+ virStorageVolTargetPtr def,
+- const char *type) {
++ const char *type)
++{
+ virBufferAsprintf(buf, " <%s>\n", type);
+
+- if (def->path)
+- virBufferAsprintf(buf, " <path>%s</path>\n", def->path);
++ virBufferEscapeString(buf, " <path>%s</path>\n", def->path);
+
+ if (options->formatToString) {
+ const char *format = (options->formatToString)(def->format);
+@@ -1511,8 +1504,7 @@ virStorageVolTargetDefFormat(virStorageVolOptionsPtr options,
+ (unsigned int) def->perms.gid);
+
+
+- if (def->perms.label)
+- virBufferAsprintf(buf, " <label>%s</label>\n",
++ virBufferEscapeString(buf, " <label>%s</label>\n",
+ def->perms.label);
+
+ virBufferAddLit(buf, " </permissions>\n");
+@@ -1572,8 +1564,8 @@ virStorageVolDefFormat(virStoragePoolDefPtr pool,
+ return NULL;
+
+ virBufferAddLit(&buf, "<volume>\n");
+- virBufferAsprintf(&buf, " <name>%s</name>\n", def->name);
+- virBufferAsprintf(&buf, " <key>%s</key>\n", NULLSTR(def->key));
++ virBufferEscapeString(&buf, " <name>%s</name>\n", def->name);
++ virBufferEscapeString(&buf, " <key>%s</key>\n", def->key);
+ virBufferAddLit(&buf, " <source>\n");
+
+ if (def->source.nextent) {
+@@ -1585,8 +1577,8 @@ virStorageVolDefFormat(virStoragePoolDefPtr pool,
+ if (thispath != NULL)
+ virBufferAddLit(&buf, " </device>\n");
+
+- virBufferAsprintf(&buf, " <device path='%s'>\n",
+- def->source.extents[i].path);
++ virBufferEscapeString(&buf, " <device path='%s'>\n",
++ def->source.extents[i].path);
+ }
+
+ virBufferAsprintf(&buf,
+diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
+index 6c39284..702a118 100644
+--- a/src/storage/storage_driver.c
++++ b/src/storage/storage_driver.c
+@@ -1554,6 +1554,9 @@ storageVolCreateXML(virStoragePoolPtr obj,
+ goto cleanup;
+ }
+
++ /* Wipe any key the user may have suggested, as volume creation
++ * will generate the canonical key. */
++ VIR_FREE(voldef->key);
+ if (backend->createVol(obj->conn, pool, voldef) < 0) {
+ goto cleanup;
+ }
+@@ -1729,7 +1732,10 @@ storageVolCreateXMLFrom(virStoragePoolPtr obj,
+ pool->volumes.count+1) < 0)
+ goto cleanup;
+
+- /* 'Define' the new volume so we get async progress reporting */
++ /* 'Define' the new volume so we get async progress reporting.
++ * Wipe any key the user may have suggested, as volume creation
++ * will generate the canonical key. */
++ VIR_FREE(newvol->key);
+ if (backend->createVol(obj->conn, pool, newvol) < 0) {
+ goto cleanup;
+ }
+diff --git a/tests/storagepoolxml2xmlin/pool-dir-naming.xml b/tests/storagepoolxml2xmlin/pool-dir-naming.xml
+new file mode 100644
+index 0000000..aa043be
+--- /dev/null
++++ b/tests/storagepoolxml2xmlin/pool-dir-naming.xml
+@@ -0,0 +1,18 @@
++<pool type='dir'>
++ <name>virtimages</name>
++ <uuid>70a7eb15-6c34-ee9c-bf57-69e8e5ff3fb2</uuid>
++ <capacity>0</capacity>
++ <allocation>0</allocation>
++ <available>0</available>
++ <source>
++ </source>
++ <target>
++ <path>///var/////lib/libvirt/<images>//</path>
++ <permissions>
++ <mode>0700</mode>
++ <owner>-1</owner>
++ <group>-1</group>
++ <label>some_label_t</label>
++ </permissions>
++ </target>
++</pool>
+diff --git a/tests/storagepoolxml2xmlout/pool-dir-naming.xml b/tests/storagepoolxml2xmlout/pool-dir-naming.xml
+new file mode 100644
+index 0000000..536f58c
+--- /dev/null
++++ b/tests/storagepoolxml2xmlout/pool-dir-naming.xml
+@@ -0,0 +1,18 @@
++<pool type='dir'>
++ <name>virtimages</name>
++ <uuid>70a7eb15-6c34-ee9c-bf57-69e8e5ff3fb2</uuid>
++ <capacity unit='bytes'>0</capacity>
++ <allocation unit='bytes'>0</allocation>
++ <available unit='bytes'>0</available>
++ <source>
++ </source>
++ <target>
++ <path>/var/lib/libvirt/<images></path>
++ <permissions>
++ <mode>0700</mode>
++ <owner>-1</owner>
++ <group>-1</group>
++ <label>some_label_t</label>
++ </permissions>
++ </target>
++</pool>
+diff --git a/tests/storagepoolxml2xmltest.c b/tests/storagepoolxml2xmltest.c
+index d59cff9..c7159c6 100644
+--- a/tests/storagepoolxml2xmltest.c
++++ b/tests/storagepoolxml2xmltest.c
+@@ -85,6 +85,7 @@ mymain(void)
+ ret = -1
+
+ DO_TEST("pool-dir");
++ DO_TEST("pool-dir-naming");
+ DO_TEST("pool-fs");
+ DO_TEST("pool-logical");
+ DO_TEST("pool-logical-nopath");
+diff --git a/tests/storagevolxml2xmlin/vol-file-backing.xml b/tests/storagevolxml2xmlin/vol-file-backing.xml
+index d23349e..73e7f28 100644
+--- a/tests/storagevolxml2xmlin/vol-file-backing.xml
++++ b/tests/storagevolxml2xmlin/vol-file-backing.xml
+@@ -1,5 +1,6 @@
+ <volume>
+ <name>sparse.img</name>
++ <key>/var/lib/libvirt/images/sparse.img</key>
+ <source/>
+ <capacity unit='GB'>10</capacity>
+ <allocation unit='MiB'>0</allocation>
+diff --git a/tests/storagevolxml2xmlin/vol-file-naming.xml b/tests/storagevolxml2xmlin/vol-file-naming.xml
+new file mode 100644
+index 0000000..9a33e2b
+--- /dev/null
++++ b/tests/storagevolxml2xmlin/vol-file-naming.xml
+@@ -0,0 +1,20 @@
++<volume>
++ <name><sparse>.img</name>
++ <source/>
++ <capacity unit="TiB">1</capacity>
++ <allocation unit="bytes">0</allocation>
++ <target>
++ <path>/var/lib/libvirt/images/<sparse>.img</path>
++ <permissions>
++ <mode>0</mode>
++ <owner>0744</owner>
++ <group>0</group>
++ <label>virt_image_t</label>
++ </permissions>
++ <timestamps>
++ <atime>1341933637.273190990</atime>
++ <mtime>1341930622.047245868</mtime>
++ <ctime>1341930622.047245868</ctime>
++ </timestamps>
++ </target>
++</volume>
+diff --git a/tests/storagevolxml2xmlout/vol-file-backing.xml b/tests/storagevolxml2xmlout/vol-file-backing.xml
+index c0f152e..8d2fb57 100644
+--- a/tests/storagevolxml2xmlout/vol-file-backing.xml
++++ b/tests/storagevolxml2xmlout/vol-file-backing.xml
+@@ -1,6 +1,6 @@
+ <volume>
+ <name>sparse.img</name>
+- <key>(null)</key>
++ <key>/var/lib/libvirt/images/sparse.img</key>
+ <source>
+ </source>
+ <capacity unit='bytes'>10000000000</capacity>
+diff --git a/tests/storagevolxml2xmlout/vol-file-naming.xml b/tests/storagevolxml2xmlout/vol-file-naming.xml
+new file mode 100644
+index 0000000..7022b02
+--- /dev/null
++++ b/tests/storagevolxml2xmlout/vol-file-naming.xml
+@@ -0,0 +1,17 @@
++<volume>
++ <name><sparse>.img</name>
++ <source>
++ </source>
++ <capacity unit='bytes'>1099511627776</capacity>
++ <allocation unit='bytes'>0</allocation>
++ <target>
++ <path>/var/lib/libvirt/images/<sparse>.img</path>
++ <format type='raw'/>
++ <permissions>
++ <mode>00</mode>
++ <owner>744</owner>
++ <group>0</group>
++ <label>virt_image_t</label>
++ </permissions>
++ </target>
++</volume>
+diff --git a/tests/storagevolxml2xmlout/vol-file.xml b/tests/storagevolxml2xmlout/vol-file.xml
+index a3d6473..b97dd50 100644
+--- a/tests/storagevolxml2xmlout/vol-file.xml
++++ b/tests/storagevolxml2xmlout/vol-file.xml
+@@ -1,6 +1,5 @@
+ <volume>
+ <name>sparse.img</name>
+- <key>(null)</key>
+ <source>
+ </source>
+ <capacity unit='bytes'>1099511627776</capacity>
+diff --git a/tests/storagevolxml2xmlout/vol-logical-backing.xml b/tests/storagevolxml2xmlout/vol-logical-backing.xml
+index 6b010e3..bf34b08 100644
+--- a/tests/storagevolxml2xmlout/vol-logical-backing.xml
++++ b/tests/storagevolxml2xmlout/vol-logical-backing.xml
+@@ -1,6 +1,6 @@
+ <volume>
+ <name>Swap</name>
+- <key>(null)</key>
++ <key>r4xkCv-MQhr-WKIT-R66x-Epn2-e8hG-1Z5gY0</key>
+ <source>
+ </source>
+ <capacity unit='bytes'>2080374784</capacity>
+diff --git a/tests/storagevolxml2xmlout/vol-logical.xml b/tests/storagevolxml2xmlout/vol-logical.xml
+index 7bf309e..e9b4e4b 100644
+--- a/tests/storagevolxml2xmlout/vol-logical.xml
++++ b/tests/storagevolxml2xmlout/vol-logical.xml
+@@ -1,6 +1,6 @@
+ <volume>
+ <name>Swap</name>
+- <key>(null)</key>
++ <key>r4xkCv-MQhr-WKIT-R66x-Epn2-e8hG-1Z5gY0</key>
+ <source>
+ </source>
+ <capacity unit='bytes'>2080374784</capacity>
+diff --git a/tests/storagevolxml2xmlout/vol-partition.xml b/tests/storagevolxml2xmlout/vol-partition.xml
+index 271964f..9be1cf1 100644
+--- a/tests/storagevolxml2xmlout/vol-partition.xml
++++ b/tests/storagevolxml2xmlout/vol-partition.xml
+@@ -1,6 +1,6 @@
+ <volume>
+ <name>sda1</name>
+- <key>(null)</key>
++ <key>/dev/sda1</key>
+ <source>
+ </source>
+ <capacity unit='bytes'>106896384</capacity>
+diff --git a/tests/storagevolxml2xmlout/vol-qcow2-0.10-lazy.xml b/tests/storagevolxml2xmlout/vol-qcow2-0.10-lazy.xml
+index a7b5fed..fd3d606 100644
+--- a/tests/storagevolxml2xmlout/vol-qcow2-0.10-lazy.xml
++++ b/tests/storagevolxml2xmlout/vol-qcow2-0.10-lazy.xml
+@@ -1,6 +1,6 @@
+ <volume>
+ <name>OtherDemo.img</name>
+- <key>(null)</key>
++ <key>/var/lib/libvirt/images/OtherDemo.img</key>
+ <source>
+ </source>
+ <capacity unit='bytes'>5368709120</capacity>
+diff --git a/tests/storagevolxml2xmlout/vol-qcow2-1.1.xml b/tests/storagevolxml2xmlout/vol-qcow2-1.1.xml
+index b7df8a6..99fb5ac 100644
+--- a/tests/storagevolxml2xmlout/vol-qcow2-1.1.xml
++++ b/tests/storagevolxml2xmlout/vol-qcow2-1.1.xml
+@@ -1,6 +1,6 @@
+ <volume>
+ <name>OtherDemo.img</name>
+- <key>(null)</key>
++ <key>/var/lib/libvirt/images/OtherDemo.img</key>
+ <source>
+ </source>
+ <capacity unit='bytes'>5368709120</capacity>
+diff --git a/tests/storagevolxml2xmlout/vol-qcow2-lazy.xml b/tests/storagevolxml2xmlout/vol-qcow2-lazy.xml
+index 92b7875..3708ea7 100644
+--- a/tests/storagevolxml2xmlout/vol-qcow2-lazy.xml
++++ b/tests/storagevolxml2xmlout/vol-qcow2-lazy.xml
+@@ -1,6 +1,6 @@
+ <volume>
+ <name>OtherDemo.img</name>
+- <key>(null)</key>
++ <key>/var/lib/libvirt/images/OtherDemo.img</key>
+ <source>
+ </source>
+ <capacity unit='bytes'>5368709120</capacity>
+diff --git a/tests/storagevolxml2xmlout/vol-qcow2-nobacking.xml b/tests/storagevolxml2xmlout/vol-qcow2-nobacking.xml
+index e2da702..f6a2e21 100644
+--- a/tests/storagevolxml2xmlout/vol-qcow2-nobacking.xml
++++ b/tests/storagevolxml2xmlout/vol-qcow2-nobacking.xml
+@@ -1,6 +1,6 @@
+ <volume>
+ <name>OtherDemo.img</name>
+- <key>(null)</key>
++ <key>/var/lib/libvirt/images/OtherDemo.img</key>
+ <source>
+ </source>
+ <capacity unit='bytes'>5368709120</capacity>
+diff --git a/tests/storagevolxml2xmlout/vol-qcow2.xml b/tests/storagevolxml2xmlout/vol-qcow2.xml
+index f931a62..b9adcb4 100644
+--- a/tests/storagevolxml2xmlout/vol-qcow2.xml
++++ b/tests/storagevolxml2xmlout/vol-qcow2.xml
+@@ -1,6 +1,6 @@
+ <volume>
+ <name>OtherDemo.img</name>
+- <key>(null)</key>
++ <key>/var/lib/libvirt/images/OtherDemo.img</key>
+ <source>
+ </source>
+ <capacity unit='bytes'>5368709120</capacity>
+diff --git a/tests/storagevolxml2xmlout/vol-sheepdog.xml b/tests/storagevolxml2xmlout/vol-sheepdog.xml
+index 2f19af8..bd5d6d8 100644
+--- a/tests/storagevolxml2xmlout/vol-sheepdog.xml
++++ b/tests/storagevolxml2xmlout/vol-sheepdog.xml
+@@ -1,6 +1,5 @@
+ <volume>
+ <name>test2</name>
+- <key>(null)</key>
+ <source>
+ </source>
+ <capacity unit='bytes'>1024</capacity>
+diff --git a/tests/storagevolxml2xmltest.c b/tests/storagevolxml2xmltest.c
+index 5b0a60b..d62c29f 100644
+--- a/tests/storagevolxml2xmltest.c
++++ b/tests/storagevolxml2xmltest.c
+@@ -110,6 +110,7 @@ mymain(void)
+ while (0);
+
+ DO_TEST("pool-dir", "vol-file");
++ DO_TEST("pool-dir", "vol-file-naming");
+ DO_TEST("pool-dir", "vol-file-backing");
+ DO_TEST("pool-dir", "vol-qcow2");
+ DO_TEST("pool-dir", "vol-qcow2-1.1");
diff --git a/libvirt.spec b/libvirt.spec
index 1c1482e..f63aa7b 100644
--- a/libvirt.spec
+++ b/libvirt.spec
@@ -367,7 +367,7 @@
Summary: Library providing a simple virtualization API
Name: libvirt
Version: 1.1.3.4
-Release: 2%{?dist}%{?extra_release}
+Release: 3%{?dist}%{?extra_release}
License: LGPLv2+
Group: Development/Libraries
BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root
@@ -383,6 +383,9 @@ Patch0001: 0001-Add-Documentation-fields-to-systemd-service-files.patch
Patch0002: 0002-virSystemdCreateMachine-Set-dependencies-for-slices.patch
Patch0003: 0003-libvirt-guests-Wait-for-libvirtd-to-initialize.patch
Patch0004: 0004-virNetServerRun-Notify-systemd-that-we-re-accepting-.patch
+# Escape XML characters in volume XML (bz #1074528)
+Patch0005: 0005-maint-fix-comma-style-issues-conf.patch
+Patch0006: 0006-storage-use-valid-XML-for-awkward-volume-names.patch
%if %{with_libvirtd}
Requires: libvirt-daemon = %{version}-%{release}
@@ -1172,6 +1175,9 @@ of recent versions of Linux (and other OSes).
%patch0002 -p1
%patch0003 -p1
%patch0004 -p1
+# Escape XML characters in volume XML (bz #1074528)
+%patch0005 -p1
+%patch0006 -p1
%build
%if ! %{with_xen}
@@ -2130,6 +2136,9 @@ fi
%endif
%changelog
+* Mon Mar 10 2014 Cole Robinson <crobinso at redhat.com> - 1.1.3.4-3
+- Escape XML characters in volume XML (bz #1074528)
+
* Wed Mar 05 2014 Cole Robinson <crobinso at redhat.com> - 1.1.3.4-2
- Fix libvirt-guests.service on host boot (bz #1031696)
More information about the scm-commits
mailing list