9:37 a.m.
Fixup and cleanup the disk source parsing code and make it usable in
other parts of the code by exporting it.
Peter Krempa (10):
conf: Remove unnecessary condition from
virDomainDiskSourceFormatInternal
conf: Refactor seclabel formatting in
virDomainDiskSourceFormatInternal
conf: Remove virDomainDiskSourceDefFormatSeclabel
conf: Refactor formatting of startupPolicy in
virDomainDiskSourceFormatInternal
conf: disk: Separate virStorageSource formatting
conf: Validate disk source configuration also for the backing store
conf: Separate seclabel validation from parsing
conf: Parse and validate disk source seclabels together with the
source
conf: Extract parsing of storage source related data
conf: Add and export wrapper for parsing storage source XML
src/conf/domain_conf.c | 402 ++++++++++++++++++++++++++++-------------------
src/conf/domain_conf.h | 13 ++
src/libvirt_private.syms | 2 +
3 files changed, 254 insertions(+), 163 deletions(-)
--
2.16.2
9:37 a.m.
New subject: [libvirt] [PATCH 01/10] conf: Remove unnecessary condition from virDomainDiskSourceFormatInternal
Now that the function is using virXMLFormatElement we don't need to
conditionally format anything, since we'll format the element according
to the presence of content.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/conf/domain_conf.c | 126 ++++++++++++++++++++++++-------------------------
1 file changed, 62 insertions(+), 64 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 04a6ee77af..ddabc77a9b 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -22832,81 +22832,79 @@ virDomainDiskSourceFormatInternal(virBufferPtr buf,
if (policy)
startupPolicy = virDomainStartupPolicyTypeToString(policy);
- if (src->path || src->nhosts > 0 || src->srcpool || startupPolicy) {
- switch ((virStorageType)src->type) {
- case VIR_STORAGE_TYPE_FILE:
- virBufferEscapeString(&attrBuf, " file='%s'",
src->path);
- virBufferEscapeString(&attrBuf, " startupPolicy='%s'",
startupPolicy);
-
- virDomainDiskSourceDefFormatSeclabel(&childBuf, src->nseclabels,
- src->seclabels, flags,
- skipSeclabels);
- break;
-
- case VIR_STORAGE_TYPE_BLOCK:
- virBufferEscapeString(&attrBuf, " dev='%s'",
src->path);
- virBufferEscapeString(&attrBuf, " startupPolicy='%s'",
startupPolicy);
-
- virDomainDiskSourceDefFormatSeclabel(&childBuf, src->nseclabels,
- src->seclabels, flags,
- skipSeclabels);
- break;
+ switch ((virStorageType)src->type) {
+ case VIR_STORAGE_TYPE_FILE:
+ virBufferEscapeString(&attrBuf, " file='%s'",
src->path);
+ virBufferEscapeString(&attrBuf, " startupPolicy='%s'",
startupPolicy);
- case VIR_STORAGE_TYPE_DIR:
- virBufferEscapeString(&attrBuf, " dir='%s'",
src->path);
- virBufferEscapeString(&attrBuf, " startupPolicy='%s'",
startupPolicy);
- break;
+ virDomainDiskSourceDefFormatSeclabel(&childBuf, src->nseclabels,
+ src->seclabels, flags,
+ skipSeclabels);
+ break;
- case VIR_STORAGE_TYPE_NETWORK:
- if (virDomainDiskSourceFormatNetwork(&attrBuf, &childBuf,
- src, flags) < 0)
- goto error;
- break;
+ case VIR_STORAGE_TYPE_BLOCK:
+ virBufferEscapeString(&attrBuf, " dev='%s'",
src->path);
+ virBufferEscapeString(&attrBuf, " startupPolicy='%s'",
startupPolicy);
- case VIR_STORAGE_TYPE_VOLUME:
- if (src->srcpool) {
- virBufferEscapeString(&attrBuf, " pool='%s'",
src->srcpool->pool);
- virBufferEscapeString(&attrBuf, " volume='%s'",
- src->srcpool->volume);
- if (src->srcpool->mode)
- virBufferAsprintf(&attrBuf, " mode='%s'",
-
virStorageSourcePoolModeTypeToString(src->srcpool->mode));
- }
- virBufferEscapeString(&attrBuf, " startupPolicy='%s'",
startupPolicy);
+ virDomainDiskSourceDefFormatSeclabel(&childBuf, src->nseclabels,
+ src->seclabels, flags,
+ skipSeclabels);
+ break;
- virDomainDiskSourceDefFormatSeclabel(&childBuf, src->nseclabels,
- src->seclabels, flags,
- skipSeclabels);
- break;
+ case VIR_STORAGE_TYPE_DIR:
+ virBufferEscapeString(&attrBuf, " dir='%s'",
src->path);
+ virBufferEscapeString(&attrBuf, " startupPolicy='%s'",
startupPolicy);
+ break;
- case VIR_STORAGE_TYPE_NONE:
- case VIR_STORAGE_TYPE_LAST:
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("unexpected disk type %d"), src->type);
+ case VIR_STORAGE_TYPE_NETWORK:
+ if (virDomainDiskSourceFormatNetwork(&attrBuf, &childBuf,
+ src, flags) < 0)
goto error;
+ break;
+
+ case VIR_STORAGE_TYPE_VOLUME:
+ if (src->srcpool) {
+ virBufferEscapeString(&attrBuf, " pool='%s'",
src->srcpool->pool);
+ virBufferEscapeString(&attrBuf, " volume='%s'",
+ src->srcpool->volume);
+ if (src->srcpool->mode)
+ virBufferAsprintf(&attrBuf, " mode='%s'",
+
virStorageSourcePoolModeTypeToString(src->srcpool->mode));
}
+ virBufferEscapeString(&attrBuf, " startupPolicy='%s'",
startupPolicy);
- /* Storage Source formatting will not carry through the blunder
- * that disk source formatting had at one time to format the
- * <auth> for a volume source type. The <auth> information is
- * kept in the storage pool and would be overwritten anyway.
- * So avoid formatting it for volumes. */
- if (src->auth && src->authInherited &&
- src->type != VIR_STORAGE_TYPE_VOLUME)
- virStorageAuthDefFormat(&childBuf, src->auth);
+ virDomainDiskSourceDefFormatSeclabel(&childBuf, src->nseclabels,
+ src->seclabels, flags,
+ skipSeclabels);
+ break;
- /* 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)
- goto error;
+ case VIR_STORAGE_TYPE_NONE:
+ case VIR_STORAGE_TYPE_LAST:
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("unexpected disk type %d"), src->type);
+ goto error;
+ }
- if (virDomainDiskSourceFormatPrivateData(&childBuf, src, flags, xmlopt) <
0)
- goto error;
+ /* Storage Source formatting will not carry through the blunder
+ * that disk source formatting had at one time to format the
+ * <auth> for a volume source type. The <auth> information is
+ * kept in the storage pool and would be overwritten anyway.
+ * So avoid formatting it for volumes. */
+ if (src->auth && src->authInherited &&
+ src->type != VIR_STORAGE_TYPE_VOLUME)
+ virStorageAuthDefFormat(&childBuf, src->auth);
- if (virXMLFormatElement(buf, "source", &attrBuf, &childBuf)
< 0)
- goto error;
- }
+ /* 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)
+ goto error;
+
+ if (virDomainDiskSourceFormatPrivateData(&childBuf, src, flags, xmlopt) < 0)
+ goto error;
+
+ if (virXMLFormatElement(buf, "source", &attrBuf, &childBuf) <
0)
+ goto error;
return 0;
--
2.16.2
9:37 a.m.
New subject: [libvirt] [PATCH 02/10] conf: Refactor seclabel formatting in virDomainDiskSourceFormatInternal
Call the formatter function only once.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/conf/domain_conf.c | 17 ++++++-----------
1 file changed, 6 insertions(+), 11 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index ddabc77a9b..ebe1172fd2 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -22836,19 +22836,11 @@ virDomainDiskSourceFormatInternal(virBufferPtr buf,
case VIR_STORAGE_TYPE_FILE:
virBufferEscapeString(&attrBuf, " file='%s'",
src->path);
virBufferEscapeString(&attrBuf, " startupPolicy='%s'",
startupPolicy);
-
- virDomainDiskSourceDefFormatSeclabel(&childBuf, src->nseclabels,
- src->seclabels, flags,
- skipSeclabels);
break;
case VIR_STORAGE_TYPE_BLOCK:
virBufferEscapeString(&attrBuf, " dev='%s'",
src->path);
virBufferEscapeString(&attrBuf, " startupPolicy='%s'",
startupPolicy);
-
- virDomainDiskSourceDefFormatSeclabel(&childBuf, src->nseclabels,
- src->seclabels, flags,
- skipSeclabels);
break;
case VIR_STORAGE_TYPE_DIR:
@@ -22873,9 +22865,6 @@ virDomainDiskSourceFormatInternal(virBufferPtr buf,
}
virBufferEscapeString(&attrBuf, " startupPolicy='%s'",
startupPolicy);
- virDomainDiskSourceDefFormatSeclabel(&childBuf, src->nseclabels,
- src->seclabels, flags,
- skipSeclabels);
break;
case VIR_STORAGE_TYPE_NONE:
@@ -22885,6 +22874,12 @@ virDomainDiskSourceFormatInternal(virBufferPtr buf,
goto error;
}
+ if (src->type != VIR_STORAGE_TYPE_NETWORK) {
+ virDomainDiskSourceDefFormatSeclabel(&childBuf, src->nseclabels,
+ src->seclabels, flags,
+ skipSeclabels);
+ }
+
/* Storage Source formatting will not carry through the blunder
* that disk source formatting had at one time to format the
* <auth> for a volume source type. The <auth> information is
--
2.16.2
9:37 a.m.
New subject: [libvirt] [PATCH 03/10] conf: Remove virDomainDiskSourceDefFormatSeclabel
The wrapper functionality can be moved to the only user
virDomainDiskSourceFormatInternal. Also removes comment which does not
reflect the truth any more.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/conf/domain_conf.c | 31 +++++++------------------------
1 file changed, 7 insertions(+), 24 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index ebe1172fd2..4aa66fe09c 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -22704,33 +22704,16 @@ virDomainDiskBlockIoDefFormat(virBufferPtr buf,
}
-/* virDomainSourceDefFormatSeclabel:
- *
- * This function automatically closes the <source> element and formats any
- * possible seclabels.
- */
-static void
-virDomainDiskSourceDefFormatSeclabel(virBufferPtr buf,
- size_t nseclabels,
- virSecurityDeviceLabelDefPtr *seclabels,
- unsigned int flags,
- bool skipSeclables)
-{
- size_t n;
-
- if (nseclabels && !skipSeclables) {
- for (n = 0; n < nseclabels; n++)
- virSecurityDeviceLabelDefFormat(buf, seclabels[n], flags);
- }
-}
-
static void
virDomainSourceDefFormatSeclabel(virBufferPtr buf,
size_t nseclabels,
virSecurityDeviceLabelDefPtr *seclabels,
unsigned int flags)
{
- virDomainDiskSourceDefFormatSeclabel(buf, nseclabels, seclabels, flags, false);
+ size_t n;
+
+ for (n = 0; n < nseclabels; n++)
+ virSecurityDeviceLabelDefFormat(buf, seclabels[n], flags);
}
@@ -22875,9 +22858,9 @@ virDomainDiskSourceFormatInternal(virBufferPtr buf,
}
if (src->type != VIR_STORAGE_TYPE_NETWORK) {
- virDomainDiskSourceDefFormatSeclabel(&childBuf, src->nseclabels,
- src->seclabels, flags,
- skipSeclabels);
+ if (!skipSeclabels)
+ virDomainSourceDefFormatSeclabel(&childBuf, src->nseclabels,
+ src->seclabels, flags);
}
/* Storage Source formatting will not carry through the blunder
--
2.16.2
9:37 a.m.
New subject: [libvirt] [PATCH 04/10] conf: Refactor formatting of startupPolicy in virDomainDiskSourceFormatInternal
Move it to a single location which also allows to get rid of the
temporrary variable.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
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 4aa66fe09c..b77cc8ed9f 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -22806,29 +22806,22 @@ virDomainDiskSourceFormatInternal(virBufferPtr buf,
bool skipSeclabels,
virDomainXMLOptionPtr xmlopt)
{
- const char *startupPolicy = NULL;
virBuffer attrBuf = VIR_BUFFER_INITIALIZER;
virBuffer childBuf = VIR_BUFFER_INITIALIZER;
virBufferSetChildIndent(&childBuf, buf);
- if (policy)
- startupPolicy = virDomainStartupPolicyTypeToString(policy);
-
switch ((virStorageType)src->type) {
case VIR_STORAGE_TYPE_FILE:
virBufferEscapeString(&attrBuf, " file='%s'",
src->path);
- virBufferEscapeString(&attrBuf, " startupPolicy='%s'",
startupPolicy);
break;
case VIR_STORAGE_TYPE_BLOCK:
virBufferEscapeString(&attrBuf, " dev='%s'",
src->path);
- virBufferEscapeString(&attrBuf, " startupPolicy='%s'",
startupPolicy);
break;
case VIR_STORAGE_TYPE_DIR:
virBufferEscapeString(&attrBuf, " dir='%s'",
src->path);
- virBufferEscapeString(&attrBuf, " startupPolicy='%s'",
startupPolicy);
break;
case VIR_STORAGE_TYPE_NETWORK:
@@ -22846,7 +22839,6 @@ virDomainDiskSourceFormatInternal(virBufferPtr buf,
virBufferAsprintf(&attrBuf, " mode='%s'",
virStorageSourcePoolModeTypeToString(src->srcpool->mode));
}
- virBufferEscapeString(&attrBuf, " startupPolicy='%s'",
startupPolicy);
break;
@@ -22858,6 +22850,10 @@ virDomainDiskSourceFormatInternal(virBufferPtr buf,
}
if (src->type != VIR_STORAGE_TYPE_NETWORK) {
+ if (policy)
+ virBufferEscapeString(&attrBuf, " startupPolicy='%s'",
+ virDomainStartupPolicyTypeToString(policy));
+
if (!skipSeclabels)
virDomainSourceDefFormatSeclabel(&childBuf, src->nseclabels,
src->seclabels, flags);
--
2.16.2
9:37 a.m.
New subject: [libvirt] [PATCH 05/10] conf: disk: Separate virStorageSource formatting
Move out formatting of 'startuPolicy' which is a property of the disk
out of the <source> element. Extracting the code formating the content
and attributes will also allow reuse in other parts of the code.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/conf/domain_conf.c | 90 ++++++++++++++++++++++++++++--------------------
src/conf/domain_conf.h | 7 ++++
src/libvirt_private.syms | 1 +
3 files changed, 60 insertions(+), 38 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b77cc8ed9f..1c79d2b49b 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -22798,45 +22798,39 @@ virDomainDiskSourceFormatPrivateData(virBufferPtr buf,
}
-static int
-virDomainDiskSourceFormatInternal(virBufferPtr buf,
- virStorageSourcePtr src,
- int policy,
- unsigned int flags,
- bool skipSeclabels,
- virDomainXMLOptionPtr xmlopt)
+int
+virDomainStorageSourceFormat(virBufferPtr attrBuf,
+ virBufferPtr childBuf,
+ virStorageSourcePtr src,
+ unsigned int flags,
+ bool skipSeclabels)
{
- virBuffer attrBuf = VIR_BUFFER_INITIALIZER;
- 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)
- goto error;
+ 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));
}
@@ -22846,18 +22840,12 @@ virDomainDiskSourceFormatInternal(virBufferPtr buf,
case VIR_STORAGE_TYPE_LAST:
virReportError(VIR_ERR_INTERNAL_ERROR,
_("unexpected disk type %d"), src->type);
- goto error;
+ return -1;
}
- if (src->type != VIR_STORAGE_TYPE_NETWORK) {
- if (policy)
- virBufferEscapeString(&attrBuf, " startupPolicy='%s'",
- virDomainStartupPolicyTypeToString(policy));
-
- if (!skipSeclabels)
- virDomainSourceDefFormatSeclabel(&childBuf, src->nseclabels,
- src->seclabels, flags);
- }
+ if (!skipSeclabels && src->type != VIR_STORAGE_TYPE_NETWORK)
+ virDomainSourceDefFormatSeclabel(childBuf, src->nseclabels,
+ src->seclabels, flags);
/* Storage Source formatting will not carry through the blunder
* that disk source formatting had at one time to format the
@@ -22866,26 +22854,52 @@ virDomainDiskSourceFormatInternal(virBufferPtr buf,
* 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)
- goto error;
+ virStorageEncryptionFormat(childBuf, src->encryption) < 0)
+ return -1;
+
+ return 0;
+}
+
+
+static int
+virDomainDiskSourceFormatInternal(virBufferPtr buf,
+ virStorageSourcePtr src,
+ int policy,
+ unsigned int flags,
+ bool skipSeclabels,
+ virDomainXMLOptionPtr xmlopt)
+{
+ virBuffer attrBuf = VIR_BUFFER_INITIALIZER;
+ virBuffer childBuf = VIR_BUFFER_INITIALIZER;
+ int ret = -1;
+
+ virBufferSetChildIndent(&childBuf, buf);
+
+ if (virDomainStorageSourceFormat(&attrBuf, &childBuf, src, flags,
+ skipSeclabels) < 0)
+ goto cleanup;
+
+ if (policy && src->type != VIR_STORAGE_TYPE_NETWORK)
+ virBufferEscapeString(&attrBuf, " startupPolicy='%s'",
+ virDomainStartupPolicyTypeToString(policy));
if (virDomainDiskSourceFormatPrivateData(&childBuf, src, flags, xmlopt) < 0)
- goto error;
+ goto cleanup;
if (virXMLFormatElement(buf, "source", &attrBuf, &childBuf) <
0)
- goto error;
+ goto cleanup;
- return 0;
+ ret = 0;
- error:
+ cleanup:
virBufferFreeAndReset(&attrBuf);
virBufferFreeAndReset(&childBuf);
- return -1;
+ return ret;
}
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 337ce79425..61379e50fe 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -3427,6 +3427,13 @@ 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,
+ bool skipSeclabels)
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
+
int virDomainDefGetVcpuPinInfoHelper(virDomainDefPtr def,
int maplen,
int ncpumaps,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 3766e20d3b..c67bce7389 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -542,6 +542,7 @@ virDomainStateReasonFromString;
virDomainStateReasonToString;
virDomainStateTypeFromString;
virDomainStateTypeToString;
+virDomainStorageSourceFormat;
virDomainTaintTypeFromString;
virDomainTaintTypeToString;
virDomainTimerModeTypeFromString;
--
2.16.2
9:37 a.m.
New subject: [libvirt] [PATCH 06/10] conf: Validate disk source configuration also for the backing store
Since we already parse the <backingStore> of a disk source, we should
also validate the configuration for the whole backing chain and not only
for the top level image.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/conf/domain_conf.c | 24 ++++++++++++++----------
1 file changed, 14 insertions(+), 10 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 1c79d2b49b..8cd41edb5e 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -8974,6 +8974,8 @@ virDomainDiskSourceDefParseAuthValidate(const virStorageSource
*src)
static int
virDomainDiskDefParseValidate(const virDomainDiskDef *def)
{
+ virStorageSourcePtr next;
+
if (def->bus != VIR_DOMAIN_DISK_BUS_VIRTIO) {
if (def->event_idx != VIR_TRISTATE_SWITCH_ABSENT) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
@@ -9044,19 +9046,21 @@ virDomainDiskDefParseValidate(const virDomainDiskDef *def)
}
}
- if (virDomainDiskSourceDefParseAuthValidate(def->src) < 0)
- return -1;
+ for (next = def->src; next; next = next->backingStore) {
+ if (virDomainDiskSourceDefParseAuthValidate(next) < 0)
+ return -1;
- if (def->src->encryption) {
- virStorageEncryptionPtr encryption = def->src->encryption;
+ if (next->encryption) {
+ virStorageEncryptionPtr encryption = next->encryption;
- if (encryption->format == VIR_STORAGE_ENCRYPTION_FORMAT_LUKS &&
- encryption->encinfo.cipher_name) {
+ if (encryption->format == VIR_STORAGE_ENCRYPTION_FORMAT_LUKS &&
+ encryption->encinfo.cipher_name) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("supplying <cipher> for domain disk definition
"
- "is unnecessary"));
- return -1;
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("supplying <cipher> for domain disk
definition "
+ "is unnecessary"));
+ return -1;
+ }
}
}
--
2.16.2
9:37 a.m.
New subject: [libvirt] [PATCH 07/10] conf: Separate seclabel validation from parsing
Rather than checking that the security label is legal when parsing it
move the code into a separate function.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/conf/domain_conf.c | 68 ++++++++++++++++++++++++++++++++------------------
1 file changed, 44 insertions(+), 24 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 8cd41edb5e..6c2a2f3a75 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -8214,8 +8214,7 @@ virSecurityLabelDefsParseXML(virDomainDefPtr def,
static int
virSecurityDeviceLabelDefParseXML(virSecurityDeviceLabelDefPtr **seclabels_rtn,
size_t *nseclabels_rtn,
- virSecurityLabelDefPtr *vmSeclabels,
- int nvmSeclabels, xmlXPathContextPtr ctxt,
+ xmlXPathContextPtr ctxt,
unsigned int flags)
{
virSecurityDeviceLabelDefPtr *seclabels = NULL;
@@ -8223,7 +8222,6 @@ virSecurityDeviceLabelDefParseXML(virSecurityDeviceLabelDefPtr
**seclabels_rtn,
int n;
size_t i, j;
xmlNodePtr *list = NULL;
- virSecurityLabelDefPtr vmDef = NULL;
char *model, *relabel, *label, *labelskip;
if ((n = virXPathNodeSet("./seclabel", ctxt, &list)) < 0)
@@ -8243,14 +8241,6 @@ virSecurityDeviceLabelDefParseXML(virSecurityDeviceLabelDefPtr
**seclabels_rtn,
/* get model associated to this override */
model = virXMLPropString(list[i], "model");
if (model) {
- /* find the security label that it's being overridden */
- for (j = 0; j < nvmSeclabels; j++) {
- if (STREQ(vmSeclabels[j]->model, model)) {
- vmDef = vmSeclabels[j];
- break;
- }
- }
-
/* check for duplicate seclabels */
for (j = 0; j < i; j++) {
if (STREQ_NULLABLE(model, seclabels[j]->model)) {
@@ -8262,14 +8252,6 @@ virSecurityDeviceLabelDefParseXML(virSecurityDeviceLabelDefPtr
**seclabels_rtn,
seclabels[i]->model = model;
}
- /* Can't use overrides if top-level doesn't allow relabeling. */
- if (vmDef && !vmDef->relabel) {
- virReportError(VIR_ERR_XML_ERROR, "%s",
- _("label overrides require relabeling to be "
- "enabled at the domain level"));
- goto error;
- }
-
relabel = virXMLPropString(list[i], "relabel");
if (relabel != NULL) {
if (STREQ(relabel, "yes")) {
@@ -8324,6 +8306,37 @@ virSecurityDeviceLabelDefParseXML(virSecurityDeviceLabelDefPtr
**seclabels_rtn,
}
+static int
+virSecurityDeviceLabelDefValidateXML(virSecurityDeviceLabelDefPtr *seclabels,
+ size_t nseclabels,
+ virSecurityLabelDefPtr *vmSeclabels,
+ size_t nvmSeclabels)
+{
+ virSecurityDeviceLabelDefPtr seclabel;
+ size_t i;
+ size_t j;
+
+ for (i = 0; i < nseclabels; i++) {
+ seclabel = seclabels[i];
+
+ /* find the security label that it's being overridden */
+ for (j = 0; j < nvmSeclabels; j++) {
+ if (STRNEQ_NULLABLE(vmSeclabels[j]->model, seclabel->model))
+ continue;
+
+ if (!vmSeclabels[j]->relabel) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("label overrides require relabeling to be "
+ "enabled at the domain level"));
+ return -1;
+ }
+ }
+ }
+
+ return 0;
+}
+
+
/* Parse the XML definition for a lease
*/
static virDomainLeaseDefPtr
@@ -9453,11 +9466,16 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
ctxt->node = sourceNode;
if (virSecurityDeviceLabelDefParseXML(&def->src->seclabels,
&def->src->nseclabels,
- vmSeclabels,
- nvmSeclabels,
ctxt,
flags) < 0)
goto error;
+
+ if (virSecurityDeviceLabelDefValidateXML(def->src->seclabels,
+ def->src->nseclabels,
+ vmSeclabels,
+ nvmSeclabels) < 0)
+ goto error;
+
ctxt->node = saved_node;
}
@@ -12133,10 +12151,12 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def,
ctxt->node = cur;
if (virSecurityDeviceLabelDefParseXML(&def->seclabels,
&def->nseclabels,
- vmSeclabels,
- nvmSeclabels,
ctxt,
- flags) < 0) {
+ flags) < 0 ||
+ virSecurityDeviceLabelDefValidateXML(def->seclabels,
+ def->nseclabels,
+ vmSeclabels,
+ nvmSeclabels) < 0) {
ctxt->node = saved_node;
goto error;
}
--
2.16.2
9:37 a.m.
New subject: [libvirt] [PATCH 08/10] conf: Parse and validate disk source seclabels together with the source
Since seclabels are formatted along with the source element and will
also make sense to be passed for the backing chain we should parse them
in the place where we parse the disk source. Same applies for
validation.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/conf/domain_conf.c | 39 +++++++++++++++------------------------
1 file changed, 15 insertions(+), 24 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 6c2a2f3a75..d1ff80feb7 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -8631,6 +8631,10 @@ virDomainDiskSourceParse(xmlNodePtr node,
!(src->encryption = virStorageEncryptionParseNode(tmp, ctxt)))
goto cleanup;
+ if (virSecurityDeviceLabelDefParseXML(&src->seclabels,
&src->nseclabels,
+ ctxt, flags) < 0)
+ goto cleanup;
+
if (virDomainDiskSourcePrivateDataParse(ctxt, src, flags, xmlopt) < 0)
goto cleanup;
@@ -8985,7 +8989,10 @@ virDomainDiskSourceDefParseAuthValidate(const virStorageSource
*src)
static int
-virDomainDiskDefParseValidate(const virDomainDiskDef *def)
+virDomainDiskDefParseValidate(const virDomainDiskDef *def,
+ virSecurityLabelDefPtr *vmSeclabels,
+ size_t nvmSeclabels)
+
{
virStorageSourcePtr next;
@@ -9075,6 +9082,12 @@ virDomainDiskDefParseValidate(const virDomainDiskDef *def)
return -1;
}
}
+
+ if (virSecurityDeviceLabelDefValidateXML(next->seclabels,
+ next->nseclabels,
+ vmSeclabels,
+ nvmSeclabels) < 0)
+ return -1;
}
return 0;
@@ -9222,7 +9235,6 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
unsigned int flags)
{
virDomainDiskDefPtr def;
- xmlNodePtr sourceNode = NULL;
xmlNodePtr cur;
xmlNodePtr save_ctxt = ctxt->node;
char *tmp = NULL;
@@ -9281,8 +9293,6 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
continue;
if (!source && virXMLNodeNameEqual(cur, "source")) {
- sourceNode = cur;
-
if (virDomainDiskSourceParse(cur, ctxt, def->src, flags, xmlopt) < 0)
goto error;
@@ -9460,25 +9470,6 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
goto error;
}
- /* If source is present, check for an optional seclabel override. */
- if (sourceNode) {
- xmlNodePtr saved_node = ctxt->node;
- ctxt->node = sourceNode;
- if (virSecurityDeviceLabelDefParseXML(&def->src->seclabels,
- &def->src->nseclabels,
- ctxt,
- flags) < 0)
- goto error;
-
- if (virSecurityDeviceLabelDefValidateXML(def->src->seclabels,
- def->src->nseclabels,
- vmSeclabels,
- nvmSeclabels) < 0)
- goto error;
-
- ctxt->node = saved_node;
- }
-
if (!target && !(flags & VIR_DOMAIN_DEF_PARSE_DISK_SOURCE)) {
if (def->src->srcpool) {
if (virAsprintf(&tmp, "pool = '%s', volume =
'%s'",
@@ -9644,7 +9635,7 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
goto error;
}
- if (virDomainDiskDefParseValidate(def) < 0)
+ if (virDomainDiskDefParseValidate(def, vmSeclabels, nvmSeclabels) < 0)
goto error;
cleanup:
--
2.16.2
9:37 a.m.
New subject: [libvirt] [PATCH 09/10] conf: Extract parsing of storage source related data
Split out the parser and separate it from the private data part so that
it can be later reused in other parts of the code.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/conf/domain_conf.c | 43 +++++++++++++++++++++++++++++--------------
1 file changed, 29 insertions(+), 14 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index d1ff80feb7..86fc275116 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -8554,24 +8554,26 @@ virDomainDiskSourceNetworkParse(xmlNodePtr node,
static int
-virDomainDiskSourcePrivateDataParse(xmlXPathContextPtr ctxt,
+virDomainDiskSourcePrivateDataParse(xmlNodePtr node,
+ xmlXPathContextPtr ctxt,
virStorageSourcePtr src,
unsigned int flags,
virDomainXMLOptionPtr xmlopt)
{
xmlNodePtr saveNode = ctxt->node;
- xmlNodePtr node;
int ret = -1;
if (!(flags & VIR_DOMAIN_DEF_PARSE_STATUS) ||
!xmlopt || !xmlopt->privateData.storageParse)
return 0;
- if (!(node = virXPathNode("./privateData", ctxt)))
- return 0;
-
ctxt->node = node;
+ if (!(ctxt->node = virXPathNode("./privateData", ctxt))) {
+ ret = 0;
+ goto cleanup;
+ }
+
if (xmlopt->privateData.storageParse(ctxt, src) < 0)
goto cleanup;
@@ -8584,12 +8586,11 @@ virDomainDiskSourcePrivateDataParse(xmlXPathContextPtr ctxt,
}
-int
-virDomainDiskSourceParse(xmlNodePtr node,
- xmlXPathContextPtr ctxt,
- virStorageSourcePtr src,
- unsigned int flags,
- virDomainXMLOptionPtr xmlopt)
+static int
+virDomainStorageSourceParse(xmlNodePtr node,
+ xmlXPathContextPtr ctxt,
+ virStorageSourcePtr src,
+ unsigned int flags)
{
int ret = -1;
xmlNodePtr saveNode = ctxt->node;
@@ -8635,9 +8636,6 @@ virDomainDiskSourceParse(xmlNodePtr node,
ctxt, flags) < 0)
goto cleanup;
- if (virDomainDiskSourcePrivateDataParse(ctxt, src, flags, xmlopt) < 0)
- goto cleanup;
-
/* People sometimes pass a bogus '' source path when they mean to omit the
* source element completely (e.g. CDROM without media). This is just a
* little compatibility check to help those broken apps */
@@ -8652,6 +8650,23 @@ virDomainDiskSourceParse(xmlNodePtr node,
}
+int
+virDomainDiskSourceParse(xmlNodePtr node,
+ xmlXPathContextPtr ctxt,
+ virStorageSourcePtr src,
+ unsigned int flags,
+ virDomainXMLOptionPtr xmlopt)
+{
+ if (virDomainStorageSourceParse(node, ctxt, src, flags) < 0)
+ return -1;
+
+ if (virDomainDiskSourcePrivateDataParse(node, ctxt, src, flags, xmlopt) < 0)
+ return -1;
+
+ return 0;
+}
+
+
static int
virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt,
virStorageSourcePtr src,
--
2.16.2
9:37 a.m.
New subject: [libvirt] [PATCH 10/10] conf: Add and export wrapper for parsing storage source XML
Add a helper that parses a storage source XML node into a new
virStorageSource object. Since there are multiple approaches to store
the 'type' and 'format' attributes, they need to be parsed manually
prior to calling the function.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/conf/domain_conf.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++++
src/conf/domain_conf.h | 6 +++++
src/libvirt_private.syms | 1 +
3 files changed, 67 insertions(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 86fc275116..0b25c6316f 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -8650,6 +8650,66 @@ virDomainStorageSourceParse(xmlNodePtr node,
}
+/**
+ * virDomainStorageSourceParseNew:
+ * @node: XML node object to parse
+ * @ctxt: XML XPath context
+ * @flags: virDomainDefParseFlags
+ *
+ * Parses the XML @node and returns a virStorageSource object with the parsed
+ * data. Note that 'format' and 'type' attributes need to be members of
the same
+ * object and need to be provided.
+ */
+virStorageSourcePtr
+virDomainStorageSourceParseNew(xmlNodePtr node,
+ xmlXPathContextPtr ctxt,
+ unsigned int flags)
+{
+ virStorageSourcePtr src = NULL;
+ virStorageSourcePtr ret = NULL;
+ char *format = NULL;
+ char *type = NULL;
+
+ if (VIR_ALLOC(src) < 0)
+ return NULL;
+
+ if (!(type = virXMLPropString(node, "type"))) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("missing storage source type"));
+ goto cleanup;
+ }
+
+ if (!(format = virXMLPropString(node, "format"))) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ ("missing storage source format"));
+ goto cleanup;
+ }
+
+ if ((src->type = virStorageTypeFromString(type)) <= 0) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unknown storage source type '%s'"), type);
+ goto cleanup;
+ }
+
+ if ((src->format = virStorageFileFormatTypeFromString(format)) <= 0) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unknown storage source format '%s'"),
format);
+ goto cleanup;
+ }
+
+ if (virDomainStorageSourceParse(node, ctxt, src, flags) < 0)
+ goto cleanup;
+
+ VIR_STEAL_PTR(ret, src);
+
+ cleanup:
+ virStorageSourceFree(src);
+ VIR_FREE(format);
+ VIR_FREE(type);
+ return ret;
+}
+
+
int
virDomainDiskSourceParse(xmlNodePtr node,
xmlXPathContextPtr ctxt,
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 61379e50fe..c82a23d220 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -3434,6 +3434,12 @@ int virDomainStorageSourceFormat(virBufferPtr attrBuf,
bool skipSeclabels)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
+virStorageSourcePtr
+virDomainStorageSourceParseNew(xmlNodePtr node,
+ xmlXPathContextPtr ctxt,
+ unsigned int flags)
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
+
int virDomainDefGetVcpuPinInfoHelper(virDomainDefPtr def,
int maplen,
int ncpumaps,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index c67bce7389..4dfcbb4230 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -543,6 +543,7 @@ virDomainStateReasonToString;
virDomainStateTypeFromString;
virDomainStateTypeToString;
virDomainStorageSourceFormat;
+virDomainStorageSourceParseNew;
virDomainTaintTypeFromString;
virDomainTaintTypeToString;
virDomainTimerModeTypeFromString;
--
2.16.2
5 a.m.
New subject: [libvirt] [PATCH 10/10] conf: Add and export wrapper for parsing storage source XML
On 03/13/2018 03:37 PM, Peter Krempa wrote:
Add a helper that parses a storage source XML node into a new
virStorageSource object. Since there are multiple approaches to store
the 'type' and 'format' attributes, they need to be parsed manually
prior to calling the function.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/conf/domain_conf.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++++
src/conf/domain_conf.h | 6 +++++
src/libvirt_private.syms | 1 +
3 files changed, 67 insertions(+)
Perhaps you forgot to include next patch that uses
virDomainStorageSourceParseNew()? Because this patch does nothing more
than introducing a function that is never used.
Michal
5:57 a.m.
New subject: [libvirt] [PATCH 10/10] conf: Add and export wrapper for parsing storage source XML
On Wed, Mar 14, 2018 at 11:00:46 +0100, Michal Privoznik wrote:
On 03/13/2018 03:37 PM, Peter Krempa wrote:
> Add a helper that parses a storage source XML node into a new
> virStorageSource object. Since there are multiple approaches to store
> the 'type' and 'format' attributes, they need to be parsed manually
> prior to calling the function.
>
> Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
> ---
> src/conf/domain_conf.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++++
> src/conf/domain_conf.h | 6 +++++
> src/libvirt_private.syms | 1 +
> 3 files changed, 67 insertions(+)
Perhaps you forgot to include next patch that uses
virDomainStorageSourceParseNew()? Because this patch does nothing more
than introducing a function that is never used.
Yes. The patch that uses this function was not posted yet. I'm sending
these sub-series out so that I don't have to send 50+ patches at once.
2:47 a.m.
New subject: [libvirt] [PATCH 10/10] conf: Add and export wrapper for parsing storage source XML
On 03/14/2018 11:57 AM, Peter Krempa wrote:
On Wed, Mar 14, 2018 at 11:00:46 +0100, Michal Privoznik wrote:
> On 03/13/2018 03:37 PM, Peter Krempa wrote:
>> Add a helper that parses a storage source XML node into a new
>> virStorageSource object. Since there are multiple approaches to store
>> the 'type' and 'format' attributes, they need to be parsed
manually
>> prior to calling the function.
>>
>> Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
>> ---
>> src/conf/domain_conf.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++++
>> src/conf/domain_conf.h | 6 +++++
>> src/libvirt_private.syms | 1 +
>> 3 files changed, 67 insertions(+)
>
> Perhaps you forgot to include next patch that uses
> virDomainStorageSourceParseNew()? Because this patch does nothing more
> than introducing a function that is never used.
Yes. The patch that uses this function was not posted yet. I'm sending
these sub-series out so that I don't have to send 50+ patches at once.
Okay. ACK then.
Michal
5:01 a.m.
On 03/13/2018 03:37 PM, Peter Krempa wrote:
Fixup and cleanup the disk source parsing code and make it usable in
other parts of the code by exporting it.
Peter Krempa (10):
conf: Remove unnecessary condition from
virDomainDiskSourceFormatInternal
conf: Refactor seclabel formatting in
virDomainDiskSourceFormatInternal
conf: Remove virDomainDiskSourceDefFormatSeclabel
conf: Refactor formatting of startupPolicy in
virDomainDiskSourceFormatInternal
conf: disk: Separate virStorageSource formatting
conf: Validate disk source configuration also for the backing store
conf: Separate seclabel validation from parsing
conf: Parse and validate disk source seclabels together with the
source
conf: Extract parsing of storage source related data
conf: Add and export wrapper for parsing storage source XML
src/conf/domain_conf.c | 402 ++++++++++++++++++++++++++++-------------------
src/conf/domain_conf.h | 13 ++
src/libvirt_private.syms | 2 +
3 files changed, 254 insertions(+), 163 deletions(-)
ACK to patches 1-9.
Michal
2444
days inactive
2446
days old
14 comments
2 participants
participants (2)
-
Michal Privoznik -
Peter Krempa