[PATCH] When updating the PFLASH file contents, we should check for a possible failure of blk_pwrite(). Similar to 3a68829 commit

[PATCH] When updating the PFLASH file contents, we should check for a possible failure of blk_pwrite(). Similar to 3a68829 commit

[Mansour Ahmadi]

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));
+        }
     }
 }
 
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));
+        }
     }
 }
 
-- 
2.16.2

Re: [PATCH] When updating the PFLASH file contents, we should check for a possible failure of blk_pwrite(). Similar to 3a68829 commit

[no-reply]

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

Re: [PATCH] When updating the PFLASH file contents, we should check for a possible failure of blk_pwrite(). Similar to 3a68829 commit

[no-reply]

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

Re: [PATCH] When updating the PFLASH file contents, we should check for a possible failure of blk_pwrite(). Similar to 3a68829 commit

[no-reply]

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

Re: [PATCH] When updating the PFLASH file contents, we should check for a possible failure of blk_pwrite(). Similar to 3a68829 commit

[Philippe Mathieu-Daudé]

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).

>       }
>   }
>   
> 

Re: [PATCH] When updating the PFLASH file contents, we should check for a possible failure of blk_pwrite(). Similar to 3a68829 commit

[Philippe Mathieu-Daudé]

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).
>
> >       }
> >   }
> >
> >
>
>