5:22 a.m.
This patchset contains Ken ICHIKAWA's patch to fix parsing of vendor_id (with
tweaked commit message) and my cleanups to pre-existing problems in the code
which I found while reviewing Ken's patch.
Ken ICHIKAWA (1):
conf: cpu: Fix parsing of vendor_id
Peter Krempa (3):
conf: cpu: Fix memory leak when specifying cpu vendor_id manually
conf: cpu: Refactor parsing of vendor_id and fallback attributes
conf: cpu: Break some long lines
src/conf/cpu_conf.c | 82 +++++++++++++++++++++++++----------------------------
1 file changed, 38 insertions(+), 44 deletions(-)
--
1.8.0.2
5:22 a.m.
New subject: [libvirt] [PATCH 1/4] conf: cpu: Fix parsing of vendor_id
From: Ken ICHIKAWA <ichikawa.ken(a)jp.fujitsu.com>
This patch fixes a problem that vendor_id attribute can not be defined
when fallback attribute is not defined.
If I define domain xml like below:
<domain>
<cpu>
<model vendor_id='aaaabbbbcccc'>core2duo</model>
</cpu>
</domain>
In dumpxml, vendor_id is not reflected:
<domain>
<cpu mode='custom' match='exact'>
<model fallback='allow'>core2duo</model>
</cpu>
</domain>
The expected output is:
<domain>
<cpu mode='custom' match='exact'>
<model fallback='allow'
vendor_id='aaaabbbbcccc'>core2duo</model>
</cpu>
</domain>
If the fallback attribute and vendor_id attribute is defined at the same
time, it's reflected as expected.
Signed-off-by: Ken ICHIKAWA <ichikawa.ken(a)jp.fujitsu.com>
---
src/conf/cpu_conf.c | 42 +++++++++++++++++++++---------------------
1 file changed, 21 insertions(+), 21 deletions(-)
diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c
index 8cb54a3..6157ed7 100644
--- a/src/conf/cpu_conf.c
+++ b/src/conf/cpu_conf.c
@@ -300,32 +300,32 @@ virCPUDefParseXML(const xmlNodePtr node,
goto error;
}
}
+ }
- if (virXPathBoolean("boolean(./model[1]/@vendor_id)", ctxt)) {
- char *vendor_id;
-
- vendor_id = virXPathString("string(./model[1]/@vendor_id)",
- ctxt);
- if (!vendor_id ||
- strlen(vendor_id) != VIR_CPU_VENDOR_ID_LENGTH) {
- virReportError(VIR_ERR_XML_ERROR,
- _("vendor_id must be exactly"
- " %d characters long"),
- VIR_CPU_VENDOR_ID_LENGTH);
+ if (virXPathBoolean("boolean(./model[1]/@vendor_id)", ctxt)) {
+ char *vendor_id;
+
+ vendor_id = virXPathString("string(./model[1]/@vendor_id)",
+ ctxt);
+ if (!vendor_id ||
+ strlen(vendor_id) != VIR_CPU_VENDOR_ID_LENGTH) {
+ virReportError(VIR_ERR_XML_ERROR,
+ _("vendor_id must be exactly"
+ " %d characters long"),
+ VIR_CPU_VENDOR_ID_LENGTH);
+ VIR_FREE(vendor_id);
+ goto error;
+ }
+ /* ensure that the string can be passed to qemu*/
+ for (i = 0; i < strlen(vendor_id); i++) {
+ if (vendor_id[i]==',') {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("vendor id is invalid"));
VIR_FREE(vendor_id);
goto error;
}
- /* ensure that the string can be passed to qemu*/
- for (i = 0; i < strlen(vendor_id); i++) {
- if (vendor_id[i]==',') {
- virReportError(VIR_ERR_XML_ERROR, "%s",
- _("vendor id is invalid"));
- VIR_FREE(vendor_id);
- goto error;
- }
- }
- def->vendor_id = vendor_id;
}
+ def->vendor_id = vendor_id;
}
}
--
1.8.0.2
8:55 a.m.
New subject: [libvirt] [PATCH 1/4] conf: cpu: Fix parsing of vendor_id
On 12/17/2012 12:22 PM, Peter Krempa wrote:
From: Ken ICHIKAWA <ichikawa.ken(a)jp.fujitsu.com>
This patch fixes a problem that vendor_id attribute can not be defined
when fallback attribute is not defined.
If I define domain xml like below:
<domain>
<cpu>
<model vendor_id='aaaabbbbcccc'>core2duo</model>
</cpu>
</domain>
In dumpxml, vendor_id is not reflected:
<domain>
<cpu mode='custom' match='exact'>
<model fallback='allow'>core2duo</model>
</cpu>
</domain>
The expected output is:
<domain>
<cpu mode='custom' match='exact'>
<model fallback='allow'
vendor_id='aaaabbbbcccc'>core2duo</model>
</cpu>
</domain>
If the fallback attribute and vendor_id attribute is defined at the same
time, it's reflected as expected.
Signed-off-by: Ken ICHIKAWA <ichikawa.ken(a)jp.fujitsu.com>
---
src/conf/cpu_conf.c | 42 +++++++++++++++++++++---------------------
1 file changed, 21 insertions(+), 21 deletions(-)
diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c
index 8cb54a3..6157ed7 100644
--- a/src/conf/cpu_conf.c
+++ b/src/conf/cpu_conf.c
@@ -300,32 +300,32 @@ virCPUDefParseXML(const xmlNodePtr node,
goto error;
}
}
+ }
- if (virXPathBoolean("boolean(./model[1]/@vendor_id)", ctxt)) {
- char *vendor_id;
-
- vendor_id = virXPathString("string(./model[1]/@vendor_id)",
- ctxt);
- if (!vendor_id ||
- strlen(vendor_id) != VIR_CPU_VENDOR_ID_LENGTH) {
- virReportError(VIR_ERR_XML_ERROR,
- _("vendor_id must be exactly"
- " %d characters long"),
- VIR_CPU_VENDOR_ID_LENGTH);
+ if (virXPathBoolean("boolean(./model[1]/@vendor_id)", ctxt)) {
+ char *vendor_id;
+
+ vendor_id = virXPathString("string(./model[1]/@vendor_id)",
+ ctxt);
+ if (!vendor_id ||
+ strlen(vendor_id) != VIR_CPU_VENDOR_ID_LENGTH) {
+ virReportError(VIR_ERR_XML_ERROR,
+ _("vendor_id must be exactly"
+ " %d characters long"),
+ VIR_CPU_VENDOR_ID_LENGTH);
+ VIR_FREE(vendor_id);
+ goto error;
+ }
+ /* ensure that the string can be passed to qemu*/
+ for (i = 0; i < strlen(vendor_id); i++) {
+ if (vendor_id[i]==',') {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("vendor id is invalid"));
VIR_FREE(vendor_id);
goto error;
}
- /* ensure that the string can be passed to qemu*/
- for (i = 0; i < strlen(vendor_id); i++) {
- if (vendor_id[i]==',') {
- virReportError(VIR_ERR_XML_ERROR, "%s",
- _("vendor id is invalid"));
- VIR_FREE(vendor_id);
- goto error;
- }
- }
- def->vendor_id = vendor_id;
}
+ def->vendor_id = vendor_id;
}
}
ACK thanks to the fact that you are cleaning it up in next patches.
Martin
5:22 a.m.
New subject: [libvirt] [PATCH 2/4] conf: cpu: Fix memory leak when specifying cpu vendor_id manually
The field was not freed from the cpu definition.
---
src/conf/cpu_conf.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c
index 6157ed7..7528980 100644
--- a/src/conf/cpu_conf.c
+++ b/src/conf/cpu_conf.c
@@ -87,6 +87,7 @@ virCPUDefFree(virCPUDefPtr def)
VIR_FREE(def->cells[i].cpustr);
}
VIR_FREE(def->cells);
+ VIR_FREE(def->vendor_id);
VIR_FREE(def);
}
--
1.8.0.2
8:55 a.m.
New subject: [libvirt] [PATCH 2/4] conf: cpu: Fix memory leak when specifying cpu vendor_id manually
On 12/17/2012 12:22 PM, Peter Krempa wrote:
The field was not freed from the cpu definition.
---
src/conf/cpu_conf.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c
index 6157ed7..7528980 100644
--- a/src/conf/cpu_conf.c
+++ b/src/conf/cpu_conf.c
@@ -87,6 +87,7 @@ virCPUDefFree(virCPUDefPtr def)
VIR_FREE(def->cells[i].cpustr);
}
VIR_FREE(def->cells);
+ VIR_FREE(def->vendor_id);
VIR_FREE(def);
}
ACK,
Martin
5:22 a.m.
New subject: [libvirt] [PATCH 3/4] conf: cpu: Refactor parsing of vendor_id and fallback attributes
This patch simplifies the code that parses the fallback and vendor_id
attributes from the domain xml cpu definition.
Changes done:
- free temp variables in the cleanup section instead of local use
- remove checking for presence of the attribute to directly getting the
value (saving call to virXPathBoolean)
- remove loop used to check for ',' in the vendor_id string with strchr
---
src/conf/cpu_conf.c | 43 +++++++++++++++++--------------------------
1 file changed, 17 insertions(+), 26 deletions(-)
diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c
index 7528980..e5695f4 100644
--- a/src/conf/cpu_conf.c
+++ b/src/conf/cpu_conf.c
@@ -195,6 +195,8 @@ virCPUDefParseXML(const xmlNodePtr node,
int n;
unsigned int i;
char *cpuMode;
+ char *fallback = NULL;
+ char *vendor_id = NULL;
if (!xmlStrEqual(node->name, BAD_CAST "cpu")) {
virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -288,45 +290,32 @@ virCPUDefParseXML(const xmlNodePtr node,
if (def->type == VIR_CPU_TYPE_GUEST &&
def->mode != VIR_CPU_MODE_HOST_PASSTHROUGH) {
- if (virXPathBoolean("boolean(./model[1]/@fallback)", ctxt)) {
- const char *fallback;
-
- fallback = virXPathString("string(./model[1]/@fallback)", ctxt);
- if (fallback) {
- def->fallback = virCPUFallbackTypeFromString(fallback);
- VIR_FREE(fallback);
- if (def->fallback < 0) {
- virReportError(VIR_ERR_XML_ERROR, "%s",
- _("Invalid fallback attribute"));
- goto error;
- }
+ if ((fallback = virXPathString("string(./model[1]/@fallback)", ctxt)))
{
+ if ((def->fallback = virCPUFallbackTypeFromString(fallback)) < 0) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("Invalid fallback attribute"));
+ goto error;
}
}
- if (virXPathBoolean("boolean(./model[1]/@vendor_id)", ctxt)) {
- char *vendor_id;
-
- vendor_id = virXPathString("string(./model[1]/@vendor_id)",
- ctxt);
- if (!vendor_id ||
- strlen(vendor_id) != VIR_CPU_VENDOR_ID_LENGTH) {
+ if ((vendor_id = virXPathString("string(./model[1]/@vendor_id)",
+ ctxt))) {
+ if (strlen(vendor_id) != VIR_CPU_VENDOR_ID_LENGTH) {
virReportError(VIR_ERR_XML_ERROR,
- _("vendor_id must be exactly"
- " %d characters long"),
+ _("vendor_id must be exactly %d characters
long"),
VIR_CPU_VENDOR_ID_LENGTH);
- VIR_FREE(vendor_id);
goto error;
}
+
/* ensure that the string can be passed to qemu*/
- for (i = 0; i < strlen(vendor_id); i++) {
- if (vendor_id[i]==',') {
+ if (strchr(vendor_id, ',')) {
virReportError(VIR_ERR_XML_ERROR, "%s",
_("vendor id is invalid"));
- VIR_FREE(vendor_id);
goto error;
- }
}
+
def->vendor_id = vendor_id;
+ vendor_id = NULL;
}
}
@@ -490,6 +479,8 @@ virCPUDefParseXML(const xmlNodePtr node,
}
cleanup:
+ VIR_FREE(fallback);
+ VIR_FREE(vendor_id);
VIR_FREE(nodes);
return def;
--
1.8.0.2
8:58 a.m.
New subject: [libvirt] [PATCH 3/4] conf: cpu: Refactor parsing of vendor_id and fallback attributes
On 12/17/2012 12:22 PM, Peter Krempa wrote:
This patch simplifies the code that parses the fallback and
vendor_id
attributes from the domain xml cpu definition.
Changes done:
- free temp variables in the cleanup section instead of local use
- remove checking for presence of the attribute to directly getting the
value (saving call to virXPathBoolean)
- remove loop used to check for ',' in the vendor_id string with strchr
s/remove/replace/
ACK,
Martin
5:22 a.m.
New subject: [libvirt] [PATCH 4/4] conf: cpu: Break some long lines
---
src/conf/cpu_conf.c | 22 ++++++++++++----------
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c
index e5695f4..41c5535 100644
--- a/src/conf/cpu_conf.c
+++ b/src/conf/cpu_conf.c
@@ -264,8 +264,9 @@ virCPUDefParseXML(const xmlNodePtr node,
VIR_FREE(match);
if (def->match < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- "%s", _("Invalid match attribute for CPU
specification"));
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Invalid match attribute for CPU "
+ "specification"));
goto error;
}
}
@@ -333,8 +334,8 @@ virCPUDefParseXML(const xmlNodePtr node,
ret = virXPathULong("string(./topology[1]/@sockets)",
ctxt, &ul);
if (ret < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- "%s", _("Missing 'sockets' attribute in
CPU topology"));
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Missing 'sockets' attribute in CPU
topology"));
goto error;
}
def->sockets = (unsigned int) ul;
@@ -342,8 +343,8 @@ virCPUDefParseXML(const xmlNodePtr node,
ret = virXPathULong("string(./topology[1]/@cores)",
ctxt, &ul);
if (ret < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- "%s", _("Missing 'cores' attribute in
CPU topology"));
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Missing 'cores' attribute in CPU
topology"));
goto error;
}
def->cores = (unsigned int) ul;
@@ -351,8 +352,8 @@ virCPUDefParseXML(const xmlNodePtr node,
ret = virXPathULong("string(./topology[1]/@threads)",
ctxt, &ul);
if (ret < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- "%s", _("Missing 'threads' attribute in
CPU topology"));
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Missing 'threads' attribute in CPU
topology"));
goto error;
}
def->threads = (unsigned int) ul;
@@ -370,8 +371,9 @@ virCPUDefParseXML(const xmlNodePtr node,
if (n > 0) {
if (!def->model) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- "%s", _("Non-empty feature list specified
without CPU model"));
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Non-empty feature list specified without "
+ "CPU model"));
goto error;
}
--
1.8.0.2
4393
days inactive
4393
days old
8 comments
2 participants
participants (2)
-
Martin Kletzander -
Peter Krempa