[PATCH 0/1] IDE: Deprecate ide-drive

[PATCH 0/1] IDE: Deprecate ide-drive

[John Snow]

John Snow (1):
  IDE: deprecate ide-drive

 qemu-deprecated.texi          | 5 +++++
 hw/ide/qdev.c                 | 3 +++
 tests/qemu-iotests/051.pc.out | 6 ++++--
 3 files changed, 12 insertions(+), 2 deletions(-)

-- 
2.21.0

[PATCH 1/1] IDE: deprecate ide-drive

[John Snow]

It's an old compatibility shim that just delegates to ide-cd or ide-hd.
I'd like to refactor these some day, and getting rid of the super-object
will make that easier.

Either way, we don't need this.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 qemu-deprecated.texi          | 5 +++++
 hw/ide/qdev.c                 | 3 +++
 tests/qemu-iotests/051.pc.out | 6 ++++--
 3 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi
index 01245e0b1c4..f802d83983e 100644
--- a/qemu-deprecated.texi
+++ b/qemu-deprecated.texi
@@ -247,6 +247,11 @@ quite a bit. It will be removed without replacement unless some users speaks
 up at the @email{qemu-devel@@nongnu.org} mailing list with information about
 their usecases.
 
+@subsection ide-drive (since 4.2)
+
+The 'ide-drive' device is deprecated. Users should use 'ide-hd' or
+'ide-cd' as appropriate to get an IDE hard disk or CDROM as needed.
+
 @section System emulator machines
 
 @subsection pc-0.12, pc-0.13, pc-0.14 and pc-0.15 (since 4.0)
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index 6fba6b62b87..9ecee4da074 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -279,6 +279,9 @@ static void ide_drive_realize(IDEDevice *dev, Error **errp)
 {
     DriveInfo *dinfo = NULL;
 
+    warn_report("The 'ide-drive' device is deprecated. "
+                "Use 'ide-hd' or 'ide-cd' instead");
+
     if (dev->conf.blk) {
         dinfo = blk_legacy_dinfo(dev->conf.blk);
     }
diff --git a/tests/qemu-iotests/051.pc.out b/tests/qemu-iotests/051.pc.out
index 000557c7c83..93b9a1f82ca 100644
--- a/tests/qemu-iotests/051.pc.out
+++ b/tests/qemu-iotests/051.pc.out
@@ -158,7 +158,8 @@ QEMU X.Y.Z monitor - type 'help' for more information
 
 Testing: -drive if=none,id=disk -device ide-drive,drive=disk
 QEMU X.Y.Z monitor - type 'help' for more information
-(qemu) QEMU_PROG: -device ide-drive,drive=disk: Device needs media, but drive is empty
+(qemu) QEMU_PROG: -device ide-drive,drive=disk: warning: The 'ide-drive' device is deprecated. Use 'ide-hd' or 'ide-cd' instead
+QEMU_PROG: -device ide-drive,drive=disk: Device needs media, but drive is empty
 
 Testing: -drive if=none,id=disk -device ide-hd,drive=disk
 QEMU X.Y.Z monitor - type 'help' for more information
@@ -228,7 +229,8 @@ QEMU X.Y.Z monitor - type 'help' for more information
 
 Testing: -drive file=TEST_DIR/t.qcow2,if=none,id=disk,readonly=on -device ide-drive,drive=disk
 QEMU X.Y.Z monitor - type 'help' for more information
-(qemu) QEMU_PROG: -device ide-drive,drive=disk: Block node is read-only
+(qemu) QEMU_PROG: -device ide-drive,drive=disk: warning: The 'ide-drive' device is deprecated. Use 'ide-hd' or 'ide-cd' instead
+QEMU_PROG: -device ide-drive,drive=disk: Block node is read-only
 
 Testing: -drive file=TEST_DIR/t.qcow2,if=none,id=disk,readonly=on -device ide-hd,drive=disk
 QEMU X.Y.Z monitor - type 'help' for more information
-- 
2.21.0

Re: [libvirt] [PATCH 1/1] IDE: deprecate ide-drive

[Peter Krempa]

On Sun, Oct 06, 2019 at 16:31:50 -0400, John Snow wrote:
> It's an old compatibility shim that just delegates to ide-cd or ide-hd.
> I'd like to refactor these some day, and getting rid of the super-object
> will make that easier.
> 
> Either way, we don't need this.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  qemu-deprecated.texi          | 5 +++++
>  hw/ide/qdev.c                 | 3 +++
>  tests/qemu-iotests/051.pc.out | 6 ++++--
>  3 files changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi
> index 01245e0b1c4..f802d83983e 100644
> --- a/qemu-deprecated.texi
> +++ b/qemu-deprecated.texi
> @@ -247,6 +247,11 @@ quite a bit. It will be removed without replacement unless some users speaks
>  up at the @email{qemu-devel@@nongnu.org} mailing list with information about
>  their usecases.
>  
> +@subsection ide-drive (since 4.2)
> +
> +The 'ide-drive' device is deprecated. Users should use 'ide-hd' or
> +'ide-cd' as appropriate to get an IDE hard disk or CDROM as needed.

In libvirt we don't use ide-drive any more for command line use since
libvirt-commit 'a4cda054e7' [0]

There is a capability named 'ide-drive.wwn', but this one is actually
probed from ide-hd along with other IDE-related capabilities since
libvirt-commit 'e67b6dcf361' [1].

There is also one test file that mentions ide-drive but that is actually
not referenced from any test code so I'll just delete it.

This means that libvirt is prepared for this deprecation so I guess you
can add a vanity-by:

Libvirt-checked-by: Peter Krempa <pkrempa@redhat.com>

[0] https://libvirt.org/git/?p=libvirt.git;a=commit;h=a4cda054e7
[1] https://libvirt.org/git/?p=libvirt.git;a=commit;h=e67b6dcf361

Re: [PATCH 1/1] IDE: deprecate ide-drive

[Markus Armbruster]

John Snow <jsnow@redhat.com> writes:

> It's an old compatibility shim that just delegates to ide-cd or ide-hd.
> I'd like to refactor these some day, and getting rid of the super-object
> will make that easier.

Device "scsi-disk" is similar.  However, it's still used by the
scsi_bus_legacy_add_drive() magic.  Not sure that's fully deprecated,
yet.  If / once it is, we can deprecate "scsi-disk", too.  Anyway, not
your department.

> Either way, we don't need this.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  qemu-deprecated.texi          | 5 +++++
>  hw/ide/qdev.c                 | 3 +++
>  tests/qemu-iotests/051.pc.out | 6 ++++--
>  3 files changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi
> index 01245e0b1c4..f802d83983e 100644
> --- a/qemu-deprecated.texi
> +++ b/qemu-deprecated.texi
> @@ -247,6 +247,11 @@ quite a bit. It will be removed without replacement unless some users speaks
>  up at the @email{qemu-devel@@nongnu.org} mailing list with information about
>  their usecases.
>  
> +@subsection ide-drive (since 4.2)
> +
> +The 'ide-drive' device is deprecated. Users should use 'ide-hd' or
> +'ide-cd' as appropriate to get an IDE hard disk or CDROM as needed.

CD-ROM

> +
>  @section System emulator machines
>  
>  @subsection pc-0.12, pc-0.13, pc-0.14 and pc-0.15 (since 4.0)
> diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
> index 6fba6b62b87..9ecee4da074 100644
> --- a/hw/ide/qdev.c
> +++ b/hw/ide/qdev.c
> @@ -279,6 +279,9 @@ static void ide_drive_realize(IDEDevice *dev, Error **errp)
>  {
>      DriveInfo *dinfo = NULL;
>  
> +    warn_report("The 'ide-drive' device is deprecated. "
> +                "Use 'ide-hd' or 'ide-cd' instead");

Two sentences, where only the first one terminated with a period.

Let's say "is deprecated, please use", like we do in several other places.

> +
>      if (dev->conf.blk) {
>          dinfo = blk_legacy_dinfo(dev->conf.blk);
>      }
> diff --git a/tests/qemu-iotests/051.pc.out b/tests/qemu-iotests/051.pc.out
> index 000557c7c83..93b9a1f82ca 100644
> --- a/tests/qemu-iotests/051.pc.out
> +++ b/tests/qemu-iotests/051.pc.out
> @@ -158,7 +158,8 @@ QEMU X.Y.Z monitor - type 'help' for more information
>  
>  Testing: -drive if=none,id=disk -device ide-drive,drive=disk
>  QEMU X.Y.Z monitor - type 'help' for more information
> -(qemu) QEMU_PROG: -device ide-drive,drive=disk: Device needs media, but drive is empty
> +(qemu) QEMU_PROG: -device ide-drive,drive=disk: warning: The 'ide-drive' device is deprecated. Use 'ide-hd' or 'ide-cd' instead
> +QEMU_PROG: -device ide-drive,drive=disk: Device needs media, but drive is empty
>  
>  Testing: -drive if=none,id=disk -device ide-hd,drive=disk
>  QEMU X.Y.Z monitor - type 'help' for more information
> @@ -228,7 +229,8 @@ QEMU X.Y.Z monitor - type 'help' for more information
>  
>  Testing: -drive file=TEST_DIR/t.qcow2,if=none,id=disk,readonly=on -device ide-drive,drive=disk
>  QEMU X.Y.Z monitor - type 'help' for more information
> -(qemu) QEMU_PROG: -device ide-drive,drive=disk: Block node is read-only
> +(qemu) QEMU_PROG: -device ide-drive,drive=disk: warning: The 'ide-drive' device is deprecated. Use 'ide-hd' or 'ide-cd' instead
> +QEMU_PROG: -device ide-drive,drive=disk: Block node is read-only
>  
>  Testing: -drive file=TEST_DIR/t.qcow2,if=none,id=disk,readonly=on -device ide-hd,drive=disk
>  QEMU X.Y.Z monitor - type 'help' for more information

A few iotests still use ide-drive.  Should any of them be converted to
ide-hd or ide-cd now?

Re: [PATCH 1/1] IDE: deprecate ide-drive

[John Snow]

On 10/7/19 5:49 AM, Markus Armbruster wrote:
> John Snow <jsnow@redhat.com> writes:
> 
>> It's an old compatibility shim that just delegates to ide-cd or ide-hd.
>> I'd like to refactor these some day, and getting rid of the super-object
>> will make that easier.
> 
> Device "scsi-disk" is similar.  However, it's still used by the
> scsi_bus_legacy_add_drive() magic.  Not sure that's fully deprecated,
> yet.  If / once it is, we can deprecate "scsi-disk", too.  Anyway, not
> your department.
> 

Yeah. I just want to get rid of this to allow myself to do bolder things
later on.

I have literally no time to do this and it's not really anything that
would make anyone money, but...

I want to add a few explicit devices:

ata-hd
ata-cd
sata-hd
sata-cd

With some shared state structures that implement common feature subsets,
like ata_registers, sata_registers, atapi_registers, etc.

I'd also like to separate out frontend and backend state providing a bit
of a cleaner division between device configuration (parameters on the
hardware creation itself), emulated device state (ATA register sets and
state machine), and QEMU backend state (block_backend pointers, aio
state counters, locks, etc etc etc -- Things solely purposed for
interacting with the block module.)

I'd also like to make each device type plug into ATA or SATA bus slots
explicitly -- no more magic IDE devices.

It's like the 5-year itch I can't help but want to scratch. My name's on
this code and it's UGLY UGLY UGLY!

The biggest roadblock to me actually doing this is figuring out how it
would be even vaguely possible to migrate from ide-hd or ide-cd to the
newer models -- it might be pretty complex, but maybe I can figure
something out somehow...

Well, suggestions welcome.

>> Either way, we don't need this.
>>
>> Signed-off-by: John Snow <jsnow@redhat.com>
>> ---
>>  qemu-deprecated.texi          | 5 +++++
>>  hw/ide/qdev.c                 | 3 +++
>>  tests/qemu-iotests/051.pc.out | 6 ++++--
>>  3 files changed, 12 insertions(+), 2 deletions(-)
>>
>> diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi
>> index 01245e0b1c4..f802d83983e 100644
>> --- a/qemu-deprecated.texi
>> +++ b/qemu-deprecated.texi
>> @@ -247,6 +247,11 @@ quite a bit. It will be removed without replacement unless some users speaks
>>  up at the @email{qemu-devel@@nongnu.org} mailing list with information about
>>  their usecases.
>>  
>> +@subsection ide-drive (since 4.2)
>> +
>> +The 'ide-drive' device is deprecated. Users should use 'ide-hd' or
>> +'ide-cd' as appropriate to get an IDE hard disk or CDROM as needed.
> 
> CD-ROM
> 

>:[

>> +
>>  @section System emulator machines
>>  
>>  @subsection pc-0.12, pc-0.13, pc-0.14 and pc-0.15 (since 4.0)
>> diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
>> index 6fba6b62b87..9ecee4da074 100644
>> --- a/hw/ide/qdev.c
>> +++ b/hw/ide/qdev.c
>> @@ -279,6 +279,9 @@ static void ide_drive_realize(IDEDevice *dev, Error **errp)
>>  {
>>      DriveInfo *dinfo = NULL;
>>  
>> +    warn_report("The 'ide-drive' device is deprecated. "
>> +                "Use 'ide-hd' or 'ide-cd' instead");
> 
> Two sentences, where only the first one terminated with a period.
> 
> Let's say "is deprecated, please use", like we do in several other places.
> 

Alright.

>> +
>>      if (dev->conf.blk) {
>>          dinfo = blk_legacy_dinfo(dev->conf.blk);
>>      }
>> diff --git a/tests/qemu-iotests/051.pc.out b/tests/qemu-iotests/051.pc.out
>> index 000557c7c83..93b9a1f82ca 100644
>> --- a/tests/qemu-iotests/051.pc.out
>> +++ b/tests/qemu-iotests/051.pc.out
>> @@ -158,7 +158,8 @@ QEMU X.Y.Z monitor - type 'help' for more information
>>  
>>  Testing: -drive if=none,id=disk -device ide-drive,drive=disk
>>  QEMU X.Y.Z monitor - type 'help' for more information
>> -(qemu) QEMU_PROG: -device ide-drive,drive=disk: Device needs media, but drive is empty
>> +(qemu) QEMU_PROG: -device ide-drive,drive=disk: warning: The 'ide-drive' device is deprecated. Use 'ide-hd' or 'ide-cd' instead
>> +QEMU_PROG: -device ide-drive,drive=disk: Device needs media, but drive is empty
>>  
>>  Testing: -drive if=none,id=disk -device ide-hd,drive=disk
>>  QEMU X.Y.Z monitor - type 'help' for more information
>> @@ -228,7 +229,8 @@ QEMU X.Y.Z monitor - type 'help' for more information
>>  
>>  Testing: -drive file=TEST_DIR/t.qcow2,if=none,id=disk,readonly=on -device ide-drive,drive=disk
>>  QEMU X.Y.Z monitor - type 'help' for more information
>> -(qemu) QEMU_PROG: -device ide-drive,drive=disk: Block node is read-only
>> +(qemu) QEMU_PROG: -device ide-drive,drive=disk: warning: The 'ide-drive' device is deprecated. Use 'ide-hd' or 'ide-cd' instead
>> +QEMU_PROG: -device ide-drive,drive=disk: Block node is read-only
>>  
>>  Testing: -drive file=TEST_DIR/t.qcow2,if=none,id=disk,readonly=on -device ide-hd,drive=disk
>>  QEMU X.Y.Z monitor - type 'help' for more information
> 
> A few iotests still use ide-drive.  Should any of them be converted to
> ide-hd or ide-cd now?
> 

I only saw the use in 051; (I should fix the output for non-PC too,
actually) and in this case it can just be dropped whenever we drop the
ide-drive definition.

I'll respin to hit the tests with a stiffer scrub-brush.

--js

Re: [PATCH 1/1] IDE: deprecate ide-drive

[Markus Armbruster]

John Snow <jsnow@redhat.com> writes:

> On 10/7/19 5:49 AM, Markus Armbruster wrote:
>> John Snow <jsnow@redhat.com> writes:
>> 
>>> It's an old compatibility shim that just delegates to ide-cd or ide-hd.
>>> I'd like to refactor these some day, and getting rid of the super-object
>>> will make that easier.
>> 
>> Device "scsi-disk" is similar.  However, it's still used by the
>> scsi_bus_legacy_add_drive() magic.  Not sure that's fully deprecated,
>> yet.  If / once it is, we can deprecate "scsi-disk", too.  Anyway, not
>> your department.
>> 
>
> Yeah. I just want to get rid of this to allow myself to do bolder things
> later on.
>
> I have literally no time to do this and it's not really anything that
> would make anyone money, but...
>
> I want to add a few explicit devices:
>
> ata-hd
> ata-cd
> sata-hd
> sata-cd
>
> With some shared state structures that implement common feature subsets,
> like ata_registers, sata_registers, atapi_registers, etc.
>
> I'd also like to separate out frontend and backend state providing a bit
> of a cleaner division between device configuration (parameters on the
> hardware creation itself), emulated device state (ATA register sets and
> state machine), and QEMU backend state (block_backend pointers, aio
> state counters, locks, etc etc etc -- Things solely purposed for
> interacting with the block module.)
>
> I'd also like to make each device type plug into ATA or SATA bus slots
> explicitly -- no more magic IDE devices.
>
> It's like the 5-year itch I can't help but want to scratch. My name's on
> this code and it's UGLY UGLY UGLY!

True!  And that's after Kraxel made it *less* ugly.  Your 5-year itch is
actually a 10-year itch that evolved from an open sore.

> The biggest roadblock to me actually doing this is figuring out how it
> would be even vaguely possible to migrate from ide-hd or ide-cd to the
> newer models -- it might be pretty complex, but maybe I can figure
> something out somehow...

Yes, that's the problem that has blocked further improvement.  Doesn't
mean it's intractable, only that nobody has found the time to tackle it
seriously.

> Well, suggestions welcome.
>
>>> Either way, we don't need this.
>>>
>>> Signed-off-by: John Snow <jsnow@redhat.com>
[...]
> I'll respin to hit the tests with a stiffer scrub-brush.

Thanks!

Re: [PATCH 1/1] IDE: deprecate ide-drive

[John Snow]

On 10/8/19 2:51 AM, Markus Armbruster wrote:
>> I'll respin to hit the tests with a stiffer scrub-brush.
> Thanks!

051 is the only test I can find that uses ide-drive, and the non-pc
version of the test doesn't seem to use it, so this actually seems
sufficient.

I'd like to keep the test for ide-drive itself in until we actually
remove it, just to make sure it still works. No harm in keeping it, I think.

--js