* Re: [PATCH v2] hw/block: nvme: Fix a build error in nvme_get_feature()
2021-02-10 10:23 [PATCH v2] hw/block: nvme: Fix a build error in nvme_get_feature() Bin Meng
@ 2021-02-10 10:42 ` Klaus Jensen
2021-02-10 10:47 ` Minwoo Im
` (2 subsequent siblings)
3 siblings, 0 replies; 11+ messages in thread
From: Klaus Jensen @ 2021-02-10 10:42 UTC (permalink / raw)
To: Bin Meng
Cc: Kevin Wolf, Peter Maydell, qemu-block, qemu-trivial, Bin Meng,
qemu-devel, Max Reitz, Keith Busch
[-- Attachment #1: Type: text/plain, Size: 1302 bytes --]
CC qemu-trivial.
On Feb 10 18:23, Bin Meng wrote:
> From: Bin Meng <bin.meng@windriver.com>
>
> Current QEMU HEAD nvme.c does not compile:
>
> hw/block/nvme.c:3242:9: error: ‘result’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> trace_pci_nvme_getfeat_vwcache(result ? "enabled" : "disabled");
> ^
> hw/block/nvme.c:3150:14: note: ‘result’ was declared here
> uint32_t result;
> ^
>
> Explicitly initialize the result to fix it.
>
> Fixes: aa5e55e3b07e ("hw/block/nvme: open code for volatile write cache")
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
>
> ---
>
> Changes in v2:
> - update function name in the commit message
>
> hw/block/nvme.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> index 5ce21b7..c122ac0 100644
> --- a/hw/block/nvme.c
> +++ b/hw/block/nvme.c
> @@ -3228,6 +3228,7 @@ static uint16_t nvme_get_feature(NvmeCtrl *n, NvmeRequest *req)
> result = ns->features.err_rec;
> goto out;
> case NVME_VOLATILE_WRITE_CACHE:
> + result = 0;
> for (i = 1; i <= n->num_namespaces; i++) {
> ns = nvme_ns(n, i);
> if (!ns) {
> --
> 2.7.4
>
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v2] hw/block: nvme: Fix a build error in nvme_get_feature()
2021-02-10 10:23 [PATCH v2] hw/block: nvme: Fix a build error in nvme_get_feature() Bin Meng
2021-02-10 10:42 ` Klaus Jensen
@ 2021-02-10 10:47 ` Minwoo Im
2021-02-10 11:12 ` Philippe Mathieu-Daudé
2021-02-10 22:29 ` Peter Maydell
3 siblings, 0 replies; 11+ messages in thread
From: Minwoo Im @ 2021-02-10 10:47 UTC (permalink / raw)
To: Bin Meng
Cc: Kevin Wolf, Peter Maydell, qemu-block, Bin Meng, qemu-devel,
Max Reitz, Klaus Jensen, Keith Busch
On 21-02-10 18:23:17, Bin Meng wrote:
> From: Bin Meng <bin.meng@windriver.com>
>
> Current QEMU HEAD nvme.c does not compile:
>
> hw/block/nvme.c:3242:9: error: ‘result’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> trace_pci_nvme_getfeat_vwcache(result ? "enabled" : "disabled");
> ^
> hw/block/nvme.c:3150:14: note: ‘result’ was declared here
> uint32_t result;
> ^
>
> Explicitly initialize the result to fix it.
>
> Fixes: aa5e55e3b07e ("hw/block/nvme: open code for volatile write cache")
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
Bin,
Thanks for the fix!
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v2] hw/block: nvme: Fix a build error in nvme_get_feature()
2021-02-10 10:23 [PATCH v2] hw/block: nvme: Fix a build error in nvme_get_feature() Bin Meng
2021-02-10 10:42 ` Klaus Jensen
2021-02-10 10:47 ` Minwoo Im
@ 2021-02-10 11:12 ` Philippe Mathieu-Daudé
2021-02-10 11:15 ` Bin Meng
2021-02-10 11:15 ` Philippe Mathieu-Daudé
2021-02-10 22:29 ` Peter Maydell
3 siblings, 2 replies; 11+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-02-10 11:12 UTC (permalink / raw)
To: Bin Meng, Keith Busch, Kevin Wolf, Klaus Jensen, Max Reitz,
qemu-block, qemu-devel
Cc: Peter Maydell, Thomas Huth, Daniel P . Berrange, Bin Meng,
Minwoo Im, Alex Bennée
Hi Bin,
On 2/10/21 11:23 AM, Bin Meng wrote:
> From: Bin Meng <bin.meng@windriver.com>
>
> Current QEMU HEAD nvme.c does not compile:
>
> hw/block/nvme.c:3242:9: error: ‘result’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> trace_pci_nvme_getfeat_vwcache(result ? "enabled" : "disabled");
> ^
> hw/block/nvme.c:3150:14: note: ‘result’ was declared here
> uint32_t result;
> ^
Why isn't this catched by our CI? What is your host OS? Fedora 33?
>
> Explicitly initialize the result to fix it.
>
> Fixes: aa5e55e3b07e ("hw/block/nvme: open code for volatile write cache")
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
>
> ---
>
> Changes in v2:
> - update function name in the commit message
>
> hw/block/nvme.c | 1 +
> 1 file changed, 1 insertion(+)
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v2] hw/block: nvme: Fix a build error in nvme_get_feature()
2021-02-10 11:12 ` Philippe Mathieu-Daudé
@ 2021-02-10 11:15 ` Bin Meng
2021-02-10 11:23 ` Thomas Huth
2021-02-10 11:15 ` Philippe Mathieu-Daudé
1 sibling, 1 reply; 11+ messages in thread
From: Bin Meng @ 2021-02-10 11:15 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Kevin Wolf, Peter Maydell, Thomas Huth, Daniel P . Berrange,
Qemu-block, Bin Meng, qemu-devel@nongnu.org Developers,
Max Reitz, Klaus Jensen, Minwoo Im, Keith Busch,
Alex Bennée
Hi Philippe,
On Wed, Feb 10, 2021 at 7:12 PM Philippe Mathieu-Daudé
<philmd@redhat.com> wrote:
>
> Hi Bin,
>
> On 2/10/21 11:23 AM, Bin Meng wrote:
> > From: Bin Meng <bin.meng@windriver.com>
> >
> > Current QEMU HEAD nvme.c does not compile:
> >
> > hw/block/nvme.c:3242:9: error: ‘result’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> > trace_pci_nvme_getfeat_vwcache(result ? "enabled" : "disabled");
> > ^
> > hw/block/nvme.c:3150:14: note: ‘result’ was declared here
> > uint32_t result;
> > ^
>
> Why isn't this catched by our CI? What is your host OS? Fedora 33?
>
I am using GCC 5.4 on Ubuntu 16.04. Please see some initial analysis
from Peter about why newer version GCC does not report it.
Regards,
Bin
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] hw/block: nvme: Fix a build error in nvme_get_feature()
2021-02-10 11:15 ` Bin Meng
@ 2021-02-10 11:23 ` Thomas Huth
0 siblings, 0 replies; 11+ messages in thread
From: Thomas Huth @ 2021-02-10 11:23 UTC (permalink / raw)
To: Bin Meng, Philippe Mathieu-Daudé
Cc: Kevin Wolf, Peter Maydell, Daniel P . Berrange, Qemu-block,
Bin Meng, qemu-devel@nongnu.org Developers, Max Reitz,
Klaus Jensen, Minwoo Im, Keith Busch, Alex Bennée
On 10/02/2021 12.15, Bin Meng wrote:
> Hi Philippe,
>
> On Wed, Feb 10, 2021 at 7:12 PM Philippe Mathieu-Daudé
> <philmd@redhat.com> wrote:
>>
>> Hi Bin,
>>
>> On 2/10/21 11:23 AM, Bin Meng wrote:
>>> From: Bin Meng <bin.meng@windriver.com>
>>>
>>> Current QEMU HEAD nvme.c does not compile:
>>>
>>> hw/block/nvme.c:3242:9: error: ‘result’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
>>> trace_pci_nvme_getfeat_vwcache(result ? "enabled" : "disabled");
>>> ^
>>> hw/block/nvme.c:3150:14: note: ‘result’ was declared here
>>> uint32_t result;
>>> ^
>>
>> Why isn't this catched by our CI? What is your host OS? Fedora 33?
>>
>
> I am using GCC 5.4 on Ubuntu 16.04. Please see some initial analysis
> from Peter about why newer version GCC does not report it.
Please note that Ubuntu 16.04 is not a supported version by QEMU anymore,
see https://qemu.readthedocs.io/en/latest/system/build-platforms.html and
https://wiki.qemu.org/Supported_Build_Platforms for details.
Thomas
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] hw/block: nvme: Fix a build error in nvme_get_feature()
2021-02-10 11:12 ` Philippe Mathieu-Daudé
2021-02-10 11:15 ` Bin Meng
@ 2021-02-10 11:15 ` Philippe Mathieu-Daudé
2021-02-10 11:17 ` Bin Meng
2021-02-10 11:22 ` Daniel P. Berrangé
1 sibling, 2 replies; 11+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-02-10 11:15 UTC (permalink / raw)
To: Bin Meng, Keith Busch, Kevin Wolf, Klaus Jensen, Max Reitz,
qemu-block, qemu-devel
Cc: Peter Maydell, Thomas Huth, Daniel P . Berrange, Bin Meng,
Minwoo Im, Alex Bennée
On 2/10/21 12:12 PM, Philippe Mathieu-Daudé wrote:
> Hi Bin,
>
> On 2/10/21 11:23 AM, Bin Meng wrote:
>> From: Bin Meng <bin.meng@windriver.com>
>>
>> Current QEMU HEAD nvme.c does not compile:
>>
>> hw/block/nvme.c:3242:9: error: ‘result’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
>> trace_pci_nvme_getfeat_vwcache(result ? "enabled" : "disabled");
>> ^
>> hw/block/nvme.c:3150:14: note: ‘result’ was declared here
>> uint32_t result;
>> ^
>
> Why isn't this catched by our CI? What is your host OS? Fedora 33?
Just noticed v1 and Peter's explanation:
https://lists.gnu.org/archive/html/qemu-devel/2021-02/msg03528.html
Can you amend "default GCC 5.4 on a Ubuntu 16.04 host" information
please?
>
>>
>> Explicitly initialize the result to fix it.
>>
>> Fixes: aa5e55e3b07e ("hw/block/nvme: open code for volatile write cache")
>> Signed-off-by: Bin Meng <bin.meng@windriver.com>
>>
>> ---
>>
>> Changes in v2:
>> - update function name in the commit message
>>
>> hw/block/nvme.c | 1 +
>> 1 file changed, 1 insertion(+)
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v2] hw/block: nvme: Fix a build error in nvme_get_feature()
2021-02-10 11:15 ` Philippe Mathieu-Daudé
@ 2021-02-10 11:17 ` Bin Meng
2021-02-10 11:22 ` Daniel P. Berrangé
1 sibling, 0 replies; 11+ messages in thread
From: Bin Meng @ 2021-02-10 11:17 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Kevin Wolf, Peter Maydell, Thomas Huth, Daniel P . Berrange,
Qemu-block, Bin Meng, qemu-devel@nongnu.org Developers,
Max Reitz, Klaus Jensen, Minwoo Im, Keith Busch,
Alex Bennée
Hi Philippe,
On Wed, Feb 10, 2021 at 7:15 PM Philippe Mathieu-Daudé
<philmd@redhat.com> wrote:
>
> On 2/10/21 12:12 PM, Philippe Mathieu-Daudé wrote:
> > Hi Bin,
> >
> > On 2/10/21 11:23 AM, Bin Meng wrote:
> >> From: Bin Meng <bin.meng@windriver.com>
> >>
> >> Current QEMU HEAD nvme.c does not compile:
> >>
> >> hw/block/nvme.c:3242:9: error: ‘result’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> >> trace_pci_nvme_getfeat_vwcache(result ? "enabled" : "disabled");
> >> ^
> >> hw/block/nvme.c:3150:14: note: ‘result’ was declared here
> >> uint32_t result;
> >> ^
> >
> > Why isn't this catched by our CI? What is your host OS? Fedora 33?
>
> Just noticed v1 and Peter's explanation:
> https://lists.gnu.org/archive/html/qemu-devel/2021-02/msg03528.html
>
> Can you amend "default GCC 5.4 on a Ubuntu 16.04 host" information
> please?
>
Sure.
Regards,
Bin
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] hw/block: nvme: Fix a build error in nvme_get_feature()
2021-02-10 11:15 ` Philippe Mathieu-Daudé
2021-02-10 11:17 ` Bin Meng
@ 2021-02-10 11:22 ` Daniel P. Berrangé
2021-02-10 11:34 ` Daniel P. Berrangé
1 sibling, 1 reply; 11+ messages in thread
From: Daniel P. Berrangé @ 2021-02-10 11:22 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Kevin Wolf, Peter Maydell, Thomas Huth, qemu-block, Bin Meng,
qemu-devel, Max Reitz, Klaus Jensen, Minwoo Im, Keith Busch,
Bin Meng, Alex Bennée
On Wed, Feb 10, 2021 at 12:15:45PM +0100, Philippe Mathieu-Daudé wrote:
> On 2/10/21 12:12 PM, Philippe Mathieu-Daudé wrote:
> > Hi Bin,
> >
> > On 2/10/21 11:23 AM, Bin Meng wrote:
> >> From: Bin Meng <bin.meng@windriver.com>
> >>
> >> Current QEMU HEAD nvme.c does not compile:
> >>
> >> hw/block/nvme.c:3242:9: error: ‘result’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> >> trace_pci_nvme_getfeat_vwcache(result ? "enabled" : "disabled");
> >> ^
> >> hw/block/nvme.c:3150:14: note: ‘result’ was declared here
> >> uint32_t result;
> >> ^
> >
> > Why isn't this catched by our CI? What is your host OS? Fedora 33?
>
> Just noticed v1 and Peter's explanation:
> https://lists.gnu.org/archive/html/qemu-devel/2021-02/msg03528.html
>
> Can you amend "default GCC 5.4 on a Ubuntu 16.04 host" information
> please?
Well Ubuntu 16.04 hasn't been considered a supported build target for
QEMU for a year now.
https://qemu.readthedocs.io/en/latest/system/build-platforms.html#linux-os-macos-freebsd-netbsd-openbsd
"The project aims to support the most recent major version
at all times. Support for the previous major version will
be dropped 2 years after the new major version is released
or when the vendor itself drops support, whichever comes
first."
IOW, we only aim for QEMU to be buildable on Ubuntu LTS 20.04 and 18.04
at this point in time. 16.04 is explicitly dropped and we will increasingly
introduce incompatibilities with it.
While this specific patch is simple, trying to keep QEMU git master
working on 16.04 is not a goal, so I'd really suggest upgrading to
a newer Ubuntu version at the soonest opportunity.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] hw/block: nvme: Fix a build error in nvme_get_feature()
2021-02-10 11:22 ` Daniel P. Berrangé
@ 2021-02-10 11:34 ` Daniel P. Berrangé
0 siblings, 0 replies; 11+ messages in thread
From: Daniel P. Berrangé @ 2021-02-10 11:34 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Kevin Wolf, Peter Maydell, Thomas Huth, qemu-block, Bin Meng,
qemu-devel, Max Reitz, Keith Busch, Minwoo Im, Klaus Jensen,
Bin Meng, Alex Bennée
On Wed, Feb 10, 2021 at 11:22:19AM +0000, Daniel P. Berrangé wrote:
> On Wed, Feb 10, 2021 at 12:15:45PM +0100, Philippe Mathieu-Daudé wrote:
> > On 2/10/21 12:12 PM, Philippe Mathieu-Daudé wrote:
> > > Hi Bin,
> > >
> > > On 2/10/21 11:23 AM, Bin Meng wrote:
> > >> From: Bin Meng <bin.meng@windriver.com>
> > >>
> > >> Current QEMU HEAD nvme.c does not compile:
> > >>
> > >> hw/block/nvme.c:3242:9: error: ‘result’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> > >> trace_pci_nvme_getfeat_vwcache(result ? "enabled" : "disabled");
> > >> ^
> > >> hw/block/nvme.c:3150:14: note: ‘result’ was declared here
> > >> uint32_t result;
> > >> ^
> > >
> > > Why isn't this catched by our CI? What is your host OS? Fedora 33?
> >
> > Just noticed v1 and Peter's explanation:
> > https://lists.gnu.org/archive/html/qemu-devel/2021-02/msg03528.html
> >
> > Can you amend "default GCC 5.4 on a Ubuntu 16.04 host" information
> > please?
>
> Well Ubuntu 16.04 hasn't been considered a supported build target for
> QEMU for a year now.
>
> https://qemu.readthedocs.io/en/latest/system/build-platforms.html#linux-os-macos-freebsd-netbsd-openbsd
>
> "The project aims to support the most recent major version
> at all times. Support for the previous major version will
> be dropped 2 years after the new major version is released
> or when the vendor itself drops support, whichever comes
> first."
>
> IOW, we only aim for QEMU to be buildable on Ubuntu LTS 20.04 and 18.04
> at this point in time. 16.04 is explicitly dropped and we will increasingly
> introduce incompatibilities with it.
>
> While this specific patch is simple, trying to keep QEMU git master
> working on 16.04 is not a goal, so I'd really suggest upgrading to
> a newer Ubuntu version at the soonest opportunity.
In particular after 6.0 QEMU is released, we'll be dropping RHEL-7
and then likely setting the min required GCC to somewhere around
6.3 which will cut off Ubuntu 16.04 upfront.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] hw/block: nvme: Fix a build error in nvme_get_feature()
2021-02-10 10:23 [PATCH v2] hw/block: nvme: Fix a build error in nvme_get_feature() Bin Meng
` (2 preceding siblings ...)
2021-02-10 11:12 ` Philippe Mathieu-Daudé
@ 2021-02-10 22:29 ` Peter Maydell
3 siblings, 0 replies; 11+ messages in thread
From: Peter Maydell @ 2021-02-10 22:29 UTC (permalink / raw)
To: Bin Meng
Cc: Kevin Wolf, Qemu-block, Bin Meng, QEMU Developers, Max Reitz,
Klaus Jensen, Minwoo Im, Keith Busch
On Wed, 10 Feb 2021 at 10:23, Bin Meng <bmeng.cn@gmail.com> wrote:
>
> From: Bin Meng <bin.meng@windriver.com>
>
> Current QEMU HEAD nvme.c does not compile:
>
> hw/block/nvme.c:3242:9: error: ‘result’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> trace_pci_nvme_getfeat_vwcache(result ? "enabled" : "disabled");
> ^
> hw/block/nvme.c:3150:14: note: ‘result’ was declared here
> uint32_t result;
> ^
>
> Explicitly initialize the result to fix it.
>
> Fixes: aa5e55e3b07e ("hw/block/nvme: open code for volatile write cache")
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
>
> ---
>
> Changes in v2:
> - update function name in the commit message
>
> hw/block/nvme.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> index 5ce21b7..c122ac0 100644
> --- a/hw/block/nvme.c
> +++ b/hw/block/nvme.c
> @@ -3228,6 +3228,7 @@ static uint16_t nvme_get_feature(NvmeCtrl *n, NvmeRequest *req)
> result = ns->features.err_rec;
> goto out;
> case NVME_VOLATILE_WRITE_CACHE:
> + result = 0;
> for (i = 1; i <= n->num_namespaces; i++) {
> ns = nvme_ns(n, i);
> if (!ns) {
> --
Also spotted by Coverity: CID 1446371
-- PMM
^ permalink raw reply [flat|nested] 11+ messages in thread