On Wed, Dec 17, 2014 at 11:52 AM, Markus Armbruster <arm...@redhat.com> wrote:
> Sorry for the slow reply, I missed this one until now. Adding block
> maintainers.
>
> John Snow <js...@redhat.com> writes:
>
>> On 11/02/2014 02:21 AM, Michael Tokarev wrote:
>>> All "modern" 2-way drive/device specifications need to explicitly
>>> specify if=none for the drive for it to not be used in the default
>>> ide bus implicitly.
>>
>> Which behave like this? I am reasonably sheltered in the IDE and S/ATA
>> lands.
>
> -drive's parameter "if" defaults to the board's preferred interface.
> if=none was added to shoehorn block devices into the -device world. As
> Michael wrote, you always have to say if=none,id=FOO,... to create
> something usable with -device.
>
>>> But how about using some if=unspecified implicitly for all devices
>>> which don't specify if=, pick any devices from that list which are
>>> referenced from -device, and only add those which left (plus the
>>> ones with explicit if=ide) to the default ide bus?
>>
>> I wouldn't really be opposed to this, since making things more
>> explicit and less mysterious with the drive sugar sounds welcome to
>> me. I imagine you're trying to avoid the case where if= is omitted and
>> QEMU gets to decide what it wants to do in this (highly ambiguous)
>> case?
>>
>>> The change should be strighforward, the only (maybe significant)
>>> prob I see is a need to reorder -device/-drive initialization in
>>> vl.c. We might pick them up in drive_check_orphaned() too. Or,
>>> we can walk over -devices when adding -drives with if={unspec,explicit}.
>>>
>>> (this is a sort of a followup to 6b9e03a4e759876, "qtest/bios-tables:
>>> Correct Q35 command line", but ofcourse it's a topic by its own).
>>>
>>> Thanks,
>>>
>>> /mjt
>>>
>>
>> It may be a little too late, since it seemed as if defaulting to
>> "IF_DEFAULT" always winds up getting mapped to whatever is provided by
>> the second argument of drive_new (block_default_type) which usually
>> winds up being machine_class->block_default_type -- which for Q35 and
>> piix both is IDE. (Implicitly, because it never sets it. interface 0
>> is IF_IDE -- which makes the "true default" for if ... IDE, until it
>> is overridden.)
>>
>> Changing this behavior might break more than we fix, perhaps.
>>
>> I'll leave the masterminding of fixing up the grossly broken drive
>> sugar up to Markus, the secret architect of the recent Q35 sugar
>> patches :>
>
> IF_DEFAULT is currently used only for the non-option image argument,
> -hd[a-d] and -cdrom. It tells the desugaring function drive_add() not
> add an "if" parameter.
>
> Parameter "if" is processed by drive_new(). It defaults to argument
> block_default_type. For -drive, it's the current machine's
> block_default_type. Some machines set IF_SCSI, and two set IF_VIRTIO,
> the rest get IF_IDE via zero initialization.
>
> Board initialization code iterates over drives they know, and create
> appropriate device models. Boards never pick up if=none drives.
>
> Anything not picked up by board initialization can be used by -device.
> Ideally, these are just the if=none drives, but confusing crap like
> "-drive id=foo,if=mtd,file=tmp.qcow2 -device usb-storage,drive=foo" also
> works.
>
> Anything not used by -device either triggers an "orphaned drive"
> warning, and stays around for possible use by device_add.
>
> Michael proposes to reshuffle this a bit. Instead of resolving missing
> "if" to the machine's block_default_type in drive_new(), keep it
> undecided a bit longer. Give -device first pick. Anything left over
> defaults to the machine's block_default_type as before.
>
> Two remarks.
>
> First, I'm afraid giving -device first pick isn't quite trivial. The
> current code acting on -device creates and connects a device model.
> Requires the board to be initialized already. Either we delay the board
> iterating over drives until after -device is done. Changes failure
> modes, need to make sure the error reporting doesn't degrade. Or we do
> an early pass to claim the drives for -device, and actually connect them
> later.
>
> Second, while I too find the need for if=none annoying, I'm not sure I
> like the amount of extra magic. Could be too much. Opinions?
I agree with both these points.
Not sure it's worth the effort to implement this correctly, review, and test it.
Stefan
