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

[Ján Tomko <jtomko@xxxxxxxxxx>]

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@xxxxxxxxxx>
> ---
> 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@xxxxxxxxxx>

Jano

Attachment: signature.asc
Description: PGP signature