[libvirt] [PATCH] conf: Allocate structure for 'perf' events in virDomainDefNew
Some code paths already assume that it is allocated since it was always allocated by virDomainPerfDefParseXML. Move allocation to virDomainDefNew to handle cases that don't go through the XML parser. This fixes crash when attempting to connect to an existing process via virDomainQemuAttach. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1350688 --- src/conf/domain_conf.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ef266af..fb622cd 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2746,6 +2746,9 @@ virDomainDefNew(void) if (!(ret->numa = virDomainNumaNew())) goto error; + if (VIR_ALLOC(ret->perf) < 0) + goto error; + ret->mem.hard_limit = VIR_DOMAIN_MEMORY_PARAM_UNLIMITED; ret->mem.soft_limit = VIR_DOMAIN_MEMORY_PARAM_UNLIMITED; ret->mem.swap_hard_limit = VIR_DOMAIN_MEMORY_PARAM_UNLIMITED; @@ -13136,9 +13139,6 @@ virDomainPerfDefParseXML(virDomainDefPtr def, if ((n = virXPathNodeSet("./perf/event", ctxt, &nodes)) < 0) return n; - if (VIR_ALLOC(def->perf) < 0) - goto cleanup; - for (i = 0; i < n; i++) { if (virDomainPerfEventDefParseXML(def->perf, nodes[i]) < 0) goto cleanup; @@ -13147,8 +13147,6 @@ virDomainPerfDefParseXML(virDomainDefPtr def, ret = 0; cleanup: - if (ret < 0) - VIR_FREE(def->perf); VIR_FREE(nodes); return ret; } @@ -23388,8 +23386,7 @@ virDomainDefFormatInternal(virDomainDefPtr def, virBufferAddLit(buf, "</pm>\n"); } - if (def->perf) - virDomainPerfDefFormat(buf, def->perf); + virDomainPerfDefFormat(buf, def->perf); virBufferAddLit(buf, "<devices>\n"); virBufferAdjustIndent(buf, 2); -- 2.9.0
On Tue, Jun 28, 2016 at 02:43:59PM +0200, Peter Krempa wrote:
Some code paths already assume that it is allocated since it was always allocated by virDomainPerfDefParseXML. Move allocation to
Shouldn't we rather fix those code paths? virDomainDefNew would get ridiculously long if we started assuming this for every optional XML element. Jan
virDomainDefNew to handle cases that don't go through the XML parser.
This fixes crash when attempting to connect to an existing process via virDomainQemuAttach.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1350688 --- src/conf/domain_conf.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-)
On Wed, Jun 29, 2016 at 08:07:29 +0200, Ján Tomko wrote:
On Tue, Jun 28, 2016 at 02:43:59PM +0200, Peter Krempa wrote:
Some code paths already assume that it is allocated since it was always allocated by virDomainPerfDefParseXML. Move allocation to
Shouldn't we rather fix those code paths?
Having all the accessors deal with this lead to the crash in the first place. Additionally accessors will need to deal with that oo.
virDomainDefNew would get ridiculously long if we started assuming this for every optional XML element.
Isn't that the purpose of the allocation helper? Additionally most structs holding additional data are directly member of struct _virDomainDef. I'll make this too and get rid of the allocation completely if you think it's better that way.
participants (2)
-
Ján Tomko -
Peter Krempa