Re: [PATCH v2 1/3] fdc: Drop deprecated floppy configuration

2021-03-08 Thread Daniel P . Berrangé
On Fri, Mar 05, 2021 at 09:06:45AM +0100, Markus Armbruster wrote:
> Markus Armbruster  writes:
> 
> > Daniel P. Berrangé  writes:
> >
> >> On Thu, Mar 04, 2021 at 11:00:57AM +0100, Markus Armbruster wrote:
> >>> Drop the crap deprecated in commit 4a27a638e7 "fdc: Deprecate
> >>> configuring floppies with -global isa-fdc" (v5.1.0).
> >>> 
> >>> Signed-off-by: Markus Armbruster 
> [...]
> > Sadly, the commit's update of docs/system/deprecated.rst neglects to
> > cover this use.  Looks the series overtaxed my capacity to juggle
> > details; my apologies.
> [...]
> 
> I'm talking about commit 4a27a638e7 here.
> 
> The deprecated.rst text only covers setting floppy controller properties
> with -global.  It neglects to cover setting them with -device.  For
> onboard controllers, -global is the only way to set them.
> 
> I append a fixup.
> 
> We can put it before this patch.  This patch then moves the fixed up
> text to removed-features.rst.
> 
> Or we squash it into this patch, i.e. this patch deletes the flawed text
> from deprecated.rst and adds the fixed up text to removed-features.rst.
> 
> Got a preference?

I'm fine with either option. It isn't unusual to tweak the text when
moving it to the removed-features.rst file, as we'll be talking about
the past rather than future.


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|




Re: [PATCH v2 1/3] fdc: Drop deprecated floppy configuration

2021-03-05 Thread Markus Armbruster
Markus Armbruster  writes:

> Daniel P. Berrangé  writes:
>
>> On Thu, Mar 04, 2021 at 11:00:57AM +0100, Markus Armbruster wrote:
>>> Drop the crap deprecated in commit 4a27a638e7 "fdc: Deprecate
>>> configuring floppies with -global isa-fdc" (v5.1.0).
>>> 
>>> Signed-off-by: Markus Armbruster 
[...]
> Sadly, the commit's update of docs/system/deprecated.rst neglects to
> cover this use.  Looks the series overtaxed my capacity to juggle
> details; my apologies.
[...]

I'm talking about commit 4a27a638e7 here.

The deprecated.rst text only covers setting floppy controller properties
with -global.  It neglects to cover setting them with -device.  For
onboard controllers, -global is the only way to set them.

I append a fixup.

We can put it before this patch.  This patch then moves the fixed up
text to removed-features.rst.

Or we squash it into this patch, i.e. this patch deletes the flawed text
from deprecated.rst and adds the fixed up text to removed-features.rst.

Got a preference?



diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
index 2fcac7861e..393ede47f1 100644
--- a/docs/system/deprecated.rst
+++ b/docs/system/deprecated.rst
@@ -97,7 +97,11 @@ QEMU 5.1 has three options:
 ``Configuring floppies with ``-global``
 '''
 
-Use ``-device floppy,...`` instead:
+Floppy controllers' drive properties (since 5.1)
+
+
+Use ``-device floppy,...`` instead.  When configuring onboard floppy
+controllers
 ::
 
 -global isa-fdc.driveA=...
@@ -120,8 +124,30 @@ become
 
 -device floppy,unit=1,drive=...
 
-``-drive`` with bogus interface type
-
+When plugging in a floppy controller
+::
+
+-device isa-fdc,...,driveA=...
+
+becomes
+::
+
+-device isa-fdc,...
+-device floppy,unit=0,drive=...
+
+and
+::
+
+-device isa-fdc,...,driveB=...
+
+becomes
+::
+
+-device isa-fdc,...
+-device floppy,unit=1,drive=...
+
+``-drive`` with bogus interface type (since 5.1)
+
 
 Drives with interface types other than ``if=none`` are for onboard
 devices.  It is possible to use drives the board doesn't pick up with




Re: [PATCH v2 1/3] fdc: Drop deprecated floppy configuration

2021-03-04 Thread Markus Armbruster
Peter Krempa  writes:

> On Thu, Mar 04, 2021 at 17:23:05 +0100, Markus Armbruster wrote:
>> Daniel P. Berrangé  writes:
>> 
>> > On Thu, Mar 04, 2021 at 03:26:55PM +0100, Markus Armbruster wrote:
>> >> Daniel P. Berrangé  writes:
>> >> 
>> >> > On Thu, Mar 04, 2021 at 11:00:57AM +0100, Markus Armbruster wrote:
>> >> >> Drop the crap deprecated in commit 4a27a638e7 "fdc: Deprecate
>> >> >> configuring floppies with -global isa-fdc" (v5.1.0).
>> >> >> 
>> >> >> Signed-off-by: Markus Armbruster 
>> >> >> ---
>> >> >>  docs/system/deprecated.rst   |  26 --
>> >> >>  docs/system/removed-features.rst |  26 ++
>> >> >>  hw/block/fdc.c   |  54 +--
>> >> >>  tests/qemu-iotests/172   |  31 +-
>> >> >>  tests/qemu-iotests/172.out   | 562 +--
>> >> >>  5 files changed, 30 insertions(+), 669 deletions(-)
>
> [...]
>
>> >> 
>> >> Correct.
>> >> 
>> >> This was deprecated in commit 4a27a638e7 "fdc: Deprecate configuring
>> >> floppies with -global isa-fdc" (v5.1.0).  Since then, its use triggers a
>> >> warning:
>> >> 
>> >> $ qemu-system-x86_64 -nodefaults -M q35 -display none -drive 
>> >> if=none,id=drive-fdc0-0-0 -device 
>> >> isa-fdc,driveA=drive-fdc0-0-0,bootindexA=1
>> >> qemu-system-x86_64: -device 
>> >> isa-fdc,driveA=drive-fdc0-0-0,bootindexA=1: warning: warning: property 
>> >> isa-fdc.driveA is deprecated
>> >> Use -device floppy,unit=0,drive=... instead.
>> >> 
>> >> Note the -M q35.  Needed because the default machine type has an onboard
>> >> isa-fdc, which cannot be configured this way.
>> >> 
>> >> Sadly, the commit's update of docs/system/deprecated.rst neglects to
>> >> cover this use.  Looks the series overtaxed my capacity to juggle
>> >> details; my apologies.
>> >> 
>> >> Is libvirt still using these properties?
>> >
>> > Unfortunately yes, but it seems like it ought to be fairly easy to
>> > change the syntax. Just need to figure out what the right way to
>> > detect the availability of the new syntax is. Presumably just look
>> > for existance of the 'floppy' device type ?
>> 
>> Yes.  The device type was added in merge commit fd209e4a7, v2.8.0.
>> 
>> > Can you confirm that switching from -global to the new -device floppy
>> > does /not/ have any live migration impact ?
>> 
>> Yes, it must not affect migration.
>> 
>> When Kevin split the floppy device type off the floppy controller, he
>> had to add some moderately ugly hackery to keep the old qdev properties
>> working.  Think propagate property values to floppy from controller,
>> which otherwise ignores them.
>> 
>> The way you get the values into the floppy device cannot affect the
>> migration data.  Only different values can.
>> 
>> This patch removes a deprecated way.
>
> Note that when QEMU_CAPS_BLOCKDEV is asserted we format floppies as:
>
> -blockdev '{"driver":"file","filename":"/tmp/firmware.img",\
> "node-name":"libvirt-2-storage","auto-read-only":true,"discard":"unmap"}' \
> -blockdev '{"node-name":"libvirt-2-format","read-only":false,"driver":"raw",\
> "file":"libvirt-2-storage"}' \
> -device floppy,unit=0,drive=libvirt-2-format,id=fdc0-0-0 \
> -blockdev '{"driver":"file","filename":"/tmp/data.img",\
> "node-name":"libvirt-1-storage","auto-read-only":true,"discard":"unmap"}' \
> -blockdev 
> '{"node-name":"libvirt-1-format","read-only":false,"driver":"qcow2",\
> "file":"libvirt-1-storage"}' \
> -device floppy,unit=1,drive=libvirt-1-format,id=fdc0-0-1 \
>
> as visible in the test file:
>
> tests/qemuxml2argvdata/disk-floppy-q35-2_11.x86_64-latest.args
>
> So libvirt should be in the clear. isa-fdc with driveA/driveB is
> formatted only when the blockdev capability isn't present.

Even better: in the clear for some time already, which means a wider
range of libvirt versions keeps working with the latest QEMU.  Nice,
because keeping the coupling reasonably lose makes upgrading easier.

Thanks!




Re: [PATCH v2 1/3] fdc: Drop deprecated floppy configuration

2021-03-04 Thread Daniel P . Berrangé
On Thu, Mar 04, 2021 at 05:52:50PM +0100, Peter Krempa wrote:
> On Thu, Mar 04, 2021 at 17:23:05 +0100, Markus Armbruster wrote:
> > Daniel P. Berrangé  writes:
> > 
> > > On Thu, Mar 04, 2021 at 03:26:55PM +0100, Markus Armbruster wrote:
> > >> Daniel P. Berrangé  writes:
> > >> 
> > >> > On Thu, Mar 04, 2021 at 11:00:57AM +0100, Markus Armbruster wrote:
> > >> >> Drop the crap deprecated in commit 4a27a638e7 "fdc: Deprecate
> > >> >> configuring floppies with -global isa-fdc" (v5.1.0).
> > >> >> 
> > >> >> Signed-off-by: Markus Armbruster 
> > >> >> ---
> > >> >>  docs/system/deprecated.rst   |  26 --
> > >> >>  docs/system/removed-features.rst |  26 ++
> > >> >>  hw/block/fdc.c   |  54 +--
> > >> >>  tests/qemu-iotests/172   |  31 +-
> > >> >>  tests/qemu-iotests/172.out   | 562 
> > >> >> +--
> > >> >>  5 files changed, 30 insertions(+), 669 deletions(-)
> 
> [...]
> 
> > >> 
> > >> Correct.
> > >> 
> > >> This was deprecated in commit 4a27a638e7 "fdc: Deprecate configuring
> > >> floppies with -global isa-fdc" (v5.1.0).  Since then, its use triggers a
> > >> warning:
> > >> 
> > >> $ qemu-system-x86_64 -nodefaults -M q35 -display none -drive 
> > >> if=none,id=drive-fdc0-0-0 -device 
> > >> isa-fdc,driveA=drive-fdc0-0-0,bootindexA=1
> > >> qemu-system-x86_64: -device 
> > >> isa-fdc,driveA=drive-fdc0-0-0,bootindexA=1: warning: warning: property 
> > >> isa-fdc.driveA is deprecated
> > >> Use -device floppy,unit=0,drive=... instead.
> > >> 
> > >> Note the -M q35.  Needed because the default machine type has an onboard
> > >> isa-fdc, which cannot be configured this way.
> > >> 
> > >> Sadly, the commit's update of docs/system/deprecated.rst neglects to
> > >> cover this use.  Looks the series overtaxed my capacity to juggle
> > >> details; my apologies.
> > >> 
> > >> Is libvirt still using these properties?
> > >
> > > Unfortunately yes, but it seems like it ought to be fairly easy to
> > > change the syntax. Just need to figure out what the right way to
> > > detect the availability of the new syntax is. Presumably just look
> > > for existance of the 'floppy' device type ?
> > 
> > Yes.  The device type was added in merge commit fd209e4a7, v2.8.0.
> > 
> > > Can you confirm that switching from -global to the new -device floppy
> > > does /not/ have any live migration impact ?
> > 
> > Yes, it must not affect migration.
> > 
> > When Kevin split the floppy device type off the floppy controller, he
> > had to add some moderately ugly hackery to keep the old qdev properties
> > working.  Think propagate property values to floppy from controller,
> > which otherwise ignores them.
> > 
> > The way you get the values into the floppy device cannot affect the
> > migration data.  Only different values can.
> > 
> > This patch removes a deprecated way.
> 
> Note that when QEMU_CAPS_BLOCKDEV is asserted we format floppies as:
> 
> -blockdev '{"driver":"file","filename":"/tmp/firmware.img",\
> "node-name":"libvirt-2-storage","auto-read-only":true,"discard":"unmap"}' \
> -blockdev '{"node-name":"libvirt-2-format","read-only":false,"driver":"raw",\
> "file":"libvirt-2-storage"}' \
> -device floppy,unit=0,drive=libvirt-2-format,id=fdc0-0-0 \
> -blockdev '{"driver":"file","filename":"/tmp/data.img",\
> "node-name":"libvirt-1-storage","auto-read-only":true,"discard":"unmap"}' \
> -blockdev 
> '{"node-name":"libvirt-1-format","read-only":false,"driver":"qcow2",\
> "file":"libvirt-1-storage"}' \
> -device floppy,unit=1,drive=libvirt-1-format,id=fdc0-0-1 \
> 
> as visible in the test file:
> 
> tests/qemuxml2argvdata/disk-floppy-q35-2_11.x86_64-latest.args
> 
> So libvirt should be in the clear. isa-fdc with driveA/driveB is
> formatted only when the blockdev capability isn't present.

Ahh, excellant, I missed that detailed and worried that we still had
work todo.


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|




Re: [PATCH v2 1/3] fdc: Drop deprecated floppy configuration

2021-03-04 Thread Peter Krempa
On Thu, Mar 04, 2021 at 17:23:05 +0100, Markus Armbruster wrote:
> Daniel P. Berrangé  writes:
> 
> > On Thu, Mar 04, 2021 at 03:26:55PM +0100, Markus Armbruster wrote:
> >> Daniel P. Berrangé  writes:
> >> 
> >> > On Thu, Mar 04, 2021 at 11:00:57AM +0100, Markus Armbruster wrote:
> >> >> Drop the crap deprecated in commit 4a27a638e7 "fdc: Deprecate
> >> >> configuring floppies with -global isa-fdc" (v5.1.0).
> >> >> 
> >> >> Signed-off-by: Markus Armbruster 
> >> >> ---
> >> >>  docs/system/deprecated.rst   |  26 --
> >> >>  docs/system/removed-features.rst |  26 ++
> >> >>  hw/block/fdc.c   |  54 +--
> >> >>  tests/qemu-iotests/172   |  31 +-
> >> >>  tests/qemu-iotests/172.out   | 562 +--
> >> >>  5 files changed, 30 insertions(+), 669 deletions(-)

[...]

> >> 
> >> Correct.
> >> 
> >> This was deprecated in commit 4a27a638e7 "fdc: Deprecate configuring
> >> floppies with -global isa-fdc" (v5.1.0).  Since then, its use triggers a
> >> warning:
> >> 
> >> $ qemu-system-x86_64 -nodefaults -M q35 -display none -drive 
> >> if=none,id=drive-fdc0-0-0 -device 
> >> isa-fdc,driveA=drive-fdc0-0-0,bootindexA=1
> >> qemu-system-x86_64: -device 
> >> isa-fdc,driveA=drive-fdc0-0-0,bootindexA=1: warning: warning: property 
> >> isa-fdc.driveA is deprecated
> >> Use -device floppy,unit=0,drive=... instead.
> >> 
> >> Note the -M q35.  Needed because the default machine type has an onboard
> >> isa-fdc, which cannot be configured this way.
> >> 
> >> Sadly, the commit's update of docs/system/deprecated.rst neglects to
> >> cover this use.  Looks the series overtaxed my capacity to juggle
> >> details; my apologies.
> >> 
> >> Is libvirt still using these properties?
> >
> > Unfortunately yes, but it seems like it ought to be fairly easy to
> > change the syntax. Just need to figure out what the right way to
> > detect the availability of the new syntax is. Presumably just look
> > for existance of the 'floppy' device type ?
> 
> Yes.  The device type was added in merge commit fd209e4a7, v2.8.0.
> 
> > Can you confirm that switching from -global to the new -device floppy
> > does /not/ have any live migration impact ?
> 
> Yes, it must not affect migration.
> 
> When Kevin split the floppy device type off the floppy controller, he
> had to add some moderately ugly hackery to keep the old qdev properties
> working.  Think propagate property values to floppy from controller,
> which otherwise ignores them.
> 
> The way you get the values into the floppy device cannot affect the
> migration data.  Only different values can.
> 
> This patch removes a deprecated way.

Note that when QEMU_CAPS_BLOCKDEV is asserted we format floppies as:

-blockdev '{"driver":"file","filename":"/tmp/firmware.img",\
"node-name":"libvirt-2-storage","auto-read-only":true,"discard":"unmap"}' \
-blockdev '{"node-name":"libvirt-2-format","read-only":false,"driver":"raw",\
"file":"libvirt-2-storage"}' \
-device floppy,unit=0,drive=libvirt-2-format,id=fdc0-0-0 \
-blockdev '{"driver":"file","filename":"/tmp/data.img",\
"node-name":"libvirt-1-storage","auto-read-only":true,"discard":"unmap"}' \
-blockdev '{"node-name":"libvirt-1-format","read-only":false,"driver":"qcow2",\
"file":"libvirt-1-storage"}' \
-device floppy,unit=1,drive=libvirt-1-format,id=fdc0-0-1 \

as visible in the test file:

tests/qemuxml2argvdata/disk-floppy-q35-2_11.x86_64-latest.args

So libvirt should be in the clear. isa-fdc with driveA/driveB is
formatted only when the blockdev capability isn't present.




Re: [PATCH v2 1/3] fdc: Drop deprecated floppy configuration

2021-03-04 Thread Markus Armbruster
Daniel P. Berrangé  writes:

> On Thu, Mar 04, 2021 at 03:26:55PM +0100, Markus Armbruster wrote:
>> Daniel P. Berrangé  writes:
>> 
>> > On Thu, Mar 04, 2021 at 11:00:57AM +0100, Markus Armbruster wrote:
>> >> Drop the crap deprecated in commit 4a27a638e7 "fdc: Deprecate
>> >> configuring floppies with -global isa-fdc" (v5.1.0).
>> >> 
>> >> Signed-off-by: Markus Armbruster 
>> >> ---
>> >>  docs/system/deprecated.rst   |  26 --
>> >>  docs/system/removed-features.rst |  26 ++
>> >>  hw/block/fdc.c   |  54 +--
>> >>  tests/qemu-iotests/172   |  31 +-
>> >>  tests/qemu-iotests/172.out   | 562 +--
>> >>  5 files changed, 30 insertions(+), 669 deletions(-)
>> >> 
>> >> diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
>> >> index 2fcac7861e..6a22bc07e2 100644
>> >> --- a/docs/system/deprecated.rst
>> >> +++ b/docs/system/deprecated.rst
>> >> @@ -94,32 +94,6 @@ QEMU 5.1 has three options:
>> >>to the user to load all the images they need.
>> >>   3. ``-bios `` - Tells QEMU to load the specified file as the 
>> >> firmwrae.
>> >>  
>> >> -``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=...
>> >> -
>> >>  ``-drive`` with bogus interface type
>> >>  
>> >>  
>> >> diff --git a/docs/system/removed-features.rst 
>> >> b/docs/system/removed-features.rst
>> >> index c8481cafbd..b0e7350408 100644
>> >> --- a/docs/system/removed-features.rst
>> >> +++ b/docs/system/removed-features.rst
>> >> @@ -38,6 +38,32 @@ or ``-display default,show-cursor=on`` instead.
>> >>  QEMU 5.0 introduced an alternative syntax to specify the size of the 
>> >> translation
>> >>  block cache, ``-accel tcg,tb-size=``.
>> >>  
>> >> +``Configuring floppies with ``-global`` (removed in 6.0)
>> >> +
>> >> +
>> >> +Use ``-device floppy,...`` instead:
>> >> +::
>> >> +
>> >> +-global isa-fdc.driveA=...
>> >> +-global sysbus-fdc.driveA=...
>> >> +-global SUNW,fdtwo.drive=...
>> >
>> > It looks like we're not actually removing the use of -global, rather
>> > we're removing the driveA= and driveB= properties entirely, which
>> > simply means there's nothing to be set via -global. The distinction
>> > is important, because IIUC, it means that libvirt's use of these
>> > properties via -device is also impacted eg
>> >
>> >   -device isa-fdc,driveA=drive-fdc0-0-0,bootindexA=1,driveB=drive-fdc0-0-1
>> >
>> > will no longer work too ?
>> 
>> Correct.
>> 
>> This was deprecated in commit 4a27a638e7 "fdc: Deprecate configuring
>> floppies with -global isa-fdc" (v5.1.0).  Since then, its use triggers a
>> warning:
>> 
>> $ qemu-system-x86_64 -nodefaults -M q35 -display none -drive 
>> if=none,id=drive-fdc0-0-0 -device isa-fdc,driveA=drive-fdc0-0-0,bootindexA=1
>> qemu-system-x86_64: -device isa-fdc,driveA=drive-fdc0-0-0,bootindexA=1: 
>> warning: warning: property isa-fdc.driveA is deprecated
>> Use -device floppy,unit=0,drive=... instead.
>> 
>> Note the -M q35.  Needed because the default machine type has an onboard
>> isa-fdc, which cannot be configured this way.
>> 
>> Sadly, the commit's update of docs/system/deprecated.rst neglects to
>> cover this use.  Looks the series overtaxed my capacity to juggle
>> details; my apologies.
>> 
>> Is libvirt still using these properties?
>
> Unfortunately yes, but it seems like it ought to be fairly easy to
> change the syntax. Just need to figure out what the right way to
> detect the availability of the new syntax is. Presumably just look
> for existance of the 'floppy' device type ?

Yes.  The device type was added in merge commit fd209e4a7, v2.8.0.

> Can you confirm that switching from -global to the new -device floppy
> does /not/ have any live migration impact ?

Yes, it must not affect migration.

When Kevin split the floppy device type off the floppy controller, he
had to add some moderately ugly hackery to keep the old qdev properties
working.  Think propagate property values to floppy from controller,
which otherwise ignores them.

The way you get the values into the floppy device cannot affect the
migration data.  Only different values can.

This patch removes a deprecated way.




Re: [PATCH v2 1/3] fdc: Drop deprecated floppy configuration

2021-03-04 Thread Daniel P . Berrangé
On Thu, Mar 04, 2021 at 03:26:55PM +0100, Markus Armbruster wrote:
> Daniel P. Berrangé  writes:
> 
> > On Thu, Mar 04, 2021 at 11:00:57AM +0100, Markus Armbruster wrote:
> >> Drop the crap deprecated in commit 4a27a638e7 "fdc: Deprecate
> >> configuring floppies with -global isa-fdc" (v5.1.0).
> >> 
> >> Signed-off-by: Markus Armbruster 
> >> ---
> >>  docs/system/deprecated.rst   |  26 --
> >>  docs/system/removed-features.rst |  26 ++
> >>  hw/block/fdc.c   |  54 +--
> >>  tests/qemu-iotests/172   |  31 +-
> >>  tests/qemu-iotests/172.out   | 562 +--
> >>  5 files changed, 30 insertions(+), 669 deletions(-)
> >> 
> >> diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
> >> index 2fcac7861e..6a22bc07e2 100644
> >> --- a/docs/system/deprecated.rst
> >> +++ b/docs/system/deprecated.rst
> >> @@ -94,32 +94,6 @@ QEMU 5.1 has three options:
> >>to the user to load all the images they need.
> >>   3. ``-bios `` - Tells QEMU to load the specified file as the 
> >> firmwrae.
> >>  
> >> -``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=...
> >> -
> >>  ``-drive`` with bogus interface type
> >>  
> >>  
> >> diff --git a/docs/system/removed-features.rst 
> >> b/docs/system/removed-features.rst
> >> index c8481cafbd..b0e7350408 100644
> >> --- a/docs/system/removed-features.rst
> >> +++ b/docs/system/removed-features.rst
> >> @@ -38,6 +38,32 @@ or ``-display default,show-cursor=on`` instead.
> >>  QEMU 5.0 introduced an alternative syntax to specify the size of the 
> >> translation
> >>  block cache, ``-accel tcg,tb-size=``.
> >>  
> >> +``Configuring floppies with ``-global`` (removed in 6.0)
> >> +
> >> +
> >> +Use ``-device floppy,...`` instead:
> >> +::
> >> +
> >> +-global isa-fdc.driveA=...
> >> +-global sysbus-fdc.driveA=...
> >> +-global SUNW,fdtwo.drive=...
> >
> > It looks like we're not actually removing the use of -global, rather
> > we're removing the driveA= and driveB= properties entirely, which
> > simply means there's nothing to be set via -global. The distinction
> > is important, because IIUC, it means that libvirt's use of these
> > properties via -device is also impacted eg
> >
> >   -device isa-fdc,driveA=drive-fdc0-0-0,bootindexA=1,driveB=drive-fdc0-0-1
> >
> > will no longer work too ?
> 
> Correct.
> 
> This was deprecated in commit 4a27a638e7 "fdc: Deprecate configuring
> floppies with -global isa-fdc" (v5.1.0).  Since then, its use triggers a
> warning:
> 
> $ qemu-system-x86_64 -nodefaults -M q35 -display none -drive 
> if=none,id=drive-fdc0-0-0 -device isa-fdc,driveA=drive-fdc0-0-0,bootindexA=1
> qemu-system-x86_64: -device isa-fdc,driveA=drive-fdc0-0-0,bootindexA=1: 
> warning: warning: property isa-fdc.driveA is deprecated
> Use -device floppy,unit=0,drive=... instead.
> 
> Note the -M q35.  Needed because the default machine type has an onboard
> isa-fdc, which cannot be configured this way.
> 
> Sadly, the commit's update of docs/system/deprecated.rst neglects to
> cover this use.  Looks the series overtaxed my capacity to juggle
> details; my apologies.
> 
> Is libvirt still using these properties?

Unfortunately yes, but it seems like it ought to be fairly easy to
change the syntax. Just need to figure out what the right way to
detect the availability of the new syntax is. Presumably just look
for existance of the 'floppy' device type ?

Can you confirm that switching from -global to the new -device floppy
does /not/ have any live migration impact ?


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|




Re: [PATCH v2 1/3] fdc: Drop deprecated floppy configuration

2021-03-04 Thread Markus Armbruster
Daniel P. Berrangé  writes:

> On Thu, Mar 04, 2021 at 11:00:57AM +0100, Markus Armbruster wrote:
>> Drop the crap deprecated in commit 4a27a638e7 "fdc: Deprecate
>> configuring floppies with -global isa-fdc" (v5.1.0).
>> 
>> Signed-off-by: Markus Armbruster 
>> ---
>>  docs/system/deprecated.rst   |  26 --
>>  docs/system/removed-features.rst |  26 ++
>>  hw/block/fdc.c   |  54 +--
>>  tests/qemu-iotests/172   |  31 +-
>>  tests/qemu-iotests/172.out   | 562 +--
>>  5 files changed, 30 insertions(+), 669 deletions(-)
>> 
>> diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
>> index 2fcac7861e..6a22bc07e2 100644
>> --- a/docs/system/deprecated.rst
>> +++ b/docs/system/deprecated.rst
>> @@ -94,32 +94,6 @@ QEMU 5.1 has three options:
>>to the user to load all the images they need.
>>   3. ``-bios `` - Tells QEMU to load the specified file as the 
>> firmwrae.
>>  
>> -``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=...
>> -
>>  ``-drive`` with bogus interface type
>>  
>>  
>> diff --git a/docs/system/removed-features.rst 
>> b/docs/system/removed-features.rst
>> index c8481cafbd..b0e7350408 100644
>> --- a/docs/system/removed-features.rst
>> +++ b/docs/system/removed-features.rst
>> @@ -38,6 +38,32 @@ or ``-display default,show-cursor=on`` instead.
>>  QEMU 5.0 introduced an alternative syntax to specify the size of the 
>> translation
>>  block cache, ``-accel tcg,tb-size=``.
>>  
>> +``Configuring floppies with ``-global`` (removed in 6.0)
>> +
>> +
>> +Use ``-device floppy,...`` instead:
>> +::
>> +
>> +-global isa-fdc.driveA=...
>> +-global sysbus-fdc.driveA=...
>> +-global SUNW,fdtwo.drive=...
>
> It looks like we're not actually removing the use of -global, rather
> we're removing the driveA= and driveB= properties entirely, which
> simply means there's nothing to be set via -global. The distinction
> is important, because IIUC, it means that libvirt's use of these
> properties via -device is also impacted eg
>
>   -device isa-fdc,driveA=drive-fdc0-0-0,bootindexA=1,driveB=drive-fdc0-0-1
>
> will no longer work too ?

Correct.

This was deprecated in commit 4a27a638e7 "fdc: Deprecate configuring
floppies with -global isa-fdc" (v5.1.0).  Since then, its use triggers a
warning:

$ qemu-system-x86_64 -nodefaults -M q35 -display none -drive 
if=none,id=drive-fdc0-0-0 -device isa-fdc,driveA=drive-fdc0-0-0,bootindexA=1
qemu-system-x86_64: -device isa-fdc,driveA=drive-fdc0-0-0,bootindexA=1: 
warning: warning: property isa-fdc.driveA is deprecated
Use -device floppy,unit=0,drive=... instead.

Note the -M q35.  Needed because the default machine type has an onboard
isa-fdc, which cannot be configured this way.

Sadly, the commit's update of docs/system/deprecated.rst neglects to
cover this use.  Looks the series overtaxed my capacity to juggle
details; my apologies.

Is libvirt still using these properties?




Re: [PATCH v2 1/3] fdc: Drop deprecated floppy configuration

2021-03-04 Thread Daniel P . Berrangé


On Thu, Mar 04, 2021 at 11:00:57AM +0100, Markus Armbruster wrote:
> Drop the crap deprecated in commit 4a27a638e7 "fdc: Deprecate
> configuring floppies with -global isa-fdc" (v5.1.0).
> 
> Signed-off-by: Markus Armbruster 
> ---
>  docs/system/deprecated.rst   |  26 --
>  docs/system/removed-features.rst |  26 ++
>  hw/block/fdc.c   |  54 +--
>  tests/qemu-iotests/172   |  31 +-
>  tests/qemu-iotests/172.out   | 562 +--
>  5 files changed, 30 insertions(+), 669 deletions(-)
> 
> diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
> index 2fcac7861e..6a22bc07e2 100644
> --- a/docs/system/deprecated.rst
> +++ b/docs/system/deprecated.rst
> @@ -94,32 +94,6 @@ QEMU 5.1 has three options:
>to the user to load all the images they need.
>   3. ``-bios `` - Tells QEMU to load the specified file as the firmwrae.
>  
> -``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=...
> -
>  ``-drive`` with bogus interface type
>  
>  
> diff --git a/docs/system/removed-features.rst 
> b/docs/system/removed-features.rst
> index c8481cafbd..b0e7350408 100644
> --- a/docs/system/removed-features.rst
> +++ b/docs/system/removed-features.rst
> @@ -38,6 +38,32 @@ or ``-display default,show-cursor=on`` instead.
>  QEMU 5.0 introduced an alternative syntax to specify the size of the 
> translation
>  block cache, ``-accel tcg,tb-size=``.
>  
> +``Configuring floppies with ``-global`` (removed in 6.0)
> +
> +
> +Use ``-device floppy,...`` instead:
> +::
> +
> +-global isa-fdc.driveA=...
> +-global sysbus-fdc.driveA=...
> +-global SUNW,fdtwo.drive=...

It looks like we're not actually removing the use of -global, rather
we're removing the driveA= and driveB= properties entirely, which
simply means there's nothing to be set via -global. The distinction
is important, because IIUC, it means that libvirt's use of these
properties via -device is also impacted eg

  -device isa-fdc,driveA=drive-fdc0-0-0,bootindexA=1,driveB=drive-fdc0-0-1

will no longer work too ?


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|




[PATCH v2 1/3] fdc: Drop deprecated floppy configuration

2021-03-04 Thread Markus Armbruster
Drop the crap deprecated in commit 4a27a638e7 "fdc: Deprecate
configuring floppies with -global isa-fdc" (v5.1.0).

Signed-off-by: Markus Armbruster 
---
 docs/system/deprecated.rst   |  26 --
 docs/system/removed-features.rst |  26 ++
 hw/block/fdc.c   |  54 +--
 tests/qemu-iotests/172   |  31 +-
 tests/qemu-iotests/172.out   | 562 +--
 5 files changed, 30 insertions(+), 669 deletions(-)

diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
index 2fcac7861e..6a22bc07e2 100644
--- a/docs/system/deprecated.rst
+++ b/docs/system/deprecated.rst
@@ -94,32 +94,6 @@ QEMU 5.1 has three options:
   to the user to load all the images they need.
  3. ``-bios `` - Tells QEMU to load the specified file as the firmwrae.
 
-``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=...
-
 ``-drive`` with bogus interface type
 
 
diff --git a/docs/system/removed-features.rst b/docs/system/removed-features.rst
index c8481cafbd..b0e7350408 100644
--- a/docs/system/removed-features.rst
+++ b/docs/system/removed-features.rst
@@ -38,6 +38,32 @@ or ``-display default,show-cursor=on`` instead.
 QEMU 5.0 introduced an alternative syntax to specify the size of the 
translation
 block cache, ``-accel tcg,tb-size=``.
 
+``Configuring floppies with ``-global`` (removed in 6.0)
+
+
+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 198940e737..f978ddf647 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -870,7 +870,6 @@ struct FDCtrl {
 uint8_t num_floppies;
 FDrive drives[MAX_FD];
 struct {
-BlockBackend *blk;
 FloppyDriveType type;
 } qdev_for_drives[MAX_FD];
 int reset_sensei;
@@ -2517,56 +2516,12 @@ static void fdctrl_connect_drives(FDCtrl *fdctrl, 
DeviceState *fdc_dev,
 {
 unsigned int i;
 FDrive *drive;
-DeviceState *dev;
-BlockBackend *blk;
-bool ok;
-const char *fdc_name, *drive_suffix;
 
 for (i = 0; i < MAX_FD; i++) {
 drive = >drives[i];
 drive->fdctrl = fdctrl;
-
-/* If the drive is not present, we skip creating the qdev device, but
- * still have to initialise the controller. */
-blk = fdctrl->qdev_for_drives[i].blk;
-if (!blk) {
-fd_init(drive);
-fd_revalidate(drive);
-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;
-ok = qdev_prop_set_drive_err(dev, "drive", blk, errp);
-blk_unref(blk);
-if (!ok) {
-return;
-}
-
-if (!qdev_realize_and_unref(dev, >bus.bus, errp)) {
-return;
-}
+fd_init(drive);
+fd_revalidate(drive);
 }
 }
 
@@ -2882,8 +2837,6 @@ static Property isa_fdc_properties[] = {
 DEFINE_PROP_UINT32("iobase", FDCtrlISABus, iobase, 0x3f0),
 DEFINE_PROP_UINT32("irq", FDCtrlISABus, irq, 6),
 DEFINE_PROP_UINT32("dma", FDCtrlISABus, dma, 2),
-DEFINE_PROP_DRIVE("driveA", FDCtrlISABus, state.qdev_for_drives[0].blk),
-DEFINE_PROP_DRIVE("driveB", FDCtrlISABus, state.qdev_for_drives[1].blk),