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.
Actually, we changed .desc for old machine types before. See commit a0dba644c139907ccf6735c505fbd254010d6938. -- Eduardo
