[libvirt PATCH 07/10] qemu/lxc: plumb isolatedPort from config down through bridge attachment

Laine Stump posted 10 patches 5 years, 11 months ago
[libvirt PATCH 07/10] qemu/lxc: plumb isolatedPort from config down through bridge attachment
Posted by Laine Stump 5 years, 11 months ago
This patch pushes the isolatedPort setting from the <interface> down
all the way to the callers of virNetDevBridgeAddPort(), and sets
BR_ISOLATED on the port (using virNetDevBridgePortSetIsolated()) after
the port has been successfully added to the bridge.

Signed-off-by: Laine Stump <laine@redhat.com>
---
 src/bhyve/bhyve_command.c   |  1 +
 src/conf/domain_conf.c      |  1 +
 src/lxc/lxc_process.c       | 10 ++++++++++
 src/network/bridge_driver.c |  1 +
 src/qemu/qemu_hotplug.c     | 16 ++++++++++++++++
 src/qemu/qemu_interface.c   |  1 +
 src/util/virnetdevtap.c     | 17 ++++++++++++++++-
 src/util/virnetdevtap.h     |  3 +++
 tests/bhyvexml2argvmock.c   |  1 +
 9 files changed, 50 insertions(+), 1 deletion(-)

diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c
index a8bfc0aa72..2df7b60115 100644
--- a/src/bhyve/bhyve_command.c
+++ b/src/bhyve/bhyve_command.c
@@ -95,6 +95,7 @@ bhyveBuildNetArgStr(virConnectPtr conn,
                                            def->uuid, NULL, NULL, 0,
                                            virDomainNetGetActualVirtPortProfile(net),
                                            virDomainNetGetActualVlan(net),
+                                           virDomainNetGetActualPortOptionsIsolated(net),
                                            NULL, 0, NULL,
                                            VIR_NETDEV_TAP_CREATE_IFUP | VIR_NETDEV_TAP_CREATE_PERSIST) < 0) {
             goto cleanup;
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index dd35522370..30b2a53b83 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -31146,6 +31146,7 @@ virDomainNetNotifyActualDevice(virConnectPtr conn,
                                                 &iface->mac, dom->uuid,
                                                 virDomainNetGetActualVirtPortProfile(iface),
                                                 virDomainNetGetActualVlan(iface),
+                                                virDomainNetGetActualPortOptionsIsolated(iface),
                                                 iface->mtu, NULL));
     }
 }
diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
index da6df86834..6851b3e3e2 100644
--- a/src/lxc/lxc_process.c
+++ b/src/lxc/lxc_process.c
@@ -303,6 +303,16 @@ virLXCProcessSetupInterfaceTap(virDomainDefPtr vm,
         } else {
             if (virNetDevBridgeAddPort(brname, parentVeth) < 0)
                 return NULL;
+
+            if (virDomainNetGetActualPortOptionsIsolated(net) == VIR_TRISTATE_BOOL_YES &&
+                virNetDevBridgePortSetIsolated(brname, parentVeth, true) < 0) {
+                virErrorPtr err;
+
+                virErrorPreserveLast(&err);
+                ignore_value(virNetDevBridgeRemovePort(brname, parentVeth));
+                virErrorRestore(&err);
+                return NULL;
+            }
         }
     }
 
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index e26c5a4879..27d9a24de9 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -2489,6 +2489,7 @@ networkStartNetworkVirtual(virNetworkDriverStatePtr driver,
         if (virNetDevTapCreateInBridgePort(def->bridge,
                                            &macTapIfName, &def->mac,
                                            NULL, NULL, &tapfd, 1, NULL, NULL,
+                                           VIR_TRISTATE_BOOL_NO,
                                            NULL, def->mtu, NULL,
                                            VIR_NETDEV_TAP_CREATE_USE_MAC_FOR_BRIDGE |
                                            VIR_NETDEV_TAP_CREATE_IFUP |
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 6395826c69..af892255c7 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -3350,12 +3350,28 @@ qemuDomainChangeNetBridge(virDomainObjPtr vm,
     }
 
     ret = virNetDevBridgeAddPort(newbridge, olddev->ifname);
+    if (ret == 0 &&
+        virDomainNetGetActualPortOptionsIsolated(newdev) == VIR_TRISTATE_BOOL_YES) {
+
+        ret = virNetDevBridgePortSetIsolated(newbridge, olddev->ifname, true);
+        if (ret < 0) {
+            virErrorPtr err;
+
+            virErrorPreserveLast(&err);
+            ignore_value(virNetDevBridgeRemovePort(newbridge, olddev->ifname));
+            virErrorRestore(&err);
+        }
+    }
     virDomainAuditNet(vm, NULL, newdev, "attach", ret == 0);
     if (ret < 0) {
         virErrorPtr err;
 
         virErrorPreserveLast(&err);
         ret = virNetDevBridgeAddPort(oldbridge, olddev->ifname);
+        if (ret == 0 &&
+            virDomainNetGetActualPortOptionsIsolated(olddev) == VIR_TRISTATE_BOOL_YES) {
+            ignore_value(virNetDevBridgePortSetIsolated(newbridge, olddev->ifname, true));
+        }
         virDomainAuditNet(vm, NULL, olddev, "attach", ret == 0);
         virErrorRestore(&err);
         return -1;
diff --git a/src/qemu/qemu_interface.c b/src/qemu/qemu_interface.c
index 74d4782599..8a01eecd83 100644
--- a/src/qemu/qemu_interface.c
+++ b/src/qemu/qemu_interface.c
@@ -568,6 +568,7 @@ qemuInterfaceBridgeConnect(virDomainDefPtr def,
                                            def->uuid, tunpath, tapfd, *tapfdSize,
                                            virDomainNetGetActualVirtPortProfile(net),
                                            virDomainNetGetActualVlan(net),
+                                           virDomainNetGetActualPortOptionsIsolated(net),
                                            net->coalesce, 0, NULL,
                                            tap_create_flags) < 0) {
             virDomainAuditNetDevice(def, net, tunpath, false);
diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c
index 84d91428e7..7bd30ea0f9 100644
--- a/src/util/virnetdevtap.c
+++ b/src/util/virnetdevtap.c
@@ -505,6 +505,7 @@ virNetDevTapAttachBridge(const char *tapname,
                          const unsigned char *vmuuid,
                          const virNetDevVPortProfile *virtPortProfile,
                          const virNetDevVlan *virtVlan,
+                         virTristateBool isolatedPort,
                          unsigned int mtu,
                          unsigned int *actualMTU)
 {
@@ -545,6 +546,16 @@ virNetDevTapAttachBridge(const char *tapname,
     } else {
         if (virNetDevBridgeAddPort(brname, tapname) < 0)
             return -1;
+
+        if (isolatedPort == VIR_TRISTATE_BOOL_YES &&
+            virNetDevBridgePortSetIsolated(brname, tapname, true) < 0) {
+            virErrorPtr err;
+
+            virErrorPreserveLast(&err);
+            ignore_value(virNetDevBridgeRemovePort(brname, tapname));
+            virErrorRestore(&err);
+            return -1;
+        }
     }
 
     return 0;
@@ -574,6 +585,7 @@ virNetDevTapReattachBridge(const char *tapname,
                            const unsigned char *vmuuid,
                            const virNetDevVPortProfile *virtPortProfile,
                            const virNetDevVlan *virtVlan,
+                           virTristateBool isolatedPort,
                            unsigned int mtu,
                            unsigned int *actualMTU)
 {
@@ -611,6 +623,7 @@ virNetDevTapReattachBridge(const char *tapname,
                                  macaddr, vmuuid,
                                  virtPortProfile,
                                  virtVlan,
+                                 isolatedPort,
                                  mtu, actualMTU) < 0)
         return -1;
 
@@ -660,6 +673,7 @@ int virNetDevTapCreateInBridgePort(const char *brname,
                                    size_t tapfdSize,
                                    const virNetDevVPortProfile *virtPortProfile,
                                    const virNetDevVlan *virtVlan,
+                                   virTristateBool isolatedPort,
                                    virNetDevCoalescePtr coalesce,
                                    unsigned int mtu,
                                    unsigned int *actualMTU,
@@ -697,7 +711,8 @@ int virNetDevTapCreateInBridgePort(const char *brname,
         goto error;
 
     if (virNetDevTapAttachBridge(*ifname, brname, macaddr, vmuuid,
-                                 virtPortProfile, virtVlan, mtu, actualMTU) < 0) {
+                                 virtPortProfile, virtVlan,
+                                 isolatedPort, mtu, actualMTU) < 0) {
         goto error;
     }
 
diff --git a/src/util/virnetdevtap.h b/src/util/virnetdevtap.h
index cae8e61861..c6bd9285ba 100644
--- a/src/util/virnetdevtap.h
+++ b/src/util/virnetdevtap.h
@@ -65,6 +65,7 @@ virNetDevTapAttachBridge(const char *tapname,
                          const unsigned char *vmuuid,
                          const virNetDevVPortProfile *virtPortProfile,
                          const virNetDevVlan *virtVlan,
+                         virTristateBool isolatedPort,
                          unsigned int mtu,
                          unsigned int *actualMTU)
     ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3)
@@ -77,6 +78,7 @@ virNetDevTapReattachBridge(const char *tapname,
                            const unsigned char *vmuuid,
                            const virNetDevVPortProfile *virtPortProfile,
                            const virNetDevVlan *virtVlan,
+                           virTristateBool isolatedPort,
                            unsigned int mtu,
                            unsigned int *actualMTU)
     ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3)
@@ -91,6 +93,7 @@ int virNetDevTapCreateInBridgePort(const char *brname,
                                    size_t tapfdSize,
                                    const virNetDevVPortProfile *virtPortProfile,
                                    const virNetDevVlan *virtVlan,
+                                   virTristateBool isolatedPort,
                                    virNetDevCoalescePtr coalesce,
                                    unsigned int mtu,
                                    unsigned int *actualMTU,
diff --git a/tests/bhyvexml2argvmock.c b/tests/bhyvexml2argvmock.c
index 2a552f9f47..25b97f5e04 100644
--- a/tests/bhyvexml2argvmock.c
+++ b/tests/bhyvexml2argvmock.c
@@ -28,6 +28,7 @@ int virNetDevTapCreateInBridgePort(const char *brname G_GNUC_UNUSED,
                                    size_t tapfdSize G_GNUC_UNUSED,
                                    const virNetDevVPortProfile *virtPortProfile G_GNUC_UNUSED,
                                    const virNetDevVlan *virtVlan G_GNUC_UNUSED,
+                                   virTristateBool isolatedPort G_GNUC_UNUSED,
                                    virNetDevCoalescePtr coalesce G_GNUC_UNUSED,
                                    unsigned int mtu G_GNUC_UNUSED,
                                    unsigned int *actualMTU G_GNUC_UNUSED,
-- 
2.24.1

Re: [libvirt PATCH 07/10] qemu/lxc: plumb isolatedPort from config down through bridge attachment
Posted by Ján Tomko 5 years, 11 months ago
On Sun, Feb 16, 2020 at 11:22:56PM -0500, Laine Stump wrote:
>This patch pushes the isolatedPort setting from the <interface> down
>all the way to the callers of virNetDevBridgeAddPort(), and sets
>BR_ISOLATED on the port (using virNetDevBridgePortSetIsolated()) after
>the port has been successfully added to the bridge.
>
>Signed-off-by: Laine Stump <laine@redhat.com>
>---
> src/bhyve/bhyve_command.c   |  1 +
> src/conf/domain_conf.c      |  1 +
> src/lxc/lxc_process.c       | 10 ++++++++++
> src/network/bridge_driver.c |  1 +
> src/qemu/qemu_hotplug.c     | 16 ++++++++++++++++
> src/qemu/qemu_interface.c   |  1 +
> src/util/virnetdevtap.c     | 17 ++++++++++++++++-
> src/util/virnetdevtap.h     |  3 +++
> tests/bhyvexml2argvmock.c   |  1 +
> 9 files changed, 50 insertions(+), 1 deletion(-)
>
>diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
>index 6395826c69..af892255c7 100644
>--- a/src/qemu/qemu_hotplug.c
>+++ b/src/qemu/qemu_hotplug.c
>@@ -3350,12 +3350,28 @@ qemuDomainChangeNetBridge(virDomainObjPtr vm,
>     }
>
>     ret = virNetDevBridgeAddPort(newbridge, olddev->ifname);
>+    if (ret == 0 &&
>+        virDomainNetGetActualPortOptionsIsolated(newdev) == VIR_TRISTATE_BOOL_YES) {
>+
>+        ret = virNetDevBridgePortSetIsolated(newbridge, olddev->ifname, true);
>+        if (ret < 0) {
>+            virErrorPtr err;
>+
>+            virErrorPreserveLast(&err);
>+            ignore_value(virNetDevBridgeRemovePort(newbridge, olddev->ifname));
>+            virErrorRestore(&err);
>+        }
>+    }
>     virDomainAuditNet(vm, NULL, newdev, "attach", ret == 0);
>     if (ret < 0) {
>         virErrorPtr err;
>
>         virErrorPreserveLast(&err);
>         ret = virNetDevBridgeAddPort(oldbridge, olddev->ifname);
>+        if (ret == 0 &&
>+            virDomainNetGetActualPortOptionsIsolated(olddev) == VIR_TRISTATE_BOOL_YES) {
>+            ignore_value(virNetDevBridgePortSetIsolated(newbridge, olddev->ifname, true));

Should this use 'oldbridge' instead of 'newbridge'?

>+        }
>         virDomainAuditNet(vm, NULL, olddev, "attach", ret == 0);
>         virErrorRestore(&err);
>         return -1;

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

Jano
Re: [libvirt PATCH 07/10] qemu/lxc: plumb isolatedPort from config down through bridge attachment
Posted by Laine Stump 5 years, 11 months ago
On 2/18/20 12:46 PM, Ján Tomko wrote:
> On Sun, Feb 16, 2020 at 11:22:56PM -0500, Laine Stump wrote:
>> This patch pushes the isolatedPort setting from the <interface> down
>> all the way to the callers of virNetDevBridgeAddPort(), and sets
>> BR_ISOLATED on the port (using virNetDevBridgePortSetIsolated()) after
>> the port has been successfully added to the bridge.
>>
>> Signed-off-by: Laine Stump <laine@redhat.com>
>> ---
>> src/bhyve/bhyve_command.c   |  1 +
>> src/conf/domain_conf.c      |  1 +
>> src/lxc/lxc_process.c       | 10 ++++++++++
>> src/network/bridge_driver.c |  1 +
>> src/qemu/qemu_hotplug.c     | 16 ++++++++++++++++
>> src/qemu/qemu_interface.c   |  1 +
>> src/util/virnetdevtap.c     | 17 ++++++++++++++++-
>> src/util/virnetdevtap.h     |  3 +++
>> tests/bhyvexml2argvmock.c   |  1 +
>> 9 files changed, 50 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
>> index 6395826c69..af892255c7 100644
>> --- a/src/qemu/qemu_hotplug.c
>> +++ b/src/qemu/qemu_hotplug.c
>> @@ -3350,12 +3350,28 @@ qemuDomainChangeNetBridge(virDomainObjPtr vm,
>>     }
>>
>>     ret = virNetDevBridgeAddPort(newbridge, olddev->ifname);
>> +    if (ret == 0 &&
>> +        virDomainNetGetActualPortOptionsIsolated(newdev) == 
>> VIR_TRISTATE_BOOL_YES) {
>> +
>> +        ret = virNetDevBridgePortSetIsolated(newbridge, 
>> olddev->ifname, true);
>> +        if (ret < 0) {
>> +            virErrorPtr err;
>> +
>> +            virErrorPreserveLast(&err);
>> +            ignore_value(virNetDevBridgeRemovePort(newbridge, 
>> olddev->ifname));
>> +            virErrorRestore(&err);
>> +        }
>> +    }
>>     virDomainAuditNet(vm, NULL, newdev, "attach", ret == 0);
>>     if (ret < 0) {
>>         virErrorPtr err;
>>
>>         virErrorPreserveLast(&err);
>>         ret = virNetDevBridgeAddPort(oldbridge, olddev->ifname);
>> +        if (ret == 0 &&
>> +            virDomainNetGetActualPortOptionsIsolated(olddev) == 
>> VIR_TRISTATE_BOOL_YES) {
>> +            ignore_value(virNetDevBridgePortSetIsolated(newbridge, 
>> olddev->ifname, true));
> 
> Should this use 'oldbridge' instead of 'newbridge'?

Whoops! Cut/paste error. (At least I removed the part about being a Navy 
Seal and having a certain set of skills)

> 
>> +        }
>>         virDomainAuditNet(vm, NULL, olddev, "attach", ret == 0);
>>         virErrorRestore(&err);
>>         return -1;
> 
> Reviewed-by: Ján Tomko <jtomko@redhat.com>
> 
> Jano