3:17 a.m.
Peter Krempa (2):
qemu: capabilities: Drop unused function virQEMUCapsGetMachineTypes
qemu: caps: Sanitize storage of machine type related data
src/qemu/qemu_capabilities.c | 125 ++++++++++++++++---------------------------
src/qemu/qemu_capabilities.h | 2 -
2 files changed, 46 insertions(+), 81 deletions(-)
--
2.9.2
3:17 a.m.
New subject: [libvirt] [PATCH 1/2] qemu: capabilities: Drop unused function virQEMUCapsGetMachineTypes
---
src/qemu/qemu_capabilities.c | 8 --------
src/qemu/qemu_capabilities.h | 2 --
2 files changed, 10 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index d5b73e6..f8e2c55 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -2240,14 +2240,6 @@ size_t virQEMUCapsGetCPUDefinitions(virQEMUCapsPtr qemuCaps,
}
-size_t virQEMUCapsGetMachineTypes(virQEMUCapsPtr qemuCaps,
- char ***names)
-{
- if (names)
- *names = qemuCaps->machineTypes;
- return qemuCaps->nmachineTypes;
-}
-
int virQEMUCapsGetMachineTypesCaps(virQEMUCapsPtr qemuCaps,
size_t *nmachines,
virCapsGuestMachinePtr **machines)
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index bd5c6d9..72e763a 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -419,8 +419,6 @@ int virQEMUCapsAddCPUDefinition(virQEMUCapsPtr qemuCaps,
const char *name);
size_t virQEMUCapsGetCPUDefinitions(virQEMUCapsPtr qemuCaps,
char ***names);
-size_t virQEMUCapsGetMachineTypes(virQEMUCapsPtr qemuCaps,
- char ***names);
const char *virQEMUCapsGetCanonicalMachine(virQEMUCapsPtr qemuCaps,
const char *name);
int virQEMUCapsGetMachineMaxCpus(virQEMUCapsPtr qemuCaps,
--
2.9.2
3:17 a.m.
New subject: [libvirt] [PATCH 2/2] qemu: caps: Sanitize storage of machine type related data
Add a structure to store the data and use a single array of the
structures rather than having 3 separate arrays with shared indexes.
---
src/qemu/qemu_capabilities.c | 117 +++++++++++++++++--------------------------
1 file changed, 46 insertions(+), 71 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index f8e2c55..23d4a65 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -342,6 +342,11 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
);
+struct virQEMUCapsMachineType {
+ char *name;
+ char *alias;
+ unsigned int maxCpus;
+};
/*
* Update the XML parser/formatter when adding more
* information to this struct so that it gets cached
@@ -369,9 +374,7 @@ struct _virQEMUCaps {
char **cpuDefinitions;
size_t nmachineTypes;
- char **machineTypes;
- char **machineAliases;
- unsigned int *machineMaxCpus;
+ struct virQEMUCapsMachineType *machineTypes;
size_t ngicCapabilities;
virGICCapability *gicCapabilities;
@@ -473,22 +476,13 @@ static void
virQEMUCapsSetDefaultMachine(virQEMUCapsPtr qemuCaps,
size_t defIdx)
{
- char *name = qemuCaps->machineTypes[defIdx];
- char *alias = qemuCaps->machineAliases[defIdx];
- unsigned int maxCpus = qemuCaps->machineMaxCpus[defIdx];
+ struct virQEMUCapsMachineType tmp = qemuCaps->machineTypes[defIdx];
memmove(qemuCaps->machineTypes + 1,
qemuCaps->machineTypes,
sizeof(qemuCaps->machineTypes[0]) * defIdx);
- memmove(qemuCaps->machineAliases + 1,
- qemuCaps->machineAliases,
- sizeof(qemuCaps->machineAliases[0]) * defIdx);
- memmove(qemuCaps->machineMaxCpus + 1,
- qemuCaps->machineMaxCpus,
- sizeof(qemuCaps->machineMaxCpus[0]) * defIdx);
- qemuCaps->machineTypes[0] = name;
- qemuCaps->machineAliases[0] = alias;
- qemuCaps->machineMaxCpus[0] = maxCpus;
+
+ qemuCaps->machineTypes[0] = tmp;
}
/* Format is:
@@ -536,23 +530,21 @@ virQEMUCapsParseMachineTypesStr(const char *output,
}
}
- if (VIR_REALLOC_N(qemuCaps->machineTypes, qemuCaps->nmachineTypes + 1) <
0 ||
- VIR_REALLOC_N(qemuCaps->machineAliases, qemuCaps->nmachineTypes + 1)
< 0 ||
- VIR_REALLOC_N(qemuCaps->machineMaxCpus, qemuCaps->nmachineTypes + 1)
< 0) {
+ if (VIR_REALLOC_N(qemuCaps->machineTypes, qemuCaps->nmachineTypes + 1) <
0) {
VIR_FREE(name);
VIR_FREE(canonical);
return -1;
}
qemuCaps->nmachineTypes++;
if (canonical) {
- qemuCaps->machineTypes[qemuCaps->nmachineTypes-1] = canonical;
- qemuCaps->machineAliases[qemuCaps->nmachineTypes-1] = name;
+ qemuCaps->machineTypes[qemuCaps->nmachineTypes-1].name = canonical;
+ qemuCaps->machineTypes[qemuCaps->nmachineTypes-1].alias = name;
} else {
- qemuCaps->machineTypes[qemuCaps->nmachineTypes-1] = name;
- qemuCaps->machineAliases[qemuCaps->nmachineTypes-1] = NULL;
+ qemuCaps->machineTypes[qemuCaps->nmachineTypes-1].name = name;
+ qemuCaps->machineTypes[qemuCaps->nmachineTypes-1].alias = NULL;
}
/* When parsing from command line we don't have information about maxCpus */
- qemuCaps->machineMaxCpus[qemuCaps->nmachineTypes-1] = 0;
+ qemuCaps->machineTypes[qemuCaps->nmachineTypes-1].maxCpus = 0;
} while ((p = next));
@@ -2042,16 +2034,12 @@ virQEMUCapsPtr virQEMUCapsNewCopy(virQEMUCapsPtr qemuCaps)
if (VIR_ALLOC_N(ret->machineTypes, qemuCaps->nmachineTypes) < 0)
goto error;
- if (VIR_ALLOC_N(ret->machineAliases, qemuCaps->nmachineTypes) < 0)
- goto error;
- if (VIR_ALLOC_N(ret->machineMaxCpus, qemuCaps->nmachineTypes) < 0)
- goto error;
ret->nmachineTypes = qemuCaps->nmachineTypes;
for (i = 0; i < qemuCaps->nmachineTypes; i++) {
- if (VIR_STRDUP(ret->machineTypes[i], qemuCaps->machineTypes[i]) < 0 ||
- VIR_STRDUP(ret->machineAliases[i], qemuCaps->machineAliases[i]) <
0)
+ if (VIR_STRDUP(ret->machineTypes[i].name, qemuCaps->machineTypes[i].name)
< 0 ||
+ VIR_STRDUP(ret->machineTypes[i].alias, qemuCaps->machineTypes[i].alias)
< 0)
goto error;
- ret->machineMaxCpus[i] = qemuCaps->machineMaxCpus[i];
+ ret->machineTypes[i].maxCpus = qemuCaps->machineTypes[i].maxCpus;
}
return ret;
@@ -2068,12 +2056,10 @@ void virQEMUCapsDispose(void *obj)
size_t i;
for (i = 0; i < qemuCaps->nmachineTypes; i++) {
- VIR_FREE(qemuCaps->machineTypes[i]);
- VIR_FREE(qemuCaps->machineAliases[i]);
+ VIR_FREE(qemuCaps->machineTypes[i].name);
+ VIR_FREE(qemuCaps->machineTypes[i].alias);
}
VIR_FREE(qemuCaps->machineTypes);
- VIR_FREE(qemuCaps->machineAliases);
- VIR_FREE(qemuCaps->machineMaxCpus);
for (i = 0; i < qemuCaps->ncpuDefinitions; i++)
VIR_FREE(qemuCaps->cpuDefinitions[i]);
@@ -2258,15 +2244,15 @@ int virQEMUCapsGetMachineTypesCaps(virQEMUCapsPtr qemuCaps,
if (VIR_ALLOC(mach) < 0)
goto error;
(*machines)[i] = mach;
- if (qemuCaps->machineAliases[i]) {
- if (VIR_STRDUP(mach->name, qemuCaps->machineAliases[i]) < 0 ||
- VIR_STRDUP(mach->canonical, qemuCaps->machineTypes[i]) < 0)
+ if (qemuCaps->machineTypes[i].alias) {
+ if (VIR_STRDUP(mach->name, qemuCaps->machineTypes[i].alias) < 0 ||
+ VIR_STRDUP(mach->canonical, qemuCaps->machineTypes[i].name) <
0)
goto error;
} else {
- if (VIR_STRDUP(mach->name, qemuCaps->machineTypes[i]) < 0)
+ if (VIR_STRDUP(mach->name, qemuCaps->machineTypes[i].name) < 0)
goto error;
}
- mach->maxCpus = qemuCaps->machineMaxCpus[i];
+ mach->maxCpus = qemuCaps->machineTypes[i].maxCpus;
}
/* Make sure all canonical machine types also have their own entry so that
@@ -2328,10 +2314,10 @@ const char *virQEMUCapsGetCanonicalMachine(virQEMUCapsPtr
qemuCaps,
return NULL;
for (i = 0; i < qemuCaps->nmachineTypes; i++) {
- if (!qemuCaps->machineAliases[i])
+ if (!qemuCaps->machineTypes[i].alias)
continue;
- if (STREQ(qemuCaps->machineAliases[i], name))
- return qemuCaps->machineTypes[i];
+ if (STREQ(qemuCaps->machineTypes[i].alias, name))
+ return qemuCaps->machineTypes[i].name;
}
return name;
@@ -2347,10 +2333,10 @@ int virQEMUCapsGetMachineMaxCpus(virQEMUCapsPtr qemuCaps,
return 0;
for (i = 0; i < qemuCaps->nmachineTypes; i++) {
- if (!qemuCaps->machineMaxCpus[i])
+ if (!qemuCaps->machineTypes[i].maxCpus)
continue;
- if (STREQ(qemuCaps->machineTypes[i], name))
- return qemuCaps->machineMaxCpus[i];
+ if (STREQ(qemuCaps->machineTypes[i].name, name))
+ return qemuCaps->machineTypes[i].maxCpus;
}
return 0;
@@ -2498,23 +2484,19 @@ virQEMUCapsProbeQMPMachineTypes(virQEMUCapsPtr qemuCaps,
if (VIR_ALLOC_N(qemuCaps->machineTypes, nmachines) < 0)
goto cleanup;
- if (VIR_ALLOC_N(qemuCaps->machineAliases, nmachines) < 0)
- goto cleanup;
- if (VIR_ALLOC_N(qemuCaps->machineMaxCpus, nmachines) < 0)
- goto cleanup;
for (i = 0; i < nmachines; i++) {
if (STREQ(machines[i]->name, "none"))
continue;
qemuCaps->nmachineTypes++;
- if (VIR_STRDUP(qemuCaps->machineAliases[qemuCaps->nmachineTypes -1],
+ if (VIR_STRDUP(qemuCaps->machineTypes[qemuCaps->nmachineTypes -1].alias,
machines[i]->alias) < 0 ||
- VIR_STRDUP(qemuCaps->machineTypes[qemuCaps->nmachineTypes - 1],
+ VIR_STRDUP(qemuCaps->machineTypes[qemuCaps->nmachineTypes - 1].name,
machines[i]->name) < 0)
goto cleanup;
if (machines[i]->isDefault)
defIdx = qemuCaps->nmachineTypes - 1;
- qemuCaps->machineMaxCpus[qemuCaps->nmachineTypes - 1] =
+ qemuCaps->machineTypes[qemuCaps->nmachineTypes - 1].maxCpus =
machines[i]->maxCpus;
}
@@ -2909,25 +2891,20 @@ virQEMUCapsLoadCache(virQEMUCapsPtr qemuCaps, const char
*filename,
}
if (n > 0) {
qemuCaps->nmachineTypes = n;
- if (VIR_ALLOC_N(qemuCaps->machineTypes,
- qemuCaps->nmachineTypes) < 0 ||
- VIR_ALLOC_N(qemuCaps->machineAliases,
- qemuCaps->nmachineTypes) < 0 ||
- VIR_ALLOC_N(qemuCaps->machineMaxCpus,
- qemuCaps->nmachineTypes) < 0)
+ if (VIR_ALLOC_N(qemuCaps->machineTypes, qemuCaps->nmachineTypes) < 0)
goto cleanup;
for (i = 0; i < n; i++) {
- if (!(qemuCaps->machineTypes[i] = virXMLPropString(nodes[i],
"name"))) {
+ if (!(qemuCaps->machineTypes[i].name = virXMLPropString(nodes[i],
"name"))) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("missing machine name in QEMU capabilities
cache"));
goto cleanup;
}
- qemuCaps->machineAliases[i] = virXMLPropString(nodes[i],
"alias");
+ qemuCaps->machineTypes[i].alias = virXMLPropString(nodes[i],
"alias");
str = virXMLPropString(nodes[i], "maxCpus");
if (str &&
- virStrToLong_ui(str, NULL, 10, &(qemuCaps->machineMaxCpus[i]))
< 0) {
+ virStrToLong_ui(str, NULL, 10,
&(qemuCaps->machineTypes[i].maxCpus)) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("malformed machine cpu count in QEMU capabilities
cache"));
goto cleanup;
@@ -3062,12 +3039,12 @@ virQEMUCapsFormatCache(virQEMUCapsPtr qemuCaps,
for (i = 0; i < qemuCaps->nmachineTypes; i++) {
virBufferEscapeString(&buf, "<machine name='%s'",
- qemuCaps->machineTypes[i]);
- if (qemuCaps->machineAliases[i])
+ qemuCaps->machineTypes[i].name);
+ if (qemuCaps->machineTypes[i].alias)
virBufferEscapeString(&buf, " alias='%s'",
- qemuCaps->machineAliases[i]);
+ qemuCaps->machineTypes[i].alias);
virBufferAsprintf(&buf, " maxCpus='%u'/>\n",
- qemuCaps->machineMaxCpus[i]);
+ qemuCaps->machineTypes[i].maxCpus);
}
for (i = 0; i < qemuCaps->ngicCapabilities; i++) {
@@ -3179,12 +3156,10 @@ virQEMUCapsReset(virQEMUCapsPtr qemuCaps)
qemuCaps->ncpuDefinitions = 0;
for (i = 0; i < qemuCaps->nmachineTypes; i++) {
- VIR_FREE(qemuCaps->machineTypes[i]);
- VIR_FREE(qemuCaps->machineAliases[i]);
+ VIR_FREE(qemuCaps->machineTypes[i].name);
+ VIR_FREE(qemuCaps->machineTypes[i].alias);
}
VIR_FREE(qemuCaps->machineTypes);
- VIR_FREE(qemuCaps->machineAliases);
- VIR_FREE(qemuCaps->machineMaxCpus);
qemuCaps->nmachineTypes = 0;
VIR_FREE(qemuCaps->gicCapabilities);
@@ -4050,7 +4025,7 @@ virQEMUCapsIsMachineSupported(virQEMUCapsPtr qemuCaps,
size_t i;
for (i = 0; i < qemuCaps->nmachineTypes; i++) {
- if (STREQ(canonical_machine, qemuCaps->machineTypes[i]))
+ if (STREQ(canonical_machine, qemuCaps->machineTypes[i].name))
return true;
}
return false;
@@ -4062,7 +4037,7 @@ virQEMUCapsGetDefaultMachine(virQEMUCapsPtr qemuCaps)
{
if (!qemuCaps->nmachineTypes)
return NULL;
- return qemuCaps->machineTypes[0];
+ return qemuCaps->machineTypes[0].name;
}
--
2.9.2
3:40 a.m.
New subject: [libvirt] [PATCH 3/2] qemu: cap: Refactor access to array in virQEMUCapsProbeQMPMachineTypes
Use a temporary pointer rather than always recalculating the index in a
very verbose way.
---
src/qemu/qemu_capabilities.c | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 23d4a65..c612960 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -2473,6 +2473,7 @@ static int
virQEMUCapsProbeQMPMachineTypes(virQEMUCapsPtr qemuCaps,
qemuMonitorPtr mon)
{
+ struct virQEMUCapsMachineType *mach;
qemuMonitorMachineInfoPtr *machines = NULL;
int nmachines = 0;
int ret = -1;
@@ -2488,16 +2489,17 @@ virQEMUCapsProbeQMPMachineTypes(virQEMUCapsPtr qemuCaps,
for (i = 0; i < nmachines; i++) {
if (STREQ(machines[i]->name, "none"))
continue;
- qemuCaps->nmachineTypes++;
- if (VIR_STRDUP(qemuCaps->machineTypes[qemuCaps->nmachineTypes -1].alias,
- machines[i]->alias) < 0 ||
- VIR_STRDUP(qemuCaps->machineTypes[qemuCaps->nmachineTypes - 1].name,
- machines[i]->name) < 0)
+
+ mach = &(qemuCaps->machineTypes[qemuCaps->nmachineTypes++]);
+
+ if (VIR_STRDUP(mach->alias, machines[i]->alias) < 0 ||
+ VIR_STRDUP(mach->name, machines[i]->name) < 0)
goto cleanup;
+
+ mach->maxCpus = machines[i]->maxCpus;
+
if (machines[i]->isDefault)
defIdx = qemuCaps->nmachineTypes - 1;
- qemuCaps->machineTypes[qemuCaps->nmachineTypes - 1].maxCpus =
- machines[i]->maxCpus;
}
if (defIdx)
--
2.9.2
2:33 a.m.
New subject: [libvirt] [PATCH 3/2] qemu: cap: Refactor access to array in virQEMUCapsProbeQMPMachineTypes
On Fri, Jul 29, 2016 at 10:40:57AM +0200, Peter Krempa wrote:
Use a temporary pointer rather than always recalculating the index in
a
very verbose way.
---
src/qemu/qemu_capabilities.c | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 23d4a65..c612960 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -2473,6 +2473,7 @@ static int
virQEMUCapsProbeQMPMachineTypes(virQEMUCapsPtr qemuCaps,
qemuMonitorPtr mon)
{
+ struct virQEMUCapsMachineType *mach;
Declaring this variable inside the for loop would be nicer, since it's
only used there.
Jan
2:34 a.m.
On Fri, Jul 29, 2016 at 10:17:19AM +0200, Peter Krempa wrote:
Peter Krempa (2):
qemu: capabilities: Drop unused function virQEMUCapsGetMachineTypes
qemu: caps: Sanitize storage of machine type related data
src/qemu/qemu_capabilities.c | 125 ++++++++++++++++---------------------------
src/qemu/qemu_capabilities.h | 2 -
2 files changed, 46 insertions(+), 81 deletions(-)
ACK to all three.
Jan
3034
days inactive
3037
days old
5 comments
2 participants
participants (2)
-
Ján Tomko -
Peter Krempa