Re: [Qemu-devel] [PATCH] pc: reduce duplication, fix PIIX descriptions

[Eduardo Habkost]

On Tue, Aug 27, 2013 at 07:13:49PM +0300, Michael S. Tsirkin wrote:
> On Tue, Aug 27, 2013 at 01:09:23PM -0300, Eduardo Habkost wrote:
> > On Tue, Aug 27, 2013 at 09:51:22AM +0200, Markus Armbruster wrote:
> > > "Michael S. Tsirkin" <address@hidden> writes:
> > > 
> > > > We have a lot of code duplication between machine types,
> > > > this increases with each new machine type
> > > > and each new field.
> > > >
> > > > This has already introduced a minor bug: description
> > > > for pc-1.3 says "Standard PC" while description for
> > > > pc-1.4 is "Standard PC (i440FX + PIIX, 1996)"
> > > > which makes you think 1.3 is somehow more standard,
> > > > or newer, while in fact it's a revision of the same PC.
> > > 
> > > I wouldn't call it a bug.  We're in the habit of never changing old
> > > machine types, so when we created pc-i440fx-1.4 and pc-q35-1.4 with a
> > > .desc that clearly explains the difference, we didn't change the older
> > > versions the PC machine types as well.
> > > 
> > > That may have been overly cautious.  As long as .desc is not exposed to
> > > the guest, it's not ABI, thus can be changed.
> > 
> > I just confirmed that on qemu-system-x86_64 the only place where .desc
> > is used is on the machine_parse() help/error code at vl.c. I don't know
> > about other architectures.
> 
> Generally, one would hope we won't expose random
> internal stuff to guests :).

Exactly: we won't expose random internal stuff to guests, but there was
nothing about the field that made it clear that it was internal.

In this specific case, somebody could one day suggest adding .desc to
some SMBIOS field, and the suggestion would even make sense to me.

> 
> > I suggest we explicitly document the field as a human-readable
> > machine-type description that should be never exposed to the guest (just
> > in case somebody decides to use .desc on some guest-visible table one
> > day).
> 
> Where would you document this?

Just above the field declaration on struct QEMUMachine at hw/boards.h.

-- 
Eduardo