From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58621) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Vbkq7-0000FF-RD for qemu-devel@nongnu.org; Thu, 31 Oct 2013 01:31:04 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Vbkpr-0006J6-Mn for qemu-devel@nongnu.org; Thu, 31 Oct 2013 01:30:39 -0400 Received: from mx1.redhat.com ([209.132.183.28]:57416) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Vbkpr-0006IM-ET for qemu-devel@nongnu.org; Thu, 31 Oct 2013 01:30:23 -0400 From: Markus Armbruster 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> <20131030230117.GA14011@redhat.com> Date: Thu, 31 Oct 2013 06:30:16 +0100 In-Reply-To: <20131030230117.GA14011@redhat.com> (Michael S. Tsirkin's message of "Thu, 31 Oct 2013 01:01:17 +0200") Message-ID: <87eh720zyv.fsf@blackfin.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain 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: "Michael S. Tsirkin" Cc: Eduardo Habkost , qemu-devel@nongnu.org, aliguori@amazon.com, pbonzini@redhat.com, lersek@redhat.com, afaerber@suse.de "Michael S. Tsirkin" writes: > 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. Yes, it has value, but you're not going to succeed into blackmailing me to do that work now.