[libvirt] [PATCH 2/2] network: Bring netdevs online later
Martin Kletzander
mkletzan at redhat.com
Wed Jul 23 13:49:02 UTC 2014
On Tue, Jul 01, 2014 at 02:00:57PM -0400, Matthew Rosato wrote:
>Defer MAC registration until net devices are actually going
>to be used by the guest. This patch does so by setting the
>devices online just before starting guest CPUs.
>
Does this have some upside/downside? Are you trying to fix some
problem? It would be nice to describe it in the commit message, so I
know what to focus on or why it's needed. Depending on the answer
there might be a way how to unit-test it.
>Signed-off-by: Matthew Rosato <mjrosato at linux.vnet.ibm.com>
>---
> src/Makefile.am | 1 +
> src/conf/domain_conf.h | 2 ++
> src/lxc/lxc_process.c | 3 +-
> src/qemu/qemu_command.c | 6 +++-
> src/qemu/qemu_hotplug.c | 7 +++++
> src/qemu/qemu_interface.c | 65 +++++++++++++++++++++++++++++++++++++++++++
> src/qemu/qemu_interface.h | 33 ++++++++++++++++++++++
> src/qemu/qemu_process.c | 4 +++
> src/util/virnetdevmacvlan.c | 8 ++++--
> src/util/virnetdevmacvlan.h | 2 ++
> 10 files changed, 126 insertions(+), 5 deletions(-)
> create mode 100644 src/qemu/qemu_interface.c
> create mode 100644 src/qemu/qemu_interface.h
>
>diff --git a/src/Makefile.am b/src/Makefile.am
>index 35720be..e3d2e36 100644
>--- a/src/Makefile.am
>+++ b/src/Makefile.am
>@@ -687,6 +687,7 @@ QEMU_DRIVER_SOURCES = \
> qemu/qemu_domain.c qemu/qemu_domain.h \
> qemu/qemu_cgroup.c qemu/qemu_cgroup.h \
> qemu/qemu_hostdev.c qemu/qemu_hostdev.h \
>+ qemu/qemu_interface.c qemu/qemu_interface.h \
> qemu/qemu_hotplug.c qemu/qemu_hotplug.h \
> qemu/qemu_hotplugpriv.h \
> qemu/qemu_conf.c qemu/qemu_conf.h \
>diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
>index 1122eb2..8375106 100644
>--- a/src/conf/domain_conf.h
>+++ b/src/conf/domain_conf.h
>@@ -921,6 +921,8 @@ struct _virDomainNetDef {
> virNetDevBandwidthPtr bandwidth;
> virNetDevVlan vlan;
> int linkstate;
>+ /* vmOp value saved if deferring interface start */
>+ virNetDevVPortProfileOp vmOp;
> };
>
> /* Used for prefix of ifname of any network name generated dynamically
>diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
>index ab0c5d0..3083ed3 100644
>--- a/src/lxc/lxc_process.c
>+++ b/src/lxc/lxc_process.c
>@@ -302,6 +302,7 @@ char *virLXCProcessSetupInterfaceDirect(virConnectPtr conn,
> virNetDevBandwidthPtr bw;
> virNetDevVPortProfilePtr prof;
> virLXCDriverConfigPtr cfg = virLXCDriverGetConfig(driver);
>+ unsigned int macvlan_create_flags = VIR_NETDEV_MACVLAN_CREATE_IFUP;
>
> /* XXX how todo bandwidth controls ?
> * Since the 'net-ifname' is about to be moved to a different
>@@ -333,7 +334,7 @@ char *virLXCProcessSetupInterfaceDirect(virConnectPtr conn,
> net->ifname, &net->mac,
> virDomainNetGetActualDirectDev(net),
> virDomainNetGetActualDirectMode(net),
>- 0, false, def->uuid,
>+ macvlan_create_flags, false, def->uuid,
> virDomainNetGetActualVirtPortProfile(net),
> &res_ifname,
> VIR_NETDEV_VPORT_PROFILE_OP_CREATE,
>diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
>index 314d8a3..a5662f5 100644
>--- a/src/qemu/qemu_command.c
>+++ b/src/qemu/qemu_command.c
>@@ -196,6 +196,9 @@ qemuPhysIfaceConnect(virDomainDefPtr def,
> net->ifname = res_ifname;
> }
>
>+ /* Save vport profile op for later */
>+ net->vmOp = vmop;
>+
> virObjectUnref(cfg);
> return rc;
>
>@@ -294,7 +297,7 @@ qemuNetworkIfaceConnect(virDomainDefPtr def,
> {
> char *brname = NULL;
> int ret = -1;
>- unsigned int tap_create_flags = VIR_NETDEV_TAP_CREATE_IFUP;
>+ unsigned int tap_create_flags = 0;
> bool template_ifname = false;
> int actualType = virDomainNetGetActualType(net);
> virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
>@@ -354,6 +357,7 @@ qemuNetworkIfaceConnect(virDomainDefPtr def,
> goto cleanup;
> }
> } else {
>+ tap_create_flags |= VIR_NETDEV_TAP_CREATE_IFUP;
> if (qemuCreateInBridgePortWithHelper(cfg, brname,
> &net->ifname,
> tapfd, tap_create_flags) < 0) {
>diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
>index 5e8aa4e..98e7764 100644
>--- a/src/qemu/qemu_hotplug.c
>+++ b/src/qemu/qemu_hotplug.c
>@@ -49,6 +49,7 @@
> #include "virstoragefile.h"
> #include "virstring.h"
> #include "virtime.h"
>+#include "qemu_interface.h"
>
> #define VIR_FROM_THIS VIR_FROM_QEMU
>
>@@ -854,6 +855,9 @@ int qemuDomainAttachNetDevice(virConnectPtr conn,
> if (networkAllocateActualDevice(vm->def, net) < 0)
> goto cleanup;
>
>+ /* Set device online immediately */
>+ qemuInterfaceStartDevice(net);
>+
> actualType = virDomainNetGetActualType(net);
>
> if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV) {
>@@ -2030,6 +2034,9 @@ qemuDomainChangeNet(virQEMUDriverPtr driver,
> goto cleanup;
> }
>
>+ /* Set device online immediately */
>+ qemuInterfaceStartDevice(newdev);
>+
> newType = virDomainNetGetActualType(newdev);
>
> if (newType == VIR_DOMAIN_NET_TYPE_HOSTDEV) {
>diff --git a/src/qemu/qemu_interface.c b/src/qemu/qemu_interface.c
>new file mode 100644
>index 0000000..b70b6eb
>--- /dev/null
>+++ b/src/qemu/qemu_interface.c
>@@ -0,0 +1,65 @@
>+/*
>+ * qemu_interface.c: QEMU interface management
>+ *
>+ * Copyright (C) 2014 Red Hat, Inc.
>+ * Copyright IBM Corp. 2014
>+ *
I don't understand this double copyright here, copy-paste mistake?
>+ * This library is free software; you can redistribute it and/or
>+ * modify it under the terms of the GNU Lesser General Public
>+ * License as published by the Free Software Foundation; either
>+ * version 2.1 of the License, or (at your option) any later version.
>+ *
>+ * This library is distributed in the hope that it will be useful,
>+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
>+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
>+ * Lesser General Public License for more details.
>+ *
>+ * You should have received a copy of the GNU Lesser General Public
>+ * License along with this library. If not, see
>+ * <http://www.gnu.org/licenses/>.
>+ *
>+ * Author: Matthew J. Rosato <mjrosato at linux.vnet.ibm.com>
>+ */
>+
>+#include <config.h>
>+
>+#include "qemu_interface.h"
>+#include "virnetdev.h"
>+#include "virnetdevtap.h"
>+#include "virnetdevmacvlan.h"
>+#include "virnetdevvportprofile.h"
>+
>+void
>+qemuInterfaceStartDevice(virDomainNetDefPtr net)
>+{
>+ switch (virDomainNetGetActualType(net)) {
>+ case VIR_DOMAIN_NET_TYPE_BRIDGE:
>+ case VIR_DOMAIN_NET_TYPE_NETWORK:
>+ if (virNetDevSetOnline(net->ifname, true) < 0) {
>+ ignore_value(virNetDevTapDelete(net->ifname));
>+ }
>+ break;
>+ case VIR_DOMAIN_NET_TYPE_DIRECT:
>+ if (virNetDevSetOnline(net->ifname, true) < 0) {
>+ ignore_value(virNetDevVPortProfileDisassociate(net->ifname,
>+ virDomainNetGetActualVirtPortProfile(net),
>+ &net->mac,
>+ virDomainNetGetActualDirectDev(net),
>+ -1,
>+ net->vmOp));
>+ }
>+ break;
>+ }
>+}
>+
>+void
>+qemuInterfaceStartDevices(virDomainDefPtr def)
>+{
>+ size_t i;
>+
>+ for (i = 0; i < def->nnets; i++) {
>+ qemuInterfaceStartDevice(def->nets[i]);
>+ }
>+
>+ return;
>+}
>diff --git a/src/qemu/qemu_interface.h b/src/qemu/qemu_interface.h
>new file mode 100644
>index 0000000..dd14556
>--- /dev/null
>+++ b/src/qemu/qemu_interface.h
>@@ -0,0 +1,33 @@
>+/*
>+ * qemu_interface.h: QEMU interface management
>+ *
>+ * Copyright (C) 2014 Red Hat, Inc.
>+ * Copyright IBM Corp. 2014
>+ *
The same as in 'qemu_interface.c'.
>+ * This library is free software; you can redistribute it and/or
>+ * modify it under the terms of the GNU Lesser General Public
>+ * License as published by the Free Software Foundation; either
>+ * version 2.1 of the License, or (at your option) any later version.
>+ *
>+ * This library is distributed in the hope that it will be useful,
>+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
>+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
>+ * Lesser General Public License for more details.
>+ *
>+ * You should have received a copy of the GNU Lesser General Public
>+ * License along with this library. If not, see
>+ * <http://www.gnu.org/licenses/>.
>+ *
>+ * Author: Matthew J. Rosato <mjrosato at linux.vnet.ibm.com>
>+ */
>+
>+#ifndef __QEMU_INTERFACE_H__
>+#define __QEMU_INTERFACE_H__
Improper indentation.
>+
>+//# include "qemu_conf.h"
Possible leftover?
>+# include "domain_conf.h"
>+
>+void qemuInterfaceStartDevice(virDomainNetDefPtr net);
>+void qemuInterfaceStartDevices(virDomainDefPtr def);
>+
>+#endif /* __QEMU_INTERFACE_H__ */
Why are these qemu_ when they have nothing to do with qemu in
particular?
>diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
>index 5b598be..26bd494 100644
>--- a/src/qemu/qemu_process.c
>+++ b/src/qemu/qemu_process.c
>@@ -42,6 +42,7 @@
> #include "qemu_hostdev.h"
> #include "qemu_hotplug.h"
> #include "qemu_migration.h"
>+#include "qemu_interface.h"
>
> #include "cpu/cpu.h"
> #include "datatypes.h"
>@@ -2775,6 +2776,9 @@ qemuProcessStartCPUs(virQEMUDriverPtr driver, virDomainObjPtr vm,
> qemuDomainObjPrivatePtr priv = vm->privateData;
> virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
>
>+ /* Bring up netdevs before starting CPUs */
>+ qemuInterfaceStartDevices(vm->def);
>+
> VIR_DEBUG("Using lock state '%s'", NULLSTR(priv->lockState));
> if (virDomainLockProcessResume(driver->lockManager, cfg->uri,
> vm, priv->lockState) < 0) {
>diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c
>index bfbecbf..efb31aa 100644
>--- a/src/util/virnetdevmacvlan.c
>+++ b/src/util/virnetdevmacvlan.c
>@@ -903,9 +903,11 @@ int virNetDevMacVLanCreateWithVPortProfile(const char *tgifname,
> goto link_del_exit;
> }
>
>- if (virNetDevSetOnline(cr_ifname, true) < 0) {
>- rc = -1;
>- goto disassociate_exit;
>+ if (flags & VIR_NETDEV_MACVLAN_CREATE_IFUP) {
>+ if (virNetDevSetOnline(cr_ifname, true) < 0) {
>+ rc = -1;
>+ goto disassociate_exit;
>+ }
> }
>
> if (flags & VIR_NETDEV_MACVLAN_CREATE_WITH_TAP) {
>diff --git a/src/util/virnetdevmacvlan.h b/src/util/virnetdevmacvlan.h
>index f7895b5..d43446f7 100644
>--- a/src/util/virnetdevmacvlan.h
>+++ b/src/util/virnetdevmacvlan.h
>@@ -44,6 +44,8 @@ typedef enum {
> VIR_NETDEV_MACVLAN_CREATE_NONE = 0,
> /* Create with a tap device */
> VIR_NETDEV_MACVLAN_CREATE_WITH_TAP = 1 << 0,
>+ /* Bring the interface up */
>+ VIR_NETDEV_MACVLAN_CREATE_IFUP = 1 << 1,
> } virNetDevMacVLanCreateFlags;
>
> int virNetDevMacVLanCreate(const char *ifname,
>--
>1.7.9.5
>
>--
>libvir-list mailing list
>libvir-list at redhat.com
>https://www.redhat.com/mailman/listinfo/libvir-list
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140723/1e1ebd36/attachment-0001.sig>
More information about the libvir-list
mailing list