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

Laine Stump posted 10 patches 5 years, 11 months ago
[libvirt PATCH 05/10] conf: parse/format <portOptions isolated='yes|no'/>
Posted by Laine Stump 5 years, 11 months ago
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@redhat.com>
---
 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

diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 29b6b95357..5bb8281a59 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -3159,6 +3159,9 @@
       <optional>
         <ref name="vlan"/>
       </optional>
+      <optional>
+        <ref name="portOptions"/>
+      </optional>
       <optional>
         <element name="teaming">
           <choice>
diff --git a/docs/schemas/network.rng b/docs/schemas/network.rng
index 677ec77724..60453225d6 100644
--- a/docs/schemas/network.rng
+++ b/docs/schemas/network.rng
@@ -332,6 +332,9 @@
         <optional>
           <ref name="vlan"/>
         </optional>
+        <optional>
+          <ref name="portOptions"/>
+        </optional>
 
         <!-- <ip> element -->
         <zeroOrMore>
diff --git a/docs/schemas/networkcommon.rng b/docs/schemas/networkcommon.rng
index fd1aac6485..c0eeb5f2c5 100644
--- a/docs/schemas/networkcommon.rng
+++ b/docs/schemas/networkcommon.rng
@@ -280,4 +280,15 @@
       </attribute>
     </element>
   </define>
+
+  <define name="portOptions">
+    <element name="portOptions">
+      <optional>
+        <attribute name="isolated">
+          <ref name="virYesNo"/>
+        </attribute>
+      </optional>
+    </element>
+  </define>
+
 </grammar>
diff --git a/docs/schemas/networkport.rng b/docs/schemas/networkport.rng
index ea43c03d41..031c5241f0 100644
--- a/docs/schemas/networkport.rng
+++ b/docs/schemas/networkport.rng
@@ -32,6 +32,9 @@
         <optional>
           <ref name="vlan"/>
         </optional>
+        <optional>
+          <ref name="portOptions"/>
+        </optional>
         <optional>
           <ref name="plug"/>
         </optional>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 7a0d126784..176550b62f 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -11480,6 +11480,9 @@ virDomainActualNetDefParseXML(xmlNodePtr node,
     if (vlanNode && virNetDevVlanParse(vlanNode, ctxt, &actual->vlan) < 0)
         goto error;
 
+    if (virNetworkPortOptionsParseXML(ctxt, &actual->isolatedPort) < 0)
+        goto error;
+
     *def = g_steal_pointer(&actual);
     ret = 0;
  error:
@@ -12376,6 +12379,9 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
             goto error;
     }
 
+    if (virNetworkPortOptionsParseXML(ctxt, &def->isolatedPort) < 0)
+        goto error;
+
  cleanup:
     virDomainActualNetDefFree(actual);
     virHashFree(filterparams);
@@ -25453,6 +25459,7 @@ virDomainActualNetDefContentsFormat(virBufferPtr buf,
         return -1;
     if (virNetDevBandwidthFormat(virDomainNetGetActualBandwidth(def), 0, buf) < 0)
         return -1;
+    virNetworkPortOptionsFormat(virDomainNetGetActualPortOptionsIsolated(def), buf);
     return 0;
 }
 
@@ -25829,6 +25836,7 @@ virDomainNetDefFormat(virBufferPtr buf,
             return -1;
         if (virNetDevBandwidthFormat(def->bandwidth, 0, buf) < 0)
             return -1;
+        virNetworkPortOptionsFormat(def->isolatedPort, buf);
 
         /* ONLY for internal status storage - format the ActualNetDef
          * as a subelement of <interface> so that no persistent config
@@ -29906,6 +29914,17 @@ virDomainNetGetActualVlan(const virDomainNetDef *iface)
 }
 
 
+virTristateBool
+virDomainNetGetActualPortOptionsIsolated(const virDomainNetDef *iface)
+{
+    if (iface->type == VIR_DOMAIN_NET_TYPE_NETWORK &&
+        iface->data.network.actual) {
+        return iface->data.network.actual->isolatedPort;
+    }
+    return iface->isolatedPort;
+}
+
+
 bool
 virDomainNetGetActualTrustGuestRxFilters(const virDomainNetDef *iface)
 {
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 867a9c7661..cdc4d25700 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -928,6 +928,7 @@ struct _virDomainActualNetDef {
     virNetDevBandwidthPtr bandwidth;
     virNetDevVlan vlan;
     int trustGuestRxFilters; /* enum virTristateBool */
+    virTristateBool isolatedPort;
     unsigned int class_id; /* class ID for bandwidth 'floor' */
 };
 
@@ -1032,6 +1033,7 @@ struct _virDomainNetDef {
     virNetDevBandwidthPtr bandwidth;
     virNetDevVlan vlan;
     int trustGuestRxFilters; /* enum virTristateBool */
+    virTristateBool isolatedPort;
     int linkstate;
     unsigned int mtu;
     virNetDevCoalescePtr coalesce;
@@ -3239,6 +3241,8 @@ const virNetDevBandwidth *
 virDomainNetGetActualBandwidth(const virDomainNetDef *iface);
 const virNetDevVlan *virDomainNetGetActualVlan(const virDomainNetDef *iface);
 bool virDomainNetGetActualTrustGuestRxFilters(const virDomainNetDef *iface);
+virTristateBool
+virDomainNetGetActualPortOptionsIsolated(const virDomainNetDef *iface);
 const char *virDomainNetGetModelString(const virDomainNetDef *net);
 int virDomainNetSetModelString(virDomainNetDefPtr et,
                                const char *model);
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 1f14a964a2..e9cc9bb55a 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -1172,6 +1172,26 @@ virNetworkIPDefParseXML(const char *networkName,
 }
 
 
+int
+virNetworkPortOptionsParseXML(xmlXPathContextPtr ctxt,
+                              virTristateBool *isolatedPort)
+{
+    g_autofree char *str = NULL;
+    int tmp = VIR_TRISTATE_BOOL_ABSENT;
+
+    if ((str = virXPathString("string(./portOptions/@isolated)", ctxt))) {
+        if ((tmp = virTristateBoolTypeFromString(str)) <= 0) {
+            virReportError(VIR_ERR_XML_ERROR,
+                           _("unknown port isolated value '%s'"), str);
+            return -1;
+        }
+    }
+
+    *isolatedPort = tmp;
+    return 0;
+}
+
+
 static int
 virNetworkPortGroupParseXML(virPortGroupDefPtr def,
                             xmlNodePtr node,
@@ -1725,6 +1745,9 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt,
     if (vlanNode && virNetDevVlanParse(vlanNode, ctxt, &def->vlan) < 0)
         goto error;
 
+    if (virNetworkPortOptionsParseXML(ctxt, &def->isolatedPort) < 0)
+        goto error;
+
     /* Parse bridge information */
     def->bridge = virXPathString("string(./bridge[1]/@name)", ctxt);
     def->bridgeZone = virXPathString("string(./bridge[1]/@zone)", ctxt);
@@ -2331,6 +2354,14 @@ virNetworkIPDefFormat(virBufferPtr buf,
     return 0;
 }
 
+void
+virNetworkPortOptionsFormat(virTristateBool isolatedPort,
+                            virBufferPtr buf)
+{
+    if (isolatedPort != VIR_TRISTATE_BOOL_ABSENT)
+        virBufferAsprintf(buf, "<portOptions isolated='%s'/>\n",
+                          virTristateBoolTypeToString(isolatedPort));
+}
 
 static int
 virPortGroupDefFormat(virBufferPtr buf,
@@ -2608,6 +2639,7 @@ virNetworkDefFormatBuf(virBufferPtr buf,
         return -1;
     if (virNetDevBandwidthFormat(def->bandwidth, 0, buf) < 0)
         return -1;
+    virNetworkPortOptionsFormat(def->isolatedPort, buf);
 
     for (i = 0; i < def->nips; i++) {
         if (virNetworkIPDefFormat(buf, &def->ips[i]) < 0)
diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
index d5dd8480db..db7243eef5 100644
--- a/src/conf/network_conf.h
+++ b/src/conf/network_conf.h
@@ -272,6 +272,7 @@ struct _virNetworkDef {
     virNetDevBandwidthPtr bandwidth;
     virNetDevVlan vlan;
     int trustGuestRxFilters; /* enum virTristateBool */
+    virTristateBool isolatedPort;
 
     /* Application-specific custom metadata */
     xmlNodePtr metadata;
@@ -377,6 +378,14 @@ virNetworkConfigFile(const char *dir,
 void
 virNetworkSetBridgeMacAddr(virNetworkDefPtr def);
 
+int
+virNetworkPortOptionsParseXML(xmlXPathContextPtr ctxt,
+                              virTristateBool *isolatedPort);
+
+void
+virNetworkPortOptionsFormat(virTristateBool isolatedPort,
+                            virBufferPtr buf);
+
 VIR_ENUM_DECL(virNetworkForward);
 
 #define VIR_CONNECT_LIST_NETWORKS_FILTERS_ACTIVE \
diff --git a/src/conf/virnetworkportdef.c b/src/conf/virnetworkportdef.c
index 28a58ad8f8..a0705a8322 100644
--- a/src/conf/virnetworkportdef.c
+++ b/src/conf/virnetworkportdef.c
@@ -161,6 +161,8 @@ virNetworkPortDefParseXML(xmlXPathContextPtr ctxt)
     if (vlanNode && virNetDevVlanParse(vlanNode, ctxt, &def->vlan) < 0)
         return NULL;
 
+    if (virNetworkPortOptionsParseXML(ctxt, &def->isolatedPort) < 0)
+        return NULL;
 
     trustGuestRxFilters
         = virXPathString("string(./rxfilters/@trustGuest)", ctxt);
@@ -360,6 +362,7 @@ virNetworkPortDefFormatBuf(virBufferPtr buf,
         virNetDevBandwidthFormat(def->bandwidth, def->class_id, buf);
     if (virNetDevVlanFormat(&def->vlan, buf) < 0)
         return -1;
+    virNetworkPortOptionsFormat(def->isolatedPort, buf);
     if (def->trustGuestRxFilters)
         virBufferAsprintf(buf, "<rxfilters trustGuest='%s'/>\n",
                           virTristateBoolTypeToString(def->trustGuestRxFilters));
diff --git a/src/conf/virnetworkportdef.h b/src/conf/virnetworkportdef.h
index f5ba337fc9..78cf2c1ba4 100644
--- a/src/conf/virnetworkportdef.h
+++ b/src/conf/virnetworkportdef.h
@@ -60,6 +60,7 @@ struct _virNetworkPortDef {
     unsigned int class_id; /* class ID for bandwidth 'floor' */
     virNetDevVlan vlan;
     int trustGuestRxFilters; /* enum virTristateBool */
+    virTristateBool isolatedPort;
 
     int plugtype; /* virNetworkPortPlugType */
     union {
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 5d043041e0..8f3312d0df 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -513,6 +513,7 @@ virDomainNetGetActualBridgeName;
 virDomainNetGetActualDirectDev;
 virDomainNetGetActualDirectMode;
 virDomainNetGetActualHostdev;
+virDomainNetGetActualPortOptionsIsolated;
 virDomainNetGetActualTrustGuestRxFilters;
 virDomainNetGetActualType;
 virDomainNetGetActualVirtPortProfile;
diff --git a/tests/networkxml2xmlin/isolated-ports.xml b/tests/networkxml2xmlin/isolated-ports.xml
new file mode 100644
index 0000000000..be21f155b2
--- /dev/null
+++ b/tests/networkxml2xmlin/isolated-ports.xml
@@ -0,0 +1,7 @@
+<network>
+  <name>port-isolation-test</name>
+  <uuid>81ff0d90-c91e-6742-64da-4a736edb9a9b</uuid>
+  <bridge name="br0"/>
+  <forward mode="bridge"/>
+  <portOptions isolated="yes"/>
+</network>
diff --git a/tests/networkxml2xmlout/isolated-ports.xml b/tests/networkxml2xmlout/isolated-ports.xml
new file mode 100644
index 0000000000..eed4461574
--- /dev/null
+++ b/tests/networkxml2xmlout/isolated-ports.xml
@@ -0,0 +1,7 @@
+<network>
+  <name>port-isolation-test</name>
+  <uuid>81ff0d90-c91e-6742-64da-4a736edb9a9b</uuid>
+  <forward mode='bridge'/>
+  <bridge name='br0'/>
+  <portOptions isolated='yes'/>
+</network>
diff --git a/tests/networkxml2xmltest.c b/tests/networkxml2xmltest.c
index f784b90c69..ec679e72ee 100644
--- a/tests/networkxml2xmltest.c
+++ b/tests/networkxml2xmltest.c
@@ -160,6 +160,7 @@ mymain(void)
     DO_TEST("metadata");
     DO_TEST("set-mtu");
     DO_TEST("dnsmasq-options");
+    DO_TEST("isolated-ports");
 
     return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
 }
diff --git a/tests/qemuxml2argvdata/net-isolated-port.xml b/tests/qemuxml2argvdata/net-isolated-port.xml
new file mode 100644
index 0000000000..1d53c0cd6b
--- /dev/null
+++ b/tests/qemuxml2argvdata/net-isolated-port.xml
@@ -0,0 +1,34 @@
+<domain type='qemu'>
+  <name>q35-test</name>
+  <uuid>11dbdcdd-4c3b-482b-8903-9bdb8c0a2774</uuid>
+  <memory unit='KiB'>2097152</memory>
+  <currentMemory unit='KiB'>2097152</currentMemory>
+  <vcpu placement='static' cpuset='0-1'>2</vcpu>
+  <os>
+    <type arch='x86_64' machine='q35'>hvm</type>
+    <boot dev='hd'/>
+  </os>
+  <clock offset='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>destroy</on_crash>
+  <devices>
+    <emulator>/usr/bin/qemu-system-x86_64</emulator>
+    <disk type='block' device='disk'>
+      <source dev='/dev/HostVG/QEMUGuest1'/>
+      <target dev='sda' bus='sata'/>
+      <address type='drive' controller='0' bus='0' target='0' unit='0'/>
+    </disk>
+    <controller type='pci' index='0' model='pcie-root'/>
+    <interface type='network'>
+      <mac address='52:54:00:d6:c0:0b'/>
+      <source network='default'/>
+      <portOptions isolated='yes'/>
+      <model type='virtio'/>
+    </interface>
+    <video>
+      <model type='qxl' ram='65536' vram='32768' vgamem='8192' heads='1'/>
+    </video>
+    <memballoon model='none'/>
+  </devices>
+</domain>
diff --git a/tests/qemuxml2xmloutdata/net-isolated-port.x86_64-latest.xml b/tests/qemuxml2xmloutdata/net-isolated-port.x86_64-latest.xml
new file mode 100644
index 0000000000..0605d36a87
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/net-isolated-port.x86_64-latest.xml
@@ -0,0 +1,63 @@
+<domain type='qemu'>
+  <name>q35-test</name>
+  <uuid>11dbdcdd-4c3b-482b-8903-9bdb8c0a2774</uuid>
+  <memory unit='KiB'>2097152</memory>
+  <currentMemory unit='KiB'>2097152</currentMemory>
+  <vcpu placement='static' cpuset='0-1'>2</vcpu>
+  <os>
+    <type arch='x86_64' machine='q35'>hvm</type>
+    <boot dev='hd'/>
+  </os>
+  <cpu mode='custom' match='exact' check='none'>
+    <model fallback='forbid'>qemu64</model>
+  </cpu>
+  <clock offset='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>destroy</on_crash>
+  <devices>
+    <emulator>/usr/bin/qemu-system-x86_64</emulator>
+    <disk type='block' device='disk'>
+      <driver name='qemu' type='raw'/>
+      <source dev='/dev/HostVG/QEMUGuest1'/>
+      <target dev='sda' bus='sata'/>
+      <address type='drive' controller='0' bus='0' target='0' unit='0'/>
+    </disk>
+    <controller type='pci' index='0' model='pcie-root'/>
+    <controller type='usb' index='0' model='qemu-xhci'>
+      <address type='pci' domain='0x0000' bus='0x02' slot='0x00' function='0x0'/>
+    </controller>
+    <controller type='sata' index='0'>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x1f' function='0x2'/>
+    </controller>
+    <controller type='pci' index='1' model='pcie-root-port'>
+      <model name='pcie-root-port'/>
+      <target chassis='1' port='0x10'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0' multifunction='on'/>
+    </controller>
+    <controller type='pci' index='2' model='pcie-root-port'>
+      <model name='pcie-root-port'/>
+      <target chassis='2' port='0x11'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x1'/>
+    </controller>
+    <controller type='pci' index='3' model='pcie-root-port'>
+      <model name='pcie-root-port'/>
+      <target chassis='3' port='0x12'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x2'/>
+    </controller>
+    <interface type='network'>
+      <mac address='52:54:00:d6:c0:0b'/>
+      <source network='default'/>
+      <portOptions isolated='yes'/>
+      <model type='virtio'/>
+      <address type='pci' domain='0x0000' bus='0x01' slot='0x00' function='0x0'/>
+    </interface>
+    <input type='mouse' bus='ps2'/>
+    <input type='keyboard' bus='ps2'/>
+    <video>
+      <model type='qxl' ram='65536' vram='32768' vgamem='8192' heads='1' primary='yes'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/>
+    </video>
+    <memballoon model='none'/>
+  </devices>
+</domain>
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 4b1699db7e..073a7e5565 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -462,6 +462,7 @@ mymain(void)
     DO_TEST("net-virtio-teaming-network",
             QEMU_CAPS_VIRTIO_NET_FAILOVER,
             QEMU_CAPS_DEVICE_VFIO_PCI);
+    DO_TEST_CAPS_LATEST("net-isolated-port");
     DO_TEST("net-hostdev", NONE);
     DO_TEST("net-hostdev-bootorder", NONE);
     DO_TEST("net-hostdev-vfio", QEMU_CAPS_DEVICE_VFIO_PCI);
-- 
2.24.1

Re: [libvirt PATCH 05/10] conf: parse/format <portOptions isolated='yes|no'/>
Posted by Ján Tomko 5 years, 11 months ago
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@redhat.com>
>---
> 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.

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?

Code-wise:
Reviewed-by: Ján Tomko <jtomko@redhat.com>

Jano
Re: [libvirt PATCH 05/10] conf: parse/format <portOptions isolated='yes|no'/>
Posted by Laine Stump 5 years, 11 months ago
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@redhat.com>
>> ---
>> 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@redhat.com>
> 
> Jano

Re: [libvirt PATCH 05/10] conf: parse/format <portOptions isolated='yes|no'/>
Posted by Laine Stump 5 years, 11 months ago
On 2/18/20 10:14 PM, Laine Stump wrote:
> 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@redhat.com>
>>> ---
>>> 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 :-)
>

Okay, I've come up with nothing better, so unless anyone else thinks of 
something else that's better between now and when I wake up tomorrow, 
I'm going to push it using "<port isolated='blah'/>" as Jan suggests 
rather than "<portOptions isolated='yes'/>".


Last chance to object! (unless you feel like reverting patches between 
now and the end of the month)


>>
>> Code-wise:
>> Reviewed-by: Ján Tomko <jtomko@redhat.com>
>>
>> Jano
>