[Qemu-devel] the need for if=none for -drive?

[Qemu-devel] the need for if=none for -drive?

[Michael Tokarev]

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.

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?

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

Re: [Qemu-devel] the need for if=none for -drive?

[John Snow]

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.

> 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 :>

Re: [Qemu-devel] the need for if=none for -drive?

[Markus Armbruster]

Sorry for the slow reply, I missed this one until now.  Adding block
maintainers.

John Snow <jsnow@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?

Re: [Qemu-devel] the need for if=none for -drive?

[Stefan Hajnoczi]

On Wed, Dec 17, 2014 at 11:52 AM, Markus Armbruster <armbru@redhat.com> wrote:
> Sorry for the slow reply, I missed this one until now.  Adding block
> maintainers.
>
> John Snow <jsnow@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