Validate that users don't try to disable vcpu 0 and reject attempt to
modify a vcpu to the state it is currently in.
---
 src/qemu/qemu_hotplug.c | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 5488b1dd4..18a8df33a 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -5875,6 +5875,37 @@ qemuDomainFilterHotplugVcpuEntities(virDomainDefPtr def,
 }


+static int
+qemuDomainVcpuValidateConfig(virDomainDefPtr def,
+                             virBitmapPtr map,
+                             bool state)
+{
+    virDomainVcpuDefPtr vcpu;
+    ssize_t next = -1;
+
+    /* vcpu 0 can't be disabled */
+    if (!state && virBitmapIsBitSet(map, 0)) {
+        virReportError(VIR_ERR_INVALID_ARG, "%s",
+                       _("vCPU '0' must be enabled"));
+        return -1;
+    }
+
+    /* make sure that all selected vcpus are in the correct state */
+    while ((next = virBitmapNextSetBit(map, next)) >= 0) {
+        if (!(vcpu = virDomainDefGetVcpu(def, next)))
+            continue;
+
+        if (vcpu->online == state) {
+            virReportError(VIR_ERR_INVALID_ARG,
+                           _("vcpu '%zd' is already in requested state"), next);
+            return -1;
+        }
+    }
+
+    return 0;
+}
+
+
 int
 qemuDomainSetVcpuInternal(virQEMUDriverPtr driver,
                           virDomainObjPtr vm,
@@ -5909,6 +5940,11 @@ qemuDomainSetVcpuInternal(virQEMUDriverPtr driver,
         }
     }

+    if (persistentDef) {
+        if (qemuDomainVcpuValidateConfig(persistentDef, map, state) < 0)
+            goto cleanup;
+    }
+
     if (livevcpus &&
         qemuDomainSetVcpusLive(driver, cfg, vm, livevcpus, state) < 0)
         goto cleanup;
-- 
2.12.2

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

On 03/31/2017 07:52 AM, Peter Krempa wrote:
> Validate that users don't try to disable vcpu 0 and reject attempt to
> modify a vcpu to the state it is currently in.
> ---
>  src/qemu/qemu_hotplug.c | 36 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 36 insertions(+)
> 
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 5488b1dd4..18a8df33a 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -5875,6 +5875,37 @@ qemuDomainFilterHotplugVcpuEntities(virDomainDefPtr def,
>  }
> 
> 
> +static int
> +qemuDomainVcpuValidateConfig(virDomainDefPtr def,
> +                             virBitmapPtr map,
> +                             bool state)
> +{
> +    virDomainVcpuDefPtr vcpu;
> +    ssize_t next = -1;
> +
> +    /* vcpu 0 can't be disabled */
> +    if (!state && virBitmapIsBitSet(map, 0)) {
> +        virReportError(VIR_ERR_INVALID_ARG, "%s",
> +                       _("vCPU '0' must be enabled"));
> +        return -1;
> +    }
> +
> +    /* make sure that all selected vcpus are in the correct state */
> +    while ((next = virBitmapNextSetBit(map, next)) >= 0) {
> +        if (!(vcpu = virDomainDefGetVcpu(def, next)))
> +            continue;
> +
> +        if (vcpu->online == state) {
> +            virReportError(VIR_ERR_INVALID_ARG,
> +                           _("vcpu '%zd' is already in requested state"), next);
> +            return -1;
> +        }

Does this really matter for this path? (config file). Wouldn't they just
be changing to what they already have and is that really a big deal.
IDC either way, but since there was no bz attached to this patch, I was
just curious why the check.

John

> +    }
> +
> +    return 0;
> +}
> +
> +
>  int
>  qemuDomainSetVcpuInternal(virQEMUDriverPtr driver,
>                            virDomainObjPtr vm,
> @@ -5909,6 +5940,11 @@ qemuDomainSetVcpuInternal(virQEMUDriverPtr driver,
>          }
>      }
> 
> +    if (persistentDef) {
> +        if (qemuDomainVcpuValidateConfig(persistentDef, map, state) < 0)
> +            goto cleanup;
> +    }
> +
>      if (livevcpus &&
>          qemuDomainSetVcpusLive(driver, cfg, vm, livevcpus, state) < 0)
>          goto cleanup;
> 

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

On Mon, Apr 03, 2017 at 18:24:59 -0400, John Ferlan wrote:
> 
> 
> On 03/31/2017 07:52 AM, Peter Krempa wrote:
> > Validate that users don't try to disable vcpu 0 and reject attempt to
> > modify a vcpu to the state it is currently in.
> > ---
> >  src/qemu/qemu_hotplug.c | 36 ++++++++++++++++++++++++++++++++++++
> >  1 file changed, 36 insertions(+)
> > 
> > diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> > index 5488b1dd4..18a8df33a 100644
> > --- a/src/qemu/qemu_hotplug.c
> > +++ b/src/qemu/qemu_hotplug.c
> > @@ -5875,6 +5875,37 @@ qemuDomainFilterHotplugVcpuEntities(virDomainDefPtr def,
> >  }
> > 
> > 
> > +static int
> > +qemuDomainVcpuValidateConfig(virDomainDefPtr def,
> > +                             virBitmapPtr map,
> > +                             bool state)
> > +{
> > +    virDomainVcpuDefPtr vcpu;
> > +    ssize_t next = -1;
> > +
> > +    /* vcpu 0 can't be disabled */
> > +    if (!state && virBitmapIsBitSet(map, 0)) {
> > +        virReportError(VIR_ERR_INVALID_ARG, "%s",
> > +                       _("vCPU '0' must be enabled"));
> > +        return -1;
> > +    }
> > +
> > +    /* make sure that all selected vcpus are in the correct state */
> > +    while ((next = virBitmapNextSetBit(map, next)) >= 0) {
> > +        if (!(vcpu = virDomainDefGetVcpu(def, next)))
> > +            continue;
> > +
> > +        if (vcpu->online == state) {
> > +            virReportError(VIR_ERR_INVALID_ARG,
> > +                           _("vcpu '%zd' is already in requested state"), next);
> > +            return -1;
> > +        }
> 
> Does this really matter for this path? (config file). Wouldn't they just
> be changing to what they already have and is that really a big deal.

I used the same check as in the online path, but you are right, it does
not make much sense. I'll drop this check.

> IDC either way, but since there was no bz attached to this patch, I was
> just curious why the check.

Bugs are there even when nobody reports them :)
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list