diff mbox

[v4,01/38] block: Remove host floppy support

Message ID 1437414365-11881-2-git-send-email-mreitz@redhat.com
State New
Headers show

Commit Message

Max Reitz July 20, 2015, 5:45 p.m. UTC
It has been deprecated as of 2.3, so we can now remove it.

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/raw-posix.c    | 217 ++-------------------------------------------------
 qapi/block-core.json |   9 +--
 2 files changed, 11 insertions(+), 215 deletions(-)

Comments

Kevin Wolf Sept. 7, 2015, 3:59 p.m. UTC | #1
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
Max Reitz Sept. 7, 2015, 4:26 p.m. UTC | #2
On 07.09.2015 17:59, Kevin Wolf wrote:
> 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,

Me neither, so I just decided to keep it the way it was.

> 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.

Seems reasonable, will do.

>> 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

I should have grep'ed through it. :-)

Thanks for reviewing!

Max
diff mbox

Patch

diff --git a/block/raw-posix.c b/block/raw-posix.c
index 855febe..83d7054 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -127,11 +127,6 @@  do { \
 
 #define FTYPE_FILE   0
 #define FTYPE_CD     1
-#define FTYPE_FD     2
-
-/* if the FD is not accessed during that time (in ns), we try to
-   reopen it to see if the disk has been changed */
-#define FD_OPEN_TIMEOUT (1000000000)
 
 #define MAX_BLOCKSIZE	4096
 
@@ -141,13 +136,6 @@  typedef struct BDRVRawState {
     int open_flags;
     size_t buf_align;
 
-#if defined(__linux__)
-    /* linux floppy specific */
-    int64_t fd_open_time;
-    int64_t fd_error_time;
-    int fd_got_error;
-    int fd_media_changed;
-#endif
 #ifdef CONFIG_LINUX_AIO
     int use_aio;
     void *aio_ctx;
@@ -626,7 +614,7 @@  static int raw_reopen_prepare(BDRVReopenState *state,
     }
 #endif
 
-    if (s->type == FTYPE_FD || s->type == FTYPE_CD) {
+    if (s->type == FTYPE_CD) {
         raw_s->open_flags |= O_NONBLOCK;
     }
 
@@ -2172,47 +2160,6 @@  static int hdev_open(BlockDriverState *bs, QDict *options, int flags,
 }
 
 #if defined(__linux__)
-/* Note: we do not have a reliable method to detect if the floppy is
-   present. The current method is to try to open the floppy at every
-   I/O and to keep it opened during a few hundreds of ms. */
-static int fd_open(BlockDriverState *bs)
-{
-    BDRVRawState *s = bs->opaque;
-    int last_media_present;
-
-    if (s->type != FTYPE_FD)
-        return 0;
-    last_media_present = (s->fd >= 0);
-    if (s->fd >= 0 &&
-        (qemu_clock_get_ns(QEMU_CLOCK_REALTIME) - s->fd_open_time) >= FD_OPEN_TIMEOUT) {
-        qemu_close(s->fd);
-        s->fd = -1;
-        DPRINTF("Floppy closed\n");
-    }
-    if (s->fd < 0) {
-        if (s->fd_got_error &&
-            (qemu_clock_get_ns(QEMU_CLOCK_REALTIME) - s->fd_error_time) < FD_OPEN_TIMEOUT) {
-            DPRINTF("No floppy (open delayed)\n");
-            return -EIO;
-        }
-        s->fd = qemu_open(bs->filename, s->open_flags & ~O_NONBLOCK);
-        if (s->fd < 0) {
-            s->fd_error_time = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
-            s->fd_got_error = 1;
-            if (last_media_present)
-                s->fd_media_changed = 1;
-            DPRINTF("No floppy\n");
-            return -EIO;
-        }
-        DPRINTF("Floppy opened\n");
-    }
-    if (!last_media_present)
-        s->fd_media_changed = 1;
-    s->fd_open_time = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
-    s->fd_got_error = 0;
-    return 0;
-}
-
 static int hdev_ioctl(BlockDriverState *bs, unsigned long int req, void *buf)
 {
     BDRVRawState *s = bs->opaque;
@@ -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)
 {
@@ -2303,14 +2251,13 @@  static int hdev_create(const char *filename, QemuOpts *opts,
     int64_t total_size = 0;
     bool has_prefix;
 
-    /* This function is used by all three protocol block drivers and therefore
-     * any of these three prefixes may be given.
+    /* This function is used by both protocol block drivers and therefore either
+     * of these prefixes may be given.
      * The return value has to be stored somewhere, otherwise this is an error
      * due to -Werror=unused-value. */
     has_prefix =
         strstart(filename, "host_device:", &filename) ||
-        strstart(filename, "host_cdrom:" , &filename) ||
-        strstart(filename, "host_floppy:", &filename);
+        strstart(filename, "host_cdrom:" , &filename);
 
     (void)has_prefix;
 
@@ -2384,155 +2331,6 @@  static BlockDriver bdrv_host_device = {
 #endif
 };
 
-#ifdef __linux__
-static void floppy_parse_filename(const char *filename, QDict *options,
-                                  Error **errp)
-{
-    /* The prefix is optional, just as for "file". */
-    strstart(filename, "host_floppy:", &filename);
-
-    qdict_put_obj(options, "filename", QOBJECT(qstring_from_str(filename)));
-}
-
-static int floppy_open(BlockDriverState *bs, QDict *options, int flags,
-                       Error **errp)
-{
-    BDRVRawState *s = bs->opaque;
-    Error *local_err = NULL;
-    int ret;
-
-    s->type = FTYPE_FD;
-
-    /* open will not fail even if no floppy is inserted, so add O_NONBLOCK */
-    ret = raw_open_common(bs, options, flags, O_NONBLOCK, &local_err);
-    if (ret) {
-        if (local_err) {
-            error_propagate(errp, local_err);
-        }
-        return ret;
-    }
-
-    /* close fd so that we can reopen it as needed */
-    qemu_close(s->fd);
-    s->fd = -1;
-    s->fd_media_changed = 1;
-
-    error_report("Host floppy pass-through is deprecated");
-    error_printf("Support for it will be removed in a future release.\n");
-    return 0;
-}
-
-static int floppy_probe_device(const char *filename)
-{
-    int fd, ret;
-    int prio = 0;
-    struct floppy_struct fdparam;
-    struct stat st;
-
-    if (strstart(filename, "/dev/fd", NULL) &&
-        !strstart(filename, "/dev/fdset/", NULL) &&
-        !strstart(filename, "/dev/fd/", NULL)) {
-        prio = 50;
-    }
-
-    fd = qemu_open(filename, O_RDONLY | O_NONBLOCK);
-    if (fd < 0) {
-        goto out;
-    }
-    ret = fstat(fd, &st);
-    if (ret == -1 || !S_ISBLK(st.st_mode)) {
-        goto outc;
-    }
-
-    /* Attempt to detect via a floppy specific ioctl */
-    ret = ioctl(fd, FDGETPRM, &fdparam);
-    if (ret >= 0)
-        prio = 100;
-
-outc:
-    qemu_close(fd);
-out:
-    return prio;
-}
-
-
-static int floppy_is_inserted(BlockDriverState *bs)
-{
-    return fd_open(bs) >= 0;
-}
-
-static int floppy_media_changed(BlockDriverState *bs)
-{
-    BDRVRawState *s = bs->opaque;
-    int ret;
-
-    /*
-     * XXX: we do not have a true media changed indication.
-     * It does not work if the floppy is changed without trying to read it.
-     */
-    fd_open(bs);
-    ret = s->fd_media_changed;
-    s->fd_media_changed = 0;
-    DPRINTF("Floppy changed=%d\n", ret);
-    return ret;
-}
-
-static void floppy_eject(BlockDriverState *bs, bool eject_flag)
-{
-    BDRVRawState *s = bs->opaque;
-    int fd;
-
-    if (s->fd >= 0) {
-        qemu_close(s->fd);
-        s->fd = -1;
-    }
-    fd = qemu_open(bs->filename, s->open_flags | O_NONBLOCK);
-    if (fd >= 0) {
-        if (ioctl(fd, FDEJECT, 0) < 0)
-            perror("FDEJECT");
-        qemu_close(fd);
-    }
-}
-
-static BlockDriver bdrv_host_floppy = {
-    .format_name        = "host_floppy",
-    .protocol_name      = "host_floppy",
-    .instance_size      = sizeof(BDRVRawState),
-    .bdrv_needs_filename = true,
-    .bdrv_probe_device	= floppy_probe_device,
-    .bdrv_parse_filename = floppy_parse_filename,
-    .bdrv_file_open     = floppy_open,
-    .bdrv_close         = raw_close,
-    .bdrv_reopen_prepare = raw_reopen_prepare,
-    .bdrv_reopen_commit  = raw_reopen_commit,
-    .bdrv_reopen_abort   = raw_reopen_abort,
-    .bdrv_create         = hdev_create,
-    .create_opts         = &raw_create_opts,
-
-    .bdrv_aio_readv     = raw_aio_readv,
-    .bdrv_aio_writev    = raw_aio_writev,
-    .bdrv_aio_flush	= raw_aio_flush,
-    .bdrv_refresh_limits = raw_refresh_limits,
-    .bdrv_io_plug = raw_aio_plug,
-    .bdrv_io_unplug = raw_aio_unplug,
-    .bdrv_flush_io_queue = raw_aio_flush_io_queue,
-
-    .bdrv_truncate      = raw_truncate,
-    .bdrv_getlength      = raw_getlength,
-    .has_variable_length = true,
-    .bdrv_get_allocated_file_size
-                        = raw_get_allocated_file_size,
-
-    .bdrv_detach_aio_context = raw_detach_aio_context,
-    .bdrv_attach_aio_context = raw_attach_aio_context,
-
-    /* removable device support */
-    .bdrv_is_inserted   = floppy_is_inserted,
-    .bdrv_media_changed = floppy_media_changed,
-    .bdrv_eject         = floppy_eject,
-};
-#endif
-
 #if defined(__linux__) || defined(__FreeBSD__) || defined(__FreeBSD_kernel__)
 static void cdrom_parse_filename(const char *filename, QDict *options,
                                  Error **errp)
@@ -2810,7 +2608,6 @@  static void bdrv_file_init(void)
     bdrv_register(&bdrv_file);
     bdrv_register(&bdrv_host_device);
 #ifdef __linux__
-    bdrv_register(&bdrv_host_floppy);
     bdrv_register(&bdrv_host_cdrom);
 #endif
 #if defined(__FreeBSD__) || defined(__FreeBSD_kernel__)
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
 #
 # @backing_file: #optional the name of the backing file (for copy-on-write)
 #
@@ -1373,15 +1374,14 @@ 
 #
 # Drivers that are supported in block device operations.
 #
-# @host_device, @host_cdrom, @host_floppy: Since 2.1
-# @host_floppy: deprecated since 2.3
+# @host_device, @host_cdrom: Since 2.1
 #
 # Since: 2.0
 ##
 { 'enum': 'BlockdevDriver',
   'data': [ 'archipelago', 'blkdebug', 'blkverify', 'bochs', 'cloop',
             'dmg', 'file', 'ftp', 'ftps', 'host_cdrom', 'host_device',
-            'host_floppy', 'http', 'https', 'null-aio', 'null-co', 'parallels',
+            'http', 'https', 'null-aio', 'null-co', 'parallels',
             'qcow', 'qcow2', 'qed', 'quorum', 'raw', 'tftp', 'vdi', 'vhdx',
             'vmdk', 'vpc', 'vvfat' ] }
 
@@ -1811,7 +1811,6 @@ 
 # TODO gluster: Wait for structured options
       'host_cdrom': 'BlockdevOptionsFile',
       'host_device':'BlockdevOptionsFile',
-      'host_floppy':'BlockdevOptionsFile',
       'http':       'BlockdevOptionsFile',
       'https':      'BlockdevOptionsFile',
 # TODO iscsi: Wait for structured options