Re: [Qemu-devel] [PATCH] machine: Fix replacement of '_' by '-' in machine property names

[Eduardo Habkost]

On Mon, Oct 17, 2016 at 07:10:01PM +0200, Markus Armbruster wrote:
> Eduardo Habkost <address@hidden> writes:
> 
> > On Mon, Oct 17, 2016 at 01:00:29PM +0200, Markus Armbruster wrote:
> >> Eduardo Habkost <address@hidden> writes:
> >> 
> >> > On Thu, Oct 13, 2016 at 06:44:14PM +0200, Markus Armbruster wrote:
> >> >> machine_set_property() replaces '_' by '-' in the property name.
> >> >> Except it fails to replace an initial '_'.  Screwed up in commit
> >> >> b0ddb8b.  Reproducer: "-M pc,__foo_bar=true" produces "Property
> >> >> '._-foo-bar' not found".
> >> >> 
> >> >> Error messages using a mangled name rather than the name the user
> >> >> actually wrote is user-hostile, but that's a different topic.
> >> >> 
> >> >> Signed-off-by: Markus Armbruster <address@hidden>
> >> >
> >> > Reviewed-by: Eduardo Habkost <address@hidden>
> >> >
> >> > I suggest we follow the same approach we used in the x86 CPU
> >> > code: instead of requiring a special parser that magically
> >> > translate strings, just add property aliases for the old names
> >> > that contained "_". It would fix the user-hostile error messages
> >> > as well.
> >> 
> >> Adding the aliases is slightly annoying, but it's probably the easiest
> >> way to get decent error messages.
> >
> > I started working on it, but then I noticed I had do check for
> > "_" in all TYPE_MACHINE subclasses and it would take more time
> > than I expected. Then I put it at the end of my queue and went
> > work on something else.
> >
> >> How can we ensure no new alias-requiring names get added?
> >
> > Being a style issue, maybe checkpatch.pl could detect some cases.
> > It's not different from the other QOM classes.
> 
> Can we adopt the rule for all of QOM, and enforce it right when
> properties get added?  Or at some other suitable time?

You mean, at runtime? We could, but if there are existing
properties that would fail the test we need to fix them first.

-- 
Eduardo