9:58 a.m.
While looking at users of virCPU.*Free, I found some code
that is supposed to deal with Xen < 4.7.
The minimum version is 4.9 since the following commit:
commit 68940b3fb3c43b8aa03cb6fd2f1d00b1737c9b2c
Author: Jim Fehlig <jfehlig(a)suse.com>
CommitDate: 2021-06-17 10:11:56 -0600
Xen: Bump minimum supported Xen version to 4.9
Remove the code dealing with the old version and use g_auto moer.
Ján Tomko (6):
libxl: capabilities: assume Xen version >= 4.7
libxl: remove enum libxlHwcapVersion
libxl: capsInitCPU: fail if we can't initialize host features
libxl: refactor libxlCapsInitCPU
libxl: use g_auto in libxlCapsNodeData
libxl: use g_auto in libxlDomainGetEmulatorType
src/libxl/libxl_capabilities.c | 119 ++++++++++-----------------------
1 file changed, 37 insertions(+), 82 deletions(-)
--
2.31.1
9:58 a.m.
New subject: [libvirt PATCH 1/6] libxl: capabilities: assume Xen version >= 4.7
Remove the code handling old Xen's hwcap words,
as well as the comment describing it.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/libxl/libxl_capabilities.c | 54 +++++++++++++---------------------
1 file changed, 20 insertions(+), 34 deletions(-)
diff --git a/src/libxl/libxl_capabilities.c b/src/libxl/libxl_capabilities.c
index b4bd1d7e62..1953d7a87a 100644
--- a/src/libxl/libxl_capabilities.c
+++ b/src/libxl/libxl_capabilities.c
@@ -90,41 +90,33 @@ libxlCapsAddCPUID(virCPUData *data, virCPUx86CPUID *cpuid, ssize_t
ncaps)
* across all supported versions of the libxl driver until libxl exposes a
* stable representation of these capabilities. Fortunately not a lot of
* variation happened so it's still trivial to keep track of these leafs
- * to describe host CPU in libvirt capabilities. v0 stands for Xen 4.4
- * up to 4.6, while v1 is meant for Xen 4.7, as depicted in the table below:
+ * to describe host CPU in libvirt capabilities.
*
- * | v0 (Xen 4.4 - 4.6) | v1 (Xen >= 4.7) |
- * ---------------------------------------------
- * word 0 | CPUID.00000001.EDX | CPUID.00000001.EDX |
- * word 1 | CPUID.80000001.EDX | CPUID.00000001.ECX |
- * word 2 | CPUID.80860001 | CPUID.80000001.EDX |
- * word 3 | - Linux - | CPUID.80000001.ECX |
- * word 4 | CPUID.00000001.ECX | CPUID.0000000D:1.EAX |
- * word 5 | CPUID.C0000001 | CPUID.00000007:0.EBX |
- * word 6 | CPUID.80000001.ECX | CPUID.00000007:0.ECX |
- * word 7 | CPUID.00000007.EBX | CPUID.80000007.EDX |
- * word 8 | - Non existent - | CPUID.80000008.EBX |
+ * | Xen >= 4.7 |
+ * ------------------------
+ * word 0 | CPUID.00000001.EDX |
+ * word 1 | CPUID.00000001.ECX |
+ * word 2 | CPUID.80000001.EDX |
+ * word 3 | CPUID.80000001.ECX |
+ * word 4 | CPUID.0000000D:1.EAX |
+ * word 5 | CPUID.00000007:0.EBX |
+ * word 6 | CPUID.00000007:0.ECX |
+ * word 7 | CPUID.80000007.EDX |
+ * word 8 | CPUID.80000008.EBX |
*
*/
static virCPUData *
libxlCapsNodeData(virCPUDef *cpu, libxl_hwcap hwcap,
- enum libxlHwcapVersion version)
+ enum libxlHwcapVersion version G_GNUC_UNUSED)
{
ssize_t ncaps;
virCPUData *cpudata = NULL;
virCPUx86CPUID cpuid[] = {
- { .eax_in = 0x00000001,
- .edx = hwcap[0] },
- { .eax_in = 0x00000001,
- .ecx = (version > LIBXL_HWCAP_V0 ? hwcap[1] : hwcap[4]) },
- { .eax_in = 0x80000001,
- .edx = (version > LIBXL_HWCAP_V0 ? hwcap[2] : hwcap[1]) },
- { .eax_in = 0x80000001,
- .ecx = (version > LIBXL_HWCAP_V0 ? hwcap[3] : hwcap[6]) },
- { .eax_in = 0x00000007,
- .ebx = (version > LIBXL_HWCAP_V0 ? hwcap[5] : hwcap[7]) },
- };
- virCPUx86CPUID cpuid_ver1[] = {
+ { .eax_in = 0x00000001, .edx = hwcap[0] },
+ { .eax_in = 0x00000001, .ecx = hwcap[1] },
+ { .eax_in = 0x80000001, .edx = hwcap[2] },
+ { .eax_in = 0x80000001, .ecx = hwcap[3] },
+ { .eax_in = 0x00000007, .ebx = hwcap[5] },
{ .eax_in = 0x0000000D, .ecx_in = 1U, .eax = hwcap[4] },
{ .eax_in = 0x00000007, .ecx_in = 0U, .ecx = hwcap[6] },
{ .eax_in = 0x80000007, .ecx_in = 0U, .edx = hwcap[7] },
@@ -137,11 +129,6 @@ libxlCapsNodeData(virCPUDef *cpu, libxl_hwcap hwcap,
if (libxlCapsAddCPUID(cpudata, cpuid, ncaps) < 0)
goto error;
- ncaps = G_N_ELEMENTS(cpuid_ver1);
- if (version > LIBXL_HWCAP_V0 &&
- libxlCapsAddCPUID(cpudata, cpuid_ver1, ncaps) < 0)
- goto error;
-
return cpudata;
error:
@@ -156,7 +143,7 @@ libxlCapsNodeData(virCPUDef *cpu, libxl_hwcap hwcap,
*/
static int
libxlCapsInitCPU(virCaps *caps, libxl_physinfo *phy_info,
- enum libxlHwcapVersion version)
+ enum libxlHwcapVersion version G_GNUC_UNUSED)
{
virCPUData *data = NULL;
virCPUDef *cpu = NULL;
@@ -175,8 +162,7 @@ libxlCapsInitCPU(virCaps *caps, libxl_physinfo *phy_info,
virCapabilitiesAddHostFeature(caps, "pae") < 0)
goto error;
- host_lm = (phy_info->hw_cap[version > LIBXL_HWCAP_V0 ? 2 : 1]
- & LIBXL_X86_FEATURE_LM_MASK);
+ host_lm = (phy_info->hw_cap[2] & LIBXL_X86_FEATURE_LM_MASK);
if (host_lm)
cpu->arch = VIR_ARCH_X86_64;
else
--
2.31.1
9:58 a.m.
New subject: [libvirt PATCH 2/6] libxl: remove enum libxlHwcapVersion
As well as the code probing for the version in libxlCapsInitHost.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/libxl/libxl_capabilities.c | 24 ++++--------------------
1 file changed, 4 insertions(+), 20 deletions(-)
diff --git a/src/libxl/libxl_capabilities.c b/src/libxl/libxl_capabilities.c
index 1953d7a87a..d98109ea86 100644
--- a/src/libxl/libxl_capabilities.c
+++ b/src/libxl/libxl_capabilities.c
@@ -45,11 +45,6 @@ VIR_LOG_INIT("libxl.libxl_capabilities");
#define LIBXL_X86_FEATURE_PAE_MASK (1 << 6)
#define LIBXL_X86_FEATURE_LM_MASK (1 << 29)
-enum libxlHwcapVersion {
- LIBXL_HWCAP_V0 = 0, /* for Xen 4.4 .. 4.6 */
- LIBXL_HWCAP_V1, /* for Xen 4.7 and up */
-};
-
struct guest_arch {
virArch arch;
int hvm;
@@ -106,8 +101,7 @@ libxlCapsAddCPUID(virCPUData *data, virCPUx86CPUID *cpuid, ssize_t
ncaps)
*
*/
static virCPUData *
-libxlCapsNodeData(virCPUDef *cpu, libxl_hwcap hwcap,
- enum libxlHwcapVersion version G_GNUC_UNUSED)
+libxlCapsNodeData(virCPUDef *cpu, libxl_hwcap hwcap)
{
ssize_t ncaps;
virCPUData *cpudata = NULL;
@@ -142,8 +136,7 @@ libxlCapsNodeData(virCPUDef *cpu, libxl_hwcap hwcap,
* the X'th 32-bit word of hw_cap.
*/
static int
-libxlCapsInitCPU(virCaps *caps, libxl_physinfo *phy_info,
- enum libxlHwcapVersion version G_GNUC_UNUSED)
+libxlCapsInitCPU(virCaps *caps, libxl_physinfo *phy_info)
{
virCPUData *data = NULL;
virCPUDef *cpu = NULL;
@@ -177,7 +170,7 @@ libxlCapsInitCPU(virCaps *caps, libxl_physinfo *phy_info,
ret = 0;
- if (!(data = libxlCapsNodeData(cpu, phy_info->hw_cap, version)) ||
+ if (!(data = libxlCapsNodeData(cpu, phy_info->hw_cap)) ||
cpuDecode(cpu, data, NULL) < 0) {
VIR_WARN("Failed to initialize host cpu features");
goto error;
@@ -196,8 +189,6 @@ libxlCapsInitCPU(virCaps *caps, libxl_physinfo *phy_info,
static int
libxlCapsInitHost(libxl_ctx *ctx, virCaps *caps)
{
- const libxl_version_info *ver_info;
- enum libxlHwcapVersion version;
libxl_physinfo phy_info;
int ret = -1;
@@ -208,14 +199,7 @@ libxlCapsInitHost(libxl_ctx *ctx, virCaps *caps)
goto cleanup;
}
- if ((ver_info = libxl_get_version_info(ctx)) == NULL) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("Failed to get version info from libxenlight"));
- goto cleanup;
- }
-
- version = (ver_info->xen_version_minor >= 7);
- if (libxlCapsInitCPU(caps, &phy_info, version) < 0)
+ if (libxlCapsInitCPU(caps, &phy_info) < 0)
goto cleanup;
if (virCapabilitiesSetNetPrefix(caps, LIBXL_GENERATED_PREFIX_XEN) < 0)
--
2.31.1
9:58 a.m.
New subject: [libvirt PATCH 3/6] libxl: capsInitCPU: fail if we can't initialize host features
Introduced by:
commit 17322e551861a5bc4dd464a7c5399204dc8c7caa
libxl: describe host cpu features based on hwcaps
with the justification that libxl_hwcaps does not have a stable
format across all version.
Even though the code would return '0' in the case of such failure,
it frees the 'cpu' pointer, while keeping it in caps->host.
Based on that, assume it does not happen in current usage.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/libxl/libxl_capabilities.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/src/libxl/libxl_capabilities.c b/src/libxl/libxl_capabilities.c
index d98109ea86..9d0ed921bd 100644
--- a/src/libxl/libxl_capabilities.c
+++ b/src/libxl/libxl_capabilities.c
@@ -166,16 +166,17 @@ libxlCapsInitCPU(virCaps *caps, libxl_physinfo *phy_info)
cpu->threads = phy_info->threads_per_core;
cpu->dies = 1;
cpu->sockets = phy_info->nr_cpus / (cpu->cores * cpu->threads);
- caps->host.cpu = cpu;
-
- ret = 0;
if (!(data = libxlCapsNodeData(cpu, phy_info->hw_cap)) ||
cpuDecode(cpu, data, NULL) < 0) {
- VIR_WARN("Failed to initialize host cpu features");
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Failed to initialize host cpu features"));
goto error;
}
+ caps->host.cpu = cpu;
+ ret = 0;
+
cleanup:
virCPUDataFree(data);
--
2.31.1
9:58 a.m.
New subject: [libvirt PATCH 4/6] libxl: refactor libxlCapsInitCPU
Use g_auto, g_steal_pointer and remove unnecessary labels.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/libxl/libxl_capabilities.c | 22 ++++++----------------
1 file changed, 6 insertions(+), 16 deletions(-)
diff --git a/src/libxl/libxl_capabilities.c b/src/libxl/libxl_capabilities.c
index 9d0ed921bd..38a9b36d8b 100644
--- a/src/libxl/libxl_capabilities.c
+++ b/src/libxl/libxl_capabilities.c
@@ -138,9 +138,8 @@ libxlCapsNodeData(virCPUDef *cpu, libxl_hwcap hwcap)
static int
libxlCapsInitCPU(virCaps *caps, libxl_physinfo *phy_info)
{
- virCPUData *data = NULL;
- virCPUDef *cpu = NULL;
- int ret = -1;
+ g_autoptr(virCPUData) data = NULL;
+ g_autoptr(virCPUDef) cpu = NULL;
int host_pae;
int host_lm;
@@ -153,7 +152,7 @@ libxlCapsInitCPU(virCaps *caps, libxl_physinfo *phy_info)
host_pae = phy_info->hw_cap[0] & LIBXL_X86_FEATURE_PAE_MASK;
if (host_pae &&
virCapabilitiesAddHostFeature(caps, "pae") < 0)
- goto error;
+ return -1;
host_lm = (phy_info->hw_cap[2] & LIBXL_X86_FEATURE_LM_MASK);
if (host_lm)
@@ -171,20 +170,11 @@ libxlCapsInitCPU(virCaps *caps, libxl_physinfo *phy_info)
cpuDecode(cpu, data, NULL) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Failed to initialize host cpu features"));
- goto error;
+ return -1;
}
- caps->host.cpu = cpu;
- ret = 0;
-
- cleanup:
- virCPUDataFree(data);
-
- return ret;
-
- error:
- virCPUDefFree(cpu);
- goto cleanup;
+ caps->host.cpu = g_steal_pointer(&cpu);
+ return 0;
}
static int
--
2.31.1
9:58 a.m.
New subject: [libvirt PATCH 5/6] libxl: use g_auto in libxlCapsNodeData
Also remove pointless labels.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/libxl/libxl_capabilities.c | 12 ++++--------
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/src/libxl/libxl_capabilities.c b/src/libxl/libxl_capabilities.c
index 38a9b36d8b..ef77bec7a0 100644
--- a/src/libxl/libxl_capabilities.c
+++ b/src/libxl/libxl_capabilities.c
@@ -104,7 +104,7 @@ static virCPUData *
libxlCapsNodeData(virCPUDef *cpu, libxl_hwcap hwcap)
{
ssize_t ncaps;
- virCPUData *cpudata = NULL;
+ g_autoptr(virCPUData) cpudata = NULL;
virCPUx86CPUID cpuid[] = {
{ .eax_in = 0x00000001, .edx = hwcap[0] },
{ .eax_in = 0x00000001, .ecx = hwcap[1] },
@@ -117,17 +117,13 @@ libxlCapsNodeData(virCPUDef *cpu, libxl_hwcap hwcap)
};
if (!(cpudata = virCPUDataNew(cpu->arch)))
- goto error;
+ return NULL;
ncaps = G_N_ELEMENTS(cpuid);
if (libxlCapsAddCPUID(cpudata, cpuid, ncaps) < 0)
- goto error;
+ return NULL;
- return cpudata;
-
- error:
- virCPUDataFree(cpudata);
- return NULL;
+ return g_steal_pointer(&cpudata);
}
/* hw_caps is an array of 32-bit words whose meaning is listed in
--
2.31.1
9:58 a.m.
New subject: [libvirt PATCH 6/6] libxl: use g_auto in libxlDomainGetEmulatorType
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/libxl/libxl_capabilities.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/src/libxl/libxl_capabilities.c b/src/libxl/libxl_capabilities.c
index ef77bec7a0..a516910215 100644
--- a/src/libxl/libxl_capabilities.c
+++ b/src/libxl/libxl_capabilities.c
@@ -715,13 +715,13 @@ int
libxlDomainGetEmulatorType(const virDomainDef *def)
{
int ret = LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN;
- virCommand *cmd = NULL;
+ g_autoptr(virCommand) cmd = NULL;
g_autofree char *output = NULL;
if (def->os.type == VIR_DOMAIN_OSTYPE_HVM) {
if (def->emulator) {
if (!virFileExists(def->emulator))
- goto cleanup;
+ return ret;
cmd = virCommandNew(def->emulator);
@@ -729,14 +729,12 @@ libxlDomainGetEmulatorType(const virDomainDef *def)
virCommandSetOutputBuffer(cmd, &output);
if (virCommandRun(cmd, NULL) < 0)
- goto cleanup;
+ return ret;
if (strstr(output, LIBXL_QEMU_DM_STR))
ret = LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL;
}
}
- cleanup:
- virCommandFree(cmd);
return ret;
}
--
2.31.1
6:33 p.m.
On 9/6/21 08:58, Ján Tomko wrote:
While looking at users of virCPU.*Free, I found some code
that is supposed to deal with Xen < 4.7.
The minimum version is 4.9 since the following commit:
commit 68940b3fb3c43b8aa03cb6fd2f1d00b1737c9b2c
Author: Jim Fehlig <jfehlig(a)suse.com>
CommitDate: 2021-06-17 10:11:56 -0600
Xen: Bump minimum supported Xen version to 4.9
Remove the code dealing with the old version and use g_auto moer.
Ján Tomko (6):
libxl: capabilities: assume Xen version >= 4.7
libxl: remove enum libxlHwcapVersion
libxl: capsInitCPU: fail if we can't initialize host features
libxl: refactor libxlCapsInitCPU
libxl: use g_auto in libxlCapsNodeData
libxl: use g_auto in libxlDomainGetEmulatorType
src/libxl/libxl_capabilities.c | 119 ++++++++++-----------------------
1 file changed, 37 insertions(+), 82 deletions(-)
Reviewed-by: Jim Fehlig <jfehlig(a)suse.com>
Unrelated to anything in this series, but while testing the patches I noticed
odd values for the 'die_id' attribute in 'virsh capabilities'
<cpus num='24'>
<cpu id='0' socket_id='0' die_id='0' core_id='0'
siblings='0-1'/>
<cpu id='1' socket_id='0' die_id='-1073069552'
core_id='0' siblings='0-1'/>
<cpu id='2' socket_id='0' die_id='-1073741696'
core_id='1' siblings='2-3'/>
<cpu id='3' socket_id='0' die_id='-1073317232'
core_id='1' siblings='2-3'/>
<cpu id='4' socket_id='0' die_id='0' core_id='2'
siblings='4-5'/>
...
</cpus>
I see commit 7b79ee2f78b uses /sys/devices/system/cpu/cpu$num/topology/die_id to
determine the value. On my test machine, all
/sys/devices/system/cpu/cpu$num/topology/die_id files contain 0. I'll need to
dig a little deeper to determine why this doesn't work on xen.
Regards,
Jim
1206
days inactive
1207
days old
7 comments
2 participants
participants (2)
-
Jim Fehlig -
Ján Tomko