[PATCH 0/9] qemu: Properly support 'volume' type backing stores
We didn't bother to translate the disk source when an user used <backingStore type='volume'>. Peter Krempa (9): virDomainDiskAddISCSIPoolSourceHost: Sanitize handling of string list virDomainDiskAddISCSIPoolSourceHost: use g_new0 instead of VIR_ALLOC_N virDomainDiskAddISCSIPoolSourceHost: Remove 'cleanup' label virDomainDiskAddISCSIPoolSourceHost: Remove ternary operator virDomainDiskAddISCSIPoolSourceHost: Take virStorageSourcePtr instead of virDomainDiskDefPtr virDomainDiskTranslateSourcePoolAuth: Take virStorageSourcePtr instead of virDomainDiskDefPtr virDomainDiskTranslateISCSIDirect: Take virStorageSourcePtr instead of virDomainDiskDefPtr virDomainDiskTranslateSourcePool: split code to setup one storage source virDomainDiskTranslateSourcePool: Translate 'VOLUME' disks in whole backing chain src/conf/domain_conf.c | 188 ++++++++++++++++++++--------------------- 1 file changed, 92 insertions(+), 96 deletions(-) -- 2.24.1
Use virStringSplitCount instead of virStringSplit so that we can drop the call to virStringListLength and use VIR_AUTOSTRINGLIST to declare it and allow removal of the cleanup section. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index c95bd34fb5..6124c7520c 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -31327,7 +31327,8 @@ virDomainDiskAddISCSIPoolSourceHost(virDomainDiskDefPtr def, virStoragePoolDefPtr pooldef) { int ret = -1; - char **tokens = NULL; + VIR_AUTOSTRINGLIST tokens = NULL; + size_t ntokens; /* Only support one host */ if (pooldef->source.nhost != 1) { @@ -31348,10 +31349,10 @@ virDomainDiskAddISCSIPoolSourceHost(virDomainDiskDefPtr def, pooldef->source.hosts[0].port : 3260; /* iscsi volume has name like "unit:0:0:1" */ - if (!(tokens = virStringSplit(def->src->srcpool->volume, ":", 0))) + if (!(tokens = virStringSplitCount(def->src->srcpool->volume, ":", 0, &ntokens))) goto cleanup; - if (virStringListLength((const char * const *)tokens) != 4) { + if (ntokens != 4) { virReportError(VIR_ERR_INTERNAL_ERROR, _("unexpected iscsi volume name '%s'"), def->src->srcpool->volume); @@ -31373,7 +31374,6 @@ virDomainDiskAddISCSIPoolSourceHost(virDomainDiskDefPtr def, ret = 0; cleanup: - virStringListFree(tokens); return ret; } -- 2.24.1
On Wed, Feb 05, 2020 at 02:40:52PM +0100, Peter Krempa wrote:
Use virStringSplitCount instead of virStringSplit so that we can drop the call to virStringListLength and use VIR_AUTOSTRINGLIST to declare it and allow removal of the cleanup section.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6124c7520c..7cf555596f 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -31339,9 +31339,7 @@ virDomainDiskAddISCSIPoolSourceHost(virDomainDiskDefPtr def, /* iscsi pool only supports one host */ def->src->nhosts = 1; - - if (VIR_ALLOC_N(def->src->hosts, def->src->nhosts) < 0) - goto cleanup; + def->src->hosts = g_new0(virStorageNetHostDef, 1); def->src->hosts[0].name = g_strdup(pooldef->source.hosts[0].name); -- 2.24.1
On Wed, Feb 05, 2020 at 02:40:53PM +0100, Peter Krempa wrote:
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Signed-off-by: Peter Krempa <pkrempa@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 7cf555596f..1c52d7f347 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -31326,7 +31326,6 @@ static int virDomainDiskAddISCSIPoolSourceHost(virDomainDiskDefPtr def, virStoragePoolDefPtr pooldef) { - int ret = -1; VIR_AUTOSTRINGLIST tokens = NULL; size_t ntokens; @@ -31334,7 +31333,7 @@ virDomainDiskAddISCSIPoolSourceHost(virDomainDiskDefPtr def, if (pooldef->source.nhost != 1) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Expected exactly 1 host for the storage pool")); - goto cleanup; + return -1; } /* iscsi pool only supports one host */ @@ -31348,13 +31347,13 @@ virDomainDiskAddISCSIPoolSourceHost(virDomainDiskDefPtr def, /* iscsi volume has name like "unit:0:0:1" */ if (!(tokens = virStringSplitCount(def->src->srcpool->volume, ":", 0, &ntokens))) - goto cleanup; + return -1; if (ntokens != 4) { virReportError(VIR_ERR_INTERNAL_ERROR, _("unexpected iscsi volume name '%s'"), def->src->srcpool->volume); - goto cleanup; + return -1; } /* iscsi pool has only one source device path */ @@ -31369,10 +31368,7 @@ virDomainDiskAddISCSIPoolSourceHost(virDomainDiskDefPtr def, def->src->protocol = VIR_STORAGE_NET_PROTOCOL_ISCSI; - ret = 0; - - cleanup: - return ret; + return 0; } -- 2.24.1
On Wed, Feb 05, 2020 at 02:40:54PM +0100, Peter Krempa wrote:
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 1c52d7f347..5ef12c7ee7 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -31342,8 +31342,10 @@ virDomainDiskAddISCSIPoolSourceHost(virDomainDiskDefPtr def, def->src->hosts[0].name = g_strdup(pooldef->source.hosts[0].name); - def->src->hosts[0].port = pooldef->source.hosts[0].port ? - pooldef->source.hosts[0].port : 3260; + if (pooldef->source.hosts[0].port != 0) + def->src->hosts[0].port = pooldef->source.hosts[0].port; + else + def->src->hosts[0].port = 3260; /* iscsi volume has name like "unit:0:0:1" */ if (!(tokens = virStringSplitCount(def->src->srcpool->volume, ":", 0, &ntokens))) -- 2.24.1
On Wed, Feb 05, 2020 at 02:40:55PM +0100, Peter Krempa wrote:
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Only 'def->src' was ever used in this function. Use the source directly. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 5ef12c7ee7..bcb17dcc6e 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -31323,7 +31323,7 @@ virDomainNetResolveActualType(virDomainNetDefPtr iface) static int -virDomainDiskAddISCSIPoolSourceHost(virDomainDiskDefPtr def, +virDomainDiskAddISCSIPoolSourceHost(virStorageSourcePtr src, virStoragePoolDefPtr pooldef) { VIR_AUTOSTRINGLIST tokens = NULL; @@ -31337,38 +31337,38 @@ virDomainDiskAddISCSIPoolSourceHost(virDomainDiskDefPtr def, } /* iscsi pool only supports one host */ - def->src->nhosts = 1; - def->src->hosts = g_new0(virStorageNetHostDef, 1); + src->nhosts = 1; + src->hosts = g_new0(virStorageNetHostDef, 1); - def->src->hosts[0].name = g_strdup(pooldef->source.hosts[0].name); + src->hosts[0].name = g_strdup(pooldef->source.hosts[0].name); if (pooldef->source.hosts[0].port != 0) - def->src->hosts[0].port = pooldef->source.hosts[0].port; + src->hosts[0].port = pooldef->source.hosts[0].port; else - def->src->hosts[0].port = 3260; + src->hosts[0].port = 3260; /* iscsi volume has name like "unit:0:0:1" */ - if (!(tokens = virStringSplitCount(def->src->srcpool->volume, ":", 0, &ntokens))) + if (!(tokens = virStringSplitCount(src->srcpool->volume, ":", 0, &ntokens))) return -1; if (ntokens != 4) { virReportError(VIR_ERR_INTERNAL_ERROR, _("unexpected iscsi volume name '%s'"), - def->src->srcpool->volume); + src->srcpool->volume); return -1; } /* iscsi pool has only one source device path */ - def->src->path = g_strdup_printf("%s/%s", pooldef->source.devices[0].path, - tokens[3]); + src->path = g_strdup_printf("%s/%s", pooldef->source.devices[0].path, + tokens[3]); /* Storage pool have not supported these 2 attributes yet, * use the defaults. */ - def->src->hosts[0].transport = VIR_STORAGE_NET_HOST_TRANS_TCP; - def->src->hosts[0].socket = NULL; + src->hosts[0].transport = VIR_STORAGE_NET_HOST_TRANS_TCP; + src->hosts[0].socket = NULL; - def->src->protocol = VIR_STORAGE_NET_PROTOCOL_ISCSI; + src->protocol = VIR_STORAGE_NET_PROTOCOL_ISCSI; return 0; } @@ -31411,7 +31411,7 @@ virDomainDiskTranslateISCSIDirect(virDomainDiskDefPtr def, def->src->auth->secrettype = g_strdup(secrettype); } - if (virDomainDiskAddISCSIPoolSourceHost(def, pooldef) < 0) + if (virDomainDiskAddISCSIPoolSourceHost(def->src, pooldef) < 0) return -1; if (!def->src->initiator.iqn && pooldef->source.initiator.iqn && -- 2.24.1
On Wed, Feb 05, 2020 at 02:40:56PM +0100, Peter Krempa wrote:
Only 'def->src' was ever used in this function. Use the source directly.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Only 'def->src' was ever used in this function. Use the source directly. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index bcb17dcc6e..4545c6b314 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -31375,18 +31375,18 @@ virDomainDiskAddISCSIPoolSourceHost(virStorageSourcePtr src, static int -virDomainDiskTranslateSourcePoolAuth(virDomainDiskDefPtr def, +virDomainDiskTranslateSourcePoolAuth(virStorageSourcePtr src, virStoragePoolSourcePtr source) { /* Only necessary when authentication set */ if (!source->auth) return 0; - def->src->auth = virStorageAuthDefCopy(source->auth); - if (!def->src->auth) + src->auth = virStorageAuthDefCopy(source->auth); + if (!src->auth) return -1; /* A <disk> doesn't use <auth type='%s', so clear that out for the disk */ - def->src->auth->authType = VIR_STORAGE_AUTH_TYPE_NONE; + src->auth->authType = VIR_STORAGE_AUTH_TYPE_NONE; return 0; } @@ -31398,7 +31398,7 @@ virDomainDiskTranslateISCSIDirect(virDomainDiskDefPtr def, def->src->srcpool->actualtype = VIR_STORAGE_TYPE_NETWORK; def->src->protocol = VIR_STORAGE_NET_PROTOCOL_ISCSI; - if (virDomainDiskTranslateSourcePoolAuth(def, + if (virDomainDiskTranslateSourcePoolAuth(def->src, &pooldef->source) < 0) return -1; -- 2.24.1
On Wed, Feb 05, 2020 at 02:40:57PM +0100, Peter Krempa wrote:
Only 'def->src' was ever used in this function. Use the source directly.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Only 'def->src' was ever used in this function. Use the source directly. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 4545c6b314..531c765032 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -31392,30 +31392,30 @@ virDomainDiskTranslateSourcePoolAuth(virStorageSourcePtr src, static int -virDomainDiskTranslateISCSIDirect(virDomainDiskDefPtr def, +virDomainDiskTranslateISCSIDirect(virStorageSourcePtr src, virStoragePoolDefPtr pooldef) { - def->src->srcpool->actualtype = VIR_STORAGE_TYPE_NETWORK; - def->src->protocol = VIR_STORAGE_NET_PROTOCOL_ISCSI; + src->srcpool->actualtype = VIR_STORAGE_TYPE_NETWORK; + src->protocol = VIR_STORAGE_NET_PROTOCOL_ISCSI; - if (virDomainDiskTranslateSourcePoolAuth(def->src, + if (virDomainDiskTranslateSourcePoolAuth(src, &pooldef->source) < 0) return -1; /* Source pool may not fill in the secrettype field, * so we need to do so here */ - if (def->src->auth && !def->src->auth->secrettype) { + if (src->auth && !src->auth->secrettype) { const char *secrettype = virSecretUsageTypeToString(VIR_SECRET_USAGE_TYPE_ISCSI); - def->src->auth->secrettype = g_strdup(secrettype); + src->auth->secrettype = g_strdup(secrettype); } - if (virDomainDiskAddISCSIPoolSourceHost(def->src, pooldef) < 0) + if (virDomainDiskAddISCSIPoolSourceHost(src, pooldef) < 0) return -1; - if (!def->src->initiator.iqn && pooldef->source.initiator.iqn && - virStorageSourceInitiatorCopy(&def->src->initiator, + if (!src->initiator.iqn && pooldef->source.initiator.iqn && + virStorageSourceInitiatorCopy(&src->initiator, &pooldef->source.initiator) < 0) { return -1; } @@ -31540,7 +31540,7 @@ virDomainDiskTranslateSourcePool(virDomainDiskDefPtr def) return -1; } - if (virDomainDiskTranslateISCSIDirect(def, pooldef) < 0) + if (virDomainDiskTranslateISCSIDirect(def->src, pooldef) < 0) return -1; break; @@ -31565,7 +31565,7 @@ virDomainDiskTranslateSourcePool(virDomainDiskDefPtr def) break; case VIR_STORAGE_SOURCE_POOL_MODE_DIRECT: - if (virDomainDiskTranslateISCSIDirect(def, pooldef) < 0) + if (virDomainDiskTranslateISCSIDirect(def->src, pooldef) < 0) return -1; break; } -- 2.24.1
On Wed, Feb 05, 2020 at 02:40:58PM +0100, Peter Krempa wrote:
Only 'def->src' was ever used in this function. Use the source directly.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Extract all the code setting up one storage source from the rest which sets up the whole disk. This will allow us to prepare the whole backing chain. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 112 ++++++++++++++++++++--------------------- 1 file changed, 55 insertions(+), 57 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 531c765032..7bd86d67e5 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -31424,37 +31424,28 @@ virDomainDiskTranslateISCSIDirect(virStorageSourcePtr src, } -int -virDomainDiskTranslateSourcePool(virDomainDiskDefPtr def) +static int +virDomainStorageSourceTranslateSourcePool(virStorageSourcePtr src, + virConnectPtr conn) { virStorageVolInfo info; g_autoptr(virStoragePoolDef) pooldef = NULL; g_autofree char *poolxml = NULL; - g_autoptr(virConnect) conn = NULL; g_autoptr(virStoragePool) pool = NULL; g_autoptr(virStorageVol) vol = NULL; - if (def->src->type != VIR_STORAGE_TYPE_VOLUME) - return 0; - - if (!def->src->srcpool) - return 0; - - if (!(conn = virGetConnectStorage())) - return -1; - - if (!(pool = virStoragePoolLookupByName(conn, def->src->srcpool->pool))) + if (!(pool = virStoragePoolLookupByName(conn, src->srcpool->pool))) return -1; if (virStoragePoolIsActive(pool) != 1) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("storage pool '%s' containing volume '%s' " "is not active"), - def->src->srcpool->pool, def->src->srcpool->volume); + src->srcpool->pool, src->srcpool->volume); return -1; } - if (!(vol = virStorageVolLookupByName(pool, def->src->srcpool->volume))) + if (!(vol = virStorageVolLookupByName(pool, src->srcpool->volume))) return -1; if (virStorageVolGetInfo(vol, &info) < 0) @@ -31466,22 +31457,22 @@ virDomainDiskTranslateSourcePool(virDomainDiskDefPtr def) if (!(pooldef = virStoragePoolDefParseString(poolxml))) return -1; - def->src->srcpool->pooltype = pooldef->type; - def->src->srcpool->voltype = info.type; + src->srcpool->pooltype = pooldef->type; + src->srcpool->voltype = info.type; - if (def->src->srcpool->mode && pooldef->type != VIR_STORAGE_POOL_ISCSI) { + if (src->srcpool->mode && pooldef->type != VIR_STORAGE_POOL_ISCSI) { virReportError(VIR_ERR_XML_ERROR, "%s", _("disk source mode is only valid when " "storage pool is of iscsi type")); return -1; } - VIR_FREE(def->src->path); - virStorageNetHostDefFree(def->src->nhosts, def->src->hosts); - def->src->nhosts = 0; - def->src->hosts = NULL; - virStorageAuthDefFree(def->src->auth); - def->src->auth = NULL; + VIR_FREE(src->path); + virStorageNetHostDefFree(src->nhosts, src->hosts); + src->nhosts = 0; + src->hosts = NULL; + virStorageAuthDefFree(src->auth); + src->auth = NULL; switch ((virStoragePoolType) pooldef->type) { case VIR_STORAGE_POOL_DIR: @@ -31492,32 +31483,24 @@ virDomainDiskTranslateSourcePool(virDomainDiskDefPtr def) case VIR_STORAGE_POOL_SCSI: case VIR_STORAGE_POOL_ZFS: case VIR_STORAGE_POOL_VSTORAGE: - if (!(def->src->path = virStorageVolGetPath(vol))) + if (!(src->path = virStorageVolGetPath(vol))) return -1; - if (def->startupPolicy && info.type != VIR_STORAGE_VOL_FILE) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("'startupPolicy' is only valid for " - "'file' type volume")); - return -1; - } - - switch (info.type) { case VIR_STORAGE_VOL_FILE: - def->src->srcpool->actualtype = VIR_STORAGE_TYPE_FILE; + src->srcpool->actualtype = VIR_STORAGE_TYPE_FILE; break; case VIR_STORAGE_VOL_DIR: - def->src->srcpool->actualtype = VIR_STORAGE_TYPE_DIR; + src->srcpool->actualtype = VIR_STORAGE_TYPE_DIR; break; case VIR_STORAGE_VOL_BLOCK: - def->src->srcpool->actualtype = VIR_STORAGE_TYPE_BLOCK; + src->srcpool->actualtype = VIR_STORAGE_TYPE_BLOCK; break; case VIR_STORAGE_VOL_PLOOP: - def->src->srcpool->actualtype = VIR_STORAGE_TYPE_FILE; + src->srcpool->actualtype = VIR_STORAGE_TYPE_FILE; break; case VIR_STORAGE_VOL_NETWORK: @@ -31533,39 +31516,25 @@ virDomainDiskTranslateSourcePool(virDomainDiskDefPtr def) break; case VIR_STORAGE_POOL_ISCSI_DIRECT: - if (def->startupPolicy) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("'startupPolicy' is only valid for " - "'file' type volume")); - return -1; - } - - if (virDomainDiskTranslateISCSIDirect(def->src, pooldef) < 0) + if (virDomainDiskTranslateISCSIDirect(src, pooldef) < 0) return -1; break; case VIR_STORAGE_POOL_ISCSI: - if (def->startupPolicy) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("'startupPolicy' is only valid for " - "'file' type volume")); - return -1; - } - - switch (def->src->srcpool->mode) { + switch (src->srcpool->mode) { case VIR_STORAGE_SOURCE_POOL_MODE_DEFAULT: case VIR_STORAGE_SOURCE_POOL_MODE_LAST: - def->src->srcpool->mode = VIR_STORAGE_SOURCE_POOL_MODE_HOST; + src->srcpool->mode = VIR_STORAGE_SOURCE_POOL_MODE_HOST; G_GNUC_FALLTHROUGH; case VIR_STORAGE_SOURCE_POOL_MODE_HOST: - def->src->srcpool->actualtype = VIR_STORAGE_TYPE_BLOCK; - if (!(def->src->path = virStorageVolGetPath(vol))) + src->srcpool->actualtype = VIR_STORAGE_TYPE_BLOCK; + if (!(src->path = virStorageVolGetPath(vol))) return -1; break; case VIR_STORAGE_SOURCE_POOL_MODE_DIRECT: - if (virDomainDiskTranslateISCSIDirect(def->src, pooldef) < 0) + if (virDomainDiskTranslateISCSIDirect(src, pooldef) < 0) return -1; break; } @@ -31587,6 +31556,35 @@ virDomainDiskTranslateSourcePool(virDomainDiskDefPtr def) } +int +virDomainDiskTranslateSourcePool(virDomainDiskDefPtr def) +{ + g_autoptr(virConnect) conn = NULL; + + if (def->src->type != VIR_STORAGE_TYPE_VOLUME) + return 0; + + if (!def->src->srcpool) + return 0; + + if (!(conn = virGetConnectStorage())) + return -1; + + if (virDomainStorageSourceTranslateSourcePool(def->src, conn) < 0) + return -1; + + if (def->startupPolicy != 0 && + virStorageSourceGetActualType(def->src) != VIR_STORAGE_VOL_FILE) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("'startupPolicy' is only valid for " + "'file' type volume")); + return -1; + } + + return 0; +} + + /** * virDomainDiskGetDetectZeroesMode: * @discard: disk/image sector discard setting -- 2.24.1
On Wed, Feb 05, 2020 at 02:40:59PM +0100, Peter Krempa wrote:
Extract all the code setting up one storage source from the rest which sets up the whole disk. This will allow us to prepare the whole backing chain.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 112 ++++++++++++++++++++--------------------- 1 file changed, 55 insertions(+), 57 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Now that we accept full backing chains on input nothing should prevent users from also using disk type 'VOLUME' for specifying the backing images. Do the translation for the whole backing chain. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 7bd86d67e5..46062e3969 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -31560,18 +31560,20 @@ int virDomainDiskTranslateSourcePool(virDomainDiskDefPtr def) { g_autoptr(virConnect) conn = NULL; + virStorageSourcePtr n; - if (def->src->type != VIR_STORAGE_TYPE_VOLUME) - return 0; - - if (!def->src->srcpool) - return 0; + for (n = def->src; virStorageSourceIsBacking(n); n = n->backingStore) { + if (n->type != VIR_STORAGE_TYPE_VOLUME || !n-> srcpool) + continue; - if (!(conn = virGetConnectStorage())) - return -1; + if (!conn) { + if (!(conn = virGetConnectStorage())) + return -1; + } - if (virDomainStorageSourceTranslateSourcePool(def->src, conn) < 0) - return -1; + if (virDomainStorageSourceTranslateSourcePool(n, conn) < 0) + return -1; + } if (def->startupPolicy != 0 && virStorageSourceGetActualType(def->src) != VIR_STORAGE_VOL_FILE) { -- 2.24.1
On Wed, Feb 05, 2020 at 02:41:00PM +0100, Peter Krempa wrote:
Now that we accept full backing chains on input nothing should prevent users from also using disk type 'VOLUME' for specifying the backing images.
s/VOLUME/volume/g No need to yell in the commit summary if the XML value is lowercase.
Do the translation for the whole backing chain.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 7bd86d67e5..46062e3969 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -31560,18 +31560,20 @@ int virDomainDiskTranslateSourcePool(virDomainDiskDefPtr def) { g_autoptr(virConnect) conn = NULL; + virStorageSourcePtr n;
- if (def->src->type != VIR_STORAGE_TYPE_VOLUME) - return 0; - - if (!def->src->srcpool) - return 0; + for (n = def->src; virStorageSourceIsBacking(n); n = n->backingStore) { + if (n->type != VIR_STORAGE_TYPE_VOLUME || !n-> srcpool)
Extra space ------------------------------------------```
+ continue;
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (2)
-
Ján Tomko -
Peter Krempa