8:45 a.m.
qemu documents that we should pass in all fields of
'CpuInstanceProperties' back when hotplugging cpus, but we didn't do
that. It turns out that qemu wanted to add new fields which were
mandatory and thus broke the interop with libvirt.
Let's honor their reques.
Peter Krempa (2):
qemu: Extract and store vCPU properties as qemu returned them
qemu: command: Use all vCPU properties when creating args for vCPU
hotplug
src/qemu/qemu_command.c | 19 +++----------------
src/qemu/qemu_domain.c | 3 +++
src/qemu/qemu_domain.h | 3 +++
src/qemu/qemu_monitor.c | 2 ++
src/qemu/qemu_monitor.h | 6 ++++++
src/qemu/qemu_monitor_json.c | 4 ++++
.../x86-modern-bulk-monitor.json | 8 ++++----
.../x86-modern-individual-add-monitor.json | 4 ++--
8 files changed, 27 insertions(+), 22 deletions(-)
--
2.21.0
8:45 a.m.
New subject: [libvirt] [PATCH 1/2] qemu: Extract and store vCPU properties as qemu returned them
In addition to the data that libvirt needs and extracts internally,
copy and store the whole 'props' JSON sub-object of the data returned by
query-hotpluggable-cpus for future use.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_domain.c | 3 +++
src/qemu/qemu_domain.h | 3 +++
src/qemu/qemu_monitor.c | 2 ++
src/qemu/qemu_monitor.h | 6 ++++++
src/qemu/qemu_monitor_json.c | 4 ++++
5 files changed, 18 insertions(+)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 4998474dc9..657f3ecfe4 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1144,6 +1144,7 @@ qemuDomainVcpuPrivateDispose(void *obj)
VIR_FREE(priv->type);
VIR_FREE(priv->alias);
+ virJSONValueFree(priv->props);
return;
}
@@ -11920,6 +11921,8 @@ qemuDomainRefreshVcpuInfo(virQEMUDriverPtr driver,
VIR_STEAL_PTR(vcpupriv->type, info[i].type);
VIR_FREE(vcpupriv->alias);
VIR_STEAL_PTR(vcpupriv->alias, info[i].alias);
+ virJSONValueFree(vcpupriv->props);
+ VIR_STEAL_PTR(vcpupriv->props, info[i].props);
vcpupriv->enable_id = info[i].id;
vcpupriv->qemu_id = info[i].qemu_id;
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 37a00323a7..d097f23342 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -449,6 +449,9 @@ struct _qemuDomainVcpuPrivate {
char *alias;
virTristateBool halted;
+ /* copy of the data that qemu returned */
+ virJSONValuePtr props;
+
/* information for hotpluggable cpus */
char *type;
int socket_id;
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index a880da3ab6..58ad109680 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -1780,6 +1780,7 @@ qemuMonitorCPUInfoClear(qemuMonitorCPUInfoPtr cpus,
VIR_FREE(cpus[i].qom_path);
VIR_FREE(cpus[i].alias);
VIR_FREE(cpus[i].type);
+ virJSONValueFree(cpus[i].props);
}
}
@@ -1931,6 +1932,7 @@ qemuMonitorGetCPUInfoHotplug(struct
qemuMonitorQueryHotpluggableCpusEntry *hotpl
VIR_STEAL_PTR(vcpus[mastervcpu].qom_path, hotplugvcpus[i].qom_path);
VIR_STEAL_PTR(vcpus[mastervcpu].alias, hotplugvcpus[i].alias);
VIR_STEAL_PTR(vcpus[mastervcpu].type, hotplugvcpus[i].type);
+ VIR_STEAL_PTR(vcpus[mastervcpu].props, hotplugvcpus[i].props);
vcpus[mastervcpu].id = hotplugvcpus[i].enable_id;
/* copy state information to slave vcpus */
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 88c9702530..de85a3ba0d 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -570,6 +570,9 @@ struct qemuMonitorQueryHotpluggableCpusEntry {
char *qom_path; /* full device qom path only present for online cpus */
char *alias; /* device alias, may be NULL for non-hotpluggable entities */
+ /* verbatim copy of the JSON data representing the CPU which must be used for hotplug
*/
+ virJSONValuePtr props;
+
/* topology information -1 if qemu didn't report given parameter */
int node_id;
int socket_id;
@@ -603,6 +606,9 @@ struct _qemuMonitorCPUInfo {
/* name of the qemu type to add in case of hotplug */
char *type;
+ /* verbatim copy of the returned data from qemu which should be used when plugging
*/
+ virJSONValuePtr props;
+
/* alias of an hotpluggable entry. Entries with alias can be hot-unplugged */
char *alias;
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index d38b2f2cbe..da1e89dded 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -8378,6 +8378,7 @@ qemuMonitorQueryHotpluggableCpusFree(struct
qemuMonitorQueryHotpluggableCpusEntr
VIR_FREE(entry->type);
VIR_FREE(entry->qom_path);
VIR_FREE(entry->alias);
+ virJSONValueFree(entry->props);
}
VIR_FREE(entries);
@@ -8426,6 +8427,9 @@ qemuMonitorJSONProcessHotpluggableCpusReply(virJSONValuePtr vcpu,
return -1;
}
+ if (!(entry->props = virJSONValueCopy(props)))
+ return -1;
+
entry->node_id = -1;
entry->socket_id = -1;
entry->core_id = -1;
--
2.21.0
8:45 a.m.
New subject: [libvirt] [PATCH 2/2] qemu: command: Use all vCPU properties when creating args for vCPU hotplug
As qemu documents we should use everything in the 'props' sub-object of
the data returned by query-hotpluggable-cpus. Until now we only used
everything we recognized, but that may break in cases when qemu
introduces new fields.
This change requires a fix to the test data as some fields were
reordered.
https://bugzilla.redhat.com/show_bug.cgi?id=1741658
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_command.c | 19 +++----------------
.../x86-modern-bulk-monitor.json | 8 ++++----
.../x86-modern-individual-add-monitor.json | 4 ++--
3 files changed, 9 insertions(+), 22 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 373ebd6d1a..1ca1ecd2f0 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -10609,24 +10609,11 @@ qemuBuildHotpluggableCPUProps(const virDomainVcpuDef *vcpu)
qemuDomainVcpuPrivatePtr vcpupriv = QEMU_DOMAIN_VCPU_PRIVATE(vcpu);
VIR_AUTOPTR(virJSONValue) ret = NULL;
- if (virJSONValueObjectCreate(&ret, "s:driver", vcpupriv->type,
- "s:id", vcpupriv->alias, NULL) <
0)
+ if (!(ret = virJSONValueCopy(vcpupriv->props)))
return NULL;
- if (vcpupriv->socket_id != -1 &&
- virJSONValueObjectAdd(ret, "i:socket-id", vcpupriv->socket_id, NULL)
< 0)
- return NULL;
-
- if (vcpupriv->core_id != -1 &&
- virJSONValueObjectAdd(ret, "i:core-id", vcpupriv->core_id, NULL)
< 0)
- return NULL;
-
- if (vcpupriv->thread_id != -1 &&
- virJSONValueObjectAdd(ret, "i:thread-id", vcpupriv->thread_id, NULL)
< 0)
- return NULL;
-
- if (vcpupriv->node_id != -1 &&
- virJSONValueObjectAdd(ret, "i:node-id", vcpupriv->node_id, NULL)
< 0)
+ if (virJSONValueObjectPrependString(ret, "id", vcpupriv->alias) < 0
||
+ virJSONValueObjectPrependString(ret, "driver", vcpupriv->type) <
0)
return NULL;
VIR_RETURN_PTR(ret);
diff --git a/tests/qemuhotplugtestcpus/x86-modern-bulk-monitor.json
b/tests/qemuhotplugtestcpus/x86-modern-bulk-monitor.json
index 4dcb6a2cf5..b1d02b8d43 100644
--- a/tests/qemuhotplugtestcpus/x86-modern-bulk-monitor.json
+++ b/tests/qemuhotplugtestcpus/x86-modern-bulk-monitor.json
@@ -141,9 +141,9 @@
"arguments": {
"driver": "qemu64-x86_64-cpu",
"id": "vcpu5",
- "socket-id": 1,
"core-id": 0,
- "thread-id": 1
+ "thread-id": 1,
+ "socket-id": 1
},
"id": "libvirt-3"
}
@@ -303,9 +303,9 @@
"arguments": {
"driver": "qemu64-x86_64-cpu",
"id": "vcpu6",
- "socket-id": 1,
"core-id": 1,
- "thread-id": 0
+ "thread-id": 0,
+ "socket-id": 1
},
"id": "libvirt-6"
}
diff --git a/tests/qemuhotplugtestcpus/x86-modern-individual-add-monitor.json
b/tests/qemuhotplugtestcpus/x86-modern-individual-add-monitor.json
index 294198b270..df05e5e3a9 100644
--- a/tests/qemuhotplugtestcpus/x86-modern-individual-add-monitor.json
+++ b/tests/qemuhotplugtestcpus/x86-modern-individual-add-monitor.json
@@ -141,9 +141,9 @@
"arguments": {
"driver": "qemu64-x86_64-cpu",
"id": "vcpu7",
- "socket-id": 1,
"core-id": 1,
- "thread-id": 1
+ "thread-id": 1,
+ "socket-id": 1
},
"id": "libvirt-3"
}
--
2.21.0
9:03 a.m.
On Thu, Aug 29, 2019 at 03:45:15PM +0200, Peter Krempa wrote:
qemu documents that we should pass in all fields of
'CpuInstanceProperties' back when hotplugging cpus, but we didn't do
that. It turns out that qemu wanted to add new fields which were
mandatory and thus broke the interop with libvirt.
Let's honor their reques.
s/reques/request/
Peter Krempa (2):
qemu: Extract and store vCPU properties as qemu returned them
qemu: command: Use all vCPU properties when creating args for vCPU
hotplug
src/qemu/qemu_command.c | 19 +++----------------
src/qemu/qemu_domain.c | 3 +++
src/qemu/qemu_domain.h | 3 +++
src/qemu/qemu_monitor.c | 2 ++
src/qemu/qemu_monitor.h | 6 ++++++
src/qemu/qemu_monitor_json.c | 4 ++++
.../x86-modern-bulk-monitor.json | 8 ++++----
.../x86-modern-individual-add-monitor.json | 4 ++--
8 files changed, 27 insertions(+), 22 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
2138
days inactive
2138
days old
3 comments
2 participants
participants (2)
-
Ján Tomko -
Peter Krempa