Re: [libvirt PATCH 05/10] conf: parse/format <portOptions isolated='yes|no'/>

On 2/18/20 12:39 PM, Ján Tomko wrote: > On Sun, Feb 16, 2020 at 11:22:54PM -0500, Laine Stump wrote: >> This is a very simple thing to parse and format, but needs to be done >> in 4 places, so two trivial utility functions have been made that can >> be called from all the higher level parser/formatters: >> >>  <domain><interface> >>  <domain><interface><actual> (only in domain status) >>  <network> >>  <networkport> >> >> Signed-off-by: Laine Stump <laine(a)redhat.com&gt; >> --- >> docs/schemas/domaincommon.rng                 |  3 + >> docs/schemas/network.rng                      |  3 + >> docs/schemas/networkcommon.rng                | 11 ++++ >> docs/schemas/networkport.rng                  |  3 + >> src/conf/domain_conf.c                        | 19 ++++++ >> src/conf/domain_conf.h                        |  4 ++ >> src/conf/network_conf.c                       | 32 ++++++++++ >> src/conf/network_conf.h                       |  9 +++ >> src/conf/virnetworkportdef.c                  |  3 + >> src/conf/virnetworkportdef.h                  |  1 + >> src/libvirt_private.syms                      |  1 + >> tests/networkxml2xmlin/isolated-ports.xml     |  7 +++ >> tests/networkxml2xmlout/isolated-ports.xml    |  7 +++ >> tests/networkxml2xmltest.c                    |  1 + >> tests/qemuxml2argvdata/net-isolated-port.xml  | 34 ++++++++++ >> .../net-isolated-port.x86_64-latest.xml       | 63 +++++++++++++++++++ >> tests/qemuxml2xmltest.c                       |  1 + >> 17 files changed, 202 insertions(+) >> create mode 100644 tests/networkxml2xmlin/isolated-ports.xml >> create mode 100644 tests/networkxml2xmlout/isolated-ports.xml >> create mode 100644 tests/qemuxml2argvdata/net-isolated-port.xml >> create mode 100644 >> tests/qemuxml2xmloutdata/net-isolated-port.x86_64-latest.xml >> > > Not a fan of multi-word elements, because they bring up our > inconsistency in using camelCase vs snake_case. Yeah, it always bothers me when I see a multiword element or attribute for that reason. I always use camelCase because I remember asking about which is preferred > 10 years ago and being told that we wanted to have camelCase in libvirt XML. That could even be a false memory, but it has always stuck with me. > > But I assume you chose the name to make it compatible with all four > containing elements. > > Would something like: > <networkport> >    <port isolated='yes'/> > </networkport> > look too odd? ummmm.... God I *HATE* coming up with element and attribute names! (That's the only response I can think of right now since it's late in the day. Let me sleep on it, but in the end I was expecting, even *hoping* someone would object to portOptions and propose an alternative, and yours doesn't really sound any *worse* than mine, so it might be worthwhile to use it just so I wouldn't have to shoulder the blame :-)

> > Code-wise: > Reviewed-by: Ján Tomko <jtomko(a)redhat.com&gt; > > Jano