Escape XML characters in volume XML (bz #1074528)

remotes/origin/f20
Cole Robinson 2014-03-10 09:25:58 -04:00
parent 0ae45cb5ee
commit 2b7e43f135
3 changed files with 924 additions and 1 deletions

View File

@ -0,0 +1,342 @@
From d3e5327fed75feeb262f4571f280a68625561a82 Mon Sep 17 00:00:00 2001
From: Eric Blake <eblake@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@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;

View File

@ -0,0 +1,572 @@
From a947da33c35e07bb68829c68b0c7c95a002a1407 Mon Sep 17 00:00:00 2001
From: Eric Blake <eblake@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@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/&lt;images&gt;//</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/&lt;images&gt;</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>&lt;sparse&gt;.img</name>
+ <source/>
+ <capacity unit="TiB">1</capacity>
+ <allocation unit="bytes">0</allocation>
+ <target>
+ <path>/var/lib/libvirt/images/&lt;sparse&gt;.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>&lt;sparse&gt;.img</name>
+ <source>
+ </source>
+ <capacity unit='bytes'>1099511627776</capacity>
+ <allocation unit='bytes'>0</allocation>
+ <target>
+ <path>/var/lib/libvirt/images/&lt;sparse&gt;.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");

View File

@ -367,7 +367,7 @@
Summary: Library providing a simple virtualization API Summary: Library providing a simple virtualization API
Name: libvirt Name: libvirt
Version: 1.1.3.4 Version: 1.1.3.4
Release: 2%{?dist}%{?extra_release} Release: 3%{?dist}%{?extra_release}
License: LGPLv2+ License: LGPLv2+
Group: Development/Libraries Group: Development/Libraries
BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root 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 Patch0002: 0002-virSystemdCreateMachine-Set-dependencies-for-slices.patch
Patch0003: 0003-libvirt-guests-Wait-for-libvirtd-to-initialize.patch Patch0003: 0003-libvirt-guests-Wait-for-libvirtd-to-initialize.patch
Patch0004: 0004-virNetServerRun-Notify-systemd-that-we-re-accepting-.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} %if %{with_libvirtd}
Requires: libvirt-daemon = %{version}-%{release} Requires: libvirt-daemon = %{version}-%{release}
@ -1172,6 +1175,9 @@ of recent versions of Linux (and other OSes).
%patch0002 -p1 %patch0002 -p1
%patch0003 -p1 %patch0003 -p1
%patch0004 -p1 %patch0004 -p1
# Escape XML characters in volume XML (bz #1074528)
%patch0005 -p1
%patch0006 -p1
%build %build
%if ! %{with_xen} %if ! %{with_xen}
@ -2130,6 +2136,9 @@ fi
%endif %endif
%changelog %changelog
* Mon Mar 10 2014 Cole Robinson <crobinso@redhat.com> - 1.1.3.4-3
- Escape XML characters in volume XML (bz #1074528)
* Wed Mar 05 2014 Cole Robinson <crobinso@redhat.com> - 1.1.3.4-2 * Wed Mar 05 2014 Cole Robinson <crobinso@redhat.com> - 1.1.3.4-2
- Fix libvirt-guests.service on host boot (bz #1031696) - Fix libvirt-guests.service on host boot (bz #1031696)