[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 2/3] setvcpus: extend qemuDomainSetVcpusFlags() to support current flag



On 07/15/2011 01:01 AM, Taku Izumi wrote:
> 
> This patch extends qemudDomainSetVcpusFlags() function to support
> VIR_DOMAIN_AFFECT_CURRENT flag.

We've been renaming qemudDomain to qemuDomain lately, so now is as good
a time to make the change for this function as any.  Especially since
you used that shorter name in your subject line :)

> Signed-off-by: Taku Izumi <izumi taku jp fujitsu com>
> ---
>  src/qemu/qemu_driver.c |   37 +++++++++++++++++++++++++++----------
>  1 file changed, 27 insertions(+), 10 deletions(-)
> 
> @@ -2950,7 +2942,32 @@ qemudDomainSetVcpusFlags(virDomainPtr do
>      if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
>          goto cleanup;
>  
> -    if (!virDomainObjIsActive(vm) && (flags & VIR_DOMAIN_AFFECT_LIVE)) {
> +    isActive = virDomainObjIsActive(vm);
> +
> +    if (flags == VIR_DOMAIN_AFFECT_CURRENT) {
> +        if (isActive)
> +            flags = VIR_DOMAIN_AFFECT_LIVE;
> +        else
> +            flags = VIR_DOMAIN_AFFECT_CONFIG;
> +    }
> +    if (flags == VIR_DOMAIN_VCPU_MAXIMUM) {
> +        if (isActive)
> +            flags |= VIR_DOMAIN_AFFECT_LIVE;
> +        else
> +            flags |= VIR_DOMAIN_AFFECT_CONFIG;
> +    }

That's a bit redundant.  How about:

if ((flags & (VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG)) == 0)

and a single set of flags |=.

Or even better, peel off VIR_DOMAIN_VCPU_MAXIMUM into a helper bool
before doing anything else.

> +
> +    /* At least one of LIVE or CONFIG must be set.  MAXIMUM cannot be
> +     * mixed with LIVE.  */
> +    if ((flags & (VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG)) == 0 ||

This condition is now guaranteed to be false, given the earlier
manipulations.

> +        (flags & (VIR_DOMAIN_VCPU_MAXIMUM | VIR_DOMAIN_AFFECT_LIVE)) ==
> +         (VIR_DOMAIN_VCPU_MAXIMUM | VIR_DOMAIN_AFFECT_LIVE)) {
> +        qemuReportError(VIR_ERR_INVALID_ARG,
> +                        _("invalid flag combination: (0x%x)"), flags);

This error message is inaccurate - we've possibly modified flags, so it
might not match what the user passed in.  Rather, we should just state
the restriction.

> +        goto endjob;
> +    }
> +
> +    if (!isActive && (flags & VIR_DOMAIN_AFFECT_LIVE)) {

ACK with nits fixed, so I'm squashing this in before pushing:

diff --git i/src/qemu/qemu_driver.c w/src/qemu/qemu_driver.c
index 0b1d5ec..f3a495f 100644
--- i/src/qemu/qemu_driver.c
+++ w/src/qemu/qemu_driver.c
@@ -2906,8 +2906,8 @@ unsupported:


 static int
-qemudDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus,
-                         unsigned int flags)
+qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus,
+                        unsigned int flags)
 {
     struct qemud_driver *driver = dom->conn->privateData;
     virDomainObjPtr vm;
@@ -2916,6 +2916,7 @@ qemudDomainSetVcpusFlags(virDomainPtr dom,
unsigned int nvcpus,
     int max;
     int ret = -1;
     bool isActive;
+    bool maximum;

     virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
                   VIR_DOMAIN_AFFECT_CONFIG |
@@ -2943,27 +2944,20 @@ qemudDomainSetVcpusFlags(virDomainPtr dom,
unsigned int nvcpus,
         goto cleanup;

     isActive = virDomainObjIsActive(vm);
+    maximum = (flags & VIR_DOMAIN_VCPU_MAXIMUM) != 0;
+    flags &= ~VIR_DOMAIN_VCPU_MAXIMUM;

     if (flags == VIR_DOMAIN_AFFECT_CURRENT) {
         if (isActive)
-            flags = VIR_DOMAIN_AFFECT_LIVE;
-        else
-            flags = VIR_DOMAIN_AFFECT_CONFIG;
-    }
-    if (flags == VIR_DOMAIN_VCPU_MAXIMUM) {
-        if (isActive)
             flags |= VIR_DOMAIN_AFFECT_LIVE;
         else
             flags |= VIR_DOMAIN_AFFECT_CONFIG;
     }

-    /* At least one of LIVE or CONFIG must be set.  MAXIMUM cannot be
-     * mixed with LIVE.  */
-    if ((flags & (VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG))
== 0 ||
-        (flags & (VIR_DOMAIN_VCPU_MAXIMUM | VIR_DOMAIN_AFFECT_LIVE)) ==
-         (VIR_DOMAIN_VCPU_MAXIMUM | VIR_DOMAIN_AFFECT_LIVE)) {
-        qemuReportError(VIR_ERR_INVALID_ARG,
-                        _("invalid flag combination: (0x%x)"), flags);
+    /* MAXIMUM cannot be mixed with LIVE.  */
+    if (maximum && (flags & VIR_DOMAIN_AFFECT_LIVE)) {
+        qemuReportError(VIR_ERR_INVALID_ARG, "%s",
+                        _("cannot adjust maximum on running domain"));
         goto endjob;
     }

@@ -2992,7 +2986,7 @@ qemudDomainSetVcpusFlags(virDomainPtr dom,
unsigned int nvcpus,
         goto endjob;
     }

-    if (!(flags & VIR_DOMAIN_VCPU_MAXIMUM) && vm->def->maxvcpus < max) {
+    if (!maximum && vm->def->maxvcpus < max) {
         max = vm->def->maxvcpus;
     }

@@ -3007,15 +3001,14 @@ qemudDomainSetVcpusFlags(virDomainPtr dom,
unsigned int nvcpus,
         goto endjob;

     switch (flags) {
-    case VIR_DOMAIN_VCPU_MAXIMUM | VIR_DOMAIN_AFFECT_CONFIG:
-        persistentDef->maxvcpus = nvcpus;
-        if (nvcpus < persistentDef->vcpus)
-            persistentDef->vcpus = nvcpus;
-        ret = 0;
-        break;
-
     case VIR_DOMAIN_AFFECT_CONFIG:
-        persistentDef->vcpus = nvcpus;
+        if (maximum) {
+            persistentDef->maxvcpus = nvcpus;
+            if (nvcpus < persistentDef->vcpus)
+                persistentDef->vcpus = nvcpus;
+        } else {
+            persistentDef->vcpus = nvcpus;
+        }
         ret = 0;
         break;

@@ -3046,9 +3039,9 @@ cleanup:
 }

 static int
-qemudDomainSetVcpus(virDomainPtr dom, unsigned int nvcpus)
+qemuDomainSetVcpus(virDomainPtr dom, unsigned int nvcpus)
 {
-    return qemudDomainSetVcpusFlags(dom, nvcpus, VIR_DOMAIN_AFFECT_LIVE);
+    return qemuDomainSetVcpusFlags(dom, nvcpus, VIR_DOMAIN_AFFECT_LIVE);
 }


@@ -8597,8 +8590,8 @@ static virDriver qemuDriver = {
     .domainRestore = qemuDomainRestore, /* 0.2.0 */
     .domainCoreDump = qemudDomainCoreDump, /* 0.7.0 */
     .domainScreenshot = qemuDomainScreenshot, /* 0.9.2 */
-    .domainSetVcpus = qemudDomainSetVcpus, /* 0.4.4 */
-    .domainSetVcpusFlags = qemudDomainSetVcpusFlags, /* 0.8.5 */
+    .domainSetVcpus = qemuDomainSetVcpus, /* 0.4.4 */
+    .domainSetVcpusFlags = qemuDomainSetVcpusFlags, /* 0.8.5 */
     .domainGetVcpusFlags = qemudDomainGetVcpusFlags, /* 0.8.5 */
     .domainPinVcpu = qemudDomainPinVcpu, /* 0.4.4 */
     .domainPinVcpuFlags = qemudDomainPinVcpuFlags, /* 0.9.3 */


-- 
Eric Blake   eblake redhat com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]