[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

From: Markus Armbruster
Subject: Re: [Qemu-devel] [PATCH v2 8/8] hw: Drop superfluous special checks for orphaned -drive
Date: Mon, 30 Jan 2017 09:10:05 +0100
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/25.1 (gnu/linux)

John Snow <address@hidden> writes:

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

Uh, I would've written "does not support if=scsi" then.  But I see where
you come from.

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

All I can come up with is even wordier: "Machine type doesn't support
this combination of if, bus, unit", or "if=scsi,bus=%d,unit=%d not
supported with this machine type".  Could also be confusing when the
user specified index instead of bus, unit.

Good error messages are hard...

reply via email to

[Prev in Thread] Current Thread [Next in Thread]