From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44083) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Vbeih-0002tO-7z for qemu-devel@nongnu.org; Wed, 30 Oct 2013 18:58:41 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Vbeib-0003lT-0Y for qemu-devel@nongnu.org; Wed, 30 Oct 2013 18:58:35 -0400 Received: from mx1.redhat.com ([209.132.183.28]:44834) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Vbeia-0003lC-OP for qemu-devel@nongnu.org; Wed, 30 Oct 2013 18:58:28 -0400 Date: Thu, 31 Oct 2013 01:01:17 +0200 From: "Michael S. Tsirkin" Message-ID: <20131030230117.GA14011@redhat.com> References: <1383137800-2990-1-git-send-email-armbru@redhat.com> <1383137800-2990-3-git-send-email-armbru@redhat.com> <20131030141816.GA4853@redhat.com> <20131030142916.GU14916@otherpad.lan.raisama.net> <20131030144823.GA12849@redhat.com> <874n7y3hyq.fsf@blackfin.pond.sub.org> <20131030191741.GB7330@redhat.com> <8761secxv5.fsf@blackfin.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <8761secxv5.fsf@blackfin.pond.sub.org> Subject: Re: [Qemu-devel] [PATCH v3 2/2] smbios: Set system manufacturer, product & version by default List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: Eduardo Habkost , qemu-devel@nongnu.org, aliguori@amazon.com, pbonzini@redhat.com, lersek@redhat.com, afaerber@suse.de On Wed, Oct 30, 2013 at 09:22:38PM +0100, Markus Armbruster wrote: > "Michael S. Tsirkin" writes: > > > On Wed, Oct 30, 2013 at 04:18:37PM +0100, Markus Armbruster wrote: > >> "Michael S. Tsirkin" writes: > >> > >> > On Wed, Oct 30, 2013 at 12:29:16PM -0200, Eduardo Habkost wrote: > >> >> On Wed, Oct 30, 2013 at 04:18:16PM +0200, Michael S. Tsirkin wrote: > >> >> > On Wed, Oct 30, 2013 at 01:56:40PM +0100, armbru@redhat.com wrote: > >> >> > > From: Markus Armbruster > >> >> > > > >> >> > > Currently, we get SeaBIOS defaults: manufacturer Bochs, product Bochs, > >> >> > > no version. Best SeaBIOS can do, but we can provide better defaults: > >> >> > > manufacturer QEMU, product & version taken from QEMUMachine desc and > >> >> > > name. > >> >> > > > >> >> > > Take care to do this only for new machine types, of course. > >> >> > > > >> >> > > Signed-off-by: Markus Armbruster > >> >> > > >> >> > I feel applying this one would be a mistake. > >> >> > > >> >> > Machine desc is for human readers. > >> >> > For example, it currently says "Standard PC (Q35 + ICH9, 2009)" > >> >> > but if we add a variant with IDE compatibility mode we will > >> >> > likely want to > >> >> > tweak it to say "Standard PC (Q35 + ICH9/AHCI mode, 2009)" > >> >> > and add another one saying ""Standard PC (Q35 + ICH9/compat mode, > >> >> > 2009)". > >> >> > > >> >> > In other words we want the ability to tweak > >> >> > description retroactively, and exposing it to guest will > >> >> > break this ability. > >> >> > > >> >> > So we really need a new field not tied to the human description. > >> >> > > >> >> > >> >> You have a point, but if we do that one day, then we can add a new > >> >> smbios-specific field and set it for each of the existing machine-types > >> >> so they keep the same ABI. This patch doesn't make us unable to do that > >> >> in the future. > >> > > >> > We'll likely forget and just break guest ABI. > >> > So we really need a unit test for this, too. > >> > >> More tests are good, but we I think we got bigger fish to fry than > >> writing tests to catch changes that are so obviously foolish as messing > >> with old machine type's QEMUMachine. > > > > You "messed with" old machine type's QEMUMachine in your cleanup > > patches too, didn't you? > > I've occasionally touched QEMUMachine initializers in cleanup series, > but nothing as frivolous as changing strings. And I can't find anything > as frivolous as that in git. We *are* careful and conservative there. > > >> >> As we are past hard freeze, I think this simple patch is better than a > >> >> more complex solution for a problem we still don't have (that can still > >> >> be implemented in 1.8). > >> > > >> > I don't see why we need to rush this into 1.7. > >> > Downstreams want their info in smbios for support, branding etc, > >> > but I don't see a burning need for this in upstream QEMU. > >> > It's kind of nice to have it say "QEMU", but we can afford to > >> > delay and do it properly for 1.8. > >> > >> Define "properly". I don't see what I'd like to do differently for 1.8. > > > > With proper tests going in first before we start changing things. > > Ideally with better separation between user visible and guest visible > > interfaces - though if there was a test to catch guest visible changes, > > I would be less worried about this lack of separation. > > You want me to test for unlikely developer mistakes that are far easier > to catch in review than most other guest ABI changes, and far less > harmful than pretty much any other guest ABI change. This would > multiply the size of this mini-series by a significant factor. I can't > justify this in good conscience to my (and your) employer. So this > isn't going to happen. > > If the maintainers agree with you, then I wasted my time. Sad, but I'd > rather write off the work I've already done than do much more work of no > particular value just to save it. It would be of no particular value *if we only test these strings*. But testing smbios generally has a lot of value IMHO. -- MST