Re: [Qemu-devel] '.' IDs and certain names breaks -global and -set

[Andreas Färber]

Am 14.04.2014 09:13, schrieb Markus Armbruster:
> Alistair Francis <address@hidden> writes:
> 
>> On Wed, Apr 9, 2014 at 9:58 PM, Peter Crosthwaite
>> <address@hidden> wrote:
>>> On Wed, Apr 9, 2014 at 7:56 PM, Markus Armbruster <address@hidden> wrote:
>>>> We have a number of device model names containing '.'.  They're unusable
>>>> with -global.  That's because "-global A.B.C=foo" gets parsed as
>>>>
>>>>     driver = "A"
>>>>     property = "B.C"
>>>>     value = "foo".
>>>>
>>>> Wrong when the device model name is really A.B and the property is C,
>>>> e.g. "-global cfi.pflash01.name".
>>>>
>>>> Related problem: QemuOpts ID may contain '.'.  Such IDs are in fact
>>>> common practice.  Unfortunately, they're unusable with -set.  For
>>>> instance, -set A.B.C.D=foo gets parsed as
>>>>
>>>>     group = "A"
>>>>     id = "B"
>>>>     arg = "C.D"
>>>>     value = "foo"
>>>>
>>>> Wrong when the id is really B.C and the arg is D, e.g. "-device
>>>> isa-serial,id=s.0 -set device.s.0.chardev=c0".  This issue isn't limited
>>>> to devices.
>>>>
>>>> Related historical problem: commit b560a9a.
>>>>
>>>> Possible solutions:
>>>>
>>>> * Outlaw '.' in QemuOpts parameter names (and thus device model names
>>>>   and property names, because QemuOpts is a major way for users to
>>>>   specify them).
>>>>
>>>
>>> I like the "."s because they mean "this is where the vendor stops and
>>> the IP name starts". Whereas "-" just delimits multiple words. We seem
>>> to be running our of characters however with these overloading
>>> problems. But AFAICT ":" is largely unused (within this context
>>> anyway). So how about:
>>>
>>> "-" seperates multiple words in a name
>>> ":" splits logical groupings of multiple words as appropriate ( e.g.
>>> some-vendor:their-device )
>>> "," demilits multiple command line arguments
>>> "." accesses property within an object
>>> "/" walks the canonical path
>>>
>>> I do also need to confess to my alternate agenda of device tree driven
>>> QEMU - I am still parsing device trees and directly mapping the
>>> compatible strings to QOM type names for machine creation. Ambiguity
>>> of "-" as meaning the "," or the "-" is hard to deal with.
>>>
>>> Regards,
>>> Peter
>>>
>>>> * Resolve the syntactic ambiguity semantically (ugh)
>>>>
>>>> * Extend the syntax of -global and -set to let users avoid the issue, or
>>>>   replace them outright
>>>>
>>>> * Use the old ostrich algorithm
>>>>
>>>> Andreas, what restrictions does QOM place on QOM path component names?
>>>> No '/', obviously.  Anything else?
>>>>
>>>
>>
>> What is the consensus with this? I like Peter's naming suggestions as
>> they are straight
>> forward and don't break any compatibility.
> 
> I'd like to hear a few more opinions, Andreas's in particular.  Give him
> a bit more time to chime in.

Thanks, was absent on Wednesday.

It is true that for as long as -global exists, dots in device IDs have
been problematic, and the pragmatic response of don't-do-that is not
very defensive. Nor is QOM's use of '/' path delimiter.

But I don't see how Peter's summary helps. Using ':' in IDs should work
today already, so that's no change and doesn't solve problems with '.',
',' or '/'.

More generally there are three ways bad strings can get in:

a) Code.
b) Command line.
c) HMP/QMP.

a) is easiest to tackle, by adding an assert to the relevant QOM APIs
and assuring via qtests that those code paths are tested.

b) hints at a more general problem of QemuOpts being very optimistic
about inputs and leaving escaping to users rather than to central
infrastructure IIUC. This involves whether ',' can be used in id=.
Additionally, -device code would need to check an id for validity,
-object possibly the path, etc.

c) would not want object-add etc. to abort on bad input strings, so
individual error handling would be needed.

Of course, -global could also check for additional dots after the
initial one, if no type matches the string up to the first.

In any case it'll be too late to fix this for 2.0.

Regards,
Andreas

P.S. Wasn't aware of -set, thanks for the pointer. ;)

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg