* Re: [PATCH] When updating the PFLASH file contents, we should check for a possible failure of blk_pwrite(). Similar to 3a68829 commit
2020-04-08 0:35 [PATCH] When updating the PFLASH file contents, we should check for a possible failure of blk_pwrite(). Similar to 3a68829 commit Mansour Ahmadi
@ 2020-04-08 6:36 ` no-reply
2020-04-08 6:39 ` no-reply
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: no-reply @ 2020-04-08 6:36 UTC (permalink / raw)
To: mansourweb; +Cc: mansourweb, qemu-devel
Patchew URL: https://patchew.org/QEMU/20200408003552.58095-1-mansourweb@gmail.com/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Subject: [PATCH] When updating the PFLASH file contents, we should check for a possible failure of blk_pwrite(). Similar to 3a68829 commit
Message-id: 20200408003552.58095-1-mansourweb@gmail.com
Type: series
=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===
Switched to a new branch 'test'
e2d6d6e When updating the PFLASH file contents, we should check for a possible failure of blk_pwrite(). Similar to 3a68829 commit
=== OUTPUT BEGIN ===
ERROR: code indent should never use tabs
#27: FILE: hw/block/pflash_cfi01.c:410:
+^Iif (ret < 0) {$
ERROR: code indent should never use tabs
#50: FILE: hw/block/pflash_cfi02.c:404:
+^Iif (ret < 0) {$
ERROR: code indent should never use tabs
#51: FILE: hw/block/pflash_cfi02.c:405:
+^I error_report("Could not update PFLASH: %s", strerror(-ret));$
total: 3 errors, 0 warnings, 36 lines checked
Commit e2d6d6e28daf (When updating the PFLASH file contents, we should check for a possible failure of blk_pwrite(). Similar to 3a68829 commit) has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===
Test command exited with code: 1
The full log is available at
http://patchew.org/logs/20200408003552.58095-1-mansourweb@gmail.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] When updating the PFLASH file contents, we should check for a possible failure of blk_pwrite(). Similar to 3a68829 commit
2020-04-08 0:35 [PATCH] When updating the PFLASH file contents, we should check for a possible failure of blk_pwrite(). Similar to 3a68829 commit Mansour Ahmadi
2020-04-08 6:36 ` no-reply
@ 2020-04-08 6:39 ` no-reply
2020-04-08 6:44 ` no-reply
2020-04-08 8:08 ` Philippe Mathieu-Daudé
3 siblings, 0 replies; 6+ messages in thread
From: no-reply @ 2020-04-08 6:39 UTC (permalink / raw)
To: mansourweb; +Cc: mansourweb, qemu-devel
Patchew URL: https://patchew.org/QEMU/20200408003552.58095-1-mansourweb@gmail.com/
Hi,
This series failed the docker-quick@centos7 build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.
=== TEST SCRIPT BEGIN ===
#!/bin/bash
make docker-image-centos7 V=1 NETWORK=1
time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1
=== TEST SCRIPT END ===
CC hw/core/qdev-fw.o
CC hw/core/fw-path-provider.o
/tmp/qemu-test/src/hw/block/pflash_cfi02.c: In function 'pflash_update':
/tmp/qemu-test/src/hw/block/pflash_cfi02.c:405:6: error: implicit declaration of function 'error_report' [-Werror=implicit-function-declaration]
error_report("Could not update PFLASH: %s", strerror(-ret));
^
/tmp/qemu-test/src/hw/block/pflash_cfi02.c:405:6: error: nested extern declaration of 'error_report' [-Werror=nested-externs]
cc1: all warnings being treated as errors
CC hw/core/nmi.o
make: *** [hw/block/pflash_cfi02.o] Error 1
make: *** Waiting for unfinished jobs....
Traceback (most recent call last):
File "./tests/docker/docker.py", line 664, in <module>
---
raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=9b665adde6a54183bc89110b4200cefb', '-u', '1003', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew2/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-wfaz1og4/src/docker-src.2020-04-08-02.36.44.3140:/var/tmp/qemu:z,ro', 'qemu:centos7', '/var/tmp/qemu/run', 'test-quick']' returned non-zero exit status 2.
filter=--filter=label=com.qemu.instance.uuid=9b665adde6a54183bc89110b4200cefb
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-wfaz1og4/src'
make: *** [docker-run-test-quick@centos7] Error 2
real 2m39.690s
user 0m8.678s
The full log is available at
http://patchew.org/logs/20200408003552.58095-1-mansourweb@gmail.com/testing.docker-quick@centos7/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] When updating the PFLASH file contents, we should check for a possible failure of blk_pwrite(). Similar to 3a68829 commit
2020-04-08 0:35 [PATCH] When updating the PFLASH file contents, we should check for a possible failure of blk_pwrite(). Similar to 3a68829 commit Mansour Ahmadi
2020-04-08 6:36 ` no-reply
2020-04-08 6:39 ` no-reply
@ 2020-04-08 6:44 ` no-reply
2020-04-08 8:08 ` Philippe Mathieu-Daudé
3 siblings, 0 replies; 6+ messages in thread
From: no-reply @ 2020-04-08 6:44 UTC (permalink / raw)
To: mansourweb; +Cc: mansourweb, qemu-devel
Patchew URL: https://patchew.org/QEMU/20200408003552.58095-1-mansourweb@gmail.com/
Hi,
This series failed the docker-mingw@fedora build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.
=== TEST SCRIPT BEGIN ===
#! /bin/bash
export ARCH=x86_64
make docker-image-fedora V=1 NETWORK=1
time make docker-test-mingw@fedora J=14 NETWORK=1
=== TEST SCRIPT END ===
CC hw/core/machine-hmp-cmds.o
CC hw/core/numa.o
/tmp/qemu-test/src/hw/block/pflash_cfi02.c: In function 'pflash_update':
/tmp/qemu-test/src/hw/block/pflash_cfi02.c:405:6: error: implicit declaration of function 'error_report'; did you mean 'error_report_err'? [-Werror=implicit-function-declaration]
error_report("Could not update PFLASH: %s", strerror(-ret));
^~~~~~~~~~~~
error_report_err
/tmp/qemu-test/src/hw/block/pflash_cfi02.c:405:6: error: nested extern declaration of 'error_report' [-Werror=nested-externs]
cc1: all warnings being treated as errors
make: *** [/tmp/qemu-test/src/rules.mak:69: hw/block/pflash_cfi02.o] Error 1
make: *** Waiting for unfinished jobs....
Traceback (most recent call last):
File "./tests/docker/docker.py", line 664, in <module>
---
raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=39c19ab38d98420d858a5d27fac840c4', '-u', '1003', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew2/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-p4ihd6fw/src/docker-src.2020-04-08-02.42.30.10229:/var/tmp/qemu:z,ro', 'qemu:fedora', '/var/tmp/qemu/run', 'test-mingw']' returned non-zero exit status 2.
filter=--filter=label=com.qemu.instance.uuid=39c19ab38d98420d858a5d27fac840c4
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-p4ihd6fw/src'
make: *** [docker-run-test-mingw@fedora] Error 2
real 2m13.506s
user 0m8.468s
The full log is available at
http://patchew.org/logs/20200408003552.58095-1-mansourweb@gmail.com/testing.docker-mingw@fedora/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] When updating the PFLASH file contents, we should check for a possible failure of blk_pwrite(). Similar to 3a68829 commit
2020-04-08 0:35 [PATCH] When updating the PFLASH file contents, we should check for a possible failure of blk_pwrite(). Similar to 3a68829 commit Mansour Ahmadi
` (2 preceding siblings ...)
2020-04-08 6:44 ` no-reply
@ 2020-04-08 8:08 ` Philippe Mathieu-Daudé
2020-04-12 21:35 ` Philippe Mathieu-Daudé
3 siblings, 1 reply; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-04-08 8:08 UTC (permalink / raw)
To: Mansour Ahmadi, qemu-devel, Qemu-block
Hi Mansour,
On 4/8/20 2:35 AM, Mansour Ahmadi wrote:
> Signed-off-by: Mansour Ahmadi <mansourweb@gmail.com>
> ---
> hw/block/pflash_cfi01.c | 6 +++++-
> hw/block/pflash_cfi02.c | 6 +++++-
> 2 files changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
> index 24f3bce7ef..31319cfd07 100644
> --- a/hw/block/pflash_cfi01.c
> +++ b/hw/block/pflash_cfi01.c
> @@ -399,13 +399,17 @@ static void pflash_update(PFlashCFI01 *pfl, int offset,
> int size)
> {
> int offset_end;
> + int ret;
> if (pfl->blk) {
> offset_end = offset + size;
> /* widen to sector boundaries */
> offset = QEMU_ALIGN_DOWN(offset, BDRV_SECTOR_SIZE);
> offset_end = QEMU_ALIGN_UP(offset_end, BDRV_SECTOR_SIZE);
> - blk_pwrite(pfl->blk, offset, pfl->storage + offset,
> + ret = blk_pwrite(pfl->blk, offset, pfl->storage + offset,
> offset_end - offset, 0);
> + if (ret < 0) {
> + error_report("Could not update PFLASH: %s", strerror(-ret));
Thanks for your patch. Note that it doesn't compile:
hw/block/pflash_cfi01.c:411:6: error: implicit declaration of function
'error_report' [-Werror=implicit-function-declaration]
error_report("Could not update PFLASH: %s", strerror(-ret));
^
Better than reporting the error is to set the error flag in the status
register.
> + }
> }
> }
>
> diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
> index 12f18d401a..fee5b3497c 100644
> --- a/hw/block/pflash_cfi02.c
> +++ b/hw/block/pflash_cfi02.c
> @@ -393,13 +393,17 @@ static uint64_t pflash_read(void *opaque, hwaddr offset, unsigned int width)
> static void pflash_update(PFlashCFI02 *pfl, int offset, int size)
> {
> int offset_end;
> + int ret;
> if (pfl->blk) {
> offset_end = offset + size;
> /* widen to sector boundaries */
> offset = QEMU_ALIGN_DOWN(offset, BDRV_SECTOR_SIZE);
> offset_end = QEMU_ALIGN_UP(offset_end, BDRV_SECTOR_SIZE);
> - blk_pwrite(pfl->blk, offset, pfl->storage + offset,
> + ret = blk_pwrite(pfl->blk, offset, pfl->storage + offset,
> offset_end - offset, 0);
> + if (ret < 0) {
> + error_report("Could not update PFLASH: %s", strerror(-ret));
> + }
Similar comments (does not compile, set error status register).
> }
> }
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] When updating the PFLASH file contents, we should check for a possible failure of blk_pwrite(). Similar to 3a68829 commit
2020-04-08 8:08 ` Philippe Mathieu-Daudé
@ 2020-04-12 21:35 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-04-12 21:35 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, Mansour Ahmadi
Cc: qemu-devel@nongnu.org Developers, Qemu-block
ping? rc3 is in 2 days.
On Wed, Apr 8, 2020 at 10:10 AM Philippe Mathieu-Daudé
<philmd@redhat.com> wrote:
>
> Hi Mansour,
>
> On 4/8/20 2:35 AM, Mansour Ahmadi wrote:
> > Signed-off-by: Mansour Ahmadi <mansourweb@gmail.com>
> > ---
> > hw/block/pflash_cfi01.c | 6 +++++-
> > hw/block/pflash_cfi02.c | 6 +++++-
> > 2 files changed, 10 insertions(+), 2 deletions(-)
> >
> > diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
> > index 24f3bce7ef..31319cfd07 100644
> > --- a/hw/block/pflash_cfi01.c
> > +++ b/hw/block/pflash_cfi01.c
> > @@ -399,13 +399,17 @@ static void pflash_update(PFlashCFI01 *pfl, int offset,
> > int size)
> > {
> > int offset_end;
> > + int ret;
> > if (pfl->blk) {
> > offset_end = offset + size;
> > /* widen to sector boundaries */
> > offset = QEMU_ALIGN_DOWN(offset, BDRV_SECTOR_SIZE);
> > offset_end = QEMU_ALIGN_UP(offset_end, BDRV_SECTOR_SIZE);
> > - blk_pwrite(pfl->blk, offset, pfl->storage + offset,
> > + ret = blk_pwrite(pfl->blk, offset, pfl->storage + offset,
> > offset_end - offset, 0);
> > + if (ret < 0) {
> > + error_report("Could not update PFLASH: %s", strerror(-ret));
>
> Thanks for your patch. Note that it doesn't compile:
>
> hw/block/pflash_cfi01.c:411:6: error: implicit declaration of function
> 'error_report' [-Werror=implicit-function-declaration]
> error_report("Could not update PFLASH: %s", strerror(-ret));
> ^
>
> Better than reporting the error is to set the error flag in the status
> register.
>
> > + }
> > }
> > }
> >
> > diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
> > index 12f18d401a..fee5b3497c 100644
> > --- a/hw/block/pflash_cfi02.c
> > +++ b/hw/block/pflash_cfi02.c
> > @@ -393,13 +393,17 @@ static uint64_t pflash_read(void *opaque, hwaddr offset, unsigned int width)
> > static void pflash_update(PFlashCFI02 *pfl, int offset, int size)
> > {
> > int offset_end;
> > + int ret;
> > if (pfl->blk) {
> > offset_end = offset + size;
> > /* widen to sector boundaries */
> > offset = QEMU_ALIGN_DOWN(offset, BDRV_SECTOR_SIZE);
> > offset_end = QEMU_ALIGN_UP(offset_end, BDRV_SECTOR_SIZE);
> > - blk_pwrite(pfl->blk, offset, pfl->storage + offset,
> > + ret = blk_pwrite(pfl->blk, offset, pfl->storage + offset,
> > offset_end - offset, 0);
> > + if (ret < 0) {
> > + error_report("Could not update PFLASH: %s", strerror(-ret));
> > + }
>
> Similar comments (does not compile, set error status register).
>
> > }
> > }
> >
> >
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread