From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50055) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cY7Jz-0003VB-U0 for qemu-devel@nongnu.org; Mon, 30 Jan 2017 03:28:21 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1cY7Jy-0006YF-CN for qemu-devel@nongnu.org; Mon, 30 Jan 2017 03:28:19 -0500 References: <1485443388-27253-1-git-send-email-armbru@redhat.com> <1485443388-27253-9-git-send-email-armbru@redhat.com> <9684a908-f425-82d8-046b-19cc17d01f55@redhat.com> <87r33o3emm.fsf@dusky.pond.sub.org> <2bcf4c7c-5c29-fda5-1ba3-5828600a8995@redhat.com> <8760l0ebgq.fsf@dusky.pond.sub.org> <87a8a9vuhu.fsf@dusky.pond.sub.org> From: John Snow Message-ID: <36c5c518-0f6e-d9e3-b835-9696078e870e@redhat.com> Date: Mon, 30 Jan 2017 03:28:08 -0500 MIME-Version: 1.0 In-Reply-To: <87a8a9vuhu.fsf@dusky.pond.sub.org> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2 8/8] hw: Drop superfluous special checks for orphaned -drive List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: kwolf@redhat.com, qemu-block@nongnu.org, Mark Cave-Ayland , qemu-devel@nongnu.org, mreitz@redhat.com, =?UTF-8?Q?Herv=c3=a9_Poussineau?= On 01/30/2017 03:10 AM, Markus Armbruster wrote: > John Snow writes: > >> On 01/27/2017 11:04 AM, Markus Armbruster wrote: >>> John Snow writes: >>> >>>> On 01/27/2017 06:51 AM, Markus Armbruster wrote: >>>>> John Snow 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