Re: [PATCH 06/16] fdc: Deprecate configuring floppies with -global isa-fdc

[John Snow]

On 6/5/20 10:56 AM, Markus Armbruster wrote:
> Deprecate
> 
>     -global isa-fdc.driveA=...
>     -global isa-fdc.driveB=...
> 
> in favour of
> 
>     -device floppy,unit=0,drive=...
>     -device floppy,unit=1,drive=...
> 
> Same for the other floppy controller devices.
> 

If you're not aware of any reason for why we need to keep global, then
neither am I.

> Signed-off-by: Markus Armbruster <armbru@redhat.com>

Acked-by: John Snow <jsnow@redhat.com>

> ---
>  docs/qdev-device-use.txt   | 13 ++++---------
>  docs/system/deprecated.rst | 26 ++++++++++++++++++++++++++
>  hw/block/fdc.c             | 17 +++++++++++++++++
>  tests/qemu-iotests/172.out | 30 ++++++++++++++++++++++++++++++
>  4 files changed, 77 insertions(+), 9 deletions(-)
> 
> diff --git a/docs/qdev-device-use.txt b/docs/qdev-device-use.txt
> index cc53e97dcd..3d781be547 100644
> --- a/docs/qdev-device-use.txt
> +++ b/docs/qdev-device-use.txt
> @@ -104,15 +104,10 @@ The -device argument differs in detail for each type of 
> drive:
>  
>  * if=floppy
>  
> -  -global isa-fdc.driveA=DRIVE-ID
> -  -global isa-fdc.driveB=DRIVE-ID
> +  -device floppy,unit=UNIT,drive=DRIVE-ID
>  
> -  This is -global instead of -device, because the floppy controller is
> -  created automatically, and we want to configure that one, not create
> -  a second one (which isn't possible anyway).
> -
> -  Without any -global isa-fdc,... you get an empty driveA and no
> -  driveB.  You can use -nodefaults to suppress the default driveA, see
> +  Without any -device floppy,... you get an empty unit 0 and no unit
> +  1.  You can use -nodefaults to suppress the default unit 0, see
>    "Default Devices".
>  
>  * if=virtio
> @@ -385,7 +380,7 @@ some DEVNAMEs:
>  
>      default device      suppressing DEVNAMEs
>      CD-ROM              ide-cd, ide-drive, ide-hd, scsi-cd, scsi-hd
> -    isa-fdc's driveA    floppy, isa-fdc
> +    floppy              floppy, isa-fdc
>      parallel            isa-parallel
>      serial              isa-serial
>      VGA                 VGA, cirrus-vga, isa-vga, isa-cirrus-vga,
> diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
> index f0061f94aa..9bd11c1e95 100644
> --- a/docs/system/deprecated.rst
> +++ b/docs/system/deprecated.rst
> @@ -172,6 +172,32 @@ previously available ``-tb-size`` option.
>  Use ``-display sdl,show-cursor=on`` or
>   ``-display gtk,show-cursor=on`` instead.
>  
> +``Configuring floppies with ``-global``
> +'''''''''''''''''''''''''''''''''''''''
> +
> +Use ``-device floppy,...`` instead:
> +::
> +
> +    -global isa-fdc.driveA=...
> +    -global sysbus-fdc.driveA=...
> +    -global SUNW,fdtwo.drive=...
> +
> +become
> +::
> +
> +    -device floppy,unit=0,drive=...
> +
> +and
> +::
> +
> +    -global isa-fdc.driveB=...
> +    -global sysbus-fdc.driveB=...
> +
> +become
> +::
> +
> +    -device floppy,unit=1,drive=...
> +
>  QEMU Machine Protocol (QMP) commands
>  ------------------------------------
>  
> diff --git a/hw/block/fdc.c b/hw/block/fdc.c
> index 35e734b6fb..4191d5b006 100644
> --- a/hw/block/fdc.c
> +++ b/hw/block/fdc.c
> @@ -2525,6 +2525,7 @@ static void fdctrl_connect_drives(FDCtrl *fdctrl, 
> DeviceState *fdc_dev,
>      DeviceState *dev;
>      BlockBackend *blk;
>      Error *local_err = NULL;
> +    const char *fdc_name, *drive_suffix;
>  
>      for (i = 0; i < MAX_FD; i++) {
>          drive = &fdctrl->drives[i];
> @@ -2539,10 +2540,26 @@ static void fdctrl_connect_drives(FDCtrl *fdctrl, 
> DeviceState *fdc_dev,
>              continue;
>          }
>  
> +        fdc_name = object_get_typename(OBJECT(fdc_dev));
> +        drive_suffix = !strcmp(fdc_name, "SUNW,fdtwo") ? "" : i ? "B" : "A";
> +        warn_report("warning: property %s.drive%s is deprecated",
> +                    fdc_name, drive_suffix);
> +        error_printf("Use -device floppy,unit=%d,drive=... instead.\n", i);
> +
>          dev = qdev_new("floppy");
>          qdev_prop_set_uint32(dev, "unit", i);
>          qdev_prop_set_enum(dev, "drive-type", 
> fdctrl->qdev_for_drives[i].type);
>  
> +        /*
> +         * Hack alert: we move the backend from the floppy controller
> +         * device to the floppy device.  We first need to detach the
> +         * controller, or else floppy_create()'s qdev_prop_set_drive()
> +         * will die when it attaches floppy device.  We also need to
> +         * take another reference so that blk_detach_dev() doesn't
> +         * free blk while we still need it.
> +         *
> +         * The hack is probably a bad idea.
> +         */
>          blk_ref(blk);
>          blk_detach_dev(blk, fdc_dev);
>          fdctrl->qdev_for_drives[i].blk = NULL;
> diff --git a/tests/qemu-iotests/172.out b/tests/qemu-iotests/172.out
> index ba15a85c88..253f35111d 100644
> --- a/tests/qemu-iotests/172.out
> +++ b/tests/qemu-iotests/172.out
> @@ -383,6 +383,8 @@ sd0: [not inserted]
>  === Using -drive if=none and -global ===
>  
>  Testing: -drive if=none,file=TEST_DIR/t.qcow2 -global isa-fdc.driveA=none0
> +QEMU_PROG: warning: warning: property isa-fdc.driveA is deprecated
> +Use -device floppy,unit=0,drive=... instead.
>  
>            dev: isa-fdc, id ""
>              iobase = 1008 (0x3f0)
> @@ -423,6 +425,8 @@ sd0: [not inserted]
>  
>  
>  Testing: -drive if=none,file=TEST_DIR/t.qcow2 -global isa-fdc.driveB=none0
> +QEMU_PROG: warning: warning: property isa-fdc.driveB is deprecated
> +Use -device floppy,unit=1,drive=... instead.
>  
>            dev: isa-fdc, id ""
>              iobase = 1008 (0x3f0)
> @@ -463,6 +467,10 @@ sd0: [not inserted]
>  
>  
>  Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive 
> if=none,file=TEST_DIR/t.qcow2.2 -global isa-fdc.driveA=none0 -global 
> isa-fdc.driveB=none1
> +QEMU_PROG: warning: warning: property isa-fdc.driveA is deprecated
> +Use -device floppy,unit=0,drive=... instead.
> +QEMU_PROG: warning: warning: property isa-fdc.driveB is deprecated
> +Use -device floppy,unit=1,drive=... instead.
>  
>            dev: isa-fdc, id ""
>              iobase = 1008 (0x3f0)
> @@ -661,6 +669,8 @@ sd0: [not inserted]
>  === Mixing -fdX and -global ===
>  
>  Testing: -fda TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 
> -global isa-fdc.driveB=none0
> +QEMU_PROG: warning: warning: property isa-fdc.driveB is deprecated
> +Use -device floppy,unit=1,drive=... instead.
>  
>            dev: isa-fdc, id ""
>              iobase = 1008 (0x3f0)
> @@ -717,6 +727,8 @@ sd0: [not inserted]
>  
>  
>  Testing: -fdb TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 
> -global isa-fdc.driveA=none0
> +QEMU_PROG: warning: warning: property isa-fdc.driveA is deprecated
> +Use -device floppy,unit=0,drive=... instead.
>  
>            dev: isa-fdc, id ""
>              iobase = 1008 (0x3f0)
> @@ -773,9 +785,13 @@ sd0: [not inserted]
>  
>  
>  Testing: -fda TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 
> -global isa-fdc.driveA=none0
> +QEMU_PROG: warning: warning: property isa-fdc.driveA is deprecated
> +Use -device floppy,unit=0,drive=... instead.
>  QEMU_PROG: Floppy unit 0 is in use
>  
>  Testing: -fdb TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 
> -global isa-fdc.driveB=none0
> +QEMU_PROG: warning: warning: property isa-fdc.driveB is deprecated
> +Use -device floppy,unit=1,drive=... instead.
>  QEMU_PROG: Floppy unit 1 is in use
>  
>  Testing: -fda TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 
> -global floppy.drive=none0
> @@ -1177,6 +1193,8 @@ QEMU_PROG: -device floppy,drive=none0,unit=0: Floppy 
> unit 0 is in use
>  === Mixing -global and -device ===
>  
>  Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive 
> if=none,file=TEST_DIR/t.qcow2.2 -global isa-fdc.driveA=none0 -device 
> floppy,drive=none1
> +QEMU_PROG: warning: warning: property isa-fdc.driveA is deprecated
> +Use -device floppy,unit=0,drive=... instead.
>  
>            dev: isa-fdc, id ""
>              iobase = 1008 (0x3f0)
> @@ -1233,6 +1251,8 @@ sd0: [not inserted]
>  
>  
>  Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive 
> if=none,file=TEST_DIR/t.qcow2.2 -global isa-fdc.driveA=none0 -device 
> floppy,drive=none1,unit=1
> +QEMU_PROG: warning: warning: property isa-fdc.driveA is deprecated
> +Use -device floppy,unit=0,drive=... instead.
>  
>            dev: isa-fdc, id ""
>              iobase = 1008 (0x3f0)
> @@ -1289,6 +1309,8 @@ sd0: [not inserted]
>  
>  
>  Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive 
> if=none,file=TEST_DIR/t.qcow2.2 -global isa-fdc.driveB=none0 -device 
> floppy,drive=none1
> +QEMU_PROG: warning: warning: property isa-fdc.driveB is deprecated
> +Use -device floppy,unit=1,drive=... instead.
>  
>            dev: isa-fdc, id ""
>              iobase = 1008 (0x3f0)
> @@ -1345,6 +1367,8 @@ sd0: [not inserted]
>  
>  
>  Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive 
> if=none,file=TEST_DIR/t.qcow2.2 -global isa-fdc.driveB=none0 -device 
> floppy,drive=none1,unit=0
> +QEMU_PROG: warning: warning: property isa-fdc.driveB is deprecated
> +Use -device floppy,unit=1,drive=... instead.
>  
>            dev: isa-fdc, id ""
>              iobase = 1008 (0x3f0)
> @@ -1441,9 +1465,13 @@ sd0: [not inserted]
>  
>  
>  Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive 
> if=none,file=TEST_DIR/t.qcow2.2 -global isa-fdc.driveA=none0 -device 
> floppy,drive=none1,unit=0
> +QEMU_PROG: warning: warning: property isa-fdc.driveA is deprecated
> +Use -device floppy,unit=0,drive=... instead.
>  QEMU_PROG: -device floppy,drive=none1,unit=0: Floppy unit 0 is in use
>  
>  Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive 
> if=none,file=TEST_DIR/t.qcow2.2 -global isa-fdc.driveB=none0 -device 
> floppy,drive=none1,unit=1
> +QEMU_PROG: warning: warning: property isa-fdc.driveB is deprecated
> +Use -device floppy,unit=1,drive=... instead.
>  QEMU_PROG: -device floppy,drive=none1,unit=1: Floppy unit 1 is in use
>  
>  Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive 
> if=none,file=TEST_DIR/t.qcow2.2 -global floppy.drive=none0 -device 
> floppy,drive=none1,unit=0
> @@ -1512,6 +1540,8 @@ QEMU_PROG: -device floppy,drive=floppy0: Property 
> 'floppy.drive' can't find valu
>  === Too many floppy drives ===
>  
>  Testing: -drive if=floppy,file=TEST_DIR/t.qcow2 -drive 
> if=none,file=TEST_DIR/t.qcow2.2 -drive if=none,file=TEST_DIR/t.qcow2.3 
> -global isa-fdc.driveB=none0 -device floppy,drive=none1
> +QEMU_PROG: warning: warning: property isa-fdc.driveB is deprecated
> +Use -device floppy,unit=1,drive=... instead.
>  QEMU_PROG: -device floppy,drive=none1: Can't create floppy unit 2, bus 
> supports only 2 units
>  
>  
> 

-- 
—js