Am 22.09.2012 20:53, schrieb Stefan Weil:
> 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.
Yes, I very much prefer ret < 0 checks for all block layer functions.
>>> + 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?
Basically anything that the OS can return. The block layer may
internally generate things like -EACCES for writing to read-only images,
or -ENOMEDIUM (not sure if it's possible for pflash).
> 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.
virtio-blk/IDE/scsi-disk do that.
>> 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.
Shouldn't we avoid fprintfs that can be triggered by the guest?
Kevin
