[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v3 4/4] blockdev: Drop deprecated bogus -drive interface type
From: |
Markus Armbruster |
Subject: |
Re: [PATCH v3 4/4] blockdev: Drop deprecated bogus -drive interface type |
Date: |
Thu, 11 Mar 2021 07:29:44 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) |
John Snow <jsnow@redhat.com> writes:
> 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@redhat.com>
>> ---
>> 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" ?
Yes.
>> + * 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" ?
I mean "because leaving unclaimed IF_NONE available for device_add is a
feature."
I'm embarrassingly prone to accidents when rewriting my own prose.
>> + */
>> + 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.)
It has served us well :)
Thank you, v4 coming!
- Re: [PATCH v3 3/4] fdc: Inline fdctrl_connect_drives() into fdctrl_realize_common(), (continued)