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

[John Snow]

On 10/7/19 5:49 AM, Markus Armbruster wrote:
> John Snow <jsnow at 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 at 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.
>>  
>> + at 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