- Devel - libvirt List Archives

[PATCH] util: re-add conditional for ifi_iqdrops field for macOS
by Daniel P. Berrangé 04 Sep '20

04 Sep '20

The conditional was removed in commit ebbf8ebe4fa6f9d43b40673f0f2dad6bf50e2085 Author: Ján Tomko <jtomko(a)redhat.com> Date: Tue Sep 1 22:56:37 2020 +0200 util: virnetdevtap: stats: fix txdrop on FreeBSD That commit was correct about this no longer being required for FreeBSD, but missed that the code is also built on macOS. Rather than testing for this field in meson though, we can simply use a platform conditional test in the code. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com> --- Pushed to fix macOS CI build jobs src/util/virnetdevtap.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c index cc6f254aa1..ab5959c646 100644 --- a/src/util/virnetdevtap.c +++ b/src/util/virnetdevtap.c @@ -974,12 +974,20 @@ virNetDevTapInterfaceStats(const char *ifname, stats->rx_bytes = ifd->ifi_obytes; stats->rx_packets = ifd->ifi_opackets; stats->rx_errs = ifd->ifi_oerrors; +# ifndef __APPLE__ stats->rx_drop = ifd->ifi_oqdrops; +# else + stats->rx_drop = 0; +# endif } else { stats->tx_bytes = ifd->ifi_obytes; stats->tx_packets = ifd->ifi_opackets; stats->tx_errs = ifd->ifi_oerrors; +# ifndef __APPLE__ stats->tx_drop = ifd->ifi_oqdrops; +# else + stats->tx_drop = 0; +# endif stats->rx_bytes = ifd->ifi_ibytes; stats->rx_packets = ifd->ifi_ipackets; stats->rx_errs = ifd->ifi_ierrors; -- 2.26.2

2 1

[PATCH] tests: add FreeBSD dependencies
by Roman Bogorodskiy 04 Sep '20

04 Sep '20

Add some FreeBSD-specific libraries (-lutil, -lkvm) to tests dependencies. Without that, FreeBSD 11.x, which uses the GNU ld, fails to link tests. Interestingly, newer FreeBSD versions that use LLVM ld tolerate this behaviour and builds successfully as is. Signed-off-by: Roman Bogorodskiy <bogorodskiy(a)gmail.com> --- tests/meson.build | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/meson.build b/tests/meson.build index ad13e2de60..ea2458efbc 100644 --- a/tests/meson.build +++ b/tests/meson.build @@ -10,11 +10,13 @@ tests_dep = declare_dependency( dlopen_dep, glib_dep, gnutls_dep, + kvm_dep, libnl_dep, libxml_dep, rpc_dep, sasl_dep, selinux_dep, + util_dep, xdr_dep, yajl_dep, ], -- 2.27.0

3 8

[libvirt PATCH] ci: Disable verbose mode for cirrus-run
by Andrea Bolognani 04 Sep '20

04 Sep '20

It was temporarily enabled to help the upstream author validate some changes, but now that 0.3.0 is out we can safely disable it again. This reverts commit 9b5622d31f8c3036425721ab9c6b23067aa87034. Signed-off-by: Andrea Bolognani <abologna(a)redhat.com> --- .gitlab-ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 4a42eb479f..bd42c95602 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -104,7 +104,7 @@ stages: -e "s|[@]PYPI_PKGS@|$PYPI_PKGS|g" <ci/cirrus/build.yml >ci/cirrus/$NAME.yml - cat ci/cirrus/$NAME.yml - - cirrus-run -v --show-build-log always ci/cirrus/$NAME.yml + - cirrus-run --show-build-log always ci/cirrus/$NAME.yml only: variables: - $CIRRUS_GITHUB_REPO -- 2.26.2

2 2

[libvirt PATCH v2 0/3] Add support for vDPA network devices
by Jonathon Jongsma 03 Sep '20

03 Sep '20

vDPA network devices allow high-performance networking in a virtual machine by providing a wire-speed data path. These devices require a vendor-specific host driver but the data path follows the virtio specification. The support for vDPA devices was recently added to qemu. This allows libvirt to support these devices. It requires that the device is configured on the host with the appropriate vendor-specific driver. This will create a chardev on the host at e.g. /dev/vhost-vdpa-0. That chardev path can then be used to define a new interface with type='vdpa'. Changes in v2: - split patches - add qemu capability - disallow migration for vdpa devices Jonathon Jongsma (3): conf: Add support for vDPA network devices qemu: add vhost-vdpa capability qemu: add vdpa support docs/formatdomain.rst | 23 ++++++++++++ docs/schemas/domaincommon.rng | 15 ++++++++ src/conf/domain_conf.c | 32 ++++++++++++++++ src/conf/domain_conf.h | 4 ++ src/conf/netdev_bandwidth_conf.c | 1 + src/libxl/libxl_conf.c | 1 + src/libxl/xen_common.c | 1 + src/lxc/lxc_controller.c | 1 + src/lxc/lxc_driver.c | 3 ++ src/lxc/lxc_process.c | 1 + src/qemu/qemu_capabilities.c | 4 ++ src/qemu/qemu_capabilities.h | 3 ++ src/qemu/qemu_command.c | 29 ++++++++++++++- src/qemu/qemu_command.h | 3 +- src/qemu/qemu_domain.c | 10 ++++- src/qemu/qemu_hotplug.c | 15 +++++--- src/qemu/qemu_interface.c | 25 +++++++++++++ src/qemu/qemu_interface.h | 2 + src/qemu/qemu_migration.c | 10 ++++- src/qemu/qemu_process.c | 1 + src/qemu/qemu_validate.c | 1 + src/vmx/vmx.c | 1 + .../caps_5.1.0.x86_64.xml | 1 + .../net-vdpa.x86_64-latest.args | 37 +++++++++++++++++++ tests/qemuxml2argvdata/net-vdpa.xml | 28 ++++++++++++++ tests/qemuxml2argvmock.c | 11 +++++- tests/qemuxml2argvtest.c | 1 + tests/qemuxml2xmloutdata/net-vdpa.xml | 33 +++++++++++++++++ tests/qemuxml2xmltest.c | 1 + tools/virsh-domain.c | 1 + 30 files changed, 288 insertions(+), 11 deletions(-) create mode 100644 tests/qemuxml2argvdata/net-vdpa.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/net-vdpa.xml create mode 100644 tests/qemuxml2xmloutdata/net-vdpa.xml -- 2.26.2

3 7

[PATCH] qemu_namespace: Be tolerant to non-existent files when populating /dev
by Michal Privoznik 03 Sep '20

03 Sep '20

In 6.7.0 release I've changed how domain namespace is built and populated. Previously it used to be done from a pre-exec hook (ran in the forked off child, just before dropping all privileges and exec()-ing QEMU), which not only meant we had to have two different code paths for creating a node in domain's namespace (one for this pre-exec hook, the other for hotplug ran from the daemon), it also proved problematic because it was leaking FDs into QEMU process. To mitigate this problem, we've not only ditched libdevmapper from the NS population process, I've also dropped the pre-exec code and let the NS be populated from the daemon (using the hotplug code). But, I was not careful when doing so, because the pre-exec code was tolerant to files that doesn't exist, while this new code isn't. For instance, the very first thing that is done when the new NS is created is it's populated with @defaultDeviceACL which contain files like /dev/null, /dev/zero, /dev/random and /dev/kvm (and others). While the rest will probably exist every time, /dev/kvm might not and thus the new code I wrote has to be tolerant to that. Of course, users can override the @defaultDeviceACL (by setting cgroup_device_acl in qemu.conf) and remove /dev/kvm (which is acceptable workaround), but we definitely want libvirt to work out of the box even on hosts without KVM. Fixes: 9048dc4e627ddf33996084167bece7b5fb83b0bc Reported-by: Daniel P. Berrangé <berrange(a)redhat.com> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- src/qemu/qemu_namespace.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_namespace.c b/src/qemu/qemu_namespace.c index 454d6a7b4d..87f4fd8d58 100644 --- a/src/qemu/qemu_namespace.c +++ b/src/qemu/qemu_namespace.c @@ -1094,6 +1094,9 @@ qemuNamespaceMknodItemInit(qemuNamespaceMknodItemPtr item, item->file = file; if (g_lstat(file, &item->sb) < 0) { + if (errno == ENOENT) + return -2; + virReportSystemError(errno, _("Unable to access %s"), file); return -1; @@ -1168,9 +1171,16 @@ qemuNamespacePrepareOneItem(qemuNamespaceMknodDataPtr data, while (1) { qemuNamespaceMknodItem item = { 0 }; + int rc; - if (qemuNamespaceMknodItemInit(&item, cfg, vm, next) < 0) + rc = qemuNamespaceMknodItemInit(&item, cfg, vm, next); + if (rc == -2) { + /* @file doesn't exist. We can break here. */ + break; + } else if (rc < 0) { + /* Some other (critical) error. */ return -1; + } if (STRPREFIX(next, QEMU_DEVPREFIX)) { for (i = 0; i < ndevMountsPath; i++) { -- 2.26.2

3 2

[PATCH] util: add device name in errors from ethtool ioctls
by Daniel P. Berrangé 03 Sep '20

03 Sep '20

Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com> --- src/util/virnetdev.c | 40 ++++++++++++++++++++++------------------ 1 file changed, 22 insertions(+), 18 deletions(-) diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index 3040f129d1..e1a4cc2bef 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -2849,7 +2849,7 @@ virNetDevRDMAFeature(const char *ifname, * Returns 0 on success, -1 on failure. */ static int -virNetDevSendEthtoolIoctl(int fd, struct ifreq *ifr) +virNetDevSendEthtoolIoctl(const char *ifname, int fd, struct ifreq *ifr) { int ret = -1; @@ -2857,13 +2857,13 @@ virNetDevSendEthtoolIoctl(int fd, struct ifreq *ifr) if (ret != 0) { switch (errno) { case EINVAL: /* kernel doesn't support SIOCETHTOOL */ - VIR_DEBUG("ethtool ioctl: invalid request"); + VIR_DEBUG("ethtool ioctl: invalid request on %s", ifname); break; case EOPNOTSUPP: /* kernel doesn't support specific feature */ - VIR_DEBUG("ethtool ioctl: request not supported"); + VIR_DEBUG("ethtool ioctl: request not supported on %s", ifname); break; default: - virReportSystemError(errno, "%s", _("ethtool ioctl error")); + virReportSystemError(errno, _("ethtool ioctl error on %s"), ifname); break; } } @@ -2888,10 +2888,10 @@ struct virNetDevEthtoolFeatureCmd { * Returns true if the feature is available, false otherwise. */ static bool -virNetDevFeatureAvailable(int fd, struct ifreq *ifr, struct ethtool_value *cmd) +virNetDevFeatureAvailable(const char *ifname, int fd, struct ifreq *ifr, struct ethtool_value *cmd) { ifr->ifr_data = (void*)cmd; - if (virNetDevSendEthtoolIoctl(fd, ifr) == 0 && + if (virNetDevSendEthtoolIoctl(ifname, fd, ifr) == 0 && cmd->data > 0) return true; return false; @@ -2899,7 +2899,8 @@ virNetDevFeatureAvailable(int fd, struct ifreq *ifr, struct ethtool_value *cmd) static void -virNetDevGetEthtoolFeatures(virBitmapPtr bitmap, +virNetDevGetEthtoolFeatures(const char *ifname, + virBitmapPtr bitmap, int fd, struct ifreq *ifr) { @@ -2941,13 +2942,13 @@ virNetDevGetEthtoolFeatures(virBitmapPtr bitmap, for (i = 0; i < G_N_ELEMENTS(ethtool_cmds); i++) { cmd.cmd = ethtool_cmds[i].cmd; - if (virNetDevFeatureAvailable(fd, ifr, &cmd)) + if (virNetDevFeatureAvailable(ifname, fd, ifr, &cmd)) ignore_value(virBitmapSetBit(bitmap, ethtool_cmds[i].feat)); } # if WITH_DECL_ETHTOOL_GFLAGS cmd.cmd = ETHTOOL_GFLAGS; - if (virNetDevFeatureAvailable(fd, ifr, &cmd)) { + if (virNetDevFeatureAvailable(ifname, fd, ifr, &cmd)) { for (i = 0; i < G_N_ELEMENTS(flags); i++) { if (cmd.data & flags[i].cmd) ignore_value(virBitmapSetBit(bitmap, flags[i].feat)); @@ -3133,19 +3134,21 @@ virNetDevSwitchdevFeature(const char *ifname G_GNUC_UNUSED, * Returns true if the feature is available, false otherwise. */ static bool -virNetDevGFeatureAvailable(int fd, +virNetDevGFeatureAvailable(const char *ifname, + int fd, struct ifreq *ifr, struct ethtool_gfeatures *cmd) { ifr->ifr_data = (void*)cmd; - if (virNetDevSendEthtoolIoctl(fd, ifr) == 0) + if (virNetDevSendEthtoolIoctl(ifname, fd, ifr) == 0) return !!FEATURE_BIT_IS_SET(cmd->features, TX_UDP_TNL, active); return false; } static int -virNetDevGetEthtoolGFeatures(virBitmapPtr bitmap, +virNetDevGetEthtoolGFeatures(const char *ifname, + virBitmapPtr bitmap, int fd, struct ifreq *ifr) { @@ -3157,13 +3160,14 @@ virNetDevGetEthtoolGFeatures(virBitmapPtr bitmap, g_cmd->cmd = ETHTOOL_GFEATURES; g_cmd->size = GFEATURES_SIZE; - if (virNetDevGFeatureAvailable(fd, ifr, g_cmd)) + if (virNetDevGFeatureAvailable(ifname, fd, ifr, g_cmd)) ignore_value(virBitmapSetBit(bitmap, VIR_NET_DEV_FEAT_TXUDPTNL)); return 0; } # else static int -virNetDevGetEthtoolGFeatures(virBitmapPtr bitmap G_GNUC_UNUSED, +virNetDevGetEthtoolGFeatures(const char *ifname G_GNUC_UNUSED, + virBitmapPtr bitmap G_GNUC_UNUSED, int fd G_GNUC_UNUSED, struct ifreq *ifr G_GNUC_UNUSED) { @@ -3228,7 +3232,7 @@ int virNetDevSetCoalesce(const char *ifname, ifr.ifr_data = (void *) &coal; - if (virNetDevSendEthtoolIoctl(fd, &ifr) < 0) { + if (virNetDevSendEthtoolIoctl(ifname, fd, &ifr) < 0) { virReportSystemError(errno, _("Cannot set coalesce info on '%s'"), ifname); @@ -3241,7 +3245,7 @@ int virNetDevSetCoalesce(const char *ifname, }; /* Don't fail if the update itself fails */ - if (virNetDevSendEthtoolIoctl(fd, &ifr) == 0) { + if (virNetDevSendEthtoolIoctl(ifname, fd, &ifr) == 0) { coalesce->rx_max_coalesced_frames = coal.rx_max_coalesced_frames; coalesce->rx_coalesce_usecs_irq = coal.rx_coalesce_usecs_irq; coalesce->rx_max_coalesced_frames_irq = coal.rx_max_coalesced_frames_irq; @@ -3307,9 +3311,9 @@ virNetDevGetFeatures(const char *ifname, if ((fd = virNetDevSetupControl(ifname, &ifr)) < 0) return -1; - virNetDevGetEthtoolFeatures(*out, fd, &ifr); + virNetDevGetEthtoolFeatures(ifname, *out, fd, &ifr); - if (virNetDevGetEthtoolGFeatures(*out, fd, &ifr) < 0) + if (virNetDevGetEthtoolGFeatures(ifname, *out, fd, &ifr) < 0) return -1; if (virNetDevRDMAFeature(ifname, out) < 0) -- 2.25.4

2 1

[PATCH v1 00/34] Rework building of domain's namespaces
by Michal Privoznik 03 Sep '20

03 Sep '20

There are couple of things happening in this series. The first patch fixes a bug. We are leaking /dev/mapper/control in QEMU. See the patch for detailed info. The second patch then cleans up code a bit. The third patch moves namespace handling code into a separate file. Patches 4 - 15 then prepare the code for switch to different approach in populating the namespace. Patches 16 - 26 then do the switch and finally, the rest of the patches drops and deduplicates some code. You can fetch the patches from here too: https://gitlab.com/MichalPrivoznik/libvirt/-/commits/dm_namespace/ Michal Prívozník (34): virDevMapperGetTargetsImpl: Close /dev/mapper/control in the end virDevMapperGetTargets: Don't ignore EBADF qemu: Separate out namespace handling code qemu_domain_namespace: Rename qemuDomainCreateNamespace() qemu_domain_namespace: Drop unused @cfg argument qemu_domain_namespace: Check for namespace enablement earlier qemuDomainNamespaceSetupHostdev: Create paths in one go qemuDomainAttachDeviceMknodHelper: Don't leak data->target qemu_domain_namespace.c: Rename qemuDomainAttachDeviceMknodData qemuDomainAttachDeviceMknodRecursive: Isolate bind mounted devices condition qemuDomainAttachDeviceMknodHelper: Create more files in a single go qemuDomainNamespaceMknodPaths: Create more files in one go qemuDomainNamespaceMknodPaths: Turn @paths into string list qemuDomainSetupDisk: Accept @src qemu_domain_namespace: Repurpose qemuDomainBuildNamespace() qemuDomainBuildNamespace: Populate basic /dev from daemon's namespace qemuDomainBuildNamespace: Populate disks from daemon's namespace qemuDomainBuildNamespace: Populate hostdevs from daemon's namespace qemuDomainBuildNamespace: Populate memory from daemon's namespace qemuDomainBuildNamespace: Populate chardevs from daemon's namespace qemuDomainBuildNamespace: Populate TPM from daemon's namespace qemuDomainBuildNamespace: Populate graphics from daemon's namespace qemuDomainBuildNamespace: Populate inputs from daemon's namespace qemuDomainBuildNamespace: Populate RNGs from daemon's namespace qemuDomainBuildNamespace: Populate loader from daemon's namespace qemuDomainBuildNamespace: Populate SEV from daemon's namespace qemu_domain_namespace: Drop unused functions qemuDomainDetachDeviceUnlink: Unlink paths in one go qemuDomainNamespaceUnlinkPaths: Turn @paths into string list qemuDomainNamespaceTeardownHostdev: Unlink paths in one go qemuDomainNamespaceTeardownMemory: Deduplicate code qemuDomainNamespaceTeardownChardev: Deduplicate code qemuDomainNamespaceTeardownRNG: Deduplicate code qemuDomainNamespaceTeardownInput: Deduplicate code po/POTFILES.in | 1 + src/qemu/Makefile.inc.am | 2 + src/qemu/qemu_cgroup.c | 2 +- src/qemu/qemu_conf.c | 1 + src/qemu/qemu_domain.c | 1848 +----------------------------- src/qemu/qemu_domain.h | 57 - src/qemu/qemu_domain_namespace.c | 1610 ++++++++++++++++++++++++++ src/qemu/qemu_domain_namespace.h | 86 ++ src/qemu/qemu_driver.c | 1 + src/qemu/qemu_hotplug.c | 1 + src/qemu/qemu_process.c | 23 +- src/qemu/qemu_security.c | 1 + src/util/virdevmapper.c | 4 +- 13 files changed, 1727 insertions(+), 1910 deletions(-) create mode 100644 src/qemu/qemu_domain_namespace.c create mode 100644 src/qemu/qemu_domain_namespace.h -- 2.26.2

4 74

[PATCH v2 0/2] Fix broken build with readline
by Michal Privoznik 03 Sep '20

03 Sep '20

This is a v2 of: https://www.redhat.com/archives/libvir-list/2020-September/msg00107.html diff to v1: - Patch 1/2 is completely new, it removes include of readline from virsh.c and virt-admin.c, so that patch 2/2 can be unchanged. Michal Prívozník (2): vsh: Introduce wrapper for add_history() vsh: Define HAVE_STDARG_H before including readline tools/virsh.c | 10 ++-------- tools/virt-admin.c | 10 ++-------- tools/vsh.c | 16 ++++++++++++++++ tools/vsh.h | 2 ++ 4 files changed, 22 insertions(+), 16 deletions(-) -- 2.26.2

4 6

[libvirt PATCH] remote: adapt augeas test case for dynamic polkit config change
by Daniel P. Berrangé 03 Sep '20

03 Sep '20

We need to use @default_auth@ in the augeas test case to match its use in the main libvirtd.conf.in file. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com> --- Pushed as a CI fix src/remote/test_libvirtd.aug.in | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/remote/test_libvirtd.aug.in b/src/remote/test_libvirtd.aug.in index 1e17397af6..56c4487a01 100644 --- a/src/remote/test_libvirtd.aug.in +++ b/src/remote/test_libvirtd.aug.in @@ -14,8 +14,8 @@ module Test_@DAEMON_NAME@ = { "unix_sock_rw_perms" = "0770" } { "unix_sock_admin_perms" = "0700" } { "unix_sock_dir" = "@runstatedir@/libvirt" } - { "auth_unix_ro" = "none" } - { "auth_unix_rw" = "none" } + { "auth_unix_ro" = "@default_auth@" } + { "auth_unix_rw" = "@default_auth@" } @CUT_ENABLE_IP@ { "auth_tcp" = "sasl" } { "auth_tls" = "none" } -- 2.26.2

1 0

[PATCH] network: drop use of dummy tap device in bridges
by Daniel P. Berrangé 03 Sep '20

03 Sep '20

A long time ago we introduced a dummy tap device (e.g. virbr0-nic) that we attached to the bridge device created for virtual networks: commit 5754dbd56d4738112a86776c09e810e32f7c3224 Author: Laine Stump <laine(a)redhat.com> Date: Wed Feb 9 03:28:12 2011 -0500 Give each virtual network bridge its own fixed MAC address This was a hack to workaround a Linux kernel bug where it would not honour any attempt to set a MAC address on a bridge. Instead the bridge would adopt the numerically lowest MAC address of all NICs attached to the bridge. This lead to the MAC addrss of the bridge changing over time as NICs were attached/detached. The Linux bug was actually fixed 3 years before the libvirt workaround was added in: commit 92c0574f11598c8036f81e27d2e8bdd6eed7d76d Author: Stephen Hemminger <shemminger(a)vyatta.com> Date: Tue Jun 17 16:10:06 2008 -0700 bridge: make bridge address settings sticky Normally, the bridge just chooses the smallest mac address as the bridge id and mac address of bridge device. But if the administrator has explictly set the interface address then don't change it. Signed-off-by: Stephen Hemminger <shemminger(a)vyatta.com> Signed-off-by: David S. Miller <davem(a)davemloft.net> but libvirt needed to support RHEL-5 kernels at that time, so none the less added the workaround. We have long since dropped support for RHEL-5 vintage distros, so there's no reason to keep the dummy tap device for the purpose of setting the bridge MAC address. Later the dummy TAP device was used for a second purpose related to IPv6 DAD (Duplicate Address Detection) in: commit db488c79173b240459c7754f38c3c6af9b432970 Author: Benjamin Cama <benoar(a)dolka.fr> Date: Wed Sep 26 21:02:20 2012 +0200 network: fix dnsmasq/radvd binding to IPv6 on recent kernels This was again dealing with a regression in the Linux kernel, where if there were no devices attached to the bridge in the UP state, IPv6 DAD would not be performed. The virbr0-nic was attached but in the DOWN state, so the above libvirt fix tenporarily brought the NIC online. The Linux commit causing the problem was in v2.6.38 commit 1faa4356a3bd89ea11fb92752d897cff3a20ec0e Author: stephen hemminger <shemminger(a)vyatta.com> Date: Mon Mar 7 08:34:06 2011 +0000 bridge: control carrier based on ports online A short while later Linux was tweaked so that DAD would still occur if the bridge had no attached devices at all in 3.1: commit b64b73d7d0c480f75684519c6134e79d50c1b341 Author: stephen hemminger <shemminger(a)vyatta.com> Date: Mon Oct 3 18:14:45 2011 +0000 bridge: leave carrier on for empty bridge IOW, the only reason we need the DAD hack of bringing virbr0-nic online is because virbr0-nic exists. Once it doesn't exist, then we hit the "empty bridge" case which works in Linux. We can rely on distros having Linux kernel >= 3.1, so both things that the virbr0-nic are doing are redundant. Fixes https://gitlab.com/libvirt/libvirt/-/issues/53 Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com> --- src/network/bridge_driver.c | 58 +++++-------------------------------- 1 file changed, 8 insertions(+), 50 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index b016d86b9f..5c00befc16 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -2247,8 +2247,7 @@ networkAddAddrToBridge(virNetworkObjPtr obj, static int -networkStartHandleMACTableManagerMode(virNetworkObjPtr obj, - const char *macTapIfName) +networkStartHandleMACTableManagerMode(virNetworkObjPtr obj) { virNetworkDefPtr def = virNetworkObjGetDef(obj); const char *brname = def->bridge; @@ -2257,12 +2256,6 @@ networkStartHandleMACTableManagerMode(virNetworkObjPtr obj, def->macTableManager == VIR_NETWORK_BRIDGE_MAC_TABLE_MANAGER_LIBVIRT) { if (virNetDevBridgeSetVlanFiltering(brname, true) < 0) return -1; - if (macTapIfName) { - if (virNetDevBridgePortSetLearning(brname, macTapIfName, false) < 0) - return -1; - if (virNetDevBridgePortSetUnicastFlood(brname, macTapIfName, false) < 0) - return -1; - } } return 0; } @@ -2330,10 +2323,8 @@ networkStartNetworkVirtual(virNetworkDriverStatePtr driver, virErrorPtr save_err = NULL; virNetworkIPDefPtr ipdef; virNetDevIPRoutePtr routedef; - g_autofree char *macTapIfName = NULL; virMacMapPtr macmap; g_autofree char *macMapFile = NULL; - int tapfd = -1; bool dnsmasqStarted = false; bool devOnline = false; bool firewalRulesAdded = false; @@ -2360,29 +2351,6 @@ networkStartNetworkVirtual(virNetworkDriverStatePtr driver, if (virNetDevBridgeCreate(def->bridge, &def->mac) < 0) return -1; - if (def->mac_specified) { - /* To set a mac for the bridge, we need to define a dummy tap - * device, set its mac, then attach it to the bridge. As long - * as its mac address is lower than any other interface that - * gets attached, the bridge will always maintain this mac - * address. - */ - macTapIfName = networkBridgeDummyNicName(def->bridge); - if (!macTapIfName) - goto error; - /* Keep tun fd open and interface up to allow for IPv6 DAD to happen */ - 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 | - VIR_NETDEV_TAP_CREATE_PERSIST) < 0) { - goto error; - } - } - if (!(macMapFile = virMacMapFileName(driver->dnsmasqStateDir, def->bridge)) || !(macmap = virMacMapNew(macMapFile))) @@ -2426,7 +2394,7 @@ networkStartNetworkVirtual(virNetworkDriverStatePtr driver, goto error; } - if (networkStartHandleMACTableManagerMode(obj, macTapIfName) < 0) + if (networkStartHandleMACTableManagerMode(obj) < 0) goto error; /* Bring up the bridge interface */ @@ -2482,15 +2450,6 @@ networkStartNetworkVirtual(virNetworkDriverStatePtr driver, if (v6present && networkWaitDadFinish(obj) < 0) goto error; - /* DAD has finished, dnsmasq is now bound to the - * bridge's IPv6 address, so we can set the dummy tun down. - */ - if (tapfd >= 0) { - if (virNetDevSetOnline(macTapIfName, false) < 0) - goto error; - VIR_FORCE_CLOSE(tapfd); - } - if (virNetDevBandwidthSet(def->bridge, def->bandwidth, true, true) < 0) goto error; @@ -2514,16 +2473,11 @@ networkStartNetworkVirtual(virNetworkDriverStatePtr driver, def->forward.type != VIR_NETWORK_FORWARD_OPEN) networkRemoveFirewallRules(def); - if (macTapIfName) { - VIR_FORCE_CLOSE(tapfd); - ignore_value(virNetDevTapDelete(macTapIfName, NULL)); - } virNetworkObjUnrefMacMap(obj); ignore_value(virNetDevBridgeDelete(def->bridge)); virErrorRestore(&save_err); - /* coverity[leaked_handle] - 'tapfd' is not leaked */ return -1; } @@ -2555,9 +2509,13 @@ networkShutdownNetworkVirtual(virNetworkDriverStatePtr driver, if (dnsmasqPid > 0) kill(dnsmasqPid, SIGTERM); + /* We no longer create a dummy NIC, but if we've upgraded + * from old libvirt, we still need to delete any dummy NIC + * that might exist. Keep this logic around for a while... + */ if (def->mac_specified) { g_autofree char *macTapIfName = networkBridgeDummyNicName(def->bridge); - if (macTapIfName) + if (macTapIfName && virNetDevExists(macTapIfName)) ignore_value(virNetDevTapDelete(macTapIfName, NULL)); } @@ -2597,7 +2555,7 @@ networkStartNetworkBridge(virNetworkObjPtr obj) if (virNetDevBandwidthSet(def->bridge, def->bandwidth, true, true) < 0) goto error; - if (networkStartHandleMACTableManagerMode(obj, NULL) < 0) + if (networkStartHandleMACTableManagerMode(obj) < 0) goto error; return 0; -- 2.25.4

2 3