[libvirt] [PATCH v3 0/2] Create TAP devices for ethernet vNICs
This is a 3rd version of v2: https://www.redhat.com/archives/libvir-list/2014-November/msg01066.html Michal Privoznik (1): qemuxml2argvtest: Load virSecurityDriverNop Vasiliy Tolstov (1): qemu: Automaticly create tap device for VIR_DOMAIN_NET_TYPE_ETHERNET src/qemu/qemu_command.c | 86 ++++++++++++++-------- src/qemu/qemu_hotplug.c | 10 +-- src/qemu/qemu_process.c | 4 + .../qemuxml2argv-graphics-spice-timeout.args | 2 +- .../qemuxml2argv-net-eth-names.args | 7 +- tests/qemuxml2argvdata/qemuxml2argv-net-eth.args | 4 +- tests/qemuxml2argvmock.c | 28 +++++++ tests/qemuxml2argvtest.c | 7 ++ 8 files changed, 104 insertions(+), 44 deletions(-) -- 2.0.4
In the next patch we are gonna be playing with networks, creating TUN/TAP devices. However, the code invokes virSecurityManagerSetTapFDLabel() over the FDs of TUN/TAP devices when the command line is constructed. This makes sense as we certainly want the FDs labelled correctly. However, due to the nature of how's the code written a security driver is required. So we need to load a dummy security driver that always claims success. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tests/qemuxml2argvtest.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index c7b8ecf..b22b207 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -493,6 +493,8 @@ mymain(void) { int ret = 0; bool skipLegacyCPUs = false; + virSecurityManagerPtr mgr = NULL; + virSecurityManagerPtr stack = NULL; abs_top_srcdir = getenv("abs_top_srcdir"); if (!abs_top_srcdir) @@ -544,6 +546,10 @@ mymain(void) driver.config->spiceTLS = 1; if (VIR_STRDUP_QUIET(driver.config->spicePassword, "123456") < 0) return EXIT_FAILURE; + if (!(mgr = virSecurityManagerNew("none", QEMU_DRIVER_NAME, false, false, false)) || + !(stack = virSecurityManagerNewStack(mgr))) + return EXIT_FAILURE; + driver.securityManager = stack; # define DO_TEST_FULL(name, migrateFrom, migrateFd, flags, ...) \ do { \ @@ -1527,6 +1533,7 @@ mymain(void) virObjectUnref(driver.config); virObjectUnref(driver.caps); virObjectUnref(driver.xmlopt); + virObjectUnref(driver.securityManager); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } -- 2.0.4
From: Vasiliy Tolstov <v.tolstov@selfip.ru> If a user doesn't specify script in network type ethernet, assume that he/she needs a simple tap device created by libvirt. This commit does not need to run external script to create tap device or add root to qemu process. Moreover, some functions need to be mocked now for qemuxml2argvtest, e.g. virNetDevTapCreate() or virNetDevSetOnline(). Signed-off-by: Vasiliy Tolstov <v.tolstov@selfip.ru> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_command.c | 86 ++++++++++++++-------- src/qemu/qemu_hotplug.c | 10 +-- src/qemu/qemu_process.c | 4 + .../qemuxml2argv-graphics-spice-timeout.args | 2 +- .../qemuxml2argv-net-eth-names.args | 7 +- tests/qemuxml2argvdata/qemuxml2argv-net-eth.args | 4 +- tests/qemuxml2argvmock.c | 28 +++++++ 7 files changed, 97 insertions(+), 44 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 1831323..0d29d3f 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -313,7 +313,7 @@ qemuNetworkIfaceConnect(virDomainDefPtr def, } else if (actualType == VIR_DOMAIN_NET_TYPE_BRIDGE) { if (VIR_STRDUP(brname, virDomainNetGetActualBridgeName(net)) < 0) return ret; - } else { + } else if (actualType != VIR_DOMAIN_NET_TYPE_ETHERNET) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Network type %d is not supported"), virDomainNetGetActualType(net)); @@ -335,30 +335,40 @@ qemuNetworkIfaceConnect(virDomainDefPtr def, tap_create_flags |= VIR_NETDEV_TAP_CREATE_VNET_HDR; } - if (cfg->privileged) { - if (virNetDevTapCreateInBridgePort(brname, &net->ifname, &net->mac, - def->uuid, tunpath, tapfd, *tapfdSize, - virDomainNetGetActualVirtPortProfile(net), - virDomainNetGetActualVlan(net), - tap_create_flags) < 0) { + if (actualType == VIR_DOMAIN_NET_TYPE_ETHERNET) { + if (virNetDevTapCreate(&net->ifname, tunpath, tapfd, *tapfdSize, + tap_create_flags) < 0) { virDomainAuditNetDevice(def, net, tunpath, false); goto cleanup; } + if (virNetDevSetOnline(net->ifname, !!(tap_create_flags & VIR_NETDEV_TAP_CREATE_IFUP)) < 0) + goto cleanup; } else { - if (qemuCreateInBridgePortWithHelper(cfg, brname, - &net->ifname, - tapfd, tap_create_flags) < 0) { - virDomainAuditNetDevice(def, net, tunpath, false); - goto cleanup; + if (cfg->privileged) { + if (virNetDevTapCreateInBridgePort(brname, &net->ifname, &net->mac, + def->uuid, tunpath, tapfd, *tapfdSize, + virDomainNetGetActualVirtPortProfile(net), + virDomainNetGetActualVlan(net), + tap_create_flags) < 0) { + virDomainAuditNetDevice(def, net, tunpath, false); + goto cleanup; + } + } else { + if (qemuCreateInBridgePortWithHelper(cfg, brname, + &net->ifname, + tapfd, tap_create_flags) < 0) { + virDomainAuditNetDevice(def, net, tunpath, false); + goto cleanup; + } + /* qemuCreateInBridgePortWithHelper can only create a single FD */ + if (*tapfdSize > 1) { + VIR_WARN("Ignoring multiqueue network request"); + *tapfdSize = 1; + } } - /* qemuCreateInBridgePortWithHelper can only create a single FD */ - if (*tapfdSize > 1) { - VIR_WARN("Ignoring multiqueue network request"); - *tapfdSize = 1; - } - } - virDomainAuditNetDevice(def, net, tunpath, true); + virDomainAuditNetDevice(def, net, tunpath, true); + } if (cfg->macFilter && ebtablesAddForwardAllowIn(driver->ebtables, @@ -4534,18 +4544,32 @@ qemuBuildHostNetStr(virDomainNetDefPtr net, break; case VIR_DOMAIN_NET_TYPE_ETHERNET: - virBufferAddLit(&buf, "tap"); + virBufferAddLit(&buf, "tap"); + type_sep = ','; + if (net->script) { if (net->ifname) { - virBufferAsprintf(&buf, "%cifname=%s", type_sep, net->ifname); - type_sep = ','; + virBufferAsprintf(&buf, "%cifname=%s", type_sep, net->ifname); + type_sep = ','; } - if (net->script) { - virBufferAsprintf(&buf, "%cscript=%s", type_sep, - net->script); - type_sep = ','; + virBufferAsprintf(&buf, "%cscript=%s", type_sep, net->script); + type_sep = ','; + } else { + /* for one tapfd 'fd=' shall be used, + * for more than one 'fds=' is the right choice */ + if (tapfdSize == 1) { + virBufferAsprintf(&buf, "%cfd=%s", type_sep, tapfd[0]); + } else { + virBufferAsprintf(&buf, "%cfds=", type_sep); + for (i = 0; i < tapfdSize; i++) { + if (i) + virBufferAddChar(&buf, ':'); + virBufferAdd(&buf, tapfd[i], -1); + } } - is_tap = true; - break; + type_sep = ','; + } + is_tap = true; + break; case VIR_DOMAIN_NET_TYPE_CLIENT: virBufferAsprintf(&buf, "socket%cconnect=%s:%d", @@ -7366,7 +7390,8 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd, /* Currently nothing besides TAP devices supports multiqueue. */ if (net->driver.virtio.queues > 0 && !(actualType == VIR_DOMAIN_NET_TYPE_NETWORK || - actualType == VIR_DOMAIN_NET_TYPE_BRIDGE)) { + actualType == VIR_DOMAIN_NET_TYPE_BRIDGE || + actualType == VIR_DOMAIN_NET_TYPE_ETHERNET)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Multiqueue network is not supported for: %s"), virDomainNetTypeToString(actualType)); @@ -7374,7 +7399,8 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd, } if (actualType == VIR_DOMAIN_NET_TYPE_NETWORK || - actualType == VIR_DOMAIN_NET_TYPE_BRIDGE) { + actualType == VIR_DOMAIN_NET_TYPE_BRIDGE || + actualType == VIR_DOMAIN_NET_TYPE_ETHERNET) { tapfdSize = net->driver.virtio.queues; if (!tapfdSize) tapfdSize = 1; diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 9467d7d..1175259 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -908,7 +908,8 @@ int qemuDomainAttachNetDevice(virConnectPtr conn, } if (actualType == VIR_DOMAIN_NET_TYPE_BRIDGE || - actualType == VIR_DOMAIN_NET_TYPE_NETWORK) { + actualType == VIR_DOMAIN_NET_TYPE_NETWORK || + actualType == VIR_DOMAIN_NET_TYPE_ETHERNET) { tapfdSize = vhostfdSize = net->driver.virtio.queues; if (!tapfdSize) tapfdSize = vhostfdSize = 1; @@ -939,13 +940,6 @@ int qemuDomainAttachNetDevice(virConnectPtr conn, iface_connected = true; if (qemuOpenVhostNet(vm->def, net, priv->qemuCaps, vhostfd, &vhostfdSize) < 0) goto cleanup; - } else if (actualType == VIR_DOMAIN_NET_TYPE_ETHERNET) { - vhostfdSize = 1; - if (VIR_ALLOC(vhostfd) < 0) - goto cleanup; - *vhostfd = -1; - if (qemuOpenVhostNet(vm->def, net, priv->qemuCaps, vhostfd, &vhostfdSize) < 0) - goto cleanup; } /* Set Bandwidth */ diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index a14b6f7..0dbf7fb 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -5064,6 +5064,10 @@ void qemuProcessStop(virQEMUDriverPtr driver, cfg->stateDir)); VIR_FREE(net->ifname); break; + case VIR_DOMAIN_NET_TYPE_ETHERNET: + ignore_value(virNetDevTapDelete(net->ifname, net->backend.tap)); + VIR_FREE(net->ifname); + break; case VIR_DOMAIN_NET_TYPE_BRIDGE: case VIR_DOMAIN_NET_TYPE_NETWORK: #ifdef VIR_NETDEV_TAP_REQUIRE_MANUAL_CLEANUP diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-timeout.args b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-timeout.args index 8b5d9ee..a5f1486 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-timeout.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-timeout.args @@ -10,7 +10,7 @@ id=virtio-disk0 \ media=cdrom,id=drive-ide0-1-0 \ -device ide-drive,bus=ide.1,unit=0,drive=drive-ide0-1-0,id=ide0-1-0 \ -device rtl8139,vlan=0,id=net0,mac=52:54:00:71:70:89,bus=pci.0,addr=0x7 \ --net tap,script=/etc/qemu-ifup,vlan=0,name=hostnet0 -serial pty \ +-net tap,ifname=vnet0,script=/etc/qemu-ifup,vlan=0,name=hostnet0 -serial pty \ -device usb-tablet,id=input0 \ -spice port=5900 -vga std \ -device AC97,id=sound0,bus=pci.0,addr=0x3 \ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-eth-names.args b/tests/qemuxml2argvdata/qemuxml2argv-net-eth-names.args index 6146e84..643b313 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-net-eth-names.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-eth-names.args @@ -2,7 +2,8 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ /usr/bin/qemu -S -M \ pc -m 214 -smp 1 -nographic -monitor unix:/tmp/test-monitor,server,nowait \ -no-acpi -boot c -usb -hda /dev/HostVG/QEMUGuest1 -net nic,\ -macaddr=00:11:22:33:44:55,vlan=0,model=rtl8139,name=net0 -net tap,\ -script=/etc/qemu-ifup,vlan=0,name=hostnet0 -net nic,macaddr=00:11:22:33:44:56,\ -vlan=1,model=e1000,name=net1 -net tap,script=/etc/qemu-ifup,vlan=1,name=hostnet1 \ +macaddr=00:11:22:33:44:55,vlan=0,model=rtl8139,name=net0 \ +-net tap,ifname=vnet0,script=/etc/qemu-ifup,vlan=0,name=hostnet0 \ +-net nic,macaddr=00:11:22:33:44:56,vlan=1,model=e1000,name=net1 \ +-net tap,ifname=vnet0,script=/etc/qemu-ifup,vlan=1,name=hostnet1 \ -serial none -parallel none diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-eth.args b/tests/qemuxml2argvdata/qemuxml2argv-net-eth.args index 28a094b..7a75850 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-net-eth.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-eth.args @@ -2,5 +2,5 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ /usr/bin/qemu -S -M \ pc -m 214 -smp 1 -nographic -monitor unix:/tmp/test-monitor,server,nowait \ -no-acpi -boot c -usb -hda /dev/HostVG/QEMUGuest1 -net nic,\ -macaddr=00:11:22:33:44:55,vlan=0,model=rtl8139 -net tap,script=/etc/qemu-ifup,\ -vlan=0 -serial none -parallel none +macaddr=00:11:22:33:44:55,vlan=0,model=rtl8139 \ +-net tap,ifname=vnet0,script=/etc/qemu-ifup,vlan=0 -serial none -parallel none diff --git a/tests/qemuxml2argvmock.c b/tests/qemuxml2argvmock.c index eccf4b0..2d63659 100644 --- a/tests/qemuxml2argvmock.c +++ b/tests/qemuxml2argvmock.c @@ -23,6 +23,11 @@ #include "internal.h" #include "virnuma.h" #include <time.h> +#include "virnetdevtap.h" +#include "virstring.h" +#include "virnetdev.h" + +#define VIR_FROM_THIS VIR_FROM_QEMU time_t time(time_t *t) { @@ -51,3 +56,26 @@ virNumaNodeIsAvailable(int node) return node >= 0 && node <= virNumaGetMaxNode(); } #endif /* WITH_NUMACTL && HAVE_NUMA_BITMASK_ISBITSET */ + +int virNetDevTapCreate(char **ifname, + const char *tunpath ATTRIBUTE_UNUSED, + int *tapfd, + int tapfdSize, + unsigned int fakeflags ATTRIBUTE_UNUSED) +{ + size_t i; + + if (virAsprintf(ifname, *ifname, 0) < 0) + return -1; + + for (i = 0; i < tapfdSize; i++) + tapfd[i] = i + STDERR_FILENO + 1; + + return 0; +} + +int virNetDevSetOnline(const char *ifname ATTRIBUTE_UNUSED, + bool online ATTRIBUTE_UNUSED) +{ + return 0; +} -- 2.0.4
On Fri, Dec 05, 2014 at 12:12:46PM +0100, Michal Privoznik wrote:
From: Vasiliy Tolstov <v.tolstov@selfip.ru>
If a user doesn't specify script in network type ethernet, assume that he/she needs a simple tap device created by libvirt. This commit does not need to run external script to create tap device or add root to qemu process. Moreover, some functions need to be mocked now for qemuxml2argvtest, e.g. virNetDevTapCreate() or virNetDevSetOnline().
Hmm, even if the user does provide a script, perhaps libvirt could create the TAP device *and* run the script itself. This would finally allow us to run QEMU unprivileged with type=ethernet in all cases. eg take QEMU entirely out of the picture for NIC setup Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 12/05/2014 04:18 AM, Daniel P. Berrange wrote:
On Fri, Dec 05, 2014 at 12:12:46PM +0100, Michal Privoznik wrote:
From: Vasiliy Tolstov <v.tolstov@selfip.ru>
If a user doesn't specify script in network type ethernet, assume that he/she needs a simple tap device created by libvirt. This commit does not need to run external script to create tap device or add root to qemu process. Moreover, some functions need to be mocked now for qemuxml2argvtest, e.g. virNetDevTapCreate() or virNetDevSetOnline().
Hmm, even if the user does provide a script, perhaps libvirt could create the TAP device *and* run the script itself. This would finally allow us to run QEMU unprivileged with type=ethernet in all cases. eg take QEMU entirely out of the picture for NIC setup
Don't we still have to mark things as tainted, and be careful that executing an arbitrary script is not going to hose the host if a less-privileged user (such as via fine-grained ACLs) passes a suspicious script? -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Fri, Dec 05, 2014 at 09:57:36AM -0700, Eric Blake wrote:
On 12/05/2014 04:18 AM, Daniel P. Berrange wrote:
On Fri, Dec 05, 2014 at 12:12:46PM +0100, Michal Privoznik wrote:
From: Vasiliy Tolstov <v.tolstov@selfip.ru>
If a user doesn't specify script in network type ethernet, assume that he/she needs a simple tap device created by libvirt. This commit does not need to run external script to create tap device or add root to qemu process. Moreover, some functions need to be mocked now for qemuxml2argvtest, e.g. virNetDevTapCreate() or virNetDevSetOnline().
Hmm, even if the user does provide a script, perhaps libvirt could create the TAP device *and* run the script itself. This would finally allow us to run QEMU unprivileged with type=ethernet in all cases. eg take QEMU entirely out of the picture for NIC setup
Don't we still have to mark things as tainted, and be careful that executing an arbitrary script is not going to hose the host if a less-privileged user (such as via fine-grained ACLs) passes a suspicious script?
Well the choice of script path is constrained based on the privilege required to define XML config. That is already effectively equivalent to root, so I don't think we need be concerned about what the script actually does from a security POV. I'm still somewhat inclined to leave the config as "tainted" though, simply because of the fact that the scripts are an opaque black box that we don't have visiblity into. So they definitely have the potential to screw up host or guest config in ways that people responding to support tickets should be made aware of by the tainting flag. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
2014-12-05 20:22 GMT+03:00 Daniel P. Berrange <berrange@redhat.com>:
I'm still somewhat inclined to leave the config as "tainted" though, simply because of the fact that the scripts are an opaque black box that we don't have visiblity into. So they definitely have the potential to screw up host or guest config in ways that people responding to support tickets should be made aware of by the tainting flag.
I'm resend new patch, what thread i need to reference when send patch? -- Vasiliy Tolstov, e-mail: v.tolstov@selfip.ru jabber: vase@selfip.ru
On 12/05/2014 06:12 AM, Michal Privoznik wrote:
@@ -7374,7 +7399,8 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd, }
if (actualType == VIR_DOMAIN_NET_TYPE_NETWORK || - actualType == VIR_DOMAIN_NET_TYPE_BRIDGE) { + actualType == VIR_DOMAIN_NET_TYPE_BRIDGE || + actualType == VIR_DOMAIN_NET_TYPE_ETHERNET) { tapfdSize = net->driver.virtio.queues; if (!tapfdSize) tapfdSize = 1;
So this patch causes libvirt to *always* create a tap device in the traditional manner for type='ethernet'. I wonder if we can safely do this without unknowingly breaking some strange functionality. In particular, what if someone is using type='ethernet' and a script to create some *other* kind of device that outwardly appears to be a tap device, but is created in a different manner - currently you can do this by using type='ethernet' and calling your strange "create my Franken-tap" command from the script. Once this patch is in, you'll no longer be able to do this. (As a matter of fact, I'm getting some sort of phantom memory about someone doing that for some different kind of virtual switch, or possibly some piece of hardware. I can't remember the details though, and it's possible that I'm mistaken - maybe they *were* just creating a standard tap device, but then doing strange things to it.)
On 12/5/14, 10:43 AM, "Laine Stump" <laine@laine.org> wrote:
On 12/05/2014 06:12 AM, Michal Privoznik wrote:
@@ -7374,7 +7399,8 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd, }
if (actualType == VIR_DOMAIN_NET_TYPE_NETWORK || - actualType == VIR_DOMAIN_NET_TYPE_BRIDGE) { + actualType == VIR_DOMAIN_NET_TYPE_BRIDGE || + actualType == VIR_DOMAIN_NET_TYPE_ETHERNET) { tapfdSize = net->driver.virtio.queues; if (!tapfdSize) tapfdSize = 1;
So this patch causes libvirt to *always* create a tap device in the traditional manner for type='ethernet'. I wonder if we can safely do this without unknowingly breaking some strange functionality. In particular, what if someone is using type='ethernet' and a script to create some *other* kind of device that outwardly appears to be a tap device, but is created in a different manner - currently you can do this by using type='ethernet' and calling your strange "create my Franken-tap" command from the script. Once this patch is in, you'll no longer be able to do this.
(As a matter of fact, I'm getting some sort of phantom memory about someone doing that for some different kind of virtual switch, or possibly some piece of hardware. I can't remember the details though, and it's possible that I'm mistaken - maybe they *were* just creating a standard tap device, but then doing strange things to it.)
In Open Contrail (www.opencontrail.org), we use this feature where tap interface is created first, so that we know the name of the tap device a priori, before creating the vm. So, this patch will break existing code. Anirban
2014-12-06 2:37 GMT+03:00 Anirban Chakraborty <abchak@juniper.net>:
In Open Contrail (www.opencontrail.org), we use this feature where tap interface is created first, so that we know the name of the tap device a priori, before creating the vm. So, this patch will break existing code.
May be we can add some flag like managed true/false ? and create tap only if managed ? -- Vasiliy Tolstov, e-mail: v.tolstov@selfip.ru jabber: vase@selfip.ru
On 12/6/14, 2:51 AM, "Vasiliy Tolstov" <v.tolstov@selfip.ru> wrote:
2014-12-06 2:37 GMT+03:00 Anirban Chakraborty <abchak@juniper.net>:
In Open Contrail (www.opencontrail.org), we use this feature where tap interface is created first, so that we know the name of the tap device a priori, before creating the vm. So, this patch will break existing code.
May be we can add some flag like managed true/false ? and create tap only if managed ?
Yes, that will work. Anirban
2014-12-08 23:51 GMT+03:00 Anirban Chakraborty <abchak@juniper.net>:
Yes, that will work.
So as i see two different point of view... I want to get this patch in next libvirt release.... What i need to do? -- Vasiliy Tolstov, e-mail: v.tolstov@selfip.ru jabber: vase@selfip.ru
On Fri, Dec 05, 2014 at 11:37:05PM +0000, Anirban Chakraborty wrote:
On 12/5/14, 10:43 AM, "Laine Stump" <laine@laine.org> wrote:
On 12/05/2014 06:12 AM, Michal Privoznik wrote:
@@ -7374,7 +7399,8 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd, }
if (actualType == VIR_DOMAIN_NET_TYPE_NETWORK || - actualType == VIR_DOMAIN_NET_TYPE_BRIDGE) { + actualType == VIR_DOMAIN_NET_TYPE_BRIDGE || + actualType == VIR_DOMAIN_NET_TYPE_ETHERNET) { tapfdSize = net->driver.virtio.queues; if (!tapfdSize) tapfdSize = 1;
So this patch causes libvirt to *always* create a tap device in the traditional manner for type='ethernet'. I wonder if we can safely do this without unknowingly breaking some strange functionality. In particular, what if someone is using type='ethernet' and a script to create some *other* kind of device that outwardly appears to be a tap device, but is created in a different manner - currently you can do this by using type='ethernet' and calling your strange "create my Franken-tap" command from the script. Once this patch is in, you'll no longer be able to do this.
(As a matter of fact, I'm getting some sort of phantom memory about someone doing that for some different kind of virtual switch, or possibly some piece of hardware. I can't remember the details though, and it's possible that I'm mistaken - maybe they *were* just creating a standard tap device, but then doing strange things to it.)
In Open Contrail (www.opencontrail.org), we use this feature where tap interface is created first, so that we know the name of the tap device a priori, before creating the vm. So, this patch will break existing code.
Do you actally pre-create the TAP device though, or merely ask for a particular choice of name. AFAIK, then using type=ethernet, QEMU will always attempt to create the TAP device itself, honouring any name given. Basically whatever QEMU does for type=ethernet, libvirt should copy in a 100% functionally equivalent manner. We simply want to move the functionality up a level. So there should be no regressions if done correctly. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 12/8/14, 2:10 AM, "Daniel P. Berrange" <berrange@redhat.com> wrote:
On Fri, Dec 05, 2014 at 11:37:05PM +0000, Anirban Chakraborty wrote:
On 12/5/14, 10:43 AM, "Laine Stump" <laine@laine.org> wrote:
On 12/05/2014 06:12 AM, Michal Privoznik wrote:
@@ -7374,7 +7399,8 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd, }
if (actualType == VIR_DOMAIN_NET_TYPE_NETWORK || - actualType == VIR_DOMAIN_NET_TYPE_BRIDGE) { + actualType == VIR_DOMAIN_NET_TYPE_BRIDGE || + actualType == VIR_DOMAIN_NET_TYPE_ETHERNET) { tapfdSize = net->driver.virtio.queues; if (!tapfdSize) tapfdSize = 1;
So this patch causes libvirt to *always* create a tap device in the traditional manner for type='ethernet'. I wonder if we can safely do this without unknowingly breaking some strange functionality. In particular, what if someone is using type='ethernet' and a script to create some *other* kind of device that outwardly appears to be a tap device, but is created in a different manner - currently you can do this by using type='ethernet' and calling your strange "create my Franken-tap" command from the script. Once this patch is in, you'll no longer be able to do this.
(As a matter of fact, I'm getting some sort of phantom memory about someone doing that for some different kind of virtual switch, or possibly some piece of hardware. I can't remember the details though, and it's possible that I'm mistaken - maybe they *were* just creating a standard tap device, but then doing strange things to it.)
In Open Contrail (www.opencontrail.org), we use this feature where tap interface is created first, so that we know the name of the tap device a priori, before creating the vm. So, this patch will break existing code.
Do you actally pre-create the TAP device though, or merely ask for a particular choice of name. AFAIK, then using type=ethernet, QEMU will always attempt to create the TAP device itself, honouring any name given. Basically whatever QEMU does for type=ethernet, libvirt should copy in a 100% functionally equivalent manner. We simply want to move the functionality up a level. So there should be no regressions if done correctly.
It is created as part of openstack nova vif creation process. Nova libvirt driver passes in the vif name to the network provider (in this case, contrail), which in turn uses it internally for various configuration purposes. What you are saying is that passing a preassigned name to libvirt for tap device creation would work. In that case, it should be fine. Anirban
participants (6)
-
Anirban Chakraborty -
Daniel P. Berrange -
Eric Blake -
Laine Stump -
Michal Privoznik -
Vasiliy Tolstov