Message ID | 20191006203150.13054-2-jsnow@redhat.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | IDE: Deprecate ide-drive | expand |
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
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?
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
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!
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
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
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(-)