Re: [Qemu-devel] [PATCH v2 13/17] blockdev: Remove IF_* check for read-only blockdev_init

[Eric Blake]

On 10/01/2013 07:20 AM, 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 <address@hidden>
> ---
>  blockdev.c                 | 6 ------
>  hw/block/m25p80.c          | 5 +++++
>  hw/block/xen_disk.c        | 5 +++++
>  hw/sd/milkymist-memcard.c  | 4 ++++
>  hw/sd/omap_mmc.c           | 6 ++++++
>  hw/sd/pl181.c              | 4 ++++
>  hw/sd/pxa2xx_mmci.c        | 3 +++
>  hw/sd/sd.c                 | 5 +++++
>  hw/sd/sdhci.c              | 3 +++
>  hw/sd/ssi-sd.c             | 3 +++
>  tests/qemu-iotests/051.out | 5 ++++-
>  11 files changed, 42 insertions(+), 7 deletions(-)

> +++ b/hw/sd/omap_mmc.c
> @@ -593,6 +593,9 @@ struct omap_mmc_s *omap_mmc_init(hwaddr base,
>  
>      /* Instantiate the storage */
>      s->card = sd_init(bd, false);
> +    if (s->card == NULL) {
> +        exit(1);

No error message about why the exit?  Also, is it worth using
EXIT_FAILURE instead of a magic number?

> +++ b/hw/sd/sd.c
> @@ -494,6 +494,11 @@ SDState *sd_init(BlockDriverState *bs, bool is_spi)
>  {
>      SDState *sd;
>  
> +    if (bdrv_is_read_only(bs)) {
> +        fprintf(stderr, "sd_init: Cannot use read-only drive\n");
> +        return NULL;

Oh, I guess there IS an error message before exit.

And you're not the first person to not use EXIT_FAILURE.  So I can live
with the patch as-is.

Reviewed-by: Eric Blake <address@hidden>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature