qemu-trivial
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-trivial] [PATCH] pflash: Avoid warnings from coverity


From: Stefan Weil
Subject: Re: [Qemu-trivial] [PATCH] pflash: Avoid warnings from coverity
Date: Sat, 22 Sep 2012 20:53:10 +0200
User-agent: Mozilla/5.0 (X11; Linux i686; rv:15.0) Gecko/20120827 Thunderbird/15.0

Am 22.09.2012 18:29, schrieb Stefan Hajnoczi:
On Wed, Sep 19, 2012 at 06:41:14PM +0200, Stefan Weil wrote:
[snip]
          offset_end = (offset_end + 511) >> 9;
-        bdrv_write(pfl->bs, offset, pfl->storage + (offset << 9),
-                   offset_end - offset);
+        if (bdrv_write(pfl->bs, offset, pfl->storage + (offset << 9),
+                       offset_end - offset) == -1) {
bdrv_write() returns -errno, not -1.

Thanks. It looks like we have more code which uses the wrong check
(and which I copied). So more patches are needed.

Should we also replace code which does bdrv_write() != 0 or !bdrv_write()
by bdrv_write() < 0 to get more uniform code (and the same for bdrv_read*),
even it is not strictly wrong?

Maybe Kevin as block maintainer should decide that.

+            fprintf(stderr, "pflash: Error writing to flash storage\n");
+        }
Please report the errno and possibly bdrv_get_device_name() to uniquely
identify this block device.

That would be overkill here: writing flash memory is not used very
often (even on real hardware it is typically only used for firmware
updates). I expect that anyone who does a firmware update in a
QEMU guest will know the name of the flash image file.

Usually I replace the flash image file on the QEMU host when I want
to exchange the firmware (much easier than flashing in the guest).

Reporting errno might be more reasonable.Are there other values than
EIO (e.g. defective media) and ENOSPC (disk full) which could occur?

A common solution for all users of bdrv_write in the block layer
would be even better. VirtualBox for example stops the guest when
ENOSPC (disk full) occurs, so it's possible for users to fix that
and resume the emulation.

Peter's comments about reporting errors to the guest make sense to me.
I'm not sure how much work that involves, printing the error is a step
in the right direction but we shouldn't forget the TODO.

Stefan

There is no 1:1 mapping of block write errors on the host to
an error statusin the flash controller.

Currently the code in pflash_cfi01.c sets an error status which
can be (mis-)used for block write errors. I just prepared a
patch which does this.

In pflash_cfi02.c I did not see such error status handling,
therefore I'd stick to printing an error message there.

Regards

Stefan W.




reply via email to

[Prev in Thread] Current Thread [Next in Thread]