[Qemu-devel] IDs in QOM (was: [PATCH] util: Emancipate id_wellformed() from QemuOpts)

[Markus Armbruster]

Markus Armbruster <address@hidden> writes:

> IDs have long spread beyond QemuOpts: not everything with an ID
> necessarily goes through QemuOpts.  Commit 9aebf3b is about such a
> case: block layer names are meant to be well-formed IDs, but some of
> them don't go through QemuOpts, and thus weren't checked.  The commit
> fixed that the straightforward way: rename the internal QemuOpts
> helper id_wellformed() to qemu_opts_id_wellformed() and give it
> external linkage.
>
> Instead of using it directly in block.c, the commit adds wrapper
> bdrv_is_valid_name(), probably to hide the connection to QemuOpts.
>
> Go one logical step further: emancipate IDs from QemuOpts.  Rename the
> function back to id_wellformed(), and put it in another file.  While
> there, clean up its value to bool.  Peel off the bdrv_is_valid_name()
> wrapper.

Thinking about IDs has led me to QOM and down a rabbit hole.  Here goes.

QemuOpts IDs need to be well-formed: consist of letters, digits, '-',
'.', '_', starting with a letter.  See id_wellformed(), commit b560a9a
"qemu-option: Reject anti-social IDs".  This restriction has served us
well.

The commit message above is a case where we started with QemuOpts, then
added QMP interfaces bypassing QemuOpts, which also bypassed its ID
well-formedness rule.  The bypass is a bug, and we fixed it.  The patch
cleans up the fix.  There might be more of the same.

QOM is the other way round: we started with a C API, and later added
external interfaces, namely -object and object-add.

QMP object-add has no need for QemuOpts.  See qmp_object_add(), commit
cff8b2c "monitor: add object-add (QMP) and object_add (HMP) command".
Parameter 'id' is an arbitrary string.  qmp_object_add() passes it to
object_add(), which runs

    object_property_add_child(container_get(object_get_root(), "/objects"),
                              id, obj, &local_err);

On an abstract level, this puts the newly created object in container
/objects/ with name id.  Down & concrete, the container object has a
property named name pointing the the new object.  Property names are
arbitrary strings; object_property_add() doesn't care.

However, we also have the concept of a path, which are sequences of
names separated by '/'.  See object_resolve_path_type(),
container_get().

What if a name contains '/'?  Or other funny characters?

Actually, "property names are arbitrary strings" has recently become
untrue: commit 339659 "qom: Add automatic arrayification to
object_property_add()" changed names to have embedded syntax.  A
property name ending with "[*]" makes the property special.  Normal
properties can be added just once, and an attempt to add it again is an
error.  These special properties, however, can be added any number of
times, and each time the name "[*]" is silently rewritten to "[N]",
where N counts from 0.

Is mangling array-ness into the name really a good idea?  Isn't this
type matter, not name matter?

Backtracking a bit...  Unlike QMP object-add, -object ) and HMP
object-add use QemuOpts.  See object_create(), commit 68d98d3 "vl: add
-object option to create QOM objects from the command line", and
hmp_object_add(), commit cff8b2c "monitor: add object-add (QMP) and
object_add (HMP) command".  Parameter 'id' is the QemuOpts ID, thus
bound by its well-formedness rule.

Therefore, -object and HMP object-add only support a subset of the
possible names.

In particular, they do not permit "automatic arrayification".

Should QOM names be (well-formed!) IDs?

If not, is -object and HMP object-add restricting the names to
well-formed IDs a bug?