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

[Markus Armbruster]

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?

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