"Michael S. Tsirkin" <m...@redhat.com> writes: > On Wed, Oct 30, 2013 at 09:22:38PM +0100, Markus Armbruster wrote: >> "Michael S. Tsirkin" <m...@redhat.com> writes: >> >> > On Wed, Oct 30, 2013 at 04:18:37PM +0100, Markus Armbruster wrote: >> >> "Michael S. Tsirkin" <m...@redhat.com> 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, arm...@redhat.com wrote: >> >> >> > > From: Markus Armbruster <arm...@redhat.com> >> >> >> > > >> >> >> > > 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 <arm...@redhat.com> >> >> >> > >> >> >> > 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.