Am 21.01.2015 um 10:34 hat Markus Armbruster geschrieben:
I'm afraid I forgot much of the discussion we had before the break, and
only now it's coming back, slowly.
Quoting myself on naming parameters identifying nodes[*]:
John Snow pointed out to me that we still haven't spelled out how this
single parameter should be named.
On obvious option is calling it node-name, or FOO-node-name when we have
several. However, we'd then have two subtly different kinds of
parameters called like that: the old ones accept *only* node names, the
new ones also accept backend names, which automatically resolve to the
backend's root node.
Three ways to cope with that:
* Find a better name.
* Make the old ones accept backend names, too. Only a few, not that
much work. However, there are exceptions:
- blockdev-add's node-name *defines* the node name.
- query-named-block-nodes's node-name *is* the node's name.
* Stop worrying and embrace the inconsistency. The affected commands
are headed for deprecation anyway.
I think I'd go with "node" or "FOO-node" for parameters that reference
nodes and accept both node names and backend names, and refrain from
touching the existing node-name parameters.
Wasn't the conclusion last time that we would try to find a better name
for new commands and leave old commands alone because they are going to
become deprecated? That is, a combination of your first and last option?
3. Node reference with backend names permitted for convenience
New QMP command block-dirty-bitmap-add (type BlockDirtyBitmapAdd) and
others
4. Node reference with backend names not permitted
QMP commands drive-mirror @replaces, change-backing-file
@image-node-name
We may want to support the "backend name resolves to root node"
convenience feature here, for consistency. Then this moves under 3.
Note interface wart: change-backing-file additionally requires the
backend owning the node. We need the backend to set op-blockers, we
can't easily find it from the node, so we make the user point it out
to us.
These shouldn't be existing. As you say, we should move them to 3.
5. "Pair of names" node reference, specify exactly one
QMP commands block_passwd, block_resize, blockdev-snapshot-sync
We can ignore this one, because we intend to replace the commands and
deprecate the old ones.
Agreed, these shouldn't be existing either.
If I understand you correctly, you're proposing to use @node-name or
@FOO-node-name when the value must be a node name (items 1+2 and 4), and
@node-ref or @FOO-node-ref where we additionally support the "backend
name resolves to root node" convenience feature (item 3).
Is that a fair description of your proposal?
PRO: the name makes it clear when the convenience feature is supported.
CON: if we eliminate 4 by supporting the convenience feature, we either
create ugly exceptions to the naming convention, or rename the
parameters.
Opinions?
If we don't have any cases where node names are allowed, but backend
names are not, then there is no reason to have two different names. I've
yet to see a reason for having commands that can accept node names, but
not backend names.
It's a bit different when the command can already accept both, but uses
two separate arguments for it. But I think most of them will be
deprecated, so we can ignore them here.
As for the naming, I'm not that sure that it's even useful to add
something to the field name. After all, this is really the _type_ of the
object, not the name. We don't have fields like 'read-only-bool' either.
If we're more specifically looking at things that actually refer to
block devices, you already mentioned drive-mirrors @replaces, which is a
great name in my opinion. @replaces-node-ref wouldn't improve anything.
Likewise, blockdev-add already refers to 'file' and 'backing' instead of
'file-node' or 'backing-node-ref'.
This probably means that FOO-node-{ref,name} shouldn't exist, because
just FOO is as good or better. The question is a bit harder where there
is only one node involved and we don't have a nice word to describe its
role for the command. This is where we used to use 'device' in the past,
when node-level addressing didn't exist yet. I think just 'node' would
be fine there.
Kevin