* [PATCH v2 01/18] crypto: Fix certificate file error handling crash bug
2019-12-04 9:36 [PATCH v2 00/18] Error handling fixes Markus Armbruster
@ 2019-12-04 9:36 ` Markus Armbruster
2019-12-04 9:45 ` Daniel P. Berrangé
2019-12-04 9:36 ` [PATCH v2 02/18] crypto: Fix typo in QCryptoTLSSession's <example> comment Markus Armbruster
` (19 subsequent siblings)
20 siblings, 1 reply; 31+ messages in thread
From: Markus Armbruster @ 2019-12-04 9:36 UTC (permalink / raw)
To: qemu-devel; +Cc: Daniel P. Berrangé
qcrypto_tls_creds_load_cert() passes uninitialized GError *gerr by
reference to g_file_get_contents(). When g_file_get_contents() fails,
it'll try to set a GError. Unless @gerr is null by dumb luck, this
logs a ERROR_OVERWRITTEN_WARNING warning message and leaves @gerr
unchanged. qcrypto_tls_creds_load_cert() then dereferences the
uninitialized @gerr.
Fix by initializing @gerr properly.
Fixes: 9a2fd4347c40321f5cbb4ab4220e759fcbf87d03
Cc: "Daniel P. Berrangé" <berrange@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
crypto/tlscredsx509.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/crypto/tlscredsx509.c b/crypto/tlscredsx509.c
index 01fc304e5d..53a4368f49 100644
--- a/crypto/tlscredsx509.c
+++ b/crypto/tlscredsx509.c
@@ -380,7 +380,7 @@ qcrypto_tls_creds_load_cert(QCryptoTLSCredsX509 *creds,
gnutls_x509_crt_t cert = NULL;
g_autofree char *buf = NULL;
gsize buflen;
- GError *gerr;
+ GError *gerr = NULL;
int ret = -1;
int err;
--
2.21.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH v2 02/18] crypto: Fix typo in QCryptoTLSSession's <example> comment
2019-12-04 9:36 [PATCH v2 00/18] Error handling fixes Markus Armbruster
2019-12-04 9:36 ` [PATCH v2 01/18] crypto: Fix certificate file error handling crash bug Markus Armbruster
@ 2019-12-04 9:36 ` Markus Armbruster
2019-12-04 9:45 ` Daniel P. Berrangé
2019-12-04 9:36 ` [PATCH v2 03/18] io: Fix Error usage in a comment <example> Markus Armbruster
` (18 subsequent siblings)
20 siblings, 1 reply; 31+ messages in thread
From: Markus Armbruster @ 2019-12-04 9:36 UTC (permalink / raw)
To: qemu-devel; +Cc: Daniel P. Berrangé
Cc: "Daniel P. Berrangé" <berrange@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
include/crypto/tlssession.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/crypto/tlssession.h b/include/crypto/tlssession.h
index e01e1a9dc2..15b9cef086 100644
--- a/include/crypto/tlssession.h
+++ b/include/crypto/tlssession.h
@@ -56,7 +56,7 @@
*
* static int mysock_run_tls(int sockfd,
* QCryptoTLSCreds *creds,
- * Error *errp)
+ * Error **errp)
* {
* QCryptoTLSSession *sess;
*
--
2.21.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH v2 03/18] io: Fix Error usage in a comment <example>
2019-12-04 9:36 [PATCH v2 00/18] Error handling fixes Markus Armbruster
2019-12-04 9:36 ` [PATCH v2 01/18] crypto: Fix certificate file error handling crash bug Markus Armbruster
2019-12-04 9:36 ` [PATCH v2 02/18] crypto: Fix typo in QCryptoTLSSession's <example> comment Markus Armbruster
@ 2019-12-04 9:36 ` Markus Armbruster
2019-12-04 9:45 ` Daniel P. Berrangé
2019-12-04 9:36 ` [PATCH v2 04/18] tests: Clean up initialization of Error *err variables Markus Armbruster
` (17 subsequent siblings)
20 siblings, 1 reply; 31+ messages in thread
From: Markus Armbruster @ 2019-12-04 9:36 UTC (permalink / raw)
To: qemu-devel; +Cc: Daniel P. Berrangé
Cc: "Daniel P. Berrangé" <berrange@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
include/io/task.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/io/task.h b/include/io/task.h
index 5cb9faf9f2..1abbfb8b65 100644
--- a/include/io/task.h
+++ b/include/io/task.h
@@ -119,7 +119,7 @@ typedef void (*QIOTaskWorker)(QIOTask *task,
* gboolean myobject_operation_timer(gpointer opaque)
* {
* QIOTask *task = QIO_TASK(opaque);
- * Error *err;*
+ * Error *err = NULL;
*
* ...check something important...
* if (err) {
--
2.21.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH v2 04/18] tests: Clean up initialization of Error *err variables
2019-12-04 9:36 [PATCH v2 00/18] Error handling fixes Markus Armbruster
` (2 preceding siblings ...)
2019-12-04 9:36 ` [PATCH v2 03/18] io: Fix Error usage in a comment <example> Markus Armbruster
@ 2019-12-04 9:36 ` Markus Armbruster
2019-12-04 14:27 ` Eric Blake
2019-12-04 14:53 ` Philippe Mathieu-Daudé
2019-12-04 9:36 ` [PATCH v2 05/18] exec: Fix file_ram_alloc() error API violations Markus Armbruster
` (16 subsequent siblings)
20 siblings, 2 replies; 31+ messages in thread
From: Markus Armbruster @ 2019-12-04 9:36 UTC (permalink / raw)
To: qemu-devel
Declaring a local Error *err without initializer looks suspicious.
Fuse the declaration with the initialization to avoid that.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
tests/test-qobject-output-visitor.c | 8 ++++----
tests/test-string-output-visitor.c | 4 ++--
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/tests/test-qobject-output-visitor.c b/tests/test-qobject-output-visitor.c
index 3e993e5ba8..d7761ebf84 100644
--- a/tests/test-qobject-output-visitor.c
+++ b/tests/test-qobject-output-visitor.c
@@ -145,10 +145,10 @@ static void test_visitor_out_enum_errors(TestOutputVisitorData *data,
const void *unused)
{
EnumOne i, bad_values[] = { ENUM_ONE__MAX, -1 };
- Error *err;
for (i = 0; i < ARRAY_SIZE(bad_values) ; i++) {
- err = NULL;
+ Error *err = NULL;
+
visit_type_EnumOne(data->ov, "unused", &bad_values[i], &err);
error_free_or_abort(&err);
visitor_reset(data);
@@ -240,11 +240,11 @@ static void test_visitor_out_struct_errors(TestOutputVisitorData *data,
EnumOne bad_values[] = { ENUM_ONE__MAX, -1 };
UserDefOne u = {0};
UserDefOne *pu = &u;
- Error *err;
int i;
for (i = 0; i < ARRAY_SIZE(bad_values) ; i++) {
- err = NULL;
+ Error *err = NULL;
+
u.has_enum1 = true;
u.enum1 = bad_values[i];
visit_type_UserDefOne(data->ov, "unused", &pu, &err);
diff --git a/tests/test-string-output-visitor.c b/tests/test-string-output-visitor.c
index 02766c0f65..1be1540767 100644
--- a/tests/test-string-output-visitor.c
+++ b/tests/test-string-output-visitor.c
@@ -207,10 +207,10 @@ static void test_visitor_out_enum_errors(TestOutputVisitorData *data,
const void *unused)
{
EnumOne i, bad_values[] = { ENUM_ONE__MAX, -1 };
- Error *err;
for (i = 0; i < ARRAY_SIZE(bad_values) ; i++) {
- err = NULL;
+ Error *err = NULL;
+
visit_type_EnumOne(data->ov, "unused", &bad_values[i], &err);
error_free_or_abort(&err);
}
--
2.21.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH v2 04/18] tests: Clean up initialization of Error *err variables
2019-12-04 9:36 ` [PATCH v2 04/18] tests: Clean up initialization of Error *err variables Markus Armbruster
@ 2019-12-04 14:27 ` Eric Blake
2019-12-04 14:53 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 31+ messages in thread
From: Eric Blake @ 2019-12-04 14:27 UTC (permalink / raw)
To: Markus Armbruster, qemu-devel
On 12/4/19 3:36 AM, Markus Armbruster wrote:
> Declaring a local Error *err without initializer looks suspicious.
> Fuse the declaration with the initialization to avoid that.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> tests/test-qobject-output-visitor.c | 8 ++++----
> tests/test-string-output-visitor.c | 4 ++--
> 2 files changed, 6 insertions(+), 6 deletions(-)
>
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v2 04/18] tests: Clean up initialization of Error *err variables
2019-12-04 9:36 ` [PATCH v2 04/18] tests: Clean up initialization of Error *err variables Markus Armbruster
2019-12-04 14:27 ` Eric Blake
@ 2019-12-04 14:53 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 31+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-12-04 14:53 UTC (permalink / raw)
To: Markus Armbruster, qemu-devel
On 12/4/19 10:36 AM, Markus Armbruster wrote:
> Declaring a local Error *err without initializer looks suspicious.
> Fuse the declaration with the initialization to avoid that.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> tests/test-qobject-output-visitor.c | 8 ++++----
> tests/test-string-output-visitor.c | 4 ++--
> 2 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/tests/test-qobject-output-visitor.c b/tests/test-qobject-output-visitor.c
> index 3e993e5ba8..d7761ebf84 100644
> --- a/tests/test-qobject-output-visitor.c
> +++ b/tests/test-qobject-output-visitor.c
> @@ -145,10 +145,10 @@ static void test_visitor_out_enum_errors(TestOutputVisitorData *data,
> const void *unused)
> {
> EnumOne i, bad_values[] = { ENUM_ONE__MAX, -1 };
> - Error *err;
>
> for (i = 0; i < ARRAY_SIZE(bad_values) ; i++) {
> - err = NULL;
> + Error *err = NULL;
> +
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> visit_type_EnumOne(data->ov, "unused", &bad_values[i], &err);
> error_free_or_abort(&err);
> visitor_reset(data);
> @@ -240,11 +240,11 @@ static void test_visitor_out_struct_errors(TestOutputVisitorData *data,
> EnumOne bad_values[] = { ENUM_ONE__MAX, -1 };
> UserDefOne u = {0};
> UserDefOne *pu = &u;
> - Error *err;
> int i;
>
> for (i = 0; i < ARRAY_SIZE(bad_values) ; i++) {
> - err = NULL;
> + Error *err = NULL;
> +
> u.has_enum1 = true;
> u.enum1 = bad_values[i];
> visit_type_UserDefOne(data->ov, "unused", &pu, &err);
> diff --git a/tests/test-string-output-visitor.c b/tests/test-string-output-visitor.c
> index 02766c0f65..1be1540767 100644
> --- a/tests/test-string-output-visitor.c
> +++ b/tests/test-string-output-visitor.c
> @@ -207,10 +207,10 @@ static void test_visitor_out_enum_errors(TestOutputVisitorData *data,
> const void *unused)
> {
> EnumOne i, bad_values[] = { ENUM_ONE__MAX, -1 };
> - Error *err;
>
> for (i = 0; i < ARRAY_SIZE(bad_values) ; i++) {
> - err = NULL;
> + Error *err = NULL;
> +
> visit_type_EnumOne(data->ov, "unused", &bad_values[i], &err);
> error_free_or_abort(&err);
> }
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v2 05/18] exec: Fix file_ram_alloc() error API violations
2019-12-04 9:36 [PATCH v2 00/18] Error handling fixes Markus Armbruster
` (3 preceding siblings ...)
2019-12-04 9:36 ` [PATCH v2 04/18] tests: Clean up initialization of Error *err variables Markus Armbruster
@ 2019-12-04 9:36 ` Markus Armbruster
2019-12-04 14:53 ` Philippe Mathieu-Daudé
2019-12-04 9:36 ` [PATCH v2 06/18] hw/acpi: Fix legacy CPU plug " Markus Armbruster
` (15 subsequent siblings)
20 siblings, 1 reply; 31+ messages in thread
From: Markus Armbruster @ 2019-12-04 9:36 UTC (permalink / raw)
To: qemu-devel; +Cc: Igor Mammedov
When os_mem_prealloc() fails, file_ram_alloc() calls qemu_ram_munmap()
and returns null. Except it doesn't when its @errp argument is null,
because it checks for failure with (errp && *errp). Introduced in
commit 056b68af77 "fix qemu exit on memory hotplug when allocation
fails at prealloc time".
No caller actually passes null.
Fix anyway: splice in a local Error *err, and error_propagate().
Cc: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
---
exec.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/exec.c b/exec.c
index ffdb518535..45695a5f2d 100644
--- a/exec.c
+++ b/exec.c
@@ -1841,6 +1841,7 @@ static void *file_ram_alloc(RAMBlock *block,
bool truncate,
Error **errp)
{
+ Error *err = NULL;
MachineState *ms = MACHINE(qdev_get_machine());
void *area;
@@ -1898,8 +1899,9 @@ static void *file_ram_alloc(RAMBlock *block,
}
if (mem_prealloc) {
- os_mem_prealloc(fd, area, memory, ms->smp.cpus, errp);
- if (errp && *errp) {
+ os_mem_prealloc(fd, area, memory, ms->smp.cpus, &err);
+ if (err) {
+ error_propagate(errp, err);
qemu_ram_munmap(fd, area, memory);
return NULL;
}
--
2.21.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH v2 05/18] exec: Fix file_ram_alloc() error API violations
2019-12-04 9:36 ` [PATCH v2 05/18] exec: Fix file_ram_alloc() error API violations Markus Armbruster
@ 2019-12-04 14:53 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 31+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-12-04 14:53 UTC (permalink / raw)
To: Markus Armbruster, qemu-devel; +Cc: Igor Mammedov
On 12/4/19 10:36 AM, Markus Armbruster wrote:
> When os_mem_prealloc() fails, file_ram_alloc() calls qemu_ram_munmap()
> and returns null. Except it doesn't when its @errp argument is null,
> because it checks for failure with (errp && *errp). Introduced in
> commit 056b68af77 "fix qemu exit on memory hotplug when allocation
> fails at prealloc time".
>
> No caller actually passes null.
>
> Fix anyway: splice in a local Error *err, and error_propagate().
>
> Cc: Igor Mammedov <imammedo@redhat.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> exec.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/exec.c b/exec.c
> index ffdb518535..45695a5f2d 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -1841,6 +1841,7 @@ static void *file_ram_alloc(RAMBlock *block,
> bool truncate,
> Error **errp)
> {
> + Error *err = NULL;
> MachineState *ms = MACHINE(qdev_get_machine());
> void *area;
>
> @@ -1898,8 +1899,9 @@ static void *file_ram_alloc(RAMBlock *block,
> }
>
> if (mem_prealloc) {
> - os_mem_prealloc(fd, area, memory, ms->smp.cpus, errp);
> - if (errp && *errp) {
> + os_mem_prealloc(fd, area, memory, ms->smp.cpus, &err);
> + if (err) {
> + error_propagate(errp, err);
> qemu_ram_munmap(fd, area, memory);
> return NULL;
> }
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v2 06/18] hw/acpi: Fix legacy CPU plug error API violations
2019-12-04 9:36 [PATCH v2 00/18] Error handling fixes Markus Armbruster
` (4 preceding siblings ...)
2019-12-04 9:36 ` [PATCH v2 05/18] exec: Fix file_ram_alloc() error API violations Markus Armbruster
@ 2019-12-04 9:36 ` Markus Armbruster
2019-12-09 16:01 ` Michael S. Tsirkin
2019-12-04 9:36 ` [PATCH v2 07/18] hw/core: Fix fit_load_fdt() error handling violations Markus Armbruster
` (14 subsequent siblings)
20 siblings, 1 reply; 31+ messages in thread
From: Markus Armbruster @ 2019-12-04 9:36 UTC (permalink / raw)
To: qemu-devel; +Cc: Igor Mammedov, Michael S. Tsirkin
legacy_acpi_cpu_plug_cb() dereferences @errp when
acpi_set_cpu_present_bit() fails. That's wrong; see the big comment
in error.h. Introduced in commit cc43364de7 "acpi/cpu-hotplug:
introduce helper function to keep bit setting in one place".
No caller actually passes null, and acpi_set_cpu_present_bit() can't
actually fail.
Fix anyway: drop acpi_set_cpu_present_bit()'s @errp parameter.
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
---
hw/acpi/cpu_hotplug.c | 10 +++-------
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/hw/acpi/cpu_hotplug.c b/hw/acpi/cpu_hotplug.c
index 3ac2045a95..9c3bcc84de 100644
--- a/hw/acpi/cpu_hotplug.c
+++ b/hw/acpi/cpu_hotplug.c
@@ -55,8 +55,7 @@ static const MemoryRegionOps AcpiCpuHotplug_ops = {
},
};
-static void acpi_set_cpu_present_bit(AcpiCpuHotplug *g, CPUState *cpu,
- Error **errp)
+static void acpi_set_cpu_present_bit(AcpiCpuHotplug *g, CPUState *cpu)
{
CPUClass *k = CPU_GET_CLASS(cpu);
int64_t cpu_id;
@@ -74,10 +73,7 @@ static void acpi_set_cpu_present_bit(AcpiCpuHotplug *g, CPUState *cpu,
void legacy_acpi_cpu_plug_cb(HotplugHandler *hotplug_dev,
AcpiCpuHotplug *g, DeviceState *dev, Error **errp)
{
- acpi_set_cpu_present_bit(g, CPU(dev), errp);
- if (*errp != NULL) {
- return;
- }
+ acpi_set_cpu_present_bit(g, CPU(dev));
acpi_send_event(DEVICE(hotplug_dev), ACPI_CPU_HOTPLUG_STATUS);
}
@@ -92,7 +88,7 @@ void legacy_acpi_cpu_hotplug_init(MemoryRegion *parent, Object *owner,
gpe_cpu->device = owner;
CPU_FOREACH(cpu) {
- acpi_set_cpu_present_bit(gpe_cpu, cpu, &error_abort);
+ acpi_set_cpu_present_bit(gpe_cpu, cpu);
}
}
--
2.21.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH v2 06/18] hw/acpi: Fix legacy CPU plug error API violations
2019-12-04 9:36 ` [PATCH v2 06/18] hw/acpi: Fix legacy CPU plug " Markus Armbruster
@ 2019-12-09 16:01 ` Michael S. Tsirkin
0 siblings, 0 replies; 31+ messages in thread
From: Michael S. Tsirkin @ 2019-12-09 16:01 UTC (permalink / raw)
To: Markus Armbruster; +Cc: Igor Mammedov, qemu-devel
On Wed, Dec 04, 2019 at 10:36:13AM +0100, Markus Armbruster wrote:
> legacy_acpi_cpu_plug_cb() dereferences @errp when
> acpi_set_cpu_present_bit() fails. That's wrong; see the big comment
> in error.h. Introduced in commit cc43364de7 "acpi/cpu-hotplug:
> introduce helper function to keep bit setting in one place".
>
> No caller actually passes null, and acpi_set_cpu_present_bit() can't
> actually fail.
>
> Fix anyway: drop acpi_set_cpu_present_bit()'s @errp parameter.
>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Igor Mammedov <imammedo@redhat.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Feel free to merge through your tree pls.
> ---
> hw/acpi/cpu_hotplug.c | 10 +++-------
> 1 file changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/hw/acpi/cpu_hotplug.c b/hw/acpi/cpu_hotplug.c
> index 3ac2045a95..9c3bcc84de 100644
> --- a/hw/acpi/cpu_hotplug.c
> +++ b/hw/acpi/cpu_hotplug.c
> @@ -55,8 +55,7 @@ static const MemoryRegionOps AcpiCpuHotplug_ops = {
> },
> };
>
> -static void acpi_set_cpu_present_bit(AcpiCpuHotplug *g, CPUState *cpu,
> - Error **errp)
> +static void acpi_set_cpu_present_bit(AcpiCpuHotplug *g, CPUState *cpu)
> {
> CPUClass *k = CPU_GET_CLASS(cpu);
> int64_t cpu_id;
> @@ -74,10 +73,7 @@ static void acpi_set_cpu_present_bit(AcpiCpuHotplug *g, CPUState *cpu,
> void legacy_acpi_cpu_plug_cb(HotplugHandler *hotplug_dev,
> AcpiCpuHotplug *g, DeviceState *dev, Error **errp)
> {
> - acpi_set_cpu_present_bit(g, CPU(dev), errp);
> - if (*errp != NULL) {
> - return;
> - }
> + acpi_set_cpu_present_bit(g, CPU(dev));
> acpi_send_event(DEVICE(hotplug_dev), ACPI_CPU_HOTPLUG_STATUS);
> }
>
> @@ -92,7 +88,7 @@ void legacy_acpi_cpu_hotplug_init(MemoryRegion *parent, Object *owner,
> gpe_cpu->device = owner;
>
> CPU_FOREACH(cpu) {
> - acpi_set_cpu_present_bit(gpe_cpu, cpu, &error_abort);
> + acpi_set_cpu_present_bit(gpe_cpu, cpu);
> }
> }
>
> --
> 2.21.0
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v2 07/18] hw/core: Fix fit_load_fdt() error handling violations
2019-12-04 9:36 [PATCH v2 00/18] Error handling fixes Markus Armbruster
` (5 preceding siblings ...)
2019-12-04 9:36 ` [PATCH v2 06/18] hw/acpi: Fix legacy CPU plug " Markus Armbruster
@ 2019-12-04 9:36 ` Markus Armbruster
2019-12-04 14:55 ` Philippe Mathieu-Daudé
2019-12-04 9:36 ` [PATCH v2 08/18] hw/ipmi: Fix realize() error API violations Markus Armbruster
` (13 subsequent siblings)
20 siblings, 1 reply; 31+ messages in thread
From: Markus Armbruster @ 2019-12-04 9:36 UTC (permalink / raw)
To: qemu-devel
fit_load_fdt() passes @errp to fit_image_addr(), then recovers from
ENOENT failures. Passing @errp is wrong, because it works only as
long as @errp is neither @error_fatal nor @error_abort. Messed up in
commit 3eb99edb48 "loader-fit: Wean off error_printf()".
No caller actually passes such values.
Fix anyway: splice in a local Error *err, and error_propagate().
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
hw/core/loader-fit.c | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/hw/core/loader-fit.c b/hw/core/loader-fit.c
index 953b16bc82..c465921b8f 100644
--- a/hw/core/loader-fit.c
+++ b/hw/core/loader-fit.c
@@ -178,11 +178,12 @@ static int fit_load_fdt(const struct fit_loader *ldr, const void *itb,
int cfg, void *opaque, const void *match_data,
hwaddr kernel_end, Error **errp)
{
+ Error *err = NULL;
const char *name;
const void *data;
const void *load_data;
hwaddr load_addr;
- int img_off, err;
+ int img_off;
size_t sz;
int ret;
@@ -197,13 +198,13 @@ static int fit_load_fdt(const struct fit_loader *ldr, const void *itb,
return -EINVAL;
}
- err = fit_image_addr(itb, img_off, "load", &load_addr, errp);
- if (err == -ENOENT) {
+ ret = fit_image_addr(itb, img_off, "load", &load_addr, &err);
+ if (ret == -ENOENT) {
load_addr = ROUND_UP(kernel_end, 64 * KiB) + (10 * MiB);
- error_free(*errp);
- } else if (err) {
- error_prepend(errp, "unable to read FDT load address from FIT: ");
- ret = err;
+ error_free(err);
+ } else if (ret) {
+ error_propagate_prepend(errp, err,
+ "unable to read FDT load address from FIT: ");
goto out;
}
--
2.21.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH v2 07/18] hw/core: Fix fit_load_fdt() error handling violations
2019-12-04 9:36 ` [PATCH v2 07/18] hw/core: Fix fit_load_fdt() error handling violations Markus Armbruster
@ 2019-12-04 14:55 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 31+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-12-04 14:55 UTC (permalink / raw)
To: Markus Armbruster, qemu-devel
On 12/4/19 10:36 AM, Markus Armbruster wrote:
> fit_load_fdt() passes @errp to fit_image_addr(), then recovers from
> ENOENT failures. Passing @errp is wrong, because it works only as
> long as @errp is neither @error_fatal nor @error_abort. Messed up in
> commit 3eb99edb48 "loader-fit: Wean off error_printf()".
>
> No caller actually passes such values.
>
> Fix anyway: splice in a local Error *err, and error_propagate().
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> hw/core/loader-fit.c | 15 ++++++++-------
> 1 file changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/hw/core/loader-fit.c b/hw/core/loader-fit.c
> index 953b16bc82..c465921b8f 100644
> --- a/hw/core/loader-fit.c
> +++ b/hw/core/loader-fit.c
> @@ -178,11 +178,12 @@ static int fit_load_fdt(const struct fit_loader *ldr, const void *itb,
> int cfg, void *opaque, const void *match_data,
> hwaddr kernel_end, Error **errp)
> {
> + Error *err = NULL;
> const char *name;
> const void *data;
> const void *load_data;
> hwaddr load_addr;
> - int img_off, err;
> + int img_off;
> size_t sz;
> int ret;
>
> @@ -197,13 +198,13 @@ static int fit_load_fdt(const struct fit_loader *ldr, const void *itb,
> return -EINVAL;
> }
>
> - err = fit_image_addr(itb, img_off, "load", &load_addr, errp);
> - if (err == -ENOENT) {
> + ret = fit_image_addr(itb, img_off, "load", &load_addr, &err);
> + if (ret == -ENOENT) {
> load_addr = ROUND_UP(kernel_end, 64 * KiB) + (10 * MiB);
> - error_free(*errp);
> - } else if (err) {
> - error_prepend(errp, "unable to read FDT load address from FIT: ");
> - ret = err;
> + error_free(err);
> + } else if (ret) {
> + error_propagate_prepend(errp, err,
> + "unable to read FDT load address from FIT: ");
> goto out;
> }
>
Cleaner.
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v2 08/18] hw/ipmi: Fix realize() error API violations
2019-12-04 9:36 [PATCH v2 00/18] Error handling fixes Markus Armbruster
` (6 preceding siblings ...)
2019-12-04 9:36 ` [PATCH v2 07/18] hw/core: Fix fit_load_fdt() error handling violations Markus Armbruster
@ 2019-12-04 9:36 ` Markus Armbruster
2019-12-04 9:36 ` [PATCH v2 09/18] qga: Fix guest-get-fsinfo " Markus Armbruster
` (12 subsequent siblings)
20 siblings, 0 replies; 31+ messages in thread
From: Markus Armbruster @ 2019-12-04 9:36 UTC (permalink / raw)
To: qemu-devel; +Cc: Corey Minyard
isa_ipmi_bt_realize(), ipmi_isa_realize(), pci_ipmi_bt_realize(), and
pci_ipmi_kcs_realize() dereference @errp when IPMIInterfaceClass
method init() fails. That's wrong; see the big comment in error.h.
Introduced in commit 0719029c47 "ipmi: Add an ISA KCS low-level
interface", then imitated in commit a9b74079cb "ipmi: Add a BT
low-level interface" and commit 12f983c6aa "ipmi: Add PCI IPMI
interfaces".
No caller actually passes null.
Fix anyway: splice in a local Error *err, and error_propagate().
Cc: Corey Minyard <cminyard@mvista.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
hw/ipmi/isa_ipmi_bt.c | 7 +++++--
hw/ipmi/isa_ipmi_kcs.c | 7 +++++--
hw/ipmi/pci_ipmi_bt.c | 6 ++++--
hw/ipmi/pci_ipmi_kcs.c | 6 ++++--
4 files changed, 18 insertions(+), 8 deletions(-)
diff --git a/hw/ipmi/isa_ipmi_bt.c b/hw/ipmi/isa_ipmi_bt.c
index 9a87ffd3f0..9fba5ed383 100644
--- a/hw/ipmi/isa_ipmi_bt.c
+++ b/hw/ipmi/isa_ipmi_bt.c
@@ -70,6 +70,7 @@ static void isa_ipmi_bt_lower_irq(IPMIBT *ib)
static void isa_ipmi_bt_realize(DeviceState *dev, Error **errp)
{
+ Error *err = NULL;
ISADevice *isadev = ISA_DEVICE(dev);
ISAIPMIBTDevice *iib = ISA_IPMI_BT(dev);
IPMIInterface *ii = IPMI_INTERFACE(dev);
@@ -85,9 +86,11 @@ static void isa_ipmi_bt_realize(DeviceState *dev, Error **errp)
iib->bt.bmc->intf = ii;
iib->bt.opaque = iib;
- iic->init(ii, 0, errp);
- if (*errp)
+ iic->init(ii, 0, &err);
+ if (err) {
+ error_propagate(errp, err);
return;
+ }
if (iib->isairq > 0) {
isa_init_irq(isadev, &iib->irq, iib->isairq);
diff --git a/hw/ipmi/isa_ipmi_kcs.c b/hw/ipmi/isa_ipmi_kcs.c
index ca3ea36a3f..cc6bd817f2 100644
--- a/hw/ipmi/isa_ipmi_kcs.c
+++ b/hw/ipmi/isa_ipmi_kcs.c
@@ -69,6 +69,7 @@ static void isa_ipmi_kcs_lower_irq(IPMIKCS *ik)
static void ipmi_isa_realize(DeviceState *dev, Error **errp)
{
+ Error *err = NULL;
ISADevice *isadev = ISA_DEVICE(dev);
ISAIPMIKCSDevice *iik = ISA_IPMI_KCS(dev);
IPMIInterface *ii = IPMI_INTERFACE(dev);
@@ -84,9 +85,11 @@ static void ipmi_isa_realize(DeviceState *dev, Error **errp)
iik->kcs.bmc->intf = ii;
iik->kcs.opaque = iik;
- iic->init(ii, 0, errp);
- if (*errp)
+ iic->init(ii, 0, &err);
+ if (err) {
+ error_propagate(errp, err);
return;
+ }
if (iik->isairq > 0) {
isa_init_irq(isadev, &iik->irq, iik->isairq);
diff --git a/hw/ipmi/pci_ipmi_bt.c b/hw/ipmi/pci_ipmi_bt.c
index 6ed925a665..ba9cf016b5 100644
--- a/hw/ipmi/pci_ipmi_bt.c
+++ b/hw/ipmi/pci_ipmi_bt.c
@@ -54,6 +54,7 @@ static void pci_ipmi_lower_irq(IPMIBT *ik)
static void pci_ipmi_bt_realize(PCIDevice *pd, Error **errp)
{
+ Error *err = NULL;
PCIIPMIBTDevice *pik = PCI_IPMI_BT(pd);
IPMIInterface *ii = IPMI_INTERFACE(pd);
IPMIInterfaceClass *iic = IPMI_INTERFACE_GET_CLASS(ii);
@@ -74,8 +75,9 @@ static void pci_ipmi_bt_realize(PCIDevice *pd, Error **errp)
pik->bt.raise_irq = pci_ipmi_raise_irq;
pik->bt.lower_irq = pci_ipmi_lower_irq;
- iic->init(ii, 8, errp);
- if (*errp) {
+ iic->init(ii, 8, &err);
+ if (err) {
+ error_propagate(errp, err);
return;
}
pci_register_bar(pd, 0, PCI_BASE_ADDRESS_SPACE_IO, &pik->bt.io);
diff --git a/hw/ipmi/pci_ipmi_kcs.c b/hw/ipmi/pci_ipmi_kcs.c
index eeba63baa4..99f46152f4 100644
--- a/hw/ipmi/pci_ipmi_kcs.c
+++ b/hw/ipmi/pci_ipmi_kcs.c
@@ -54,6 +54,7 @@ static void pci_ipmi_lower_irq(IPMIKCS *ik)
static void pci_ipmi_kcs_realize(PCIDevice *pd, Error **errp)
{
+ Error *err = NULL;
PCIIPMIKCSDevice *pik = PCI_IPMI_KCS(pd);
IPMIInterface *ii = IPMI_INTERFACE(pd);
IPMIInterfaceClass *iic = IPMI_INTERFACE_GET_CLASS(ii);
@@ -74,8 +75,9 @@ static void pci_ipmi_kcs_realize(PCIDevice *pd, Error **errp)
pik->kcs.raise_irq = pci_ipmi_raise_irq;
pik->kcs.lower_irq = pci_ipmi_lower_irq;
- iic->init(ii, 8, errp);
- if (*errp) {
+ iic->init(ii, 8, &err);
+ if (err) {
+ error_propagate(errp, err);
return;
}
pci_register_bar(pd, 0, PCI_BASE_ADDRESS_SPACE_IO, &pik->kcs.io);
--
2.21.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH v2 09/18] qga: Fix guest-get-fsinfo error API violations
2019-12-04 9:36 [PATCH v2 00/18] Error handling fixes Markus Armbruster
` (7 preceding siblings ...)
2019-12-04 9:36 ` [PATCH v2 08/18] hw/ipmi: Fix realize() error API violations Markus Armbruster
@ 2019-12-04 9:36 ` Markus Armbruster
2019-12-04 14:57 ` Philippe Mathieu-Daudé
2019-12-04 9:36 ` [PATCH v2 10/18] memory-device: Fix memory pre-plug " Markus Armbruster
` (11 subsequent siblings)
20 siblings, 1 reply; 31+ messages in thread
From: Markus Armbruster @ 2019-12-04 9:36 UTC (permalink / raw)
To: qemu-devel; +Cc: Michael Roth
build_guest_fsinfo_for_virtual_device() dereferences @errp when
build_guest_fsinfo_for_device() fails. That's wrong; see the big
comment in error.h. Introduced in commit 46d4c5723e "qga: Add
guest-get-fsinfo command".
No caller actually passes null.
Fix anyway: splice in a local Error *err, and error_propagate().
Cc: Michael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
qga/commands-posix.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 1c1a165dae..0be527ccb8 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -1049,6 +1049,7 @@ static void build_guest_fsinfo_for_virtual_device(char const *syspath,
GuestFilesystemInfo *fs,
Error **errp)
{
+ Error *err = NULL;
DIR *dir;
char *dirpath;
struct dirent *entry;
@@ -1078,10 +1079,11 @@ static void build_guest_fsinfo_for_virtual_device(char const *syspath,
g_debug(" slave device '%s'", entry->d_name);
path = g_strdup_printf("%s/slaves/%s", syspath, entry->d_name);
- build_guest_fsinfo_for_device(path, fs, errp);
+ build_guest_fsinfo_for_device(path, fs, &err);
g_free(path);
- if (*errp) {
+ if (err) {
+ error_propagate(errp, err);
break;
}
}
--
2.21.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH v2 09/18] qga: Fix guest-get-fsinfo error API violations
2019-12-04 9:36 ` [PATCH v2 09/18] qga: Fix guest-get-fsinfo " Markus Armbruster
@ 2019-12-04 14:57 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 31+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-12-04 14:57 UTC (permalink / raw)
To: Markus Armbruster, qemu-devel; +Cc: Michael Roth
On 12/4/19 10:36 AM, Markus Armbruster wrote:
> build_guest_fsinfo_for_virtual_device() dereferences @errp when
> build_guest_fsinfo_for_device() fails. That's wrong; see the big
> comment in error.h. Introduced in commit 46d4c5723e "qga: Add
> guest-get-fsinfo command".
>
> No caller actually passes null.
>
> Fix anyway: splice in a local Error *err, and error_propagate().
>
> Cc: Michael Roth <mdroth@linux.vnet.ibm.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> qga/commands-posix.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> index 1c1a165dae..0be527ccb8 100644
> --- a/qga/commands-posix.c
> +++ b/qga/commands-posix.c
> @@ -1049,6 +1049,7 @@ static void build_guest_fsinfo_for_virtual_device(char const *syspath,
> GuestFilesystemInfo *fs,
> Error **errp)
> {
> + Error *err = NULL;
> DIR *dir;
> char *dirpath;
> struct dirent *entry;
> @@ -1078,10 +1079,11 @@ static void build_guest_fsinfo_for_virtual_device(char const *syspath,
>
> g_debug(" slave device '%s'", entry->d_name);
> path = g_strdup_printf("%s/slaves/%s", syspath, entry->d_name);
> - build_guest_fsinfo_for_device(path, fs, errp);
> + build_guest_fsinfo_for_device(path, fs, &err);
> g_free(path);
>
> - if (*errp) {
> + if (err) {
> + error_propagate(errp, err);
> break;
> }
> }
>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v2 10/18] memory-device: Fix memory pre-plug error API violations
2019-12-04 9:36 [PATCH v2 00/18] Error handling fixes Markus Armbruster
` (8 preceding siblings ...)
2019-12-04 9:36 ` [PATCH v2 09/18] qga: Fix guest-get-fsinfo " Markus Armbruster
@ 2019-12-04 9:36 ` Markus Armbruster
2019-12-04 9:36 ` [PATCH v2 11/18] s390x/event-facility: Fix realize() " Markus Armbruster
` (10 subsequent siblings)
20 siblings, 0 replies; 31+ messages in thread
From: Markus Armbruster @ 2019-12-04 9:36 UTC (permalink / raw)
To: qemu-devel; +Cc: David Hildenbrand
memory_device_get_free_addr() dereferences @errp when
memory_device_check_addable() fails. That's wrong; see the big
comment in error.h. Introduced in commit 1b6d6af21b "pc-dimm: factor
out capacity and slot checks into MemoryDevice".
No caller actually passes null.
Fix anyway: splice in a local Error *err, and error_propagate().
Cc: David Hildenbrand <david@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
---
hw/mem/memory-device.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/hw/mem/memory-device.c b/hw/mem/memory-device.c
index aef148c1d7..4bc9cf0917 100644
--- a/hw/mem/memory-device.c
+++ b/hw/mem/memory-device.c
@@ -99,6 +99,7 @@ static uint64_t memory_device_get_free_addr(MachineState *ms,
uint64_t align, uint64_t size,
Error **errp)
{
+ Error *err = NULL;
GSList *list = NULL, *item;
Range as, new = range_empty;
@@ -123,8 +124,9 @@ static uint64_t memory_device_get_free_addr(MachineState *ms,
return 0;
}
- memory_device_check_addable(ms, size, errp);
- if (*errp) {
+ memory_device_check_addable(ms, size, &err);
+ if (err) {
+ error_propagate(errp, err);
return 0;
}
--
2.21.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH v2 11/18] s390x/event-facility: Fix realize() error API violations
2019-12-04 9:36 [PATCH v2 00/18] Error handling fixes Markus Armbruster
` (9 preceding siblings ...)
2019-12-04 9:36 ` [PATCH v2 10/18] memory-device: Fix memory pre-plug " Markus Armbruster
@ 2019-12-04 9:36 ` Markus Armbruster
2019-12-04 9:36 ` [PATCH v2 12/18] s390x/cpumodel: Fix feature property " Markus Armbruster
` (9 subsequent siblings)
20 siblings, 0 replies; 31+ messages in thread
From: Markus Armbruster @ 2019-12-04 9:36 UTC (permalink / raw)
To: qemu-devel; +Cc: Cornelia Huck, David Hildenbrand
sclp_events_bus_realize() dereferences @errp when
object_property_set_bool() fails. That's wrong; see the big comment
in error.h. Introduced in commit f6102c329c "s390/sclp: rework sclp
event facility initialization + device realization".
No caller actually passes null.
Fix anyway: splice in a local Error *err, and error_propagate().
Cc: David Hildenbrand <david@redhat.com>
Cc: Cornelia Huck <cohuck@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
---
hw/s390x/event-facility.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/hw/s390x/event-facility.c b/hw/s390x/event-facility.c
index 66205697ae..cdcf9154c4 100644
--- a/hw/s390x/event-facility.c
+++ b/hw/s390x/event-facility.c
@@ -339,14 +339,16 @@ out:
static void sclp_events_bus_realize(BusState *bus, Error **errp)
{
+ Error *err = NULL;
BusChild *kid;
/* TODO: recursive realization has to be done in common code */
QTAILQ_FOREACH(kid, &bus->children, sibling) {
DeviceState *dev = kid->child;
- object_property_set_bool(OBJECT(dev), true, "realized", errp);
- if (*errp) {
+ object_property_set_bool(OBJECT(dev), true, "realized", &err);
+ if (errp) {
+ error_propagate(errp, err);
return;
}
}
--
2.21.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH v2 12/18] s390x/cpumodel: Fix feature property error API violations
2019-12-04 9:36 [PATCH v2 00/18] Error handling fixes Markus Armbruster
` (10 preceding siblings ...)
2019-12-04 9:36 ` [PATCH v2 11/18] s390x/event-facility: Fix realize() " Markus Armbruster
@ 2019-12-04 9:36 ` Markus Armbruster
2019-12-04 9:36 ` [PATCH v2 13/18] s390x/cpumodel: Fix realize() " Markus Armbruster
` (8 subsequent siblings)
20 siblings, 0 replies; 31+ messages in thread
From: Markus Armbruster @ 2019-12-04 9:36 UTC (permalink / raw)
To: qemu-devel; +Cc: Cornelia Huck, David Hildenbrand
s390x-cpu property setters set_feature() and set_feature_group()
dereference @errp when the visitor fails. That's wrong; see the big
comment in error.h. Introduced in commit 0754f60429 "s390x/cpumodel:
expose features and feature groups as properties".
No caller actually passes null.
Fix anyway: splice in a local Error *err, and error_propagate().
Cc: David Hildenbrand <david@redhat.com>
Cc: Cornelia Huck <cohuck@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
---
target/s390x/cpu_models.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
index 7e92fb2e15..6a29fd3ab1 100644
--- a/target/s390x/cpu_models.c
+++ b/target/s390x/cpu_models.c
@@ -987,6 +987,7 @@ static void get_feature(Object *obj, Visitor *v, const char *name,
static void set_feature(Object *obj, Visitor *v, const char *name,
void *opaque, Error **errp)
{
+ Error *err = NULL;
S390Feat feat = (S390Feat) opaque;
DeviceState *dev = DEVICE(obj);
S390CPU *cpu = S390_CPU(obj);
@@ -1002,8 +1003,9 @@ static void set_feature(Object *obj, Visitor *v, const char *name,
return;
}
- visit_type_bool(v, name, &value, errp);
- if (*errp) {
+ visit_type_bool(v, name, &value, &err);
+ if (err) {
+ error_propagate(errp, err);
return;
}
if (value) {
@@ -1043,6 +1045,7 @@ static void get_feature_group(Object *obj, Visitor *v, const char *name,
static void set_feature_group(Object *obj, Visitor *v, const char *name,
void *opaque, Error **errp)
{
+ Error *err = NULL;
S390FeatGroup group = (S390FeatGroup) opaque;
const S390FeatGroupDef *def = s390_feat_group_def(group);
DeviceState *dev = DEVICE(obj);
@@ -1059,8 +1062,9 @@ static void set_feature_group(Object *obj, Visitor *v, const char *name,
return;
}
- visit_type_bool(v, name, &value, errp);
- if (*errp) {
+ visit_type_bool(v, name, &value, &err);
+ if (err) {
+ error_propagate(errp, err);
return;
}
if (value) {
--
2.21.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH v2 13/18] s390x/cpumodel: Fix realize() error API violations
2019-12-04 9:36 [PATCH v2 00/18] Error handling fixes Markus Armbruster
` (11 preceding siblings ...)
2019-12-04 9:36 ` [PATCH v2 12/18] s390x/cpumodel: Fix feature property " Markus Armbruster
@ 2019-12-04 9:36 ` Markus Armbruster
2019-12-04 9:36 ` [PATCH v2 14/18] s390x/cpumodel: Fix query-cpu-model-FOO " Markus Armbruster
` (7 subsequent siblings)
20 siblings, 0 replies; 31+ messages in thread
From: Markus Armbruster @ 2019-12-04 9:36 UTC (permalink / raw)
To: qemu-devel; +Cc: Cornelia Huck, David Hildenbrand
get_max_cpu_model() dereferences @errp when
kvm_s390_get_host_cpu_model() fails, apply_cpu_model() dereferences it
when kvm_s390_apply_cpu_model() fails, and s390_realize_cpu_model()
dereferences it when get_max_cpu_model() or check_compatibility()
fail. That's wrong; see the big comment in error.h. All three
introduced in commit 80560137cf "s390x/cpumodel: check and apply the
CPU model".
No caller actually passes null.
Fix anyway: splice in a local Error *err, and error_propagate().
Cc: David Hildenbrand <david@redhat.com>
Cc: Cornelia Huck <cohuck@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
---
target/s390x/cpu_models.c | 37 ++++++++++++++++++++++---------------
1 file changed, 22 insertions(+), 15 deletions(-)
diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
index 6a29fd3ab1..c702e34a26 100644
--- a/target/s390x/cpu_models.c
+++ b/target/s390x/cpu_models.c
@@ -870,6 +870,7 @@ static void check_compatibility(const S390CPUModel *max_model,
static S390CPUModel *get_max_cpu_model(Error **errp)
{
+ Error *err = NULL;
static S390CPUModel max_model;
static bool cached;
@@ -878,22 +879,24 @@ static S390CPUModel *get_max_cpu_model(Error **errp)
}
if (kvm_enabled()) {
- kvm_s390_get_host_cpu_model(&max_model, errp);
+ kvm_s390_get_host_cpu_model(&max_model, &err);
} else {
max_model.def = s390_find_cpu_def(QEMU_MAX_CPU_TYPE, QEMU_MAX_CPU_GEN,
QEMU_MAX_CPU_EC_GA, NULL);
bitmap_copy(max_model.features, qemu_max_cpu_feat, S390_FEAT_MAX);
- }
- if (!*errp) {
- cached = true;
- return &max_model;
}
- return NULL;
+ if (err) {
+ error_propagate(errp, err);
+ return NULL;
+ }
+ cached = true;
+ return &max_model;
}
static inline void apply_cpu_model(const S390CPUModel *model, Error **errp)
{
#ifndef CONFIG_USER_ONLY
+ Error *err = NULL;
static S390CPUModel applied_model;
static bool applied;
@@ -909,20 +912,23 @@ static inline void apply_cpu_model(const S390CPUModel *model, Error **errp)
}
if (kvm_enabled()) {
- kvm_s390_apply_cpu_model(model, errp);
+ kvm_s390_apply_cpu_model(model, &err);
+ if (err) {
+ error_propagate(errp, err);
+ return;
+ }
}
- if (!*errp) {
- applied = true;
- if (model) {
- applied_model = *model;
- }
+ applied = true;
+ if (model) {
+ applied_model = *model;
}
#endif
}
void s390_realize_cpu_model(CPUState *cs, Error **errp)
{
+ Error *err = NULL;
S390CPUClass *xcc = S390_CPU_GET_CLASS(cs);
S390CPU *cpu = S390_CPU(cs);
const S390CPUModel *max_model;
@@ -939,7 +945,7 @@ void s390_realize_cpu_model(CPUState *cs, Error **errp)
}
max_model = get_max_cpu_model(errp);
- if (*errp) {
+ if (!max_model) {
error_prepend(errp, "CPU models are not available: ");
return;
}
@@ -951,8 +957,9 @@ void s390_realize_cpu_model(CPUState *cs, Error **errp)
cpu->model->cpu_ver = max_model->cpu_ver;
check_consistency(cpu->model);
- check_compatibility(max_model, cpu->model, errp);
- if (*errp) {
+ check_compatibility(max_model, cpu->model, &err);
+ if (err) {
+ error_propagate(errp, err);
return;
}
--
2.21.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH v2 14/18] s390x/cpumodel: Fix query-cpu-model-FOO error API violations
2019-12-04 9:36 [PATCH v2 00/18] Error handling fixes Markus Armbruster
` (12 preceding siblings ...)
2019-12-04 9:36 ` [PATCH v2 13/18] s390x/cpumodel: Fix realize() " Markus Armbruster
@ 2019-12-04 9:36 ` Markus Armbruster
2019-12-04 9:36 ` [PATCH v2 15/18] s390x/cpumodel: Fix query-cpu-definitions " Markus Armbruster
` (6 subsequent siblings)
20 siblings, 0 replies; 31+ messages in thread
From: Markus Armbruster @ 2019-12-04 9:36 UTC (permalink / raw)
To: qemu-devel; +Cc: Cornelia Huck, David Hildenbrand
cpu_model_from_info() is a helper for qmp_query_cpu_model_expansion(),
qmp_query_cpu_model_comparison(), qmp_query_cpu_model_baseline(). It
dereferences @errp when the visitor or the QOM setter fails. That's
wrong; see the big comment in error.h. Introduced in commit
137974cea3 's390x/cpumodel: implement QMP interface
"query-cpu-model-expansion"'.
Its three callers have the same issue. Introduced in commit
4e82ef0502 's390x/cpumodel: implement QMP interface
"query-cpu-model-comparison"' and commit f1a47d08ef 's390x/cpumodel:
implement QMP interface "query-cpu-model-baseline"'.
No caller actually passes null.
Fix anyway: splice in a local Error *err, and error_propagate().
Cc: David Hildenbrand <david@redhat.com>
Cc: Cornelia Huck <cohuck@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
---
target/s390x/cpu_models.c | 43 ++++++++++++++++++++++++---------------
1 file changed, 27 insertions(+), 16 deletions(-)
diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
index c702e34a26..3ed301b5e5 100644
--- a/target/s390x/cpu_models.c
+++ b/target/s390x/cpu_models.c
@@ -477,6 +477,7 @@ CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp)
static void cpu_model_from_info(S390CPUModel *model, const CpuModelInfo *info,
Error **errp)
{
+ Error *err = NULL;
const QDict *qdict = NULL;
const QDictEntry *e;
Visitor *visitor;
@@ -513,24 +514,26 @@ static void cpu_model_from_info(S390CPUModel *model, const CpuModelInfo *info,
if (qdict) {
visitor = qobject_input_visitor_new(info->props);
- visit_start_struct(visitor, NULL, NULL, 0, errp);
- if (*errp) {
+ visit_start_struct(visitor, NULL, NULL, 0, &err);
+ if (err) {
+ error_propagate(errp, err);
visit_free(visitor);
object_unref(obj);
return;
}
for (e = qdict_first(qdict); e; e = qdict_next(qdict, e)) {
- object_property_set(obj, visitor, e->key, errp);
- if (*errp) {
+ object_property_set(obj, visitor, e->key, &err);
+ if (err) {
break;
}
}
- if (!*errp) {
+ if (!err) {
visit_check_struct(visitor, errp);
}
visit_end_struct(visitor, NULL);
visit_free(visitor);
- if (*errp) {
+ if (err) {
+ error_propagate(errp, err);
object_unref(obj);
return;
}
@@ -595,13 +598,15 @@ CpuModelExpansionInfo *qmp_query_cpu_model_expansion(CpuModelExpansionType type,
CpuModelInfo *model,
Error **errp)
{
+ Error *err = NULL;
CpuModelExpansionInfo *expansion_info = NULL;
S390CPUModel s390_model;
bool delta_changes = false;
/* convert it to our internal representation */
- cpu_model_from_info(&s390_model, model, errp);
- if (*errp) {
+ cpu_model_from_info(&s390_model, model, &err);
+ if (err) {
+ error_propagate(errp, err);
return NULL;
}
@@ -634,18 +639,21 @@ CpuModelCompareInfo *qmp_query_cpu_model_comparison(CpuModelInfo *infoa,
CpuModelInfo *infob,
Error **errp)
{
+ Error *err = NULL;
CpuModelCompareResult feat_result, gen_result;
CpuModelCompareInfo *compare_info;
S390FeatBitmap missing, added;
S390CPUModel modela, modelb;
/* convert both models to our internal representation */
- cpu_model_from_info(&modela, infoa, errp);
- if (*errp) {
+ cpu_model_from_info(&modela, infoa, &err);
+ if (err) {
+ error_propagate(errp, err);
return NULL;
}
- cpu_model_from_info(&modelb, infob, errp);
- if (*errp) {
+ cpu_model_from_info(&modelb, infob, &err);
+ if (err) {
+ error_propagate(errp, err);
return NULL;
}
compare_info = g_new0(CpuModelCompareInfo, 1);
@@ -707,6 +715,7 @@ CpuModelBaselineInfo *qmp_query_cpu_model_baseline(CpuModelInfo *infoa,
CpuModelInfo *infob,
Error **errp)
{
+ Error *err = NULL;
CpuModelBaselineInfo *baseline_info;
S390CPUModel modela, modelb, model;
uint16_t cpu_type;
@@ -714,13 +723,15 @@ CpuModelBaselineInfo *qmp_query_cpu_model_baseline(CpuModelInfo *infoa,
uint8_t max_gen;
/* convert both models to our internal representation */
- cpu_model_from_info(&modela, infoa, errp);
- if (*errp) {
+ cpu_model_from_info(&modela, infoa, &err);
+ if (err) {
+ error_propagate(errp, err);
return NULL;
}
- cpu_model_from_info(&modelb, infob, errp);
- if (*errp) {
+ cpu_model_from_info(&modelb, infob, &err);
+ if (err) {
+ error_propagate(errp, err);
return NULL;
}
--
2.21.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH v2 15/18] s390x/cpumodel: Fix query-cpu-definitions error API violations
2019-12-04 9:36 [PATCH v2 00/18] Error handling fixes Markus Armbruster
` (13 preceding siblings ...)
2019-12-04 9:36 ` [PATCH v2 14/18] s390x/cpumodel: Fix query-cpu-model-FOO " Markus Armbruster
@ 2019-12-04 9:36 ` Markus Armbruster
2019-12-04 9:36 ` [PATCH v2 16/18] error: Clean up unusual names of Error * variables Markus Armbruster
` (5 subsequent siblings)
20 siblings, 0 replies; 31+ messages in thread
From: Markus Armbruster @ 2019-12-04 9:36 UTC (permalink / raw)
To: qemu-devel; +Cc: Cornelia Huck, David Hildenbrand
qmp_query_cpu_definitions() passes @errp to get_max_cpu_model(), then
frees any error it gets back. This effectively ignores errors.
Dereferencing @errp is wrong; see the big comment in error.h. Passing
@errp is also wrong, because it works only as long as @errp is neither
@error_fatal nor @error_abort. Introduced in commit 38cba1f4d8
"s390x: return unavailable features via query-cpu-definitions".
No caller actually passes such @errp values.
Fix anyway: simply pass NULL to get_max_cpu_model().
Cc: David Hildenbrand <david@redhat.com>
Cc: Cornelia Huck <cohuck@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
---
target/s390x/cpu_models.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
index 3ed301b5e5..547bab8ac3 100644
--- a/target/s390x/cpu_models.c
+++ b/target/s390x/cpu_models.c
@@ -462,11 +462,7 @@ CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp)
.list = NULL,
};
- list_data.model = get_max_cpu_model(errp);
- if (*errp) {
- error_free(*errp);
- *errp = NULL;
- }
+ list_data.model = get_max_cpu_model(NULL);
object_class_foreach(create_cpu_model_list, TYPE_S390_CPU, false,
&list_data);
--
2.21.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH v2 16/18] error: Clean up unusual names of Error * variables
2019-12-04 9:36 [PATCH v2 00/18] Error handling fixes Markus Armbruster
` (14 preceding siblings ...)
2019-12-04 9:36 ` [PATCH v2 15/18] s390x/cpumodel: Fix query-cpu-definitions " Markus Armbruster
@ 2019-12-04 9:36 ` Markus Armbruster
2019-12-04 9:36 ` [PATCH v2 17/18] hw/intc/s390: Simplify error handling in kvm_s390_flic_realize() Markus Armbruster
` (4 subsequent siblings)
20 siblings, 0 replies; 31+ messages in thread
From: Markus Armbruster @ 2019-12-04 9:36 UTC (permalink / raw)
To: qemu-devel; +Cc: Philippe Mathieu-Daudé
Local Error * variables are conventionally named @err or @local_err,
and Error ** parameters @errp. Naming local variables like parameters
is confusing. Clean that up.
Naming parameters like local variables is also confusing. Left for
another day.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
hw/intc/s390_flic_kvm.c | 10 +++++-----
hw/ppc/spapr_pci.c | 16 ++++++++--------
hw/ppc/spapr_pci_nvlink2.c | 10 +++++-----
tests/test-blockjob.c | 16 ++++++++--------
4 files changed, 26 insertions(+), 26 deletions(-)
diff --git a/hw/intc/s390_flic_kvm.c b/hw/intc/s390_flic_kvm.c
index c9ee80eaae..30d50c2369 100644
--- a/hw/intc/s390_flic_kvm.c
+++ b/hw/intc/s390_flic_kvm.c
@@ -582,10 +582,10 @@ static void kvm_s390_flic_realize(DeviceState *dev, Error **errp)
struct kvm_create_device cd = {0};
struct kvm_device_attr test_attr = {0};
int ret;
- Error *errp_local = NULL;
+ Error *err = NULL;
- KVM_S390_FLIC_GET_CLASS(dev)->parent_realize(dev, &errp_local);
- if (errp_local) {
+ KVM_S390_FLIC_GET_CLASS(dev)->parent_realize(dev, &err);
+ if (err) {
goto fail;
}
flic_state->fd = -1;
@@ -593,7 +593,7 @@ static void kvm_s390_flic_realize(DeviceState *dev, Error **errp)
cd.type = KVM_DEV_TYPE_FLIC;
ret = kvm_vm_ioctl(kvm_state, KVM_CREATE_DEVICE, &cd);
if (ret < 0) {
- error_setg_errno(&errp_local, errno, "Creating the KVM device failed");
+ error_setg_errno(&err, errno, "Creating the KVM device failed");
trace_flic_create_device(errno);
goto fail;
}
@@ -605,7 +605,7 @@ static void kvm_s390_flic_realize(DeviceState *dev, Error **errp)
KVM_HAS_DEVICE_ATTR, test_attr);
return;
fail:
- error_propagate(errp, errp_local);
+ error_propagate(errp, err);
}
static void kvm_s390_flic_reset(DeviceState *dev)
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index f6fbcf99ed..723373de73 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -2042,13 +2042,13 @@ void spapr_phb_dma_reset(SpaprPhbState *sphb)
static void spapr_phb_reset(DeviceState *qdev)
{
SpaprPhbState *sphb = SPAPR_PCI_HOST_BRIDGE(qdev);
- Error *errp = NULL;
+ Error *err = NULL;
spapr_phb_dma_reset(sphb);
spapr_phb_nvgpu_free(sphb);
- spapr_phb_nvgpu_setup(sphb, &errp);
- if (errp) {
- error_report_err(errp);
+ spapr_phb_nvgpu_setup(sphb, &err);
+ if (err) {
+ error_report_err(err);
}
/* Reset the IOMMU state */
@@ -2326,7 +2326,7 @@ int spapr_dt_phb(SpaprMachineState *spapr, SpaprPhbState *phb,
cpu_to_be32(phb->numa_node)};
SpaprTceTable *tcet;
SpaprDrc *drc;
- Error *errp = NULL;
+ Error *err = NULL;
/* Start populating the FDT */
_FDT(bus_off = fdt_add_subnode(fdt, 0, phb->dtbusname));
@@ -2408,9 +2408,9 @@ int spapr_dt_phb(SpaprMachineState *spapr, SpaprPhbState *phb,
return ret;
}
- spapr_phb_nvgpu_populate_dt(phb, fdt, bus_off, &errp);
- if (errp) {
- error_report_err(errp);
+ spapr_phb_nvgpu_populate_dt(phb, fdt, bus_off, &err);
+ if (err) {
+ error_report_err(err);
}
spapr_phb_nvgpu_ram_populate_dt(phb, fdt);
diff --git a/hw/ppc/spapr_pci_nvlink2.c b/hw/ppc/spapr_pci_nvlink2.c
index 4aa89ede23..8332d5694e 100644
--- a/hw/ppc/spapr_pci_nvlink2.c
+++ b/hw/ppc/spapr_pci_nvlink2.c
@@ -57,7 +57,7 @@ struct SpaprPhbPciNvGpuConfig {
uint64_t nv2_atsd_current;
int num; /* number of non empty (i.e. tgt!=0) entries in slots[] */
SpaprPhbPciNvGpuSlot slots[NVGPU_MAX_NUM];
- Error *errp;
+ Error *err;
};
static SpaprPhbPciNvGpuSlot *
@@ -153,7 +153,7 @@ static void spapr_phb_pci_collect_nvgpu(PCIBus *bus, PCIDevice *pdev,
spapr_pci_collect_nvnpu(nvgpus, pdev, tgt, MEMORY_REGION(mr_npu),
&local_err);
}
- error_propagate(&nvgpus->errp, local_err);
+ error_propagate(&nvgpus->err, local_err);
}
if ((pci_default_read_config(pdev, PCI_HEADER_TYPE, 1) !=
PCI_HEADER_TYPE_BRIDGE)) {
@@ -187,9 +187,9 @@ void spapr_phb_nvgpu_setup(SpaprPhbState *sphb, Error **errp)
pci_for_each_device(bus, pci_bus_num(bus),
spapr_phb_pci_collect_nvgpu, sphb->nvgpus);
- if (sphb->nvgpus->errp) {
- error_propagate(errp, sphb->nvgpus->errp);
- sphb->nvgpus->errp = NULL;
+ if (sphb->nvgpus->err) {
+ error_propagate(errp, sphb->nvgpus->err);
+ sphb->nvgpus->err = NULL;
goto cleanup_exit;
}
diff --git a/tests/test-blockjob.c b/tests/test-blockjob.c
index 7844c9ffcb..e670a20617 100644
--- a/tests/test-blockjob.c
+++ b/tests/test-blockjob.c
@@ -34,13 +34,13 @@ static BlockJob *mk_job(BlockBackend *blk, const char *id,
int flags)
{
BlockJob *job;
- Error *errp = NULL;
+ Error *err = NULL;
job = block_job_create(id, drv, NULL, blk_bs(blk),
0, BLK_PERM_ALL, 0, flags, block_job_cb,
- NULL, &errp);
+ NULL, &err);
if (should_succeed) {
- g_assert_null(errp);
+ g_assert_null(err);
g_assert_nonnull(job);
if (id) {
g_assert_cmpstr(job->job.id, ==, id);
@@ -48,9 +48,9 @@ static BlockJob *mk_job(BlockBackend *blk, const char *id,
g_assert_cmpstr(job->job.id, ==, blk_name(blk));
}
} else {
- g_assert_nonnull(errp);
+ g_assert_nonnull(err);
g_assert_null(job);
- error_free(errp);
+ error_free(err);
}
return job;
@@ -80,9 +80,9 @@ static BlockBackend *create_blk(const char *name)
bdrv_unref(bs);
if (name) {
- Error *errp = NULL;
- monitor_add_blk(blk, name, &errp);
- g_assert_null(errp);
+ Error *err = NULL;
+ monitor_add_blk(blk, name, &err);
+ g_assert_null(err);
}
return blk;
--
2.21.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH v2 17/18] hw/intc/s390: Simplify error handling in kvm_s390_flic_realize()
2019-12-04 9:36 [PATCH v2 00/18] Error handling fixes Markus Armbruster
` (15 preceding siblings ...)
2019-12-04 9:36 ` [PATCH v2 16/18] error: Clean up unusual names of Error * variables Markus Armbruster
@ 2019-12-04 9:36 ` Markus Armbruster
2019-12-04 9:36 ` [PATCH v2 18/18] tests-blockjob: Use error_free_or_abort() Markus Armbruster
` (3 subsequent siblings)
20 siblings, 0 replies; 31+ messages in thread
From: Markus Armbruster @ 2019-12-04 9:36 UTC (permalink / raw)
To: qemu-devel; +Cc: Halil Pasic, Christian Borntraeger, Cornelia Huck
Cc: Halil Pasic <pasic@linux.ibm.com>
Cc: Cornelia Huck <cohuck@redhat.com>
Cc: Christian Borntraeger <borntraeger@de.ibm.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
Acked-by: Halil Pasic <pasic@linux.ibm.com>
---
hw/intc/s390_flic_kvm.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/hw/intc/s390_flic_kvm.c b/hw/intc/s390_flic_kvm.c
index 30d50c2369..dddd33ea61 100644
--- a/hw/intc/s390_flic_kvm.c
+++ b/hw/intc/s390_flic_kvm.c
@@ -586,16 +586,17 @@ static void kvm_s390_flic_realize(DeviceState *dev, Error **errp)
KVM_S390_FLIC_GET_CLASS(dev)->parent_realize(dev, &err);
if (err) {
- goto fail;
+ error_propagate(errp, err);
+ return;
}
flic_state->fd = -1;
cd.type = KVM_DEV_TYPE_FLIC;
ret = kvm_vm_ioctl(kvm_state, KVM_CREATE_DEVICE, &cd);
if (ret < 0) {
- error_setg_errno(&err, errno, "Creating the KVM device failed");
+ error_setg_errno(errp, errno, "Creating the KVM device failed");
trace_flic_create_device(errno);
- goto fail;
+ return;
}
flic_state->fd = cd.fd;
@@ -603,9 +604,6 @@ static void kvm_s390_flic_realize(DeviceState *dev, Error **errp)
test_attr.group = KVM_DEV_FLIC_CLEAR_IO_IRQ;
flic_state->clear_io_supported = !ioctl(flic_state->fd,
KVM_HAS_DEVICE_ATTR, test_attr);
- return;
-fail:
- error_propagate(errp, err);
}
static void kvm_s390_flic_reset(DeviceState *dev)
--
2.21.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PATCH v2 18/18] tests-blockjob: Use error_free_or_abort()
2019-12-04 9:36 [PATCH v2 00/18] Error handling fixes Markus Armbruster
` (16 preceding siblings ...)
2019-12-04 9:36 ` [PATCH v2 17/18] hw/intc/s390: Simplify error handling in kvm_s390_flic_realize() Markus Armbruster
@ 2019-12-04 9:36 ` Markus Armbruster
2019-12-04 9:38 ` [PATCH v2 00/18] Error handling fixes David Hildenbrand
` (2 subsequent siblings)
20 siblings, 0 replies; 31+ messages in thread
From: Markus Armbruster @ 2019-12-04 9:36 UTC (permalink / raw)
To: qemu-devel
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
tests/test-blockjob.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/tests/test-blockjob.c b/tests/test-blockjob.c
index e670a20617..4eeb184caf 100644
--- a/tests/test-blockjob.c
+++ b/tests/test-blockjob.c
@@ -48,9 +48,8 @@ static BlockJob *mk_job(BlockBackend *blk, const char *id,
g_assert_cmpstr(job->job.id, ==, blk_name(blk));
}
} else {
- g_assert_nonnull(err);
+ error_free_or_abort(&err);
g_assert_null(job);
- error_free(err);
}
return job;
--
2.21.0
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PATCH v2 00/18] Error handling fixes
2019-12-04 9:36 [PATCH v2 00/18] Error handling fixes Markus Armbruster
` (17 preceding siblings ...)
2019-12-04 9:36 ` [PATCH v2 18/18] tests-blockjob: Use error_free_or_abort() Markus Armbruster
@ 2019-12-04 9:38 ` David Hildenbrand
2019-12-04 9:49 ` Markus Armbruster
2019-12-05 16:07 ` Cornelia Huck
20 siblings, 0 replies; 31+ messages in thread
From: David Hildenbrand @ 2019-12-04 9:38 UTC (permalink / raw)
To: Markus Armbruster, qemu-devel
Cc: Corey Minyard, Daniel P. Berrangé,
Michael S. Tsirkin, Cornelia Huck, Michael Roth, Halil Pasic,
Christian Borntraeger, Igor Mammedov
On 04.12.19 10:36, Markus Armbruster wrote:
> v2:
> * Old PATCH 01-03 have been merged for 4.2
> * PATCH 05-15: Commit message rephrased [David], R-bys kept
Thanks!
--
Thanks,
David / dhildenb
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v2 00/18] Error handling fixes
2019-12-04 9:36 [PATCH v2 00/18] Error handling fixes Markus Armbruster
` (18 preceding siblings ...)
2019-12-04 9:38 ` [PATCH v2 00/18] Error handling fixes David Hildenbrand
@ 2019-12-04 9:49 ` Markus Armbruster
2019-12-05 16:07 ` Cornelia Huck
20 siblings, 0 replies; 31+ messages in thread
From: Markus Armbruster @ 2019-12-04 9:49 UTC (permalink / raw)
To: qemu-devel
Cc: Corey Minyard, Daniel P. Berrangé,
David Hildenbrand, Cornelia Huck, Michael Roth, Halil Pasic,
Christian Borntraeger, Michael S. Tsirkin, Igor Mammedov
Neglected to mention: I'm fine with taking care of getting the whole
series merged. Maintainers, feel free to take "your" parts through your
tree regardless. Whatever is left after a while I'll take through mine.
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [PATCH v2 00/18] Error handling fixes
2019-12-04 9:36 [PATCH v2 00/18] Error handling fixes Markus Armbruster
` (19 preceding siblings ...)
2019-12-04 9:49 ` Markus Armbruster
@ 2019-12-05 16:07 ` Cornelia Huck
20 siblings, 0 replies; 31+ messages in thread
From: Cornelia Huck @ 2019-12-05 16:07 UTC (permalink / raw)
To: Markus Armbruster
Cc: Corey Minyard, Daniel P. Berrangé,
David Hildenbrand, Michael S. Tsirkin, qemu-devel, Michael Roth,
Halil Pasic, Christian Borntraeger, Igor Mammedov
On Wed, 4 Dec 2019 10:36:07 +0100
Markus Armbruster <armbru@redhat.com> wrote:
> v2:
> * Old PATCH 01-03 have been merged for 4.2
> * PATCH 05-15: Commit message rephrased [David], R-bys kept
Thanks, queued patches 11-15 to s390-next.
Patch 17 depends on patch 16, so I'll leave merging that one to you.
>
> Cc: "Daniel P. Berrangé" <berrange@redhat.com>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Christian Borntraeger <borntraeger@de.ibm.com>
> Cc: Corey Minyard <cminyard@mvista.com>
> Cc: Cornelia Huck <cohuck@redhat.com>
> Cc: David Hildenbrand <david@redhat.com>
> Cc: Halil Pasic <pasic@linux.ibm.com>
> Cc: Igor Mammedov <imammedo@redhat.com>
> Cc: Michael Roth <mdroth@linux.vnet.ibm.com>
>
> Markus Armbruster (18):
> crypto: Fix certificate file error handling crash bug
> crypto: Fix typo in QCryptoTLSSession's <example> comment
> io: Fix Error usage in a comment <example>
> tests: Clean up initialization of Error *err variables
> exec: Fix file_ram_alloc() error API violations
> hw/acpi: Fix legacy CPU plug error API violations
> hw/core: Fix fit_load_fdt() error handling violations
> hw/ipmi: Fix realize() error API violations
> qga: Fix guest-get-fsinfo error API violations
> memory-device: Fix memory pre-plug error API violations
> s390x/event-facility: Fix realize() error API violations
> s390x/cpumodel: Fix feature property error API violations
> s390x/cpumodel: Fix realize() error API violations
> s390x/cpumodel: Fix query-cpu-model-FOO error API violations
> s390x/cpumodel: Fix query-cpu-definitions error API violations
> error: Clean up unusual names of Error * variables
> hw/intc/s390: Simplify error handling in kvm_s390_flic_realize()
> tests-blockjob: Use error_free_or_abort()
>
> include/crypto/tlssession.h | 2 +-
> include/io/task.h | 2 +-
> crypto/tlscredsx509.c | 2 +-
> exec.c | 6 +-
> hw/acpi/cpu_hotplug.c | 10 +--
> hw/core/loader-fit.c | 15 ++---
> hw/intc/s390_flic_kvm.c | 16 +++--
> hw/ipmi/isa_ipmi_bt.c | 7 ++-
> hw/ipmi/isa_ipmi_kcs.c | 7 ++-
> hw/ipmi/pci_ipmi_bt.c | 6 +-
> hw/ipmi/pci_ipmi_kcs.c | 6 +-
> hw/mem/memory-device.c | 6 +-
> hw/ppc/spapr_pci.c | 16 ++---
> hw/ppc/spapr_pci_nvlink2.c | 10 +--
> hw/s390x/event-facility.c | 6 +-
> qga/commands-posix.c | 6 +-
> target/s390x/cpu_models.c | 98 +++++++++++++++++------------
> tests/test-blockjob.c | 15 +++--
> tests/test-qobject-output-visitor.c | 8 +--
> tests/test-string-output-visitor.c | 4 +-
> 20 files changed, 139 insertions(+), 109 deletions(-)
>
^ permalink raw reply [flat|nested] 31+ messages in thread