[libvirt] [PATCH] qemu: reject min_guarantee at parse time
min_guarantee isn't implemented for qemu, and an explicit check was added in june 2014 to reject the VM at qemu startup time. It's a weird place to do XML validation, so move it to the post parse area where we have similar checks. Also drop a validation check against min_guarantee in the command line building; it will never see min_guarantee there. --- src/qemu/qemu_command.c | 1 - src/qemu/qemu_domain.c | 7 +++++++ src/qemu/qemu_process.c | 7 ------- 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 26c19ff..2fb967a 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -9143,7 +9143,6 @@ qemuBuildCommandLineValidate(virQEMUDriverPtr driver, if (virMemoryLimitIsSet(def->mem.hard_limit) || virMemoryLimitIsSet(def->mem.soft_limit) || - def->mem.min_guarantee || virMemoryLimitIsSet(def->mem.swap_hard_limit)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Memory tuning is not available in session mode")); diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 73187ce..f5fafc8 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1690,6 +1690,13 @@ qemuDomainDefPostParse(virDomainDefPtr def, return ret; } + if (def->mem.min_guarantee) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Parameter 'min_guarantee' " + "not supported by QEMU.")); + return -1; + } + if (def->os.loader && def->os.loader->type == VIR_DOMAIN_LOADER_TYPE_PFLASH && def->os.loader->readonly == VIR_TRISTATE_SWITCH_ON && diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 81d86c2..387aff5 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4550,13 +4550,6 @@ qemuProcessStartValidate(virQEMUDriverPtr driver, virDomainDefCheckDuplicateDiskInfo(vm->def) < 0) return -1; - if (vm->def->mem.min_guarantee) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Parameter 'min_guarantee' " - "not supported by QEMU.")); - return -1; - } - VIR_DEBUG("Checking for any possible (non-fatal) issues"); /* -- 2.7.3
On Fri, Apr 15, 2016 at 05:37:17PM -0400, Cole Robinson wrote:
min_guarantee isn't implemented for qemu, and an explicit check was added in june 2014 to reject the VM at qemu startup time. It's a weird place to do XML validation, so move it to the post parse area where we have similar checks.
NACK, it's done precisely there because if there is any domain that has that parameter already in, it will disappear after upgrade to libvirt with this patch. That's what we have that validation function for and the reason why it is called when the domain is being started. We have bunch of similar issues that I wanted to address globally, but it doesn't look like it will work in near future.
Also drop a validation check against min_guarantee in the command line building; it will never see min_guarantee there. --- src/qemu/qemu_command.c | 1 - src/qemu/qemu_domain.c | 7 +++++++ src/qemu/qemu_process.c | 7 ------- 3 files changed, 7 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 26c19ff..2fb967a 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -9143,7 +9143,6 @@ qemuBuildCommandLineValidate(virQEMUDriverPtr driver,
if (virMemoryLimitIsSet(def->mem.hard_limit) || virMemoryLimitIsSet(def->mem.soft_limit) || - def->mem.min_guarantee || virMemoryLimitIsSet(def->mem.swap_hard_limit)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Memory tuning is not available in session mode")); diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 73187ce..f5fafc8 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1690,6 +1690,13 @@ qemuDomainDefPostParse(virDomainDefPtr def, return ret; }
+ if (def->mem.min_guarantee) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Parameter 'min_guarantee' " + "not supported by QEMU.")); + return -1; + } + if (def->os.loader && def->os.loader->type == VIR_DOMAIN_LOADER_TYPE_PFLASH && def->os.loader->readonly == VIR_TRISTATE_SWITCH_ON && diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 81d86c2..387aff5 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4550,13 +4550,6 @@ qemuProcessStartValidate(virQEMUDriverPtr driver, virDomainDefCheckDuplicateDiskInfo(vm->def) < 0) return -1;
- if (vm->def->mem.min_guarantee) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Parameter 'min_guarantee' " - "not supported by QEMU.")); - return -1; - } - VIR_DEBUG("Checking for any possible (non-fatal) issues");
/* -- 2.7.3
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 04/16/2016 08:54 AM, Martin Kletzander wrote:
On Fri, Apr 15, 2016 at 05:37:17PM -0400, Cole Robinson wrote:
min_guarantee isn't implemented for qemu, and an explicit check was added in june 2014 to reject the VM at qemu startup time. It's a weird place to do XML validation, so move it to the post parse area where we have similar checks.
NACK, it's done precisely there because if there is any domain that has that parameter already in, it will disappear after upgrade to libvirt with this patch. That's what we have that validation function for and the reason why it is called when the domain is being started.
We have bunch of similar issues that I wanted to address globally, but it doesn't look like it will work in near future.
I see what you mean. In this case it's a bit fuzzy since that particular check has been there since Aug 2014, so I strongly doubt there's any configs in practice that have that param set on anything recent. But it's a fairly pedantic point, I'll change the patch to add a comment about the deliberate location choice. Thanks, Cole
participants (2)
-
Cole Robinson -
Martin Kletzander