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

[John Snow]

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.

The new error even suggests information I think is wrong and misleading:
We do support SCSI! (Just not this many of them.)

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

--js