[libvirt] [PATCH 0/6] conf: qemu: Introduce relaxed config validation
Recently I NACKed quite a few patches attempting to add checks to the post parse callback infrastructure that would inhbit configs from being loaded after daemon restart. To solve this introduce a new infrastructure that will get called only on codepaths that define and start the config leaving the code paths that load configs untouched. Peter Krempa (6): conf: disk: Rename virDomainDiskDefValidate to virDomainDiskDefParseValidate qemu: driver: Fix function header alignment of some functions conf: Introduce infrastructure to add config validation to define time qemu: Move check that validates 'min_guarantee' to qemuDomainDefValidate conf: Move check that validates disk info to virDomainDefValidate conf: Add validation infrastructure for device hot/cold plug src/conf/domain_conf.c | 128 +++++++++++++++++++++++++++++++++++++++-------- src/conf/domain_conf.h | 11 ++-- src/libvirt_private.syms | 3 +- src/qemu/qemu_conf.h | 2 + src/qemu/qemu_domain.c | 38 ++++++++++++++ src/qemu/qemu_domain.h | 3 ++ src/qemu/qemu_driver.c | 26 ++++++++-- src/qemu/qemu_process.c | 19 ++++--- 8 files changed, 190 insertions(+), 40 deletions(-) -- 2.8.2
Name the validation function distinctively since it's called in the parser. Later patches will add function that will validate disk definitions that are invalid but need to be parsed to avoid losing domains. --- src/conf/domain_conf.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b445469..12f9da2 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -6932,7 +6932,7 @@ virDomainDiskDefGeometryParse(virDomainDiskDefPtr def, static int -virDomainDiskDefValidate(const virDomainDiskDef *def) +virDomainDiskDefParseValidate(const virDomainDiskDef *def) { if (def->bus != VIR_DOMAIN_DISK_BUS_VIRTIO) { if (def->event_idx != VIR_TRISTATE_SWITCH_ABSENT) { @@ -7537,7 +7537,7 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, goto error; } - if (virDomainDiskDefValidate(def) < 0) + if (virDomainDiskDefParseValidate(def) < 0) goto error; cleanup: -- 2.8.2
On 05/17/2016 10:25 AM, Peter Krempa wrote:
Name the validation function distinctively since it's called in the parser. Later patches will add function that will validate disk definitions that are invalid but need to be parsed to avoid losing domains. --- src/conf/domain_conf.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b445469..12f9da2 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -6932,7 +6932,7 @@ virDomainDiskDefGeometryParse(virDomainDiskDefPtr def,
static int -virDomainDiskDefValidate(const virDomainDiskDef *def) +virDomainDiskDefParseValidate(const virDomainDiskDef *def) { if (def->bus != VIR_DOMAIN_DISK_BUS_VIRTIO) { if (def->event_idx != VIR_TRISTATE_SWITCH_ABSENT) { @@ -7537,7 +7537,7 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, goto error; }
- if (virDomainDiskDefValidate(def) < 0) + if (virDomainDiskDefParseValidate(def) < 0) goto error;
cleanup:
ACK, and could be pushed now - Cole
--- src/qemu/qemu_driver.c | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 37d970e..7f8dfc8 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7241,7 +7241,10 @@ qemuDomainCreate(virDomainPtr dom) return qemuDomainCreateWithFlags(dom, 0); } -static virDomainPtr qemuDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flags) +static virDomainPtr +qemuDomainDefineXMLFlags(virConnectPtr conn, + const char *xml, + unsigned int flags) { virQEMUDriverPtr driver = conn->privateData; virDomainDefPtr def = NULL; @@ -8192,8 +8195,10 @@ qemuDomainUpdateDeviceConfig(virQEMUCapsPtr qemuCaps, } -static int qemuDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, - unsigned int flags) +static int +qemuDomainAttachDeviceFlags(virDomainPtr dom, + const char *xml, + unsigned int flags) { virQEMUDriverPtr driver = dom->conn->privateData; virDomainObjPtr vm = NULL; @@ -8438,8 +8443,10 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom, } -static int qemuDomainDetachDeviceFlags(virDomainPtr dom, const char *xml, - unsigned int flags) +static int +qemuDomainDetachDeviceFlags(virDomainPtr dom, + const char *xml, + unsigned int flags) { virQEMUDriverPtr driver = dom->conn->privateData; virDomainObjPtr vm = NULL; -- 2.8.2
On 05/17/2016 10:25 AM, Peter Krempa wrote:
--- src/qemu/qemu_driver.c | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-)
ACK, and could be pushed now - Cole
Recently there were a few patches adding checks to the post parse callback infrastructure that rejected previously accepted configuration. This unfortunately was not acceptable since we would fail to load the configs from the disk and the VMs would vanish. This patch adds helpers which are called in the appropriate places after the config is parsed when defining/starting a VM but not when reloading the configs. This infrastructure is meant for invalid configurations that were accepted previously and thus it's not executed when starting a VM from any saved state. Any checks that are necessary to be executed at that point still need to be added to qemuProcessValidateXML. --- src/conf/domain_conf.c | 67 ++++++++++++++++++++++++++++++++++++++---------- src/conf/domain_conf.h | 2 ++ src/libvirt_private.syms | 1 + src/qemu/qemu_conf.h | 2 ++ src/qemu/qemu_domain.c | 21 +++++++++++++++ src/qemu/qemu_driver.c | 6 +++++ src/qemu/qemu_process.c | 22 +++++++++++----- 7 files changed, 100 insertions(+), 21 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 12f9da2..7359dc7 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4145,20 +4145,6 @@ virDomainDeviceDefPostParseInternal(virDomainDeviceDefPtr dev, } } - /* Validate LUN configuration */ - if (disk->device == VIR_DOMAIN_DISK_DEVICE_LUN) { - /* volumes haven't been translated at this point, so accept them */ - if (!(disk->src->type == VIR_STORAGE_TYPE_BLOCK || - disk->src->type == VIR_STORAGE_TYPE_VOLUME || - (disk->src->type == VIR_STORAGE_TYPE_NETWORK && - disk->src->protocol == VIR_STORAGE_NET_PROTOCOL_ISCSI))) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("disk '%s' improperly configured for a " - "device='lun'"), disk->dst); - return -1; - } - } - if (disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE && virDomainDiskDefAssignAddress(xmlopt, disk, def) < 0) return -1; @@ -24357,3 +24343,56 @@ virDomainObjGetShortName(virDomainObjPtr vm) return ret; } + + +static int +virDomainDiskDefValidate(const virDomainDiskDef *disk) +{ + /* Validate LUN configuration */ + if (disk->device == VIR_DOMAIN_DISK_DEVICE_LUN) { + /* volumes haven't been translated at this point, so accept them */ + if (!(disk->src->type == VIR_STORAGE_TYPE_BLOCK || + disk->src->type == VIR_STORAGE_TYPE_VOLUME || + (disk->src->type == VIR_STORAGE_TYPE_NETWORK && + disk->src->protocol == VIR_STORAGE_NET_PROTOCOL_ISCSI))) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("disk '%s' improperly configured for a " + "device='lun'"), disk->dst); + return -1; + } + } + + return 0; +} + + +#define VIR_DOMAIN_DEF_VALIDATE_DEVICES(dev, func) \ + for (i = 0; i < def->n ## dev ## s; i++) { \ + if (func(def->dev ## s[i]) < 0) \ + return -1; \ + } + +/** + * virDomainDefValidate: + * @def: domain definition + * + * This validation function is designed to take checks of globally invalid + * configurations that the parser needs to accept so that VMs don't vanish upon + * daemon restart. Such definition can be rejected upon startup or define, where + * this function shall be called. + * + * Returns 0 if domain definition is valid, -1 on error and reports an + * appropriate message. + */ +int +virDomainDefValidate(const virDomainDef *def) +{ + size_t i; + + /* check configuration of individual devices */ + VIR_DOMAIN_DEF_VALIDATE_DEVICES(disk, virDomainDiskDefValidate); + + return 0; +} + +#undef VIR_DOMAIN_DEF_VALIDATE_DEVICES diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index b9e696d..f2704c0 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3178,4 +3178,6 @@ bool virDomainDefHasMemballoon(const virDomainDef *def) ATTRIBUTE_NONNULL(1); char *virDomainObjGetShortName(virDomainObjPtr vm); +int virDomainDefValidate(const virDomainDef *def); + #endif /* __DOMAIN_CONF_H */ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 8408081..cc5d6c9 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -243,6 +243,7 @@ virDomainDefSetMemoryInitial; virDomainDefSetMemoryTotal; virDomainDefSetVcpus; virDomainDefSetVcpusMax; +virDomainDefValidate; virDomainDeleteConfig; virDomainDeviceAddressIsValid; virDomainDeviceAddressTypeToString; diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index a714b84..cbc5008 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -333,4 +333,6 @@ int qemuTranslateSnapshotDiskSourcePool(virConnectPtr conn, char * qemuGetHugepagePath(virHugeTLBFSPtr hugepage); char * qemuGetDefaultHugepath(virHugeTLBFSPtr hugetlbfs, size_t nhugetlbfs); + +int qemuDomainDefValidate(const virDomainDef *def); #endif /* __QEMUD_CONF_H */ diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 0733872..5ccb483 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -5343,3 +5343,24 @@ qemuDomainDefValidateDiskLunSource(const virStorageSource *src) return 0; } + + +/** + * qemuDomainDefValidate: + * @def: domain definition + * + * Validates that the domain definition is valid for the qemu driver. This check + * is run when the domain is defined or started as new. This check is explicitly + * not executed when starting a VM from snapshot/saved state/migration to allow + * partially invalid configs to still run. + * + * Returns 0 on success, -1 on error. + */ +int +qemuDomainDefValidate(const virDomainDef *def) +{ + if (virDomainDefValidate(def) < 0) + return -1; + + return 0; +} diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 7f8dfc8..e6d22ec 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1799,6 +1799,9 @@ static virDomainPtr qemuDomainCreateXML(virConnectPtr conn, if (virDomainCreateXMLEnsureACL(conn, def) < 0) goto cleanup; + if (qemuDomainDefValidate(def) < 0) + goto cleanup; + if (!(qemuCaps = virQEMUCapsCacheLookup(driver->qemuCapsCache, def->emulator))) goto cleanup; @@ -7275,6 +7278,9 @@ qemuDomainDefineXMLFlags(virConnectPtr conn, if (virDomainDefineXMLFlagsEnsureACL(conn, def) < 0) goto cleanup; + if (qemuDomainDefValidate(def) < 0) + goto cleanup; + if (!(qemuCaps = virQEMUCapsCacheLookup(driver->qemuCapsCache, def->emulator))) goto cleanup; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index eddf3a7..d079b18 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4620,13 +4620,6 @@ qemuProcessStartValidateXML(virDomainObjPtr vm, * If back compat isn't a concern, XML validation should probably * be done at parse time. */ - if (qemuValidateCpuCount(vm->def, qemuCaps) < 0) - return -1; - - if (!migration && !snapshot && - virDomainDefCheckDuplicateDiskInfo(vm->def) < 0) - return -1; - if (vm->def->mem.min_guarantee) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Parameter 'min_guarantee' " @@ -4634,6 +4627,21 @@ qemuProcessStartValidateXML(virDomainObjPtr vm, return -1; } + /* checks below should not be executed when starting a qemu process for a + * VM that was running before (migration, snapshots, save). It's more + * important to start such VM than keep the configuration clean */ + if (!migration && !snapshot) { + if (qemuDomainDefValidate(vm->def) < 0) + return -1; + + if (virDomainDefCheckDuplicateDiskInfo(vm->def) < 0) + return -1; + } + + /* checks below validate config that would make qemu fail to start */ + if (qemuValidateCpuCount(vm->def, qemuCaps) < 0) + return -1; + return 0; } -- 2.8.2
On 05/17/2016 10:25 AM, Peter Krempa wrote:
Recently there were a few patches adding checks to the post parse callback infrastructure that rejected previously accepted configuration. This unfortunately was not acceptable since we would fail to load the configs from the disk and the VMs would vanish.
This patch adds helpers which are called in the appropriate places after the config is parsed when defining/starting a VM but not when reloading the configs.
This infrastructure is meant for invalid configurations that were accepted previously and thus it's not executed when starting a VM from any saved state. Any checks that are necessary to be executed at that point still need to be added to qemuProcessValidateXML. --- src/conf/domain_conf.c | 67 ++++++++++++++++++++++++++++++++++++++---------- src/conf/domain_conf.h | 2 ++ src/libvirt_private.syms | 1 + src/qemu/qemu_conf.h | 2 ++ src/qemu/qemu_domain.c | 21 +++++++++++++++ src/qemu/qemu_driver.c | 6 +++++ src/qemu/qemu_process.c | 22 +++++++++++----- 7 files changed, 100 insertions(+), 21 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 12f9da2..7359dc7 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4145,20 +4145,6 @@ virDomainDeviceDefPostParseInternal(virDomainDeviceDefPtr dev, } }
- /* Validate LUN configuration */ - if (disk->device == VIR_DOMAIN_DISK_DEVICE_LUN) { - /* volumes haven't been translated at this point, so accept them */ - if (!(disk->src->type == VIR_STORAGE_TYPE_BLOCK || - disk->src->type == VIR_STORAGE_TYPE_VOLUME || - (disk->src->type == VIR_STORAGE_TYPE_NETWORK && - disk->src->protocol == VIR_STORAGE_NET_PROTOCOL_ISCSI))) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("disk '%s' improperly configured for a " - "device='lun'"), disk->dst); - return -1; - } - } -
Callers via virDomainDeviceDefParse now lose this check. John
if (disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE && virDomainDiskDefAssignAddress(xmlopt, disk, def) < 0) return -1; @@ -24357,3 +24343,56 @@ virDomainObjGetShortName(virDomainObjPtr vm)
return ret; } + + +static int +virDomainDiskDefValidate(const virDomainDiskDef *disk) +{ + /* Validate LUN configuration */ + if (disk->device == VIR_DOMAIN_DISK_DEVICE_LUN) { + /* volumes haven't been translated at this point, so accept them */ + if (!(disk->src->type == VIR_STORAGE_TYPE_BLOCK || + disk->src->type == VIR_STORAGE_TYPE_VOLUME || + (disk->src->type == VIR_STORAGE_TYPE_NETWORK && + disk->src->protocol == VIR_STORAGE_NET_PROTOCOL_ISCSI))) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("disk '%s' improperly configured for a " + "device='lun'"), disk->dst); + return -1; + } + } + + return 0; +} + + +#define VIR_DOMAIN_DEF_VALIDATE_DEVICES(dev, func) \ + for (i = 0; i < def->n ## dev ## s; i++) { \ + if (func(def->dev ## s[i]) < 0) \ + return -1; \ + } + +/** + * virDomainDefValidate: + * @def: domain definition + * + * This validation function is designed to take checks of globally invalid + * configurations that the parser needs to accept so that VMs don't vanish upon + * daemon restart. Such definition can be rejected upon startup or define, where + * this function shall be called. + * + * Returns 0 if domain definition is valid, -1 on error and reports an + * appropriate message. + */ +int +virDomainDefValidate(const virDomainDef *def) +{ + size_t i; + + /* check configuration of individual devices */ + VIR_DOMAIN_DEF_VALIDATE_DEVICES(disk, virDomainDiskDefValidate); + + return 0; +} + +#undef VIR_DOMAIN_DEF_VALIDATE_DEVICES diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index b9e696d..f2704c0 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3178,4 +3178,6 @@ bool virDomainDefHasMemballoon(const virDomainDef *def) ATTRIBUTE_NONNULL(1);
char *virDomainObjGetShortName(virDomainObjPtr vm);
+int virDomainDefValidate(const virDomainDef *def); + #endif /* __DOMAIN_CONF_H */ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 8408081..cc5d6c9 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -243,6 +243,7 @@ virDomainDefSetMemoryInitial; virDomainDefSetMemoryTotal; virDomainDefSetVcpus; virDomainDefSetVcpusMax; +virDomainDefValidate; virDomainDeleteConfig; virDomainDeviceAddressIsValid; virDomainDeviceAddressTypeToString; diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index a714b84..cbc5008 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -333,4 +333,6 @@ int qemuTranslateSnapshotDiskSourcePool(virConnectPtr conn, char * qemuGetHugepagePath(virHugeTLBFSPtr hugepage); char * qemuGetDefaultHugepath(virHugeTLBFSPtr hugetlbfs, size_t nhugetlbfs); + +int qemuDomainDefValidate(const virDomainDef *def); #endif /* __QEMUD_CONF_H */ diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 0733872..5ccb483 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -5343,3 +5343,24 @@ qemuDomainDefValidateDiskLunSource(const virStorageSource *src)
return 0; } + + +/** + * qemuDomainDefValidate: + * @def: domain definition + * + * Validates that the domain definition is valid for the qemu driver. This check + * is run when the domain is defined or started as new. This check is explicitly + * not executed when starting a VM from snapshot/saved state/migration to allow + * partially invalid configs to still run. + * + * Returns 0 on success, -1 on error. + */ +int +qemuDomainDefValidate(const virDomainDef *def) +{ + if (virDomainDefValidate(def) < 0) + return -1; + + return 0; +} diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 7f8dfc8..e6d22ec 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1799,6 +1799,9 @@ static virDomainPtr qemuDomainCreateXML(virConnectPtr conn, if (virDomainCreateXMLEnsureACL(conn, def) < 0) goto cleanup;
+ if (qemuDomainDefValidate(def) < 0) + goto cleanup; + if (!(qemuCaps = virQEMUCapsCacheLookup(driver->qemuCapsCache, def->emulator))) goto cleanup;
@@ -7275,6 +7278,9 @@ qemuDomainDefineXMLFlags(virConnectPtr conn, if (virDomainDefineXMLFlagsEnsureACL(conn, def) < 0) goto cleanup;
+ if (qemuDomainDefValidate(def) < 0) + goto cleanup; + if (!(qemuCaps = virQEMUCapsCacheLookup(driver->qemuCapsCache, def->emulator))) goto cleanup;
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index eddf3a7..d079b18 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4620,13 +4620,6 @@ qemuProcessStartValidateXML(virDomainObjPtr vm, * If back compat isn't a concern, XML validation should probably * be done at parse time. */ - if (qemuValidateCpuCount(vm->def, qemuCaps) < 0) - return -1; - - if (!migration && !snapshot && - virDomainDefCheckDuplicateDiskInfo(vm->def) < 0) - return -1; - if (vm->def->mem.min_guarantee) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Parameter 'min_guarantee' " @@ -4634,6 +4627,21 @@ qemuProcessStartValidateXML(virDomainObjPtr vm, return -1; }
+ /* checks below should not be executed when starting a qemu process for a + * VM that was running before (migration, snapshots, save). It's more + * important to start such VM than keep the configuration clean */ + if (!migration && !snapshot) { + if (qemuDomainDefValidate(vm->def) < 0) + return -1; + + if (virDomainDefCheckDuplicateDiskInfo(vm->def) < 0) + return -1; + } + + /* checks below validate config that would make qemu fail to start */ + if (qemuValidateCpuCount(vm->def, qemuCaps) < 0) + return -1; + return 0; }
On 05/17/2016 08:25 AM, Peter Krempa wrote:
Recently there were a few patches adding checks to the post parse callback infrastructure that rejected previously accepted configuration. This unfortunately was not acceptable since we would fail to load the configs from the disk and the VMs would vanish.
Would these patches only affect the qemu driver in this way, or other drivers as well? Regards, Jim
On Tue, May 17, 2016 at 23:04:19 -0600, Jim Fehlig wrote:
On 05/17/2016 08:25 AM, Peter Krempa wrote:
Recently there were a few patches adding checks to the post parse callback infrastructure that rejected previously accepted configuration. This unfortunately was not acceptable since we would fail to load the configs from the disk and the VMs would vanish.
Would these patches only affect the qemu driver in this way, or other drivers as well?
Currently the code is done only for the qemu driver. Other drivers need to add calls to virDomainDefValidate or as in qemu driver's case to 'drv'DomainDefValidate and call them from apropriate places where the XML definition is parsed. I currently opted to add this just to the qemu driver, but it certainly can be applied to any other driver. Peter
On Tue, May 17, 2016 at 04:25:39PM +0200, Peter Krempa wrote:
Recently there were a few patches adding checks to the post parse callback infrastructure that rejected previously accepted configuration. This unfortunately was not acceptable since we would fail to load the configs from the disk and the VMs would vanish.
This patch adds helpers which are called in the appropriate places after the config is parsed when defining/starting a VM but not when reloading the configs.
This infrastructure is meant for invalid configurations that were accepted previously and thus it's not executed when starting a VM from any saved state. Any checks that are necessary to be executed at that point still need to be added to qemuProcessValidateXML. --- src/conf/domain_conf.c | 67 ++++++++++++++++++++++++++++++++++++++---------- src/conf/domain_conf.h | 2 ++ src/libvirt_private.syms | 1 + src/qemu/qemu_conf.h | 2 ++ src/qemu/qemu_domain.c | 21 +++++++++++++++ src/qemu/qemu_driver.c | 6 +++++ src/qemu/qemu_process.c | 22 +++++++++++----- 7 files changed, 100 insertions(+), 21 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
[...]
+/** + * virDomainDefValidate: + * @def: domain definition + * + * This validation function is designed to take checks of globally invalid + * configurations that the parser needs to accept so that VMs don't vanish upon + * daemon restart. Such definition can be rejected upon startup or define, where + * this function shall be called. + * + * Returns 0 if domain definition is valid, -1 on error and reports an + * appropriate message. + */ +int +virDomainDefValidate(const virDomainDef *def) +{ + size_t i; + + /* check configuration of individual devices */ + VIR_DOMAIN_DEF_VALIDATE_DEVICES(disk, virDomainDiskDefValidate); + + return 0; +} + +#undef VIR_DOMAIN_DEF_VALIDATE_DEVICES
[...]
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c @@ -5343,3 +5343,24 @@ qemuDomainDefValidateDiskLunSource(const virStorageSource *src)
return 0; } + + +/** + * qemuDomainDefValidate: + * @def: domain definition + * + * Validates that the domain definition is valid for the qemu driver. This check + * is run when the domain is defined or started as new. This check is explicitly + * not executed when starting a VM from snapshot/saved state/migration to allow + * partially invalid configs to still run. + * + * Returns 0 on success, -1 on error. + */ +int +qemuDomainDefValidate(const virDomainDef *def) +{ + if (virDomainDefValidate(def) < 0) + return -1; + + return 0; +} diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 7f8dfc8..e6d22ec 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1799,6 +1799,9 @@ static virDomainPtr qemuDomainCreateXML(virConnectPtr conn, if (virDomainCreateXMLEnsureACL(conn, def) < 0) goto cleanup;
+ if (qemuDomainDefValidate(def) < 0) + goto cleanup; + if (!(qemuCaps = virQEMUCapsCacheLookup(driver->qemuCapsCache, def->emulator))) goto cleanup;
@@ -7275,6 +7278,9 @@ qemuDomainDefineXMLFlags(virConnectPtr conn, if (virDomainDefineXMLFlagsEnsureACL(conn, def) < 0) goto cleanup;
+ if (qemuDomainDefValidate(def) < 0) + goto cleanup; + if (!(qemuCaps = virQEMUCapsCacheLookup(driver->qemuCapsCache, def->emulator))) goto cleanup;
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
[...]
@@ -4634,6 +4627,21 @@ qemuProcessStartValidateXML(virDomainObjPtr vm, return -1; }
+ /* checks below should not be executed when starting a qemu process for a + * VM that was running before (migration, snapshots, save). It's more + * important to start such VM than keep the configuration clean */ + if (!migration && !snapshot) { + if (qemuDomainDefValidate(vm->def) < 0) + return -1; + + if (virDomainDefCheckDuplicateDiskInfo(vm->def) < 0) + return -1; + } + + /* checks below validate config that would make qemu fail to start */ + if (qemuValidateCpuCount(vm->def, qemuCaps) < 0) + return -1; + return 0; }
I would suggest to do the opposite. That would mean adding the virDomainDefValidate() after postParse callbacks and there you would call xmlopt->config.domainDefValidate(). Calling virDomainDefValidate() after postParse would be conditional based on a new VIR_DOMAIN_DEF_PARSE_... flag which would have to be set at the appropriate places so we would not run this new validation while starting a libvirtd and the migration/snapshot/save case. The motivation to do it this way is: 1. reuse existing code, 2. all places, that has to be skipped would be covered and we wouldn't miss calling the qemuDomainDefValidate() for tests like this patch do, 3. if you add something new, where the validation needs to be skipped, you would notice it while testing the new code and that would force you to think about it. Pavel
On 05/18/2016 08:43 AM, Pavel Hrdina wrote:
On Tue, May 17, 2016 at 04:25:39PM +0200, Peter Krempa wrote:
Recently there were a few patches adding checks to the post parse callback infrastructure that rejected previously accepted configuration. This unfortunately was not acceptable since we would fail to load the configs from the disk and the VMs would vanish.
This patch adds helpers which are called in the appropriate places after the config is parsed when defining/starting a VM but not when reloading the configs.
This infrastructure is meant for invalid configurations that were accepted previously and thus it's not executed when starting a VM from any saved state. Any checks that are necessary to be executed at that point still need to be added to qemuProcessValidateXML. --- src/conf/domain_conf.c | 67 ++++++++++++++++++++++++++++++++++++++---------- src/conf/domain_conf.h | 2 ++ src/libvirt_private.syms | 1 + src/qemu/qemu_conf.h | 2 ++ src/qemu/qemu_domain.c | 21 +++++++++++++++ src/qemu/qemu_driver.c | 6 +++++ src/qemu/qemu_process.c | 22 +++++++++++----- 7 files changed, 100 insertions(+), 21 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
[...]
+/** + * virDomainDefValidate: + * @def: domain definition + * + * This validation function is designed to take checks of globally invalid + * configurations that the parser needs to accept so that VMs don't vanish upon + * daemon restart. Such definition can be rejected upon startup or define, where + * this function shall be called. + * + * Returns 0 if domain definition is valid, -1 on error and reports an + * appropriate message. + */ +int +virDomainDefValidate(const virDomainDef *def) +{ + size_t i; + + /* check configuration of individual devices */ + VIR_DOMAIN_DEF_VALIDATE_DEVICES(disk, virDomainDiskDefValidate); + + return 0; +} + +#undef VIR_DOMAIN_DEF_VALIDATE_DEVICES
[...]
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c @@ -5343,3 +5343,24 @@ qemuDomainDefValidateDiskLunSource(const virStorageSource *src)
return 0; } + + +/** + * qemuDomainDefValidate: + * @def: domain definition + * + * Validates that the domain definition is valid for the qemu driver. This check + * is run when the domain is defined or started as new. This check is explicitly + * not executed when starting a VM from snapshot/saved state/migration to allow + * partially invalid configs to still run. + * + * Returns 0 on success, -1 on error. + */ +int +qemuDomainDefValidate(const virDomainDef *def) +{ + if (virDomainDefValidate(def) < 0) + return -1; + + return 0; +} diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 7f8dfc8..e6d22ec 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1799,6 +1799,9 @@ static virDomainPtr qemuDomainCreateXML(virConnectPtr conn, if (virDomainCreateXMLEnsureACL(conn, def) < 0) goto cleanup;
+ if (qemuDomainDefValidate(def) < 0) + goto cleanup; + if (!(qemuCaps = virQEMUCapsCacheLookup(driver->qemuCapsCache, def->emulator))) goto cleanup;
@@ -7275,6 +7278,9 @@ qemuDomainDefineXMLFlags(virConnectPtr conn, if (virDomainDefineXMLFlagsEnsureACL(conn, def) < 0) goto cleanup;
+ if (qemuDomainDefValidate(def) < 0) + goto cleanup; + if (!(qemuCaps = virQEMUCapsCacheLookup(driver->qemuCapsCache, def->emulator))) goto cleanup;
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
[...]
@@ -4634,6 +4627,21 @@ qemuProcessStartValidateXML(virDomainObjPtr vm, return -1; }
+ /* checks below should not be executed when starting a qemu process for a + * VM that was running before (migration, snapshots, save). It's more + * important to start such VM than keep the configuration clean */ + if (!migration && !snapshot) { + if (qemuDomainDefValidate(vm->def) < 0) + return -1; + + if (virDomainDefCheckDuplicateDiskInfo(vm->def) < 0) + return -1; + } + + /* checks below validate config that would make qemu fail to start */ + if (qemuValidateCpuCount(vm->def, qemuCaps) < 0) + return -1; + return 0; }
I would suggest to do the opposite. That would mean adding the virDomainDefValidate() after postParse callbacks and there you would call xmlopt->config.domainDefValidate(). Calling virDomainDefValidate() after postParse would be conditional based on a new VIR_DOMAIN_DEF_PARSE_... flag which would have to be set at the appropriate places so we would not run this new validation while starting a libvirtd and the migration/snapshot/save case.
The motivation to do it this way is:
1. reuse existing code, 2. all places, that has to be skipped would be covered and we wouldn't miss calling the qemuDomainDefValidate() for tests like this patch do, 3. if you add something new, where the validation needs to be skipped, you would notice it while testing the new code and that would force you to think about it.
Yes, this was my thinking too. We already have a targeted flag that does this as a one off: VIR_DOMAIN_DEF_PARSE_SKIP_OSTYPE_CHECKS. Converting that stuff to a PostParse validation function and renaming the flag could be a good initial imple, since it already has test suite representation IIRC Another topic related to this: we already have another chunk of qemu specific validation in qemuBuildCommandLineValidate, and lots of bits sprinkled within the command line builder. Do we eventually want to wire that up to PostParse too? So users will get immediate feedback if they try to define XML with some feature that their qemu binary doesn't support. Regardless, after the initial plumbing patches are pushed, we should decide what the convention will be and explicitly try to enforce it: all new generic domain conf validation checks go in function XX, qemu specific define time checks go in YY, optionally qemu startup time checks go in ZZ. Rather than the mishmash of checks sprinkled across the XML parser, cli builder, address assignment, plus specific validation functions. Thanks, Cole
Now that we have a good place for this check let's move it there. --- src/qemu/qemu_domain.c | 6 ++++++ src/qemu/qemu_process.c | 6 ------ 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 5ccb483..f172d6f 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -5362,5 +5362,11 @@ qemuDomainDefValidate(const virDomainDef *def) if (virDomainDefValidate(def) < 0) return -1; + if (def->mem.min_guarantee) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Parameter 'min_guarantee' not supported by QEMU.")); + return -1; + } + return 0; } diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index d079b18..ae435e4 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4620,12 +4620,6 @@ qemuProcessStartValidateXML(virDomainObjPtr vm, * If back compat isn't a concern, XML validation should probably * be done at parse time. */ - if (vm->def->mem.min_guarantee) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Parameter 'min_guarantee' " - "not supported by QEMU.")); - return -1; - } /* checks below should not be executed when starting a qemu process for a * VM that was running before (migration, snapshots, save). It's more -- 2.8.2
Now that we have a good place for this check let's move it there. --- src/conf/domain_conf.c | 11 +++++++---- src/conf/domain_conf.h | 6 ++---- src/libvirt_private.syms | 1 - src/qemu/qemu_process.c | 3 --- 4 files changed, 9 insertions(+), 12 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 7359dc7..2beb825 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -24263,8 +24263,8 @@ virDomainDefNeedsPlacementAdvice(virDomainDefPtr def) int -virDomainDiskDefCheckDuplicateInfo(virDomainDiskDefPtr a, - virDomainDiskDefPtr b) +virDomainDiskDefCheckDuplicateInfo(const virDomainDiskDef *a, + const virDomainDiskDef *b) { if (STREQ(a->dst, b->dst)) { virReportError(VIR_ERR_XML_ERROR, @@ -24293,8 +24293,8 @@ virDomainDiskDefCheckDuplicateInfo(virDomainDiskDefPtr a, } -int -virDomainDefCheckDuplicateDiskInfo(virDomainDefPtr def) +static int +virDomainDefCheckDuplicateDiskInfo(const virDomainDef *def) { size_t i; size_t j; @@ -24392,6 +24392,9 @@ virDomainDefValidate(const virDomainDef *def) /* check configuration of individual devices */ VIR_DOMAIN_DEF_VALIDATE_DEVICES(disk, virDomainDiskDefValidate); + if (virDomainDefCheckDuplicateDiskInfo(def) < 0) + return -1; + return 0; } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index f2704c0..3c219ea 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3160,11 +3160,9 @@ virDomainParseMemory(const char *xpath, bool virDomainDefNeedsPlacementAdvice(virDomainDefPtr def) ATTRIBUTE_NONNULL(1); -int virDomainDiskDefCheckDuplicateInfo(virDomainDiskDefPtr a, - virDomainDiskDefPtr b) +int virDomainDiskDefCheckDuplicateInfo(const virDomainDiskDef *a, + const virDomainDiskDef *b) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); -int virDomainDefCheckDuplicateDiskInfo(virDomainDefPtr def) - ATTRIBUTE_NONNULL(1); int virDomainDefGetVcpuPinInfoHelper(virDomainDefPtr def, int maplen, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index cc5d6c9..99975ed 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -206,7 +206,6 @@ virDomainDefAddController; virDomainDefAddImplicitDevices; virDomainDefAddUSBController; virDomainDefCheckABIStability; -virDomainDefCheckDuplicateDiskInfo; virDomainDefClearCCWAddresses; virDomainDefClearDeviceAliases; virDomainDefClearPCIAddresses; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index ae435e4..15c0bf9 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4627,9 +4627,6 @@ qemuProcessStartValidateXML(virDomainObjPtr vm, if (!migration && !snapshot) { if (qemuDomainDefValidate(vm->def) < 0) return -1; - - if (virDomainDefCheckDuplicateDiskInfo(vm->def) < 0) - return -1; } /* checks below validate config that would make qemu fail to start */ -- 2.8.2
Similarly to domain definition it would be possible to introduce invalid config via device coldplug. Add a similar infrastructure to do the checking. --- src/conf/domain_conf.c | 52 +++++++++++++++++++++++++++++++++++++++++++++--- src/conf/domain_conf.h | 3 +++ src/libvirt_private.syms | 1 + src/qemu/qemu_domain.c | 11 ++++++++++ src/qemu/qemu_domain.h | 3 +++ src/qemu/qemu_driver.c | 3 +++ 6 files changed, 70 insertions(+), 3 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 2beb825..3a809d4 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -24366,9 +24366,54 @@ virDomainDiskDefValidate(const virDomainDiskDef *disk) } -#define VIR_DOMAIN_DEF_VALIDATE_DEVICES(dev, func) \ +/** + * virDomainDeviceDefValidate: + * @def: domain definition + * @dev: device definition + * + */ +int +virDomainDeviceDefValidate(const virDomainDef *def ATTRIBUTE_UNUSED, + const virDomainDeviceDef *dev) +{ + switch ((virDomainDeviceType) dev->type) { + case VIR_DOMAIN_DEVICE_DISK: + return virDomainDiskDefValidate(dev->data.disk); + case VIR_DOMAIN_DEVICE_LEASE: + case VIR_DOMAIN_DEVICE_FS: + case VIR_DOMAIN_DEVICE_NET: + case VIR_DOMAIN_DEVICE_INPUT: + case VIR_DOMAIN_DEVICE_SOUND: + case VIR_DOMAIN_DEVICE_VIDEO: + case VIR_DOMAIN_DEVICE_HOSTDEV: + case VIR_DOMAIN_DEVICE_WATCHDOG: + case VIR_DOMAIN_DEVICE_CONTROLLER: + case VIR_DOMAIN_DEVICE_GRAPHICS: + case VIR_DOMAIN_DEVICE_HUB: + case VIR_DOMAIN_DEVICE_REDIRDEV: + case VIR_DOMAIN_DEVICE_SMARTCARD: + case VIR_DOMAIN_DEVICE_CHR: + case VIR_DOMAIN_DEVICE_MEMBALLOON: + case VIR_DOMAIN_DEVICE_NVRAM: + case VIR_DOMAIN_DEVICE_RNG: + case VIR_DOMAIN_DEVICE_SHMEM: + case VIR_DOMAIN_DEVICE_TPM: + case VIR_DOMAIN_DEVICE_PANIC: + case VIR_DOMAIN_DEVICE_MEMORY: + case VIR_DOMAIN_DEVICE_NONE: + case VIR_DOMAIN_DEVICE_LAST: + break; + } + + return 0; +} + + +#define VIR_DOMAIN_DEF_VALIDATE_DEVICES(dev, devtype) \ + device.type = devtype; \ for (i = 0; i < def->n ## dev ## s; i++) { \ - if (func(def->dev ## s[i]) < 0) \ + device.data.dev = def->dev ## s[i]; \ + if (virDomainDeviceDefValidate(def, &device) < 0) \ return -1; \ } @@ -24388,9 +24433,10 @@ int virDomainDefValidate(const virDomainDef *def) { size_t i; + virDomainDeviceDef device; /* check configuration of individual devices */ - VIR_DOMAIN_DEF_VALIDATE_DEVICES(disk, virDomainDiskDefValidate); + VIR_DOMAIN_DEF_VALIDATE_DEVICES(disk, VIR_DOMAIN_DEVICE_DISK); if (virDomainDefCheckDuplicateDiskInfo(def) < 0) return -1; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 3c219ea..8f40feb 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3176,6 +3176,9 @@ bool virDomainDefHasMemballoon(const virDomainDef *def) ATTRIBUTE_NONNULL(1); char *virDomainObjGetShortName(virDomainObjPtr vm); +int virDomainDeviceDefValidate(const virDomainDef *def, + const virDomainDeviceDef *dev); + int virDomainDefValidate(const virDomainDef *def); #endif /* __DOMAIN_CONF_H */ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 99975ed..08d8f54 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -249,6 +249,7 @@ virDomainDeviceAddressTypeToString; virDomainDeviceDefCopy; virDomainDeviceDefFree; virDomainDeviceDefParse; +virDomainDeviceDefValidate; virDomainDeviceFindControllerModel; virDomainDeviceGetInfo; virDomainDeviceInfoCopy; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index f172d6f..58bd412 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -5345,6 +5345,17 @@ qemuDomainDefValidateDiskLunSource(const virStorageSource *src) } +int +qemuDomainDeviceDefValidate(const virDomainDef *def, + const virDomainDeviceDef *dev) +{ + if (virDomainDeviceDefValidate(def, dev) < 0) + return -1; + + return 0; +} + + /** * qemuDomainDefValidate: * @def: domain definition diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index baf8bd8..aee38c6 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -667,6 +667,9 @@ void qemuDomainSecretDestroy(virDomainObjPtr vm) int qemuDomainSecretPrepare(virConnectPtr conn, virDomainObjPtr vm) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); +int qemuDomainDeviceDefValidate(const virDomainDef *def, + const virDomainDeviceDef *dev); + int qemuDomainDefValidateDiskLunSource(const virStorageSource *src) ATTRIBUTE_NONNULL(1); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index e6d22ec..fd2c0be 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -8248,6 +8248,9 @@ qemuDomainAttachDeviceFlags(virDomainPtr dom, if (dev == NULL) goto endjob; + if (qemuDomainDeviceDefValidate(vm->def, dev) < 0) + goto endjob; + if (flags & VIR_DOMAIN_AFFECT_CONFIG && flags & VIR_DOMAIN_AFFECT_LIVE) { /* If we are affecting both CONFIG and LIVE -- 2.8.2
On 05/17/2016 10:25 AM, Peter Krempa wrote:
Recently I NACKed quite a few patches attempting to add checks to the post parse callback infrastructure that would inhbit configs from being loaded after daemon restart.
To solve this introduce a new infrastructure that will get called only on codepaths that define and start the config leaving the code paths that load configs untouched.
Peter Krempa (6): conf: disk: Rename virDomainDiskDefValidate to virDomainDiskDefParseValidate qemu: driver: Fix function header alignment of some functions conf: Introduce infrastructure to add config validation to define time qemu: Move check that validates 'min_guarantee' to qemuDomainDefValidate conf: Move check that validates disk info to virDomainDefValidate conf: Add validation infrastructure for device hot/cold plug
src/conf/domain_conf.c | 128 +++++++++++++++++++++++++++++++++++++++-------- src/conf/domain_conf.h | 11 ++-- src/libvirt_private.syms | 3 +- src/qemu/qemu_conf.h | 2 + src/qemu/qemu_domain.c | 38 ++++++++++++++ src/qemu/qemu_domain.h | 3 ++ src/qemu/qemu_driver.c | 26 ++++++++-- src/qemu/qemu_process.c | 19 ++++--- 8 files changed, 190 insertions(+), 40 deletions(-)
Well I see that patch 6 addresses the comments from patch 3 at least from qemuAttach The genesis of what is now virDomainDiskDefValidate was to attempt to extract out one of the qemuCheckDiskConfig checks into an earlier failure. Looking at that - I would think it would be a prime candidate to become patch 7 (yeah, I know, patches welcome...). Thinking about that code though reminds me - virStorageTranslateDiskSourcePool would need to be called... I'm in favor of this, but figure a few more eyes on it can also help. John
On Tue, May 17, 2016 at 16:55:53 -0400, John Ferlan wrote:
On 05/17/2016 10:25 AM, Peter Krempa wrote:
Recently I NACKed quite a few patches attempting to add checks to the post parse callback infrastructure that would inhbit configs from being loaded after daemon restart.
To solve this introduce a new infrastructure that will get called only on codepaths that define and start the config leaving the code paths that load configs untouched.
[...]
Well I see that patch 6 addresses the comments from patch 3 at least from qemuAttach
The genesis of what is now virDomainDiskDefValidate was to attempt to extract out one of the qemuCheckDiskConfig checks into an earlier failure. Looking at that - I would think it would be a prime candidate to become patch 7 (yeah, I know, patches welcome...). Thinking about that code though reminds me - virStorageTranslateDiskSourcePool would need to be called...
At the point where you define a VM you don't really wan't to do that. The storage pool may not be accessible at that point so the translation might fail. We'd at least need to be very graceful to that failure and fall back to the check that doesn't require the pool to be translated. Peter
participants (5)
-
Cole Robinson -
Jim Fehlig -
John Ferlan -
Pavel Hrdina -
Peter Krempa