On 01/30/2017 03:10 AM, Markus Armbruster wrote: > John Snow <js...@redhat.com> writes: > >> On 01/27/2017 11:04 AM, Markus Armbruster wrote: >>> John Snow <js...@redhat.com> writes: >>> >>>> On 01/27/2017 06:51 AM, Markus Armbruster wrote: >>>>> John Snow <js...@redhat.com> 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" > > Hmm. > >> 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... >
Yes, so just follow your own instincts. Just offering a non-blocking comment. --js