* [Qemu-devel] [PATCH 0/2] s390x: cpu cleanups @ 2017-10-17 13:41 Igor Mammedov 2017-10-17 13:41 ` [Qemu-devel] [PATCH 1/2] s390x: fix cpu object referrence leak in s390x_new_cpu() Igor Mammedov 2017-10-17 13:41 ` [Qemu-devel] [PATCH 2/2] s390x: move s390x_new_cpu() into board code Igor Mammedov 0 siblings, 2 replies; 8+ messages in thread From: Igor Mammedov @ 2017-10-17 13:41 UTC (permalink / raw) To: qemu-devel Cc: Richard Henderson, Alexander Graf, Cornelia Huck, Christian Borntraeger series includes remaining in my queue non cpu_model refactoring patches that fix and cleanup cpu_init code CC: Richard Henderson <rth@twiddle.net> CC: Alexander Graf <agraf@suse.de> CC: Cornelia Huck <cohuck@redhat.com> CC: Christian Borntraeger <borntraeger@de.ibm.com> Igor Mammedov (2): s390x: fix cpu object referrence leak in s390x_new_cpu() s390x: move s390x_new_cpu() into board code target/s390x/cpu.h | 1 - hw/s390x/s390-virtio-ccw.c | 21 +++++++++++++++++++++ target/s390x/helper.c | 20 -------------------- 3 files changed, 21 insertions(+), 21 deletions(-) -- 2.7.4 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [Qemu-devel] [PATCH 1/2] s390x: fix cpu object referrence leak in s390x_new_cpu() 2017-10-17 13:41 [Qemu-devel] [PATCH 0/2] s390x: cpu cleanups Igor Mammedov @ 2017-10-17 13:41 ` Igor Mammedov 2017-10-17 14:28 ` Cornelia Huck 2017-10-17 15:49 ` Cornelia Huck 2017-10-17 13:41 ` [Qemu-devel] [PATCH 2/2] s390x: move s390x_new_cpu() into board code Igor Mammedov 1 sibling, 2 replies; 8+ messages in thread From: Igor Mammedov @ 2017-10-17 13:41 UTC (permalink / raw) To: qemu-devel Cc: Richard Henderson, Alexander Graf, Cornelia Huck, Christian Borntraeger object_new() returns cpu with refcnt == 1 and after realize refcnt == 2*. s390x_new_cpu() as an owner of the first refcnt should have released it on exit in both cases (on error and success) to avoid it leaking. Do so for both cases. Signed-off-by: Igor Mammedov <imammedo@redhat.com> --- target/s390x/helper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/target/s390x/helper.c b/target/s390x/helper.c index 97adbcc..64d4c48 100644 --- a/target/s390x/helper.c +++ b/target/s390x/helper.c @@ -80,9 +80,9 @@ S390CPU *s390x_new_cpu(const char *typename, uint32_t core_id, Error **errp) object_property_set_bool(OBJECT(cpu), true, "realized", &err); out: + object_unref(OBJECT(cpu)); if (err) { error_propagate(errp, err); - object_unref(OBJECT(cpu)); cpu = NULL; } return cpu; -- 2.7.4 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] s390x: fix cpu object referrence leak in s390x_new_cpu() 2017-10-17 13:41 ` [Qemu-devel] [PATCH 1/2] s390x: fix cpu object referrence leak in s390x_new_cpu() Igor Mammedov @ 2017-10-17 14:28 ` Cornelia Huck 2017-10-17 15:49 ` Cornelia Huck 1 sibling, 0 replies; 8+ messages in thread From: Cornelia Huck @ 2017-10-17 14:28 UTC (permalink / raw) To: Igor Mammedov Cc: qemu-devel, Richard Henderson, Alexander Graf, Christian Borntraeger, qemu-s390x On Tue, 17 Oct 2017 15:41:19 +0200 Igor Mammedov <imammedo@redhat.com> wrote: > object_new() returns cpu with refcnt == 1 and after realize > refcnt == 2*. s390x_new_cpu() as an owner of the first refcnt > should have released it on exit in both cases (on error and > success) to avoid it leaking. Do so for both cases. > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > --- > target/s390x/helper.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/target/s390x/helper.c b/target/s390x/helper.c > index 97adbcc..64d4c48 100644 > --- a/target/s390x/helper.c > +++ b/target/s390x/helper.c > @@ -80,9 +80,9 @@ S390CPU *s390x_new_cpu(const char *typename, uint32_t core_id, Error **errp) > object_property_set_bool(OBJECT(cpu), true, "realized", &err); > > out: > + object_unref(OBJECT(cpu)); > if (err) { > error_propagate(errp, err); > - object_unref(OBJECT(cpu)); > cpu = NULL; > } > return cpu; Looks reasonable. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] s390x: fix cpu object referrence leak in s390x_new_cpu() 2017-10-17 13:41 ` [Qemu-devel] [PATCH 1/2] s390x: fix cpu object referrence leak in s390x_new_cpu() Igor Mammedov 2017-10-17 14:28 ` Cornelia Huck @ 2017-10-17 15:49 ` Cornelia Huck 1 sibling, 0 replies; 8+ messages in thread From: Cornelia Huck @ 2017-10-17 15:49 UTC (permalink / raw) To: Igor Mammedov Cc: qemu-devel, Richard Henderson, Alexander Graf, Christian Borntraeger, qemu-s390x On Tue, 17 Oct 2017 15:41:19 +0200 Igor Mammedov <imammedo@redhat.com> wrote: > object_new() returns cpu with refcnt == 1 and after realize > refcnt == 2*. s390x_new_cpu() as an owner of the first refcnt > should have released it on exit in both cases (on error and > success) to avoid it leaking. Do so for both cases. > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > --- > target/s390x/helper.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/target/s390x/helper.c b/target/s390x/helper.c > index 97adbcc..64d4c48 100644 > --- a/target/s390x/helper.c > +++ b/target/s390x/helper.c > @@ -80,9 +80,9 @@ S390CPU *s390x_new_cpu(const char *typename, uint32_t core_id, Error **errp) > object_property_set_bool(OBJECT(cpu), true, "realized", &err); > > out: > + object_unref(OBJECT(cpu)); > if (err) { > error_propagate(errp, err); > - object_unref(OBJECT(cpu)); > cpu = NULL; > } > return cpu; Thanks, applied. ^ permalink raw reply [flat|nested] 8+ messages in thread
* [Qemu-devel] [PATCH 2/2] s390x: move s390x_new_cpu() into board code 2017-10-17 13:41 [Qemu-devel] [PATCH 0/2] s390x: cpu cleanups Igor Mammedov 2017-10-17 13:41 ` [Qemu-devel] [PATCH 1/2] s390x: fix cpu object referrence leak in s390x_new_cpu() Igor Mammedov @ 2017-10-17 13:41 ` Igor Mammedov 2017-10-17 14:29 ` Cornelia Huck 1 sibling, 1 reply; 8+ messages in thread From: Igor Mammedov @ 2017-10-17 13:41 UTC (permalink / raw) To: qemu-devel Cc: Richard Henderson, Alexander Graf, Cornelia Huck, Christian Borntraeger s390-virtio-ccw.c is the sole user of s390x_new_cpu(), so move this helper there. Signed-off-by: Igor Mammedov <imammedo@redhat.com> --- target/s390x/cpu.h | 1 - hw/s390x/s390-virtio-ccw.c | 21 +++++++++++++++++++++ target/s390x/helper.c | 20 -------------------- 3 files changed, 21 insertions(+), 21 deletions(-) diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h index 7e864c8..b177be9 100644 --- a/target/s390x/cpu.h +++ b/target/s390x/cpu.h @@ -695,7 +695,6 @@ void s390_cpu_list(FILE *f, fprintf_function cpu_fprintf); /* helper.c */ #define cpu_init(cpu_model) cpu_generic_init(TYPE_S390_CPU, cpu_model) -S390CPU *s390x_new_cpu(const char *typename, uint32_t core_id, Error **errp); #define S390_CPU_TYPE_SUFFIX "-" TYPE_S390_CPU #define S390_CPU_TYPE_NAME(name) (name S390_CPU_TYPE_SUFFIX) diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c index 32d3f11..f64db51 100644 --- a/hw/s390x/s390-virtio-ccw.c +++ b/hw/s390x/s390-virtio-ccw.c @@ -52,6 +52,27 @@ S390CPU *s390_cpu_addr2state(uint16_t cpu_addr) return S390_CPU(ms->possible_cpus->cpus[cpu_addr].cpu); } +static +S390CPU *s390x_new_cpu(const char *typename, uint32_t core_id, Error **errp) +{ + S390CPU *cpu = S390_CPU(object_new(typename)); + Error *err = NULL; + + object_property_set_int(OBJECT(cpu), core_id, "core-id", &err); + if (err != NULL) { + goto out; + } + object_property_set_bool(OBJECT(cpu), true, "realized", &err); + +out: + object_unref(OBJECT(cpu)); + if (err) { + error_propagate(errp, err); + cpu = NULL; + } + return cpu; +} + static void s390_init_cpus(MachineState *machine) { MachineClass *mc = MACHINE_GET_CLASS(machine); diff --git a/target/s390x/helper.c b/target/s390x/helper.c index 64d4c48..52aa64b 100644 --- a/target/s390x/helper.c +++ b/target/s390x/helper.c @@ -68,26 +68,6 @@ void s390x_cpu_timer(void *opaque) } #endif -S390CPU *s390x_new_cpu(const char *typename, uint32_t core_id, Error **errp) -{ - S390CPU *cpu = S390_CPU(object_new(typename)); - Error *err = NULL; - - object_property_set_int(OBJECT(cpu), core_id, "core-id", &err); - if (err != NULL) { - goto out; - } - object_property_set_bool(OBJECT(cpu), true, "realized", &err); - -out: - object_unref(OBJECT(cpu)); - if (err) { - error_propagate(errp, err); - cpu = NULL; - } - return cpu; -} - #ifndef CONFIG_USER_ONLY hwaddr s390_cpu_get_phys_page_debug(CPUState *cs, vaddr vaddr) -- 2.7.4 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] s390x: move s390x_new_cpu() into board code 2017-10-17 13:41 ` [Qemu-devel] [PATCH 2/2] s390x: move s390x_new_cpu() into board code Igor Mammedov @ 2017-10-17 14:29 ` Cornelia Huck 2017-10-17 15:13 ` [Qemu-devel] [PATCH v2 " Igor Mammedov 0 siblings, 1 reply; 8+ messages in thread From: Cornelia Huck @ 2017-10-17 14:29 UTC (permalink / raw) To: Igor Mammedov Cc: qemu-devel, Richard Henderson, Alexander Graf, Christian Borntraeger, qemu-s390x On Tue, 17 Oct 2017 15:41:20 +0200 Igor Mammedov <imammedo@redhat.com> wrote: > s390-virtio-ccw.c is the sole user of s390x_new_cpu(), > so move this helper there. > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > --- > target/s390x/cpu.h | 1 - > hw/s390x/s390-virtio-ccw.c | 21 +++++++++++++++++++++ > target/s390x/helper.c | 20 -------------------- > 3 files changed, 21 insertions(+), 21 deletions(-) > > diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h > index 7e864c8..b177be9 100644 > --- a/target/s390x/cpu.h > +++ b/target/s390x/cpu.h > @@ -695,7 +695,6 @@ void s390_cpu_list(FILE *f, fprintf_function cpu_fprintf); > > /* helper.c */ > #define cpu_init(cpu_model) cpu_generic_init(TYPE_S390_CPU, cpu_model) > -S390CPU *s390x_new_cpu(const char *typename, uint32_t core_id, Error **errp); > > #define S390_CPU_TYPE_SUFFIX "-" TYPE_S390_CPU > #define S390_CPU_TYPE_NAME(name) (name S390_CPU_TYPE_SUFFIX) > diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c > index 32d3f11..f64db51 100644 > --- a/hw/s390x/s390-virtio-ccw.c > +++ b/hw/s390x/s390-virtio-ccw.c > @@ -52,6 +52,27 @@ S390CPU *s390_cpu_addr2state(uint16_t cpu_addr) > return S390_CPU(ms->possible_cpus->cpus[cpu_addr].cpu); > } > > +static > +S390CPU *s390x_new_cpu(const char *typename, uint32_t core_id, Error **errp) I don't like this wrap; better wrap the argument list. > +{ > + S390CPU *cpu = S390_CPU(object_new(typename)); > + Error *err = NULL; > + > + object_property_set_int(OBJECT(cpu), core_id, "core-id", &err); > + if (err != NULL) { > + goto out; > + } > + object_property_set_bool(OBJECT(cpu), true, "realized", &err); > + > +out: > + object_unref(OBJECT(cpu)); > + if (err) { > + error_propagate(errp, err); > + cpu = NULL; > + } > + return cpu; > +} > + > static void s390_init_cpus(MachineState *machine) > { > MachineClass *mc = MACHINE_GET_CLASS(machine); > diff --git a/target/s390x/helper.c b/target/s390x/helper.c > index 64d4c48..52aa64b 100644 > --- a/target/s390x/helper.c > +++ b/target/s390x/helper.c > @@ -68,26 +68,6 @@ void s390x_cpu_timer(void *opaque) > } > #endif > > -S390CPU *s390x_new_cpu(const char *typename, uint32_t core_id, Error **errp) > -{ > - S390CPU *cpu = S390_CPU(object_new(typename)); > - Error *err = NULL; > - > - object_property_set_int(OBJECT(cpu), core_id, "core-id", &err); > - if (err != NULL) { > - goto out; > - } > - object_property_set_bool(OBJECT(cpu), true, "realized", &err); > - > -out: > - object_unref(OBJECT(cpu)); > - if (err) { > - error_propagate(errp, err); > - cpu = NULL; > - } > - return cpu; > -} > - > #ifndef CONFIG_USER_ONLY > > hwaddr s390_cpu_get_phys_page_debug(CPUState *cs, vaddr vaddr) Otherwise, looks sane. ^ permalink raw reply [flat|nested] 8+ messages in thread
* [Qemu-devel] [PATCH v2 2/2] s390x: move s390x_new_cpu() into board code 2017-10-17 14:29 ` Cornelia Huck @ 2017-10-17 15:13 ` Igor Mammedov 2017-10-17 15:50 ` Cornelia Huck 0 siblings, 1 reply; 8+ messages in thread From: Igor Mammedov @ 2017-10-17 15:13 UTC (permalink / raw) To: qemu-devel; +Cc: Cornelia Huck s390-virtio-ccw.c is the sole user of s390x_new_cpu(), so move this helper there. Signed-off-by: Igor Mammedov <imammedo@redhat.com> --- CC: Cornelia Huck <cohuck@redhat.com> v2: - style fix: wrap the argument list --- target/s390x/cpu.h | 1 - hw/s390x/s390-virtio-ccw.c | 21 +++++++++++++++++++++ target/s390x/helper.c | 20 -------------------- 3 files changed, 21 insertions(+), 21 deletions(-) diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h index 7e864c8..b177be9 100644 --- a/target/s390x/cpu.h +++ b/target/s390x/cpu.h @@ -695,7 +695,6 @@ void s390_cpu_list(FILE *f, fprintf_function cpu_fprintf); /* helper.c */ #define cpu_init(cpu_model) cpu_generic_init(TYPE_S390_CPU, cpu_model) -S390CPU *s390x_new_cpu(const char *typename, uint32_t core_id, Error **errp); #define S390_CPU_TYPE_SUFFIX "-" TYPE_S390_CPU #define S390_CPU_TYPE_NAME(name) (name S390_CPU_TYPE_SUFFIX) diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c index 32d3f11..e593c71 100644 --- a/hw/s390x/s390-virtio-ccw.c +++ b/hw/s390x/s390-virtio-ccw.c @@ -52,6 +52,27 @@ S390CPU *s390_cpu_addr2state(uint16_t cpu_addr) return S390_CPU(ms->possible_cpus->cpus[cpu_addr].cpu); } +static S390CPU *s390x_new_cpu(const char *typename, uint32_t core_id, + Error **errp) +{ + S390CPU *cpu = S390_CPU(object_new(typename)); + Error *err = NULL; + + object_property_set_int(OBJECT(cpu), core_id, "core-id", &err); + if (err != NULL) { + goto out; + } + object_property_set_bool(OBJECT(cpu), true, "realized", &err); + +out: + object_unref(OBJECT(cpu)); + if (err) { + error_propagate(errp, err); + cpu = NULL; + } + return cpu; +} + static void s390_init_cpus(MachineState *machine) { MachineClass *mc = MACHINE_GET_CLASS(machine); diff --git a/target/s390x/helper.c b/target/s390x/helper.c index 64d4c48..52aa64b 100644 --- a/target/s390x/helper.c +++ b/target/s390x/helper.c @@ -68,26 +68,6 @@ void s390x_cpu_timer(void *opaque) } #endif -S390CPU *s390x_new_cpu(const char *typename, uint32_t core_id, Error **errp) -{ - S390CPU *cpu = S390_CPU(object_new(typename)); - Error *err = NULL; - - object_property_set_int(OBJECT(cpu), core_id, "core-id", &err); - if (err != NULL) { - goto out; - } - object_property_set_bool(OBJECT(cpu), true, "realized", &err); - -out: - object_unref(OBJECT(cpu)); - if (err) { - error_propagate(errp, err); - cpu = NULL; - } - return cpu; -} - #ifndef CONFIG_USER_ONLY hwaddr s390_cpu_get_phys_page_debug(CPUState *cs, vaddr vaddr) -- 2.7.4 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH v2 2/2] s390x: move s390x_new_cpu() into board code 2017-10-17 15:13 ` [Qemu-devel] [PATCH v2 " Igor Mammedov @ 2017-10-17 15:50 ` Cornelia Huck 0 siblings, 0 replies; 8+ messages in thread From: Cornelia Huck @ 2017-10-17 15:50 UTC (permalink / raw) To: Igor Mammedov; +Cc: qemu-devel, qemu-s390x On Tue, 17 Oct 2017 17:13:23 +0200 Igor Mammedov <imammedo@redhat.com> wrote: > s390-virtio-ccw.c is the sole user of s390x_new_cpu(), > so move this helper there. > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > --- > CC: Cornelia Huck <cohuck@redhat.com> > > v2: > - style fix: wrap the argument list > > --- > target/s390x/cpu.h | 1 - > hw/s390x/s390-virtio-ccw.c | 21 +++++++++++++++++++++ > target/s390x/helper.c | 20 -------------------- > 3 files changed, 21 insertions(+), 21 deletions(-) Thanks, applied. ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2017-10-17 15:50 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-10-17 13:41 [Qemu-devel] [PATCH 0/2] s390x: cpu cleanups Igor Mammedov 2017-10-17 13:41 ` [Qemu-devel] [PATCH 1/2] s390x: fix cpu object referrence leak in s390x_new_cpu() Igor Mammedov 2017-10-17 14:28 ` Cornelia Huck 2017-10-17 15:49 ` Cornelia Huck 2017-10-17 13:41 ` [Qemu-devel] [PATCH 2/2] s390x: move s390x_new_cpu() into board code Igor Mammedov 2017-10-17 14:29 ` Cornelia Huck 2017-10-17 15:13 ` [Qemu-devel] [PATCH v2 " Igor Mammedov 2017-10-17 15:50 ` Cornelia Huck
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).