Am 23.09.2013 um 10:00 hat Max Reitz geschrieben:
> On 2013-09-20 13:54, Kevin Wolf wrote:
> >IF_NONE allows read-only, which makes forbidding it in this place
> >for other types pretty much pointless.
> >
> >Instead, make sure that all devices for which the check would have
> >errored out check in their init function that they don't get a read-only
> >BlockDriverState. This catches even cases where IF_NONE and -device is
> >used.
> >
> >Signed-off-by: Kevin Wolf <kw...@redhat.com>
> >---
> > blockdev.c | 6 ------
> > hw/block/m25p80.c | 5 +++++
> > hw/block/xen_disk.c | 6 ++++++
> > hw/sd/milkymist-memcard.c | 4 ++++
> > hw/sd/omap_mmc.c | 8 ++++++++
> > hw/sd/pl181.c | 4 ++++
> > hw/sd/pxa2xx_mmci.c | 4 ++++
> > hw/sd/sd.c | 5 +++++
> > hw/sd/sdhci.c | 4 ++++
> > hw/sd/ssi-sd.c | 3 +++
> > tests/qemu-iotests/051.out | 5 ++++-
> > 11 files changed, 47 insertions(+), 7 deletions(-)
> >
> >diff --git a/blockdev.c b/blockdev.c
> >index bbdae31..ed4ba65 100644
> >--- a/blockdev.c
> >+++ b/blockdev.c
> >@@ -529,12 +529,6 @@ static DriveInfo *blockdev_init(QDict *bs_opts,
> > if (media == MEDIA_CDROM) {
> > /* CDROM is fine for any interface, don't check. */
> > ro = 1;
> >- } else if (ro == 1) {
> >- if (type != IF_SCSI && type != IF_VIRTIO && type != IF_FLOPPY &&
> >- type != IF_NONE && type != IF_PFLASH) {
> >- error_report("read-only not supported by this bus type");
> >- goto err;
> >- }
> > }
> > bdrv_flags |= ro ? 0 : BDRV_O_RDWR;
> >diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
> >index 8c3b7f0..02a1544 100644
> >--- a/hw/block/m25p80.c
> >+++ b/hw/block/m25p80.c
> >@@ -624,6 +624,11 @@ static int m25p80_init(SSISlave *ss)
> > if (dinfo && dinfo->bdrv) {
> > DB_PRINT_L(0, "Binding to IF_MTD drive\n");
> > s->bdrv = dinfo->bdrv;
> >+ if (bdrv_is_read_only(s->bdrv)) {
> >+ fprintf(stderr, "Can't use a read-only drive");
> >+ return 1;
> >+ }
> >+
> > /* FIXME: Move to late init */
> > if (bdrv_read(s->bdrv, 0, s->storage, DIV_ROUND_UP(s->size,
> > BDRV_SECTOR_SIZE))) {
> >diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
> >index f35fc59..828c9d9 100644
> >--- a/hw/block/xen_disk.c
> >+++ b/hw/block/xen_disk.c
> >@@ -39,6 +39,7 @@
> > #include "hw/xen/xen_backend.h"
> > #include "xen_blkif.h"
> > #include "sysemu/blockdev.h"
> >+#include "qemu/error-report.h"
> > /* ------------------------------------------------------------- */
> >@@ -829,6 +830,11 @@ static int blk_connect(struct XenDevice *xendev)
> > /* setup via qemu cmdline -> already setup for us */
> > xen_be_printf(&blkdev->xendev, 2, "get configured bdrv (cmdline
> > setup)\n");
> > blkdev->bs = blkdev->dinfo->bdrv;
> >+ if (bdrv_is_read_only(blkdev->bs) && !readonly) {
> >+ error_report("Unexpected read-only drive");
> >+ blkdev->bs = NULL;
> >+ return -1;
> >+ }
> Just out of curiosity, why do you use error_report here but
> fprintf(stderr) in most other places?
Variatio delectat. ;-)
I guess fprintf + exit was a common pattern elsewhere, so I used it,
whereas error_report() is my default function. But now that I look at
this file again, maybe it should really be xen_be_printf() here...
Kevin
