Re: [PATCH v3 4/4] blockdev: Drop deprecated bogus -drive interface type

On 3/9/21 11:12 AM, Markus Armbruster wrote: > Drop the crap deprecated in commit a1b40bda08 "blockdev: Deprecate > -drive with bogus interface type" (v5.1.0). > > Signed-off-by: Markus Armbruster <armbru(a)redhat.com&gt; > --- > docs/system/deprecated.rst | 7 ------ > docs/system/removed-features.rst | 7 ++++++ > include/sysemu/blockdev.h | 1 - > blockdev.c | 37 +++++++++++++------------------- > softmmu/vl.c | 8 +------ > 5 files changed, 23 insertions(+), 37 deletions(-) > > diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst > index 601e9647a5..664ed60e9f 100644 > --- a/docs/system/deprecated.rst > +++ b/docs/system/deprecated.rst > @@ -94,13 +94,6 @@ QEMU 5.1 has three options: > to the user to load all the images they need. > 3. ``-bios <file>`` - Tells QEMU to load the specified file as the firmwrae. > > -``-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 > --device. This usage is now deprecated. Use ``if=none`` instead. > - > Short-form boolean options (since 6.0) > '''''''''''''''''''''''''''''''''''''' > > diff --git a/docs/system/removed-features.rst b/docs/system/removed-features.rst > index 77e7ba1339..e6d2fbe798 100644 > --- a/docs/system/removed-features.rst > +++ b/docs/system/removed-features.rst > @@ -87,6 +87,13 @@ becomes > -device isa-fdc,... > -device floppy,unit=1,drive=... > > +``-drive`` with bogus interface type (removed in 6.0) > +''''''''''''''''''''''''''''''''''''''''''''''''''''' > + > +Drives with interface types other than ``if=none`` are for onboard > +devices. Drives the board doesn't pick up can no longer be used with > +-device. Use ``if=none`` instead. > + > QEMU Machine Protocol (QMP) commands > ------------------------------------ > > diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h > index 3b5fcda08d..32c2d6023c 100644 > --- a/include/sysemu/blockdev.h > +++ b/include/sysemu/blockdev.h > @@ -35,7 +35,6 @@ struct DriveInfo { > bool is_default; /* Added by default_drive() ? */ > int media_cd; > QemuOpts *opts; > - bool claimed_by_board; > QTAILQ_ENTRY(DriveInfo) next; > }; > > diff --git a/blockdev.c b/blockdev.c > index cd438e60e3..2e01889cff 100644 > --- a/blockdev.c > +++ b/blockdev.c > @@ -240,19 +240,10 @@ DriveInfo *drive_get(BlockInterfaceType type, int bus, int unit) > return NULL; > } > > -void drive_mark_claimed_by_board(void) > -{ > - BlockBackend *blk; > - DriveInfo *dinfo; > - > - for (blk = blk_next(NULL); blk; blk = blk_next(blk)) { > - dinfo = blk_legacy_dinfo(blk); > - if (dinfo && blk_get_attached_dev(blk)) { > - dinfo->claimed_by_board = true; > - } > - } > -} > - > +/* > + * Check board claimed all -drive that are meant to be claimed. > + * Fatal error if any remain unclaimed. > + */ > void drive_check_orphaned(void) > { > BlockBackend *blk; > @@ -262,7 +253,17 @@ void drive_check_orphaned(void) > > for (blk = blk_next(NULL); blk; blk = blk_next(blk)) { > dinfo = blk_legacy_dinfo(blk); > - if (dinfo->is_default || dinfo->type == IF_NONE) { > + /* > + * Ignore default drives, because we create certain default > + * drives unconditionally, then leave them unclaimed. Not the > + * users fault. "user's" ?

> + * Ignore IF_VIRTIO, because it gets desugared into -device, > + * so we can leave failing to -device. > + * Ignore IF_NONE, because leaving unclaimed IF_NONE remains > + * available for device_add is a feature. Do you mean "as a feature" ?

> + */ > + if (dinfo->is_default || dinfo->type == IF_VIRTIO > + || dinfo->type == IF_NONE) { > continue; > } > if (!blk_get_attached_dev(blk)) { > @@ -273,14 +274,6 @@ void drive_check_orphaned(void) > if_name[dinfo->type], dinfo->bus, dinfo->unit); > loc_pop(&loc); > orphans = true; > - continue; > - } > - if (!dinfo->claimed_by_board && dinfo->type != IF_VIRTIO) { > - loc_push_none(&loc); > - qemu_opts_loc_restore(dinfo->opts); > - warn_report("bogus if=%s is deprecated, use if=none", > - if_name[dinfo->type]); > - loc_pop(&loc); > } > } > > diff --git a/softmmu/vl.c b/softmmu/vl.c > index ff488ea3e7..7453611152 100644 > --- a/softmmu/vl.c > +++ b/softmmu/vl.c > @@ -2460,13 +2460,7 @@ static void qemu_init_board(void) > /* From here on we enter MACHINE_PHASE_INITIALIZED. */ > machine_run_board_init(current_machine); > > - /* > - * TODO To drop support for deprecated bogus if=..., move > - * drive_check_orphaned() here, replacing this call. Also drop > - * its deprecation warning, along with DriveInfo member > - * @claimed_by_board. > - */ > - drive_mark_claimed_by_board(); > + drive_check_orphaned(); > > realtime_init(); > > Rest looks okay as far as I am concerned, pending your response to Dan's remark. (Sorry for being AWOL here. Thank you for taking the time to clean it up as you have; it feels like closing a book on some of the first reviews I ever got @ RH when I put the first orphan check in aeons ago! I think only the whitespace is left by now, haha.)