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@xxxxxxxxxxxxxxxxxx>
---
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@xxxxxxxxxxxxxxxxxx> + */ + +#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@xxxxxxxxxxxxxxxxxx> + */ + +#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@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list