[PATCH v5 0/4] delay timer_new from init to realize to fix memleaks.

[PATCH v5 0/4] delay timer_new from init to realize to fix memleaks.

[Pan Nengyuan]

This series delay timer_new from init into realize to avoid memleaks when we call 'device_list_properties'.
And do timer_free only in s390x_cpu_finalize because it's hotplugable. However, mos6522_realize is never called
at all due to the incorrect creation of it. So we fix the incorrect creation in mac_via/cuda/pmu first, then 
move the timer_new to mos6522_realize().

v1:
   - Delay timer_new() from init() to realize() to fix memleaks.
v2:
   - Similarly to other cleanups, move timer_new into realize in target/s390x/cpu.c (Suggested by Philippe Mathieu-Daudé).
   - Send these two patches as a series instead of send each as a single patch but with wrong subject in v1.
v3:
   - It's not valid in mos6522 if we move timer_new from init to realize, because it's never called at all.
     Thus, we remove null check in reset, and add calls to mos6522_realize() in mac_via_realize to make this move to be valid.
   - split patch by device to make it more clear.
v4:
   - Also do timer_free on the error path in realize() and fix some coding style. Then use device_class_set_parent_unrealize to declare unrealize.
   - split the mos6522 patch into two, one to fix incorrect creation of mos6522, the other to fix memleak.

v5: 
   - Fix two other places where we create mos6522's subclasses but forgot to realize it(macio/cuda,macio/pmu). 
     Otherwise, this will cause SEGVs during make check-qtest-ppc64.
   - Remove timer_del on the error path of s390x_cpu_realize() and simply use errp instead a temporary variable.

Pan Nengyuan (4):
  s390x: fix memleaks in cpu_finalize
  mac_via: fix incorrect creation of mos6522 device in mac_via
  hw/misc/macio: fix incorrect creation of mos6522's subclasses
  hw/misc/mos6522: move timer_new from init() into realize() to avoid
    memleaks

 hw/misc/mac_via.c      | 40 +++++++++++++++++++++++++++-------------
 hw/misc/macio/cuda.c   | 11 +++++++++--
 hw/misc/macio/pmu.c    | 11 +++++++++--
 hw/misc/mos6522.c      |  6 ++++++
 target/s390x/cpu-qom.h |  1 +
 target/s390x/cpu.c     | 30 ++++++++++++++++++++++++++----
 6 files changed, 78 insertions(+), 21 deletions(-)

-- 
2.18.2

[PATCH v5 1/4] s390x: fix memleaks in cpu_finalize

[Pan Nengyuan]

This patch fix memleaks when we call tests/qtest/cpu-plug-test on s390x. The leak stack is as follow:

Direct leak of 48 byte(s) in 1 object(s) allocated from:
    #0 0x7fb43c7cd970 in __interceptor_calloc (/lib64/libasan.so.5+0xef970)
    #1 0x7fb43be2149d in g_malloc0 (/lib64/libglib-2.0.so.0+0x5249d)
    #2 0x558ba96da716 in timer_new_full /mnt/sdb/qemu-new/qemu/include/qemu/timer.h:530
    #3 0x558ba96da716 in timer_new /mnt/sdb/qemu-new/qemu/include/qemu/timer.h:551
    #4 0x558ba96da716 in timer_new_ns /mnt/sdb/qemu-new/qemu/include/qemu/timer.h:569
    #5 0x558ba96da716 in s390_cpu_initfn /mnt/sdb/qemu-new/qemu/target/s390x/cpu.c:285
    #6 0x558ba9c969ab in object_init_with_type /mnt/sdb/qemu-new/qemu/qom/object.c:372
    #7 0x558ba9c9eb5f in object_initialize_with_type /mnt/sdb/qemu-new/qemu/qom/object.c:516
    #8 0x558ba9c9f053 in object_new_with_type /mnt/sdb/qemu-new/qemu/qom/object.c:684
    #9 0x558ba967ede6 in s390x_new_cpu /mnt/sdb/qemu-new/qemu/hw/s390x/s390-virtio-ccw.c:64
    #10 0x558ba99764b3 in hmp_cpu_add /mnt/sdb/qemu-new/qemu/hw/core/machine-hmp-cmds.c:57
    #11 0x558ba9b1c27f in handle_hmp_command /mnt/sdb/qemu-new/qemu/monitor/hmp.c:1082
    #12 0x558ba96c1b02 in qmp_human_monitor_command /mnt/sdb/qemu-new/qemu/monitor/misc.c:142

Reported-by: Euler Robot <euler.robot@huawei.com>
Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>
---
Cc: Richard Henderson <rth@twiddle.net>
Cc: David Hildenbrand <david@redhat.com>
Cc: Cornelia Huck <cohuck@redhat.com>
Cc: qemu-s390x@nongnu.org
---
v2->v1:
- Similarly to other cleanups, move timer_new into realize(Suggested by Philippe Mathieu-Daudé)
v3->v2:
- Also do the timer_free in unrealize, it seems balanced.
v4->v3:
- Also do timer_free on the error path in realize() and fix some coding style.
- Use device_class_set_parent_unrealize to declare unrealize.
v5->v4:
- remove timer_del on the error path of realize(), it's redundant. (Suggested by David Hildenbrand)
- Simply use errp instead a temporary variable. (Suggested by David Hildenbrand)
---
 target/s390x/cpu-qom.h |  1 +
 target/s390x/cpu.c     | 30 ++++++++++++++++++++++++++----
 2 files changed, 27 insertions(+), 4 deletions(-)

diff --git a/target/s390x/cpu-qom.h b/target/s390x/cpu-qom.h
index dbe5346ec9..af9ffed0d8 100644
--- a/target/s390x/cpu-qom.h
+++ b/target/s390x/cpu-qom.h
@@ -61,6 +61,7 @@ typedef struct S390CPUClass {
     const char *desc;
 
     DeviceRealize parent_realize;
+    DeviceUnrealize parent_unrealize;
     void (*parent_reset)(CPUState *cpu);
     void (*load_normal)(CPUState *cpu);
     void (*reset)(CPUState *cpu, cpu_reset_type type);
diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
index cf84d307c6..8ce38bf399 100644
--- a/target/s390x/cpu.c
+++ b/target/s390x/cpu.c
@@ -182,6 +182,12 @@ static void s390_cpu_realizefn(DeviceState *dev, Error **errp)
 #if !defined(CONFIG_USER_ONLY)
     MachineState *ms = MACHINE(qdev_get_machine());
     unsigned int max_cpus = ms->smp.max_cpus;
+
+    cpu->env.tod_timer =
+        timer_new_ns(QEMU_CLOCK_VIRTUAL, s390x_tod_timer, cpu);
+    cpu->env.cpu_timer =
+        timer_new_ns(QEMU_CLOCK_VIRTUAL, s390x_cpu_timer, cpu);
+
     if (cpu->env.core_id >= max_cpus) {
         error_setg(&err, "Unable to add CPU with core-id: %" PRIu32
                    ", maximum core-id: %d", cpu->env.core_id,
@@ -224,9 +230,27 @@ static void s390_cpu_realizefn(DeviceState *dev, Error **errp)
 
     scc->parent_realize(dev, &err);
 out:
+    timer_free(cpu->env.tod_timer);
+    timer_free(cpu->env.cpu_timer);
     error_propagate(errp, err);
 }
 
+static void s390_cpu_unrealizefn(DeviceState *dev, Error **errp)
+{
+    S390CPUClass *scc = S390_CPU_GET_CLASS(dev);
+
+#if !defined(CONFIG_USER_ONLY)
+    S390CPU *cpu = S390_CPU(dev);
+
+    timer_del(cpu->env.tod_timer);
+    timer_del(cpu->env.cpu_timer);
+    timer_free(cpu->env.tod_timer);
+    timer_free(cpu->env.cpu_timer);
+#endif
+
+    scc->parent_unrealize(dev, errp);
+}
+
 static GuestPanicInformation *s390_cpu_get_crash_info(CPUState *cs)
 {
     GuestPanicInformation *panic_info;
@@ -279,10 +303,6 @@ static void s390_cpu_initfn(Object *obj)
                         s390_cpu_get_crash_info_qom, NULL, NULL, NULL, NULL);
     s390_cpu_model_register_props(obj);
 #if !defined(CONFIG_USER_ONLY)
-    cpu->env.tod_timer =
-        timer_new_ns(QEMU_CLOCK_VIRTUAL, s390x_tod_timer, cpu);
-    cpu->env.cpu_timer =
-        timer_new_ns(QEMU_CLOCK_VIRTUAL, s390x_cpu_timer, cpu);
     s390_cpu_set_state(S390_CPU_STATE_STOPPED, cpu);
 #endif
 }
@@ -453,6 +473,8 @@ static void s390_cpu_class_init(ObjectClass *oc, void *data)
 
     device_class_set_parent_realize(dc, s390_cpu_realizefn,
                                     &scc->parent_realize);
+    device_class_set_parent_unrealize(dc, s390_cpu_unrealizefn,
+                                      &scc->parent_unrealize);
     device_class_set_props(dc, s390x_cpu_properties);
     dc->user_creatable = true;
 
-- 
2.18.2

[PATCH v5 2/4] mac_via: fix incorrect creation of mos6522 device in mac_via

[Pan Nengyuan]

This patch fix a bug in mac_via where it failed to actually realize devices it was using.
And move the init codes which inits the mos6522 objects and properties on them from realize()
into init(). However, we keep qdev_set_parent_bus in realize(), otherwise it will cause
device-introspect-test test fail. Then do the realize mos6522 device in the mac_vir_realize.

Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>
---
Cc: Laurent Vivier <laurent@vivier.eu>
Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
v4->v3:
- split v3 into two patches, this patch fix incorrect creation of mos6522, move inits and props
  from realize into init.
v5->v4:
- remove redundant code.
- As discussion in https://patchwork.kernel.org/patch/11421229/, we still keep 
  qdev_set_parent_bus in realize().
---
 hw/misc/mac_via.c | 40 +++++++++++++++++++++++++++-------------
 1 file changed, 27 insertions(+), 13 deletions(-)

diff --git a/hw/misc/mac_via.c b/hw/misc/mac_via.c
index b7d0012794..d208f0b18a 100644
--- a/hw/misc/mac_via.c
+++ b/hw/misc/mac_via.c
@@ -868,24 +868,24 @@ static void mac_via_reset(DeviceState *dev)
 static void mac_via_realize(DeviceState *dev, Error **errp)
 {
     MacVIAState *m = MAC_VIA(dev);
-    MOS6522State *ms;
     struct tm tm;
     int ret;
+    Error *err = NULL;
 
-    /* Init VIAs 1 and 2 */
-    sysbus_init_child_obj(OBJECT(dev), "via1", &m->mos6522_via1,
-                          sizeof(m->mos6522_via1), TYPE_MOS6522_Q800_VIA1);
+    qdev_set_parent_bus(DEVICE(&m->mos6522_via1), sysbus_get_default());
+    qdev_set_parent_bus(DEVICE(&m->mos6522_via2), sysbus_get_default());
 
-    sysbus_init_child_obj(OBJECT(dev), "via2", &m->mos6522_via2,
-                          sizeof(m->mos6522_via2), TYPE_MOS6522_Q800_VIA2);
+    object_property_set_bool(OBJECT(&m->mos6522_via1), true, "realized", &err);
+    if (err != NULL) {
+        error_propagate(errp, err);
+        return;
+    }
 
-    /* Pass through mos6522 output IRQs */
-    ms = MOS6522(&m->mos6522_via1);
-    object_property_add_alias(OBJECT(dev), "irq[0]", OBJECT(ms),
-                              SYSBUS_DEVICE_GPIO_IRQ "[0]", &error_abort);
-    ms = MOS6522(&m->mos6522_via2);
-    object_property_add_alias(OBJECT(dev), "irq[1]", OBJECT(ms),
-                              SYSBUS_DEVICE_GPIO_IRQ "[0]", &error_abort);
+    object_property_set_bool(OBJECT(&m->mos6522_via2), true, "realized", &err);
+    if (err != NULL) {
+        error_propagate(errp, err);
+        return;
+    }
 
     /* Pass through mos6522 input IRQs */
     qdev_pass_gpios(DEVICE(&m->mos6522_via1), dev, "via1-irq");
@@ -948,6 +948,20 @@ static void mac_via_init(Object *obj)
     /* ADB */
     qbus_create_inplace((BusState *)&m->adb_bus, sizeof(m->adb_bus),
                         TYPE_ADB_BUS, DEVICE(obj), "adb.0");
+
+    /* Init VIAs 1 and 2 */
+    object_initialize_child(OBJECT(m), "via1", &m->mos6522_via1,
+                            sizeof(m->mos6522_via1), TYPE_MOS6522_Q800_VIA1,
+                            &error_abort, NULL);
+    object_initialize_child(OBJECT(m), "via2", &m->mos6522_via2,
+                            sizeof(m->mos6522_via2), TYPE_MOS6522_Q800_VIA2,
+                            &error_abort, NULL);
+
+    /* Pass through mos6522 output IRQs */
+    object_property_add_alias(OBJECT(m), "irq[0]", OBJECT(&m->mos6522_via1),
+                              SYSBUS_DEVICE_GPIO_IRQ "[0]", &error_abort);
+    object_property_add_alias(OBJECT(m), "irq[1]", OBJECT(&m->mos6522_via2),
+                              SYSBUS_DEVICE_GPIO_IRQ "[0]", &error_abort);
 }
 
 static void postload_update_cb(void *opaque, int running, RunState state)
-- 
2.18.2

[PATCH v5 3/4] hw/misc/macio: fix incorrect creation of mos6522's subclasses

[Pan Nengyuan]

There are two other places where we create mos6522's subclasses but forgot to realize
it. This patch do the realize in these places to fix that.

Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>
---
Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Cc: David Gibson <david@gibson.dropbear.id.au>
Cc: qemu-ppc@nongnu.org
---
v5:
- Also fix incorrect creation of mos6522's subclasses on two other places.
  Not sure if there is a conversion plan, we still keep sysbus_init_child_obj in init()
  in this patch as it was.
---
 hw/misc/macio/cuda.c | 11 +++++++++--
 hw/misc/macio/pmu.c  | 11 +++++++++--
 2 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/hw/misc/macio/cuda.c b/hw/misc/macio/cuda.c
index e0cc0aac5d..ee780a8288 100644
--- a/hw/misc/macio/cuda.c
+++ b/hw/misc/macio/cuda.c
@@ -36,6 +36,7 @@
 #include "qemu/cutils.h"
 #include "qemu/log.h"
 #include "qemu/module.h"
+#include "qapi/error.h"
 #include "trace.h"
 
 /* Bits in B data register: all active low */
@@ -524,11 +525,17 @@ static void cuda_realize(DeviceState *dev, Error **errp)
     CUDAState *s = CUDA(dev);
     SysBusDevice *sbd;
     MOS6522State *ms;
-    DeviceState *d;
+    DeviceState *d = DEVICE(&s->mos6522_cuda);
     struct tm tm;
+    Error *err = NULL;
+
+    object_property_set_bool(OBJECT(d), true, "realized", &err);
+    if (err != NULL) {
+        error_propagate(errp, err);
+        return;
+    }
 
     /* Pass IRQ from 6522 */
-    d = DEVICE(&s->mos6522_cuda);
     ms = MOS6522(d);
     sbd = SYS_BUS_DEVICE(s);
     sysbus_pass_irq(sbd, SYS_BUS_DEVICE(ms));
diff --git a/hw/misc/macio/pmu.c b/hw/misc/macio/pmu.c
index b8466a4a3f..ae55992288 100644
--- a/hw/misc/macio/pmu.c
+++ b/hw/misc/macio/pmu.c
@@ -43,6 +43,7 @@
 #include "qemu/cutils.h"
 #include "qemu/log.h"
 #include "qemu/module.h"
+#include "qapi/error.h"
 #include "trace.h"
 
 
@@ -741,11 +742,17 @@ static void pmu_realize(DeviceState *dev, Error **errp)
     PMUState *s = VIA_PMU(dev);
     SysBusDevice *sbd;
     MOS6522State *ms;
-    DeviceState *d;
+    DeviceState *d = DEVICE(&s->mos6522_pmu);;
     struct tm tm;
+    Error *err = NULL;
+
+    object_property_set_bool(OBJECT(d), true, "realized", &err);
+    if (err != NULL) {
+        error_propagate(errp, err);
+        return;
+    }
 
     /* Pass IRQ from 6522 */
-    d = DEVICE(&s->mos6522_pmu);
     ms = MOS6522(d);
     sbd = SYS_BUS_DEVICE(s);
     sysbus_pass_irq(sbd, SYS_BUS_DEVICE(ms));
-- 
2.18.2

[PATCH v5 4/4] hw/misc/mos6522: move timer_new from init() into realize() to avoid memleaks

[Pan Nengyuan]

There are some memleaks when we call 'device_list_properties'.
This patch move timer_new from init into realize to fix it.

Reported-by: Euler Robot <euler.robot@huawei.com>
Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>
---
Cc: Laurent Vivier <laurent@vivier.eu>
Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Cc: David Gibson <david@gibson.dropbear.id.au>
Cc: qemu-ppc@nongnu.org
---
v2->v1:
- no changes in this patch.
v3->v2:
- remove null check in reset, and add calls to mos6522_realize() in mac_via_realize to make this move to be valid.
v4->v3:
- split patch into two, this patch fix the memleaks.
v5->v4:
- No change in this patch. But add [patch 3/4] to fix SEGVs during make check-qtest-ppc64 if apply this patch.
  This patch also depend to another fix [patch 2/4].
---
 hw/misc/mos6522.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/hw/misc/mos6522.c b/hw/misc/mos6522.c
index 19e154b870..c1cd154a84 100644
--- a/hw/misc/mos6522.c
+++ b/hw/misc/mos6522.c
@@ -485,6 +485,11 @@ static void mos6522_init(Object *obj)
     for (i = 0; i < ARRAY_SIZE(s->timers); i++) {
         s->timers[i].index = i;
     }
+}
+
+static void mos6522_realize(DeviceState *dev, Error **errp)
+{
+    MOS6522State *s = MOS6522(dev);
 
     s->timers[0].timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, mos6522_timer1, s);
     s->timers[1].timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, mos6522_timer2, s);
@@ -502,6 +507,7 @@ static void mos6522_class_init(ObjectClass *oc, void *data)
 
     dc->reset = mos6522_reset;
     dc->vmsd = &vmstate_mos6522;
+    dc->realize = mos6522_realize;
     device_class_set_props(dc, mos6522_properties);
     mdc->parent_reset = dc->reset;
     mdc->set_sr_int = mos6522_set_sr_int;
-- 
2.18.2

Re: [PATCH v5 0/4] delay timer_new from init to realize to fix memleaks.

[no-reply]

Patchew URL: https://patchew.org/QEMU/20200314084730.25876-1-pannengyuan@huawei.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Subject: [PATCH v5 0/4] delay timer_new from init to realize to fix memleaks.
Message-id: 20200314084730.25876-1-pannengyuan@huawei.com
Type: series

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 - [tag update]      patchew/20200312145900.2054-1-zhiwei_liu@c-sky.com -> patchew/20200312145900.2054-1-zhiwei_liu@c-sky.com
Switched to a new branch 'test'
57225b5 hw/misc/mos6522: move timer_new from init() into realize() to avoid memleaks
68ae0a2 hw/misc/macio: fix incorrect creation of mos6522's subclasses
5b8325b mac_via: fix incorrect creation of mos6522 device in mac_via
330de48 s390x: fix memleaks in cpu_finalize

=== OUTPUT BEGIN ===
1/4 Checking commit 330de484a515 (s390x: fix memleaks in cpu_finalize)
2/4 Checking commit 5b8325bdb752 (mac_via: fix incorrect creation of mos6522 device in mac_via)
3/4 Checking commit 68ae0a2ea95b (hw/misc/macio: fix incorrect creation of mos6522's subclasses)
ERROR: superfluous trailing semicolon
#62: FILE: hw/misc/macio/pmu.c:745:
+    DeviceState *d = DEVICE(&s->mos6522_pmu);;

total: 1 errors, 0 warnings, 52 lines checked

Patch 3/4 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

4/4 Checking commit 57225b5cea3b (hw/misc/mos6522: move timer_new from init() into realize() to avoid memleaks)
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20200314084730.25876-1-pannengyuan@huawei.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [PATCH v5 0/4] delay timer_new from init to realize to fix memleaks.

[Mark Cave-Ayland]

On 14/03/2020 08:47, Pan Nengyuan wrote:

> This series delay timer_new from init into realize to avoid memleaks when we call 'device_list_properties'.
> And do timer_free only in s390x_cpu_finalize because it's hotplugable. However, mos6522_realize is never called
> at all due to the incorrect creation of it. So we fix the incorrect creation in mac_via/cuda/pmu first, then 
> move the timer_new to mos6522_realize().
> 
> v1:
>    - Delay timer_new() from init() to realize() to fix memleaks.
> v2:
>    - Similarly to other cleanups, move timer_new into realize in target/s390x/cpu.c (Suggested by Philippe Mathieu-Daudé).
>    - Send these two patches as a series instead of send each as a single patch but with wrong subject in v1.
> v3:
>    - It's not valid in mos6522 if we move timer_new from init to realize, because it's never called at all.
>      Thus, we remove null check in reset, and add calls to mos6522_realize() in mac_via_realize to make this move to be valid.
>    - split patch by device to make it more clear.
> v4:
>    - Also do timer_free on the error path in realize() and fix some coding style. Then use device_class_set_parent_unrealize to declare unrealize.
>    - split the mos6522 patch into two, one to fix incorrect creation of mos6522, the other to fix memleak.
> 
> v5: 
>    - Fix two other places where we create mos6522's subclasses but forgot to realize it(macio/cuda,macio/pmu). 
>      Otherwise, this will cause SEGVs during make check-qtest-ppc64.
>    - Remove timer_del on the error path of s390x_cpu_realize() and simply use errp instead a temporary variable.
> 
> Pan Nengyuan (4):
>   s390x: fix memleaks in cpu_finalize
>   mac_via: fix incorrect creation of mos6522 device in mac_via
>   hw/misc/macio: fix incorrect creation of mos6522's subclasses
>   hw/misc/mos6522: move timer_new from init() into realize() to avoid
>     memleaks
> 
>  hw/misc/mac_via.c      | 40 +++++++++++++++++++++++++++-------------
>  hw/misc/macio/cuda.c   | 11 +++++++++--
>  hw/misc/macio/pmu.c    | 11 +++++++++--
>  hw/misc/mos6522.c      |  6 ++++++
>  target/s390x/cpu-qom.h |  1 +
>  target/s390x/cpu.c     | 30 ++++++++++++++++++++++++++----
>  6 files changed, 78 insertions(+), 21 deletions(-)

I just gave this a test on qemu-system-ppc -M mac99 with both cuda and pmu, and also
qemu-system-m68k for mac_via and I didn't see any crashes there, so:

Tested-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>


ATB,

Mark.

Re: [PATCH v5 1/4] s390x: fix memleaks in cpu_finalize

[Cornelia Huck]

On Sat, 14 Mar 2020 16:47:27 +0800
Pan Nengyuan <pannengyuan@huawei.com> wrote:

> This patch fix memleaks when we call tests/qtest/cpu-plug-test on s390x. The leak stack is as follow:
> 
> Direct leak of 48 byte(s) in 1 object(s) allocated from:
>     #0 0x7fb43c7cd970 in __interceptor_calloc (/lib64/libasan.so.5+0xef970)
>     #1 0x7fb43be2149d in g_malloc0 (/lib64/libglib-2.0.so.0+0x5249d)
>     #2 0x558ba96da716 in timer_new_full /mnt/sdb/qemu-new/qemu/include/qemu/timer.h:530
>     #3 0x558ba96da716 in timer_new /mnt/sdb/qemu-new/qemu/include/qemu/timer.h:551
>     #4 0x558ba96da716 in timer_new_ns /mnt/sdb/qemu-new/qemu/include/qemu/timer.h:569
>     #5 0x558ba96da716 in s390_cpu_initfn /mnt/sdb/qemu-new/qemu/target/s390x/cpu.c:285
>     #6 0x558ba9c969ab in object_init_with_type /mnt/sdb/qemu-new/qemu/qom/object.c:372
>     #7 0x558ba9c9eb5f in object_initialize_with_type /mnt/sdb/qemu-new/qemu/qom/object.c:516
>     #8 0x558ba9c9f053 in object_new_with_type /mnt/sdb/qemu-new/qemu/qom/object.c:684
>     #9 0x558ba967ede6 in s390x_new_cpu /mnt/sdb/qemu-new/qemu/hw/s390x/s390-virtio-ccw.c:64
>     #10 0x558ba99764b3 in hmp_cpu_add /mnt/sdb/qemu-new/qemu/hw/core/machine-hmp-cmds.c:57
>     #11 0x558ba9b1c27f in handle_hmp_command /mnt/sdb/qemu-new/qemu/monitor/hmp.c:1082
>     #12 0x558ba96c1b02 in qmp_human_monitor_command /mnt/sdb/qemu-new/qemu/monitor/misc.c:142
> 
> Reported-by: Euler Robot <euler.robot@huawei.com>
> Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>
> ---
> Cc: Richard Henderson <rth@twiddle.net>
> Cc: David Hildenbrand <david@redhat.com>
> Cc: Cornelia Huck <cohuck@redhat.com>
> Cc: qemu-s390x@nongnu.org
> ---
> v2->v1:
> - Similarly to other cleanups, move timer_new into realize(Suggested by Philippe Mathieu-Daudé)
> v3->v2:
> - Also do the timer_free in unrealize, it seems balanced.
> v4->v3:
> - Also do timer_free on the error path in realize() and fix some coding style.
> - Use device_class_set_parent_unrealize to declare unrealize.
> v5->v4:
> - remove timer_del on the error path of realize(), it's redundant. (Suggested by David Hildenbrand)
> - Simply use errp instead a temporary variable. (Suggested by David Hildenbrand)
> ---
>  target/s390x/cpu-qom.h |  1 +
>  target/s390x/cpu.c     | 30 ++++++++++++++++++++++++++----
>  2 files changed, 27 insertions(+), 4 deletions(-)

Patch seems fine now (more review still welcome :)

Question: should I take this through the s390-fixes branch, or will
somebody else queue the whole series?