hw/intc/xics_kvm.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-)
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
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; > } > }
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.
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
© 2016 - 2024 Red Hat, Inc.