Re: [PATCH 04/11] qemu-options: finesse the recommendations around -blockdev

[Markus Armbruster]

Alex Bennée <alex.bennee@linaro.org> writes:

> Markus Armbruster <armbru@redhat.com> writes:
>
>> Alex Bennée <alex.bennee@linaro.org> writes:
>>
>>> We are a bit premature in recommending -blockdev/-device as the best
>>> way to configure block devices, especially in the common case.
>>> Improve the language to hopefully make things clearer.
>>>
>>> Suggested-by: Michael Tokarev <mjt@tls.msk.ru>
>>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>>> ---
>>>  qemu-options.hx | 8 ++++++--
>>>  1 file changed, 6 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/qemu-options.hx b/qemu-options.hx
>>> index 59bdf67a2c..9a69ed838e 100644
>>> --- a/qemu-options.hx
>>> +++ b/qemu-options.hx
>>> @@ -1143,10 +1143,14 @@ have gone through several iterations as the feature 
>>> set and complexity
>>>  of the block layer have grown. Many online guides to QEMU often
>>>  reference older and deprecated options, which can lead to confusion.
>>>  
>>> -The recommended modern way to describe disks is to use a combination of
>>> +The most explicit way to describe disks is to use a combination of
>>>  ``-device`` to specify the hardware device and ``-blockdev`` to
>>>  describe the backend. The device defines what the guest sees and the
>>> -backend describes how QEMU handles the data.
>>> +backend describes how QEMU handles the data. The ``-drive`` option
>>> +combines the device and backend into a single command line options
>>> +which is useful in the majority of cases.
>>
>> -drive may look simpler from afar, but it really is a hot mess.  Sadly,
>> we can't get rid of it until we find a replacement for configuring
>> onboard block devices.  We might be able to clean it up some if we
>> accept compatibility breaks.  A new convenience option would be less
>> confusing, I guess.
>
> This is only a partial revert of the original wording which others have
> pointed out was a little too prescriptive. I believe the case of
> snapshot was one where a pure device/blockdev command line is hard to
> use. 
>
>>>                                            Older options like ``-hda``
>>> +bake in a lot of assumptions from the days when QEMU was emulating a
>>> +legacy PC, they are not recommended for modern configurations.
>>>  
>>>  ERST
>>
>> These older options and the non-option argument are simple macros for
>> -drive:
>>
>>     IMG-FILE                    -drive index=0,file=IMG-FILE,media=disk
>>     -hda IMG-FILE               -drive index=0,file=IMG-FILE,media=disk
>>     -hdb IMG-FILE               -drive index=1,file=IMG-FILE,media=disk
>>     -hdc IMG-FILE               -drive index=2,file=IMG-FILE,media=disk
>>     -hdd IMG-FILE               -drive index=3,file=IMG-FILE,media=disk
>>     -cdrom IMG-FILE             -drive index=2,file=IMG-FILE,media=cdrom
>>     -fda IMG-FILE               -drive if=floppy,index=0,file=IMG-FILE
>>     -fdb IMG-FILE               -drive if=floppy,index=1,file=IMG-FILE
>>     -mtdblock IMG-FILE          -drive if=mtd,file=IMG-FILE
>>     -sd IMG-FILE                -drive if=sd,file=IMG-FILE
>>     -pflash IMG-FILE            -drive if=pflash,file=IMG-FILE
>>
>> What assumptions do you have in mind?
>
> I was under the impression things like -hda wouldn't work say on an Arm
> machine because you don't know what sort of interface you might be
> using and -hda implies IDE. Where is this macro substitution done?

qemu_init() calls drive_add() for all these options.

drive_add(TYPE, INDEX, FILE, OPTSTR) creates a QemuOpts in group
"drive".  It sets "if" to if_name[TYPE] unless TYPE is IF_DEFAULT,
"index" to INDEX unless it's negative, and "file" to FILE unless it's
null.  Then it parses OPTSTR on top.

For -hdX, the call looks like

                drive_add(IF_DEFAULT, popt->index - QEMU_OPTION_hda, optarg,
                          HD_OPTS);

We pass IF_DEFAULT, so "if" remains unset.  "index" is set to 0 for
-hda, 1, for -hdb and so forth.  "file" is set to the option argument.
Since HD_OPTS is "media=disk", we set "media" to "disk".

The QemuOpts in config group "drive" get passed to drive_new() via
drive_init_func().  Unset "if" defaults to the current machine's class's
block_default_type.

If a machine doesn't set this member explicitly, it remains zero, which
is IF_NONE.  Documented in blockdev.h:

    typedef enum {
        IF_DEFAULT = -1,            /* for use with drive_add() only */
        /*
         * IF_NONE must be zero, because we want MachineClass member
--->     * block_default_type to default-initialize to IF_NONE
         */
        IF_NONE = 0,
        IF_IDE, IF_SCSI, IF_FLOPPY, IF_PFLASH, IF_MTD, IF_SD, IF_VIRTIO, IF_XEN,
        IF_COUNT
    } BlockInterfaceType;

Questions?

>> I think you need at least Kevin's Acked-by for this.
>
> In the ideal world I could convince the block maintainers to write a new
> section to the manual that explains the theory behind the block
> subsystem and how things interact and are put together. Until then this
> is merely a sticking plaster to make the manual a little more
> authoritative than then numerous example command lines our users blindly
> copy from online blog posts.
>
> Of course we could* always ask our new AI overlords:
>
[...]

> * just because we could doesn't mean we should

Heh!