6:52 a.m.
Resolve a few corner cases which would create invalid configurations or produce
bad error messages.
Peter Krempa (5):
qemu: hotplug: Iterate over vcpu 0 in individual vcpu hotplug code
qemu: hotplug: Fix formatting strings in
qemuDomainFilterHotplugVcpuEntities
qemu: hotplug: Clear vcpu ordering for coldplug of vcpus
qemu: hotplug: Add validation for coldplug of individual vcpus
qemu: hotplug: Validate that vcpu-hotplug does not break config
src/qemu/qemu_hotplug.c | 73 +++++++++++++++++++++++++++++++++++++++++++------
1 file changed, 65 insertions(+), 8 deletions(-)
--
2.12.2
6:52 a.m.
New subject: [libvirt] [PATCH 1/5] qemu: hotplug: Iterate over vcpu 0 in individual vcpu hotplug code
Buggy condition meant that vcpu0 would not be iterated in the checks.
Since it's not hotpluggable anyways we would not be able to break the
configuration of a live VM.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1437013
---
src/qemu/qemu_hotplug.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index de561da58..b5b520d8c 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -5784,7 +5784,7 @@ qemuDomainSetVcpuConfig(virDomainDefPtr def,
def->individualvcpus = true;
- while ((next = virBitmapNextSetBit(map, next)) > 0) {
+ while ((next = virBitmapNextSetBit(map, next)) >= 0) {
if (!(vcpu = virDomainDefGetVcpu(def, next)))
continue;
@@ -5817,7 +5817,7 @@ qemuDomainFilterHotplugVcpuEntities(virDomainDefPtr def,
return NULL;
/* make sure that all selected vcpus are in the correct state */
- while ((next = virBitmapNextSetBit(map, next)) > 0) {
+ while ((next = virBitmapNextSetBit(map, next)) >= 0) {
if (!(vcpu = virDomainDefGetVcpu(def, next)))
continue;
@@ -5837,7 +5837,7 @@ qemuDomainFilterHotplugVcpuEntities(virDomainDefPtr def,
/* Make sure that all vCPUs belonging to a single hotpluggable entity were
* selected and then de-select any sub-threads of it. */
next = -1;
- while ((next = virBitmapNextSetBit(map, next)) > 0) {
+ while ((next = virBitmapNextSetBit(map, next)) >= 0) {
if (!(vcpu = virDomainDefGetVcpu(def, next)))
continue;
--
2.12.2
6:52 a.m.
New subject: [libvirt] [PATCH 2/5] qemu: hotplug: Fix formatting strings in qemuDomainFilterHotplugVcpuEntities
'next' is declared as 'ssize_t' so use '%zd'
---
src/qemu/qemu_hotplug.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index b5b520d8c..48de6b815 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -5823,13 +5823,13 @@ qemuDomainFilterHotplugVcpuEntities(virDomainDefPtr def,
if (vcpu->online == state) {
virReportError(VIR_ERR_INVALID_ARG,
- _("vcpu '%zu' is already in requested
state"), next);
+ _("vcpu '%zd' is already in requested
state"), next);
goto cleanup;
}
if (vcpu->online && !vcpu->hotpluggable) {
virReportError(VIR_ERR_INVALID_ARG,
- _("vcpu '%zu' can't be hotunplugged"),
next);
+ _("vcpu '%zd' can't be hotunplugged"),
next);
goto cleanup;
}
}
@@ -5845,7 +5845,7 @@ qemuDomainFilterHotplugVcpuEntities(virDomainDefPtr def,
if (vcpupriv->vcpus == 0) {
virReportError(VIR_ERR_INVALID_ARG,
- _("vcpu '%zu' belongs to a larger hotpluggable
entity, "
+ _("vcpu '%zd' belongs to a larger hotpluggable
entity, "
"but siblings were not selected"), next);
goto cleanup;
}
@@ -5854,7 +5854,7 @@ qemuDomainFilterHotplugVcpuEntities(virDomainDefPtr def,
if (!virBitmapIsBitSet(map, i)) {
virReportError(VIR_ERR_INVALID_ARG,
_("vcpu '%zu' was not selected but it belongs
to "
- "hotpluggable entity '%zu-%zu' which was
"
+ "hotpluggable entity '%zd-%zd' which was
"
"partially selected"),
i, next, next + vcpupriv->vcpus - 1);
goto cleanup;
--
2.12.2
6:52 a.m.
New subject: [libvirt] [PATCH 3/5] qemu: hotplug: Clear vcpu ordering for coldplug of vcpus
Vcpu order is required to stay sequential. Clear the order on cpu
coldplug to avoid issues with removing vcpus out of sequence.
---
src/qemu/qemu_hotplug.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 48de6b815..5488b1dd4 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -5784,13 +5784,15 @@ qemuDomainSetVcpuConfig(virDomainDefPtr def,
def->individualvcpus = true;
+ /* ordering information may become invalid, thus clear it */
+ virDomainDefVcpuOrderClear(def);
+
while ((next = virBitmapNextSetBit(map, next)) >= 0) {
if (!(vcpu = virDomainDefGetVcpu(def, next)))
continue;
vcpu->online = state;
vcpu->hotpluggable = VIR_TRISTATE_BOOL_YES;
- vcpu->order = 0;
}
}
--
2.12.2
6:52 a.m.
New subject: [libvirt] [PATCH 4/5] qemu: hotplug: Add validation for coldplug of individual vcpus
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
5:24 p.m.
New subject: [libvirt] [PATCH 4/5] qemu: hotplug: Add validation for coldplug of individual vcpus
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;
2:14 a.m.
New subject: [libvirt] [PATCH 4/5] qemu: hotplug: Add validation for coldplug of individual vcpus
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 :)
6:52 a.m.
New subject: [libvirt] [PATCH 5/5] qemu: hotplug: Validate that vcpu-hotplug does not break config
Make sure that non-hotpluggable vcpus stay clustered at the beginning
after modifying persistent definition.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1437010
---
src/qemu/qemu_hotplug.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 18a8df33a..d5aeb6bc0 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -5881,6 +5881,7 @@ qemuDomainVcpuValidateConfig(virDomainDefPtr def,
bool state)
{
virDomainVcpuDefPtr vcpu;
+ size_t maxvcpus = virDomainDefGetVcpusMax(def);
ssize_t next = -1;
/* vcpu 0 can't be disabled */
@@ -5890,7 +5891,25 @@ qemuDomainVcpuValidateConfig(virDomainDefPtr def,
return -1;
}
+ /* non-hotpluggable vcpus need to stay clustered starting from vcpu 0 */
+ for (next = virBitmapNextSetBit(map, -1) + 1; next < maxvcpus; next++) {
+ if (!(vcpu = virDomainDefGetVcpu(def, next)))
+ continue;
+
+ /* skip vcpus being modified */
+ if (virBitmapIsBitSet(map, next))
+ continue;
+
+ if (vcpu->online && vcpu->hotpluggable == VIR_TRISTATE_BOOL_NO) {
+ virReportError(VIR_ERR_INVALID_ARG,
+ _("vcpu '%zd' can't be modified as it is
followed "
+ "by non-hotpluggable online vcpus"), next);
+ return -1;
+ }
+ }
+
/* make sure that all selected vcpus are in the correct state */
+ next = -1;
while ((next = virBitmapNextSetBit(map, next)) >= 0) {
if (!(vcpu = virDomainDefGetVcpu(def, next)))
continue;
--
2.12.2
5:25 p.m.
On 03/31/2017 07:52 AM, Peter Krempa wrote:
Resolve a few corner cases which would create invalid configurations
or produce
bad error messages.
Peter Krempa (5):
qemu: hotplug: Iterate over vcpu 0 in individual vcpu hotplug code
qemu: hotplug: Fix formatting strings in
qemuDomainFilterHotplugVcpuEntities
qemu: hotplug: Clear vcpu ordering for coldplug of vcpus
qemu: hotplug: Add validation for coldplug of individual vcpus
qemu: hotplug: Validate that vcpu-hotplug does not break config
src/qemu/qemu_hotplug.c | 73 +++++++++++++++++++++++++++++++++++++++++++------
1 file changed, 65 insertions(+), 8 deletions(-)
ACK series (left a note on patch 4)
John
2788
days inactive
2792
days old
8 comments
2 participants
participants (2)
-
John Ferlan -
Peter Krempa