Re: [Qemu-block] [Qemu-devel] [PATCH v2 8/8] hw: Drop superfluous special checks for orphaned -drive

[John Snow]

On 01/27/2017 11:04 AM, Markus Armbruster wrote:
> John Snow <address@hidden> writes:
> 
>> On 01/27/2017 06:51 AM, Markus Armbruster wrote:
>>> John Snow <address@hidden> writes:
>>>
>>>> On 01/26/2017 10:09 AM, Markus Armbruster wrote:
>>>>> We've traditionally rejected orphans here and there, but not
>>>>> systematically.  For instance, the sun4m machines have an onboard SCSI
>>>>> HBA (bus=0), and have always rejected bus>0.  Other machines with an
>>>>> onboard SCSI HBA don't.
>>>>>
>>>>> Commit a66c9dc made all orphans trigger a warning, and the previous
>>>>> commit turned this into an error.  The checks "here and there" are now
>>>>> redundant.  Drop them.
>>>>>
>>>>> Note that the one in mips_jazz.c was wrong: it rejected bus > MAX_FD,
>>>>> but MAX_FD is the number of floppy drives per bus.
>>>>>
>>>>> Error messages change from
>>>>>
>>>>>     $ qemu-system-x86_64 -drive if=ide,bus=2
>>>>>     qemu-system-x86_64: Too many IDE buses defined (3 > 2)
>>>>>     $ qemu-system-mips64 -M magnum,accel=qtest -drive 
>>>>> if=floppy,bus=2,id=fd1
>>>>>     qemu: too many floppy drives
>>>>>     $ qemu-system-sparc -M LX -drive if=scsi,bus=1
>>>>>     qemu: too many SCSI bus
>>>>>
>>>>> to
>>>>>
>>>>>     $ qemu-system-x86_64 -drive if=ide,bus=2
>>>>>     qemu-system-x86_64: -drive if=ide,bus=2: machine type does not 
>>>>> support this drive
>>>>>     $ qemu-system-mips64 -M magnum,accel=qtest -drive 
>>>>> if=floppy,bus=2,id=fd1
>>>>>     qemu-system-mips64: -drive if=floppy,bus=2,id=fd1: machine type does 
>>>>> not support this drive
>>>>>     $ qemu-system-sparc -M LX -drive if=scsi,bus=1
>>>>>     qemu-system-sparc: -drive if=scsi,bus=1: machine type does not 
>>>>> support this drive
>>>>>
>>>>
>>>> Hm, that's a lot less helpful, isn't it? Can we augment with hints?
>>>
>>> The message itself may be less specific, but it now comes with a precise
>>> location.  Personally, I'd even find
>>>
>>>     qemu-system-sparc: -drive if=scsi,bus=1: *mumble* *mumble*
>>>
>>> more helpful than
>>>
>>>     qemu: too many SCSI bus
>>>
>>> because the former tells me *which* of the options is bad.  We tend to
>>> have lots and lots of them.
>>>
>>> The deleted special case errors cover only a minority of "orphan"
>>> -drive.  If these cases need improvement, then so will the general case.
>>> If you can come up with a hint that makes the general case message more
>>> useful, I'm more than happy to squash it into PATCH 6.
>>>
>>
>> The old error had "why" and the new error has "where" but neither has
>> both. I would suggest that from the "why" you can divine the "where,"
>> but the opposite is not as easily true.
> 
> Some users will be able to divine more easily than others.  Consider my
> "too many floppy drives" example.  There's just one, and the machine
> actually supports two.  The user has to make the connection to "bus=2"
> somehow.  Now, anybody crazy enough to mess with bus= can probably be
> expected to figure this out, but still, the deleted error messages
> aren't exactly wonderful.
> 
>> The new error even suggests information I think is wrong and misleading:
>> We do support SCSI! (Just not this many of them.)
> 
> Well, the error doesn't say "machine doesn't support SCSI", only
> "doesn't support this particular -drive".  Perhaps it could be worded
> more clearly.  Ideas?
> 

Ah, I see what you mean now. I interpreted "this drive" to mean SCSI,
not this SCSI *instance*. If it can be made clearer that QEMU is simply
unable to instantiate this particular instance, that'd be fine.

Instead of "Machine type does not support this drive,"

how about

"Machine type cannot instantiate this drive instance"

Or ... follow your own best judgement. This is really YOUR wheelhouse.
My example is a little wordy.

>> No suggestions for how or where to append the hints. This is not the
>> hill I am preparing to die on.
> 
> Plenty of hills around...
> 

Some of them have flowers.