[libvirt] [PATCH 0/2] conf: DEF_FEATURE tweaks
After the net model enum patches, the test driver started rejecting non-enum model strings. It should continue to accept unknown models and generally act closer to the qemu driver. So let's add all of qemu's DEF_FEATURE bits to match while we are at it (virt-manager will want this for os firmware XML testing at least) Fix an issue in the area while I'm here Cole Robinson (2): test: match qemu VIR_DOMAIN_DEF_FEATURE* usage conf: Fix VIR_DOMAIN_DEF_FEATURE_FW_AUTOSELECT validation src/conf/domain_conf.c | 6 ++---- src/test/test_driver.c | 10 +++++++++- src/vmx/vmx.c | 3 ++- 3 files changed, 13 insertions(+), 6 deletions(-) -- 2.21.0
Match the XML feature usage of the qemu driver, so the test driver doesn't reject things like <os firmware='efi'/>. Particularly VIR_DOMAIN_DEF_FEATURE_NET_MODEL_STRING is needed to prevent regressions for test suite users with net model strings that aren't in the virDomainNetModel enum yet Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/test/test_driver.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index d5eecf4b7f..24c1dc5adf 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -390,6 +390,14 @@ testDriverNew(void) .parse = testDomainDefNamespaceParse, .free = testDomainDefNamespaceFree, }; + virDomainDefParserConfig config = { + .features = VIR_DOMAIN_DEF_FEATURE_MEMORY_HOTPLUG | + VIR_DOMAIN_DEF_FEATURE_OFFLINE_VCPUPIN | + VIR_DOMAIN_DEF_FEATURE_INDIVIDUAL_VCPUS | + VIR_DOMAIN_DEF_FEATURE_USER_ALIAS | + VIR_DOMAIN_DEF_FEATURE_FW_AUTOSELECT | + VIR_DOMAIN_DEF_FEATURE_NET_MODEL_STRING, + }; testDriverPtr ret; if (testDriverInitialize() < 0) @@ -398,7 +406,7 @@ testDriverNew(void) if (!(ret = virObjectLockableNew(testDriverClass))) return NULL; - if (!(ret->xmlopt = virDomainXMLOptionNew(NULL, NULL, &ns, NULL, NULL)) || + if (!(ret->xmlopt = virDomainXMLOptionNew(&config, NULL, &ns, NULL, NULL)) || !(ret->eventState = virObjectEventStateNew()) || !(ret->ifaces = virInterfaceObjListNew()) || !(ret->domains = virDomainObjListNew()) || -- 2.21.0
On Wed, 2019-04-17 at 10:57 -0400, Cole Robinson wrote: [...]
+ virDomainDefParserConfig config = { + .features = VIR_DOMAIN_DEF_FEATURE_MEMORY_HOTPLUG | + VIR_DOMAIN_DEF_FEATURE_OFFLINE_VCPUPIN | + VIR_DOMAIN_DEF_FEATURE_INDIVIDUAL_VCPUS | + VIR_DOMAIN_DEF_FEATURE_USER_ALIAS | + VIR_DOMAIN_DEF_FEATURE_FW_AUTOSELECT | + VIR_DOMAIN_DEF_FEATURE_NET_MODEL_STRING, + };
virDomainDefParserConfig instances in other drivers usually follow the (vir)${DRIVER}DomainDefParserConfig, so I would do the same here and call it something like testDomainDefParserConfig. Either way, Reviewed-by: Andrea Bolognani <abologna@redhat.com> and safe for freeze. The second patch in the series is not as urgent, nor is it as obviously correct, so I'll probably look at it once 5.3.0 is out. -- Andrea Bolognani / Red Hat / Virtualization
On 4/30/19 12:20 PM, Andrea Bolognani wrote:
On Wed, 2019-04-17 at 10:57 -0400, Cole Robinson wrote: [...]
+ virDomainDefParserConfig config = { + .features = VIR_DOMAIN_DEF_FEATURE_MEMORY_HOTPLUG | + VIR_DOMAIN_DEF_FEATURE_OFFLINE_VCPUPIN | + VIR_DOMAIN_DEF_FEATURE_INDIVIDUAL_VCPUS | + VIR_DOMAIN_DEF_FEATURE_USER_ALIAS | + VIR_DOMAIN_DEF_FEATURE_FW_AUTOSELECT | + VIR_DOMAIN_DEF_FEATURE_NET_MODEL_STRING, + };
virDomainDefParserConfig instances in other drivers usually follow the (vir)${DRIVER}DomainDefParserConfig, so I would do the same here and call it something like testDomainDefParserConfig.
Sorry I forgot to make this change before pushing...
Either way,
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
and safe for freeze.
Thanks, I've pushed it now
The second patch in the series is not as urgent, nor is it as obviously correct, so I'll probably look at it once 5.3.0 is out.
Cool. No rush on that one, it's minor - Cole
The check shouldn't be dependent on whether os->loader is set. Fix it, and add the XML feature to vmx.c which would start failing Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/conf/domain_conf.c | 6 ++---- src/vmx/vmx.c | 3 ++- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 51aa48f421..7c4a1ff7fa 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -6815,9 +6815,6 @@ static int virDomainDefOSValidate(const virDomainDef *def, virDomainXMLOptionPtr xmlopt) { - if (!def->os.loader) - return 0; - if (def->os.firmware && !(xmlopt->config.features & VIR_DOMAIN_DEF_FEATURE_FW_AUTOSELECT)) { virReportError(VIR_ERR_XML_DETAIL, "%s", @@ -6825,7 +6822,8 @@ virDomainDefOSValidate(const virDomainDef *def, return -1; } - if (!def->os.loader->path && + if (def->os.loader && + !def->os.loader->path && def->os.firmware == VIR_DOMAIN_OS_DEF_FIRMWARE_NONE) { virReportError(VIR_ERR_XML_DETAIL, "%s", _("no loader path specified and firmware auto selection disabled")); diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c index 35b83a2320..82418ceca2 100644 --- a/src/vmx/vmx.c +++ b/src/vmx/vmx.c @@ -555,7 +555,8 @@ static virDomainDefParserConfig virVMXDomainDefParserConfig = { .domainPostParseCallback = virVMXDomainDefPostParse, .features = (VIR_DOMAIN_DEF_FEATURE_WIDE_SCSI | VIR_DOMAIN_DEF_FEATURE_NAME_SLASH | - VIR_DOMAIN_DEF_FEATURE_NO_BOOT_ORDER), + VIR_DOMAIN_DEF_FEATURE_NO_BOOT_ORDER | + VIR_DOMAIN_DEF_FEATURE_FW_AUTOSELECT), }; struct virVMXDomainDefNamespaceData { -- 2.21.0
On 4/17/19 10:57 AM, Cole Robinson wrote:
After the net model enum patches, the test driver started rejecting non-enum model strings. It should continue to accept unknown models and generally act closer to the qemu driver. So let's add all of qemu's DEF_FEATURE bits to match while we are at it (virt-manager will want this for os firmware XML testing at least)
Fix an issue in the area while I'm here
Cole Robinson (2): test: match qemu VIR_DOMAIN_DEF_FEATURE* usage conf: Fix VIR_DOMAIN_DEF_FEATURE_FW_AUTOSELECT validation
src/conf/domain_conf.c | 6 ++---- src/test/test_driver.c | 10 +++++++++- src/vmx/vmx.c | 3 ++- 3 files changed, 13 insertions(+), 6 deletions(-)
ping, first patch at least should go into the next release at least, some valid testdriver XML usage may require it as explained above Thanks, Cole
participants (2)
-
Andrea Bolognani -
Cole Robinson