[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[libvirt] [PATCHv2 0/2] two different possibilities for virsh net-update command



The two following patches are alternate approaches that each have a
slightly different commandline syntax.

In V1 of the net-update patch, I tried having a single string option
that could contain either an xml string, or the name of a file
containing an xml string, then two options that would be used to make the decision of file vs. xml:

   net-update netname add ip-dhcp-host --xml "<host mac='xx..' .../>"
   net-update netname add ip-dhcp-host /tmp/myfile.xml
   (or 'net-update netname add ip-dhcp-host --file /tmp/myfile.xml')

(the trick here is that "--xml" is a boolean option, *not* a string
option that gets set to "<host...", and the string itself goes into a
generic, usually unnamed option called xmldata. Likewise, the filename
is being put into xmldata, and if the "--xml" boolean option isn't
given on the commandline, it's assumed that it is a file. The ability
to specifically say it is a file was just added for completeness, but
really is semantically useless.)

The problem with this is that someone looking at the syntax in the
help might come up with a confusing commandline like this:

    net-update netname add --xmldata /tmp/myfile.xml --file \
      --section ip-dhcp-host --live

or

    net-update netname add --xmldata "<host mac='x:x...'/>" --xml \
      --section ip-dhcp-host --live

or even worse, they would specify "--xmldata", but leave out --xml,
and virsh would attempt to interpret the text as a filename.

On the other hand, if I named the string option "file" and the boolean
option --xmldata, that would make things confusing even in the case
that the command was *correct*:

    net-update netname add --file "<host mac='x:x...'/>" --xmldata \
      --section ip-dhcp-host --live

Of course, in any of those cases, if the user followed the "normal"
command specification, it would look just fine, but it seems like
there is a lot of room to cause confusion.

So, for V2, I'm trying two different approaches:

PATCHv2 1/2: Have a single required "xml" arg, and "auto-detect"
whether it is xml text or a filename by looking at the first character
- if the first character is "<", interpret it as an xml element. If
not, interpret it as a filename. This leads to the simplest
commandlines, since you *never* need to specify "--file" or "--xml"
(unless you really want to):

   net-update netname add ip-dhcp-host "<host mac='xx..' .../>"
   net-update netname add ip-dhcp-host /tmp/myfile.xml

(The ambiguous case of a filename starting with "<" can be avoided by
specifying the filename as "./<....")

There may be some circumstance I'm not aware of where a fully formed XML element doesn't necessarily start with a "<" though, or someone may be 100% against auto-disambiguating the string type, so I'm also sending a different version:

PATCHv2 2/2: Have two separate optional string args, "file" and
"xml". Manually check in the code that only one of them has been
specified. Since both are optional, you must *always* provide the
option name on the commandline ("--file" or "--xml"). In this case, the commandlines look like this:

   net-update netname add ip-dhcp-host --xml "<host mac='xx..' .../>"
   net-update netname add ip-dhcp-host --file /tmp/myfile.xml

I'm (as usual) completely undecided about which of these versions to
go with (including the first, which should still be considered,
although I don't really like the possibility of creating confusing
commandlines that it creates), so all votes/opinions/etc are welcome!


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]