Re: [Qemu-devel] [PATCH v4 01/38] block: Remove host floppy support

[Kevin Wolf <kwolf@redhat.com>]

Am 20.07.2015 um 19:45 hat Max Reitz geschrieben:
> It has been deprecated as of 2.3, so we can now remove it.
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>

> @@ -2241,8 +2188,9 @@ static BlockAIOCB *hdev_aio_ioctl(BlockDriverState *bs,
>      pool = aio_get_thread_pool(bdrv_get_aio_context(bs));
>      return thread_pool_submit_aio(pool, aio_worker, acb, cb, opaque);
>  }
> +#endif /* linux */
>  
> -#elif defined(__FreeBSD__) || defined(__FreeBSD_kernel__)
> +#if defined(__FreeBSD__) || defined(__FreeBSD_kernel__)
>  static int fd_open(BlockDriverState *bs)
>  {
>      BDRVRawState *s = bs->opaque;
> @@ -2252,7 +2200,7 @@ static int fd_open(BlockDriverState *bs)
>          return 0;
>      return -EIO;
>  }
> -#else /* !linux && !FreeBSD */
> +#else /* !FreeBSD */
>  
>  static int fd_open(BlockDriverState *bs)
>  {

Full context:

    #if defined(__FreeBSD__) || defined(__FreeBSD_kernel__)
    static int fd_open(BlockDriverState *bs)
    {
        BDRVRawState *s = bs->opaque;

        /* this is just to ensure s->fd is sane (its called by io ops) */
        if (s->fd >= 0)
            return 0;
        return -EIO;
    }
    #else /* !FreeBSD */

    static int fd_open(BlockDriverState *bs)
    {
        return 0;
    }

    #endif /* !linux && !FreeBSD */

First of all, the final comment isn't accurate any more, this branch is
now for Linux, too.

But really the whole #ifdef looks dubious now. It's not clear to me why
we're checking fd >= 0 for FreeBSD at all, using an invalid file
descriptor (most likely -1, which is set explicitly in some places)
should automatically lead to failure. And conversely, I can't see why
doing the same check for non-FreeBSD platforms should hurt.

Ideally, I'd try to get rid of all the fd_open() calls, but failing that
let's use the FreeBSD version universally and get rid of the #ifdef at
least. Or perhaps get rid of the #ifdef in this patch and add another
one that removes fd_open() completely.

> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 7b2efb8..133fa38 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -215,10 +215,11 @@
>  # @drv: the name of the block format used to open the backing device. As of
>  #       0.14.0 this can be: 'blkdebug', 'bochs', 'cloop', 'cow', 'dmg',
>  #       'file', 'file', 'ftp', 'ftps', 'host_cdrom', 'host_device',
> -#       'host_floppy', 'http', 'https', 'nbd', 'parallels', 'qcow',
> +#       'http', 'https', 'nbd', 'parallels', 'qcow',
>  #       'qcow2', 'raw', 'tftp', 'vdi', 'vmdk', 'vpc', 'vvfat'
>  #       2.2: 'archipelago' added, 'cow' dropped
>  #       2.3: 'host_floppy' deprecated
> +#       2.4: 'host_floppy' dropped

2.5

Kevin