* [Qemu-devel] [PATCH v2] xics_kvm: fix a build break
@ 2018-06-12 10:11 Cédric Le Goater
2018-06-12 11:41 ` Greg Kurz
2018-06-12 23:43 ` David Gibson
0 siblings, 2 replies; 4+ messages in thread
From: Cédric Le Goater @ 2018-06-12 10:11 UTC (permalink / raw)
To: qemu-ppc
Cc: qemu-devel, David Gibson, Greg Kurz, Peter Maydell,
Cédric Le Goater
On CentOS 7.5, gcc-4.8.5-28.el7_5.1.ppc64le fails to build QEMU due to :
hw/intc/xics_kvm.c: In function ‘ics_set_kvm_state’:
hw/intc/xics_kvm.c:281:13: error: ‘ret’ may be used uninitialized in this
function [-Werror=maybe-uninitialized]
return ret;
Fix the breakage and also remove the extra error reporting as
kvm_device_access() already provides a substantial error message.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
Greg, can you please test ? RHEL7.5 does not catch the issue with the
same compiler level :/ I don't understand why.
hw/intc/xics_kvm.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/hw/intc/xics_kvm.c b/hw/intc/xics_kvm.c
index 8bdf6afe82a0..8dba2f84e71e 100644
--- a/hw/intc/xics_kvm.c
+++ b/hw/intc/xics_kvm.c
@@ -186,8 +186,7 @@ static void ics_get_kvm_state(ICSState *ics)
kvm_device_access(kernel_xics_fd, KVM_DEV_XICS_GRP_SOURCES,
i + ics->offset, &state, false, &local_err);
if (local_err) {
- error_report("Unable to retrieve KVM interrupt controller state"
- " for IRQ %d: %s", i + ics->offset, strerror(errno));
+ error_report_err(local_err);
exit(1);
}
@@ -273,11 +272,10 @@ static int ics_set_kvm_state(ICSState *ics, int version_id)
state |= KVM_XICS_QUEUED;
}
- kvm_device_access(kernel_xics_fd, KVM_DEV_XICS_GRP_SOURCES,
- i + ics->offset, &state, true, &local_err);
+ ret = kvm_device_access(kernel_xics_fd, KVM_DEV_XICS_GRP_SOURCES,
+ i + ics->offset, &state, true, &local_err);
if (local_err) {
- error_report("Unable to restore KVM interrupt controller state"
- " for IRQs %d: %s", i + ics->offset, strerror(errno));
+ error_report_err(local_err);
return ret;
}
}
--
2.13.6
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [Qemu-devel] [PATCH v2] xics_kvm: fix a build break
2018-06-12 10:11 [Qemu-devel] [PATCH v2] xics_kvm: fix a build break Cédric Le Goater
@ 2018-06-12 11:41 ` Greg Kurz
2018-06-12 12:19 ` Cédric Le Goater
2018-06-12 23:43 ` David Gibson
1 sibling, 1 reply; 4+ messages in thread
From: Greg Kurz @ 2018-06-12 11:41 UTC (permalink / raw)
To: Cédric Le Goater; +Cc: qemu-ppc, qemu-devel, David Gibson, Peter Maydell
On Tue, 12 Jun 2018 12:11:35 +0200
Cédric Le Goater <clg@kaod.org> wrote:
> On CentOS 7.5, gcc-4.8.5-28.el7_5.1.ppc64le fails to build QEMU due to :
>
> hw/intc/xics_kvm.c: In function ‘ics_set_kvm_state’:
> hw/intc/xics_kvm.c:281:13: error: ‘ret’ may be used uninitialized in this
> function [-Werror=maybe-uninitialized]
> return ret;
>
> Fix the breakage and also remove the extra error reporting as
> kvm_device_access() already provides a substantial error message.
>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>
> Greg, can you please test ? RHEL7.5 does not catch the issue with the
> same compiler level :/ I don't understand why.
>
This fixes the build breakage for me. Now this should really be squashed
into your original patch to preserve bisect on master.
Just one remark:
> hw/intc/xics_kvm.c | 10 ++++------
> 1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/hw/intc/xics_kvm.c b/hw/intc/xics_kvm.c
> index 8bdf6afe82a0..8dba2f84e71e 100644
> --- a/hw/intc/xics_kvm.c
> +++ b/hw/intc/xics_kvm.c
> @@ -186,8 +186,7 @@ static void ics_get_kvm_state(ICSState *ics)
> kvm_device_access(kernel_xics_fd, KVM_DEV_XICS_GRP_SOURCES,
> i + ics->offset, &state, false, &local_err);
> if (local_err) {
> - error_report("Unable to retrieve KVM interrupt controller state"
> - " for IRQ %d: %s", i + ics->offset, strerror(errno));
> + error_report_err(local_err);
This would change the error message from:
Unable to retrieve KVM interrupt controller state for IRQ 4096: Invalid argument
to
KVM_GET_DEVICE_ATTR failed: Group 1 attr 0x0000000000001000: Invalid argument
I find it a bit cryptic, and we loose both the IRQ number and direct indication
that it is related to XICS on KVM. That's why I had suggested to print both with
error_reportf_err().
Since this would result in a very long line, alternatively, you can
keep the current error message, using -ret instead of errno, and call
error_free(local_err).
> exit(1);
> }
>
> @@ -273,11 +272,10 @@ static int ics_set_kvm_state(ICSState *ics, int version_id)
> state |= KVM_XICS_QUEUED;
> }
>
> - kvm_device_access(kernel_xics_fd, KVM_DEV_XICS_GRP_SOURCES,
> - i + ics->offset, &state, true, &local_err);
> + ret = kvm_device_access(kernel_xics_fd, KVM_DEV_XICS_GRP_SOURCES,
> + i + ics->offset, &state, true, &local_err);
> if (local_err) {
> - error_report("Unable to restore KVM interrupt controller state"
> - " for IRQs %d: %s", i + ics->offset, strerror(errno));
> + error_report_err(local_err);
> return ret;
> }
> }
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [Qemu-devel] [PATCH v2] xics_kvm: fix a build break
2018-06-12 11:41 ` Greg Kurz
@ 2018-06-12 12:19 ` Cédric Le Goater
0 siblings, 0 replies; 4+ messages in thread
From: Cédric Le Goater @ 2018-06-12 12:19 UTC (permalink / raw)
To: Greg Kurz; +Cc: qemu-ppc, qemu-devel, David Gibson, Peter Maydell
On 06/12/2018 01:41 PM, Greg Kurz wrote:
> On Tue, 12 Jun 2018 12:11:35 +0200
> Cédric Le Goater <clg@kaod.org> wrote:
>
>> On CentOS 7.5, gcc-4.8.5-28.el7_5.1.ppc64le fails to build QEMU due to :
>>
>> hw/intc/xics_kvm.c: In function ‘ics_set_kvm_state’:
>> hw/intc/xics_kvm.c:281:13: error: ‘ret’ may be used uninitialized in this
>> function [-Werror=maybe-uninitialized]
>> return ret;
>>
>> Fix the breakage and also remove the extra error reporting as
>> kvm_device_access() already provides a substantial error message.
>>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>>
>> Greg, can you please test ? RHEL7.5 does not catch the issue with the
>> same compiler level :/ I don't understand why.
>>
>
> This fixes the build breakage for me. Now this should really be squashed
> into your original patch to preserve bisect on master.
After some analysis, you need to configure with '--enable-debug' to reproduce
the breakage. '--enable-debug' removes -O2.
With -O2, the compiler doesn't catch the error. It looks very much like a gcc
issue to me.
Thanks,
C.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH v2] xics_kvm: fix a build break
2018-06-12 10:11 [Qemu-devel] [PATCH v2] xics_kvm: fix a build break Cédric Le Goater
2018-06-12 11:41 ` Greg Kurz
@ 2018-06-12 23:43 ` David Gibson
1 sibling, 0 replies; 4+ messages in thread
From: David Gibson @ 2018-06-12 23:43 UTC (permalink / raw)
To: Cédric Le Goater; +Cc: qemu-ppc, qemu-devel, Greg Kurz, Peter Maydell
[-- Attachment #1: Type: text/plain, Size: 2414 bytes --]
On Tue, Jun 12, 2018 at 12:11:35PM +0200, Cédric Le Goater wrote:
> On CentOS 7.5, gcc-4.8.5-28.el7_5.1.ppc64le fails to build QEMU due to :
>
> hw/intc/xics_kvm.c: In function ‘ics_set_kvm_state’:
> hw/intc/xics_kvm.c:281:13: error: ‘ret’ may be used uninitialized in this
> function [-Werror=maybe-uninitialized]
> return ret;
>
> Fix the breakage and also remove the extra error reporting as
> kvm_device_access() already provides a substantial error message.
>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
Applied to ppc-for-3.0, thanks.
> ---
>
> Greg, can you please test ? RHEL7.5 does not catch the issue with the
> same compiler level :/ I don't understand why.
>
> hw/intc/xics_kvm.c | 10 ++++------
> 1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/hw/intc/xics_kvm.c b/hw/intc/xics_kvm.c
> index 8bdf6afe82a0..8dba2f84e71e 100644
> --- a/hw/intc/xics_kvm.c
> +++ b/hw/intc/xics_kvm.c
> @@ -186,8 +186,7 @@ static void ics_get_kvm_state(ICSState *ics)
> kvm_device_access(kernel_xics_fd, KVM_DEV_XICS_GRP_SOURCES,
> i + ics->offset, &state, false, &local_err);
> if (local_err) {
> - error_report("Unable to retrieve KVM interrupt controller state"
> - " for IRQ %d: %s", i + ics->offset, strerror(errno));
> + error_report_err(local_err);
> exit(1);
> }
>
> @@ -273,11 +272,10 @@ static int ics_set_kvm_state(ICSState *ics, int version_id)
> state |= KVM_XICS_QUEUED;
> }
>
> - kvm_device_access(kernel_xics_fd, KVM_DEV_XICS_GRP_SOURCES,
> - i + ics->offset, &state, true, &local_err);
> + ret = kvm_device_access(kernel_xics_fd, KVM_DEV_XICS_GRP_SOURCES,
> + i + ics->offset, &state, true, &local_err);
> if (local_err) {
> - error_report("Unable to restore KVM interrupt controller state"
> - " for IRQs %d: %s", i + ics->offset, strerror(errno));
> + error_report_err(local_err);
> return ret;
> }
> }
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-06-12 23:43 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-06-12 10:11 [Qemu-devel] [PATCH v2] xics_kvm: fix a build break Cédric Le Goater
2018-06-12 11:41 ` Greg Kurz
2018-06-12 12:19 ` Cédric Le Goater
2018-06-12 23:43 ` David Gibson
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).