diff mbox

qemu: reject min_guarantee at parse time

Message ID a236905674b334b72db09ca978180ea039a3453d.1460756237.git.crobinso@redhat.com
State New
Headers show

Commit Message

Cole Robinson April 15, 2016, 9:37 p.m. UTC 
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(-)

-- 
2.7.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Comments

Cole Robinson April 18, 2016, 12:09 p.m. UTC | #1 
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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
diff mbox

Patch

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");
 
     /*