7:43 p.m.
(NB: As I looked further into the details of this, I decided it would be
simpler for me to just post a few patches to take the place of this one
patch. I've left in the reasoning for a couple of those patches which I
had typed before I made the decision to just make a new series, but this
patch can just be dropped, as the series I just sent here:
https://www.redhat.com/archives/libvir-list/2020-December/msg00744.html
takes care of everything done here, plus some other things.)
On 12/13/20 8:50 PM, Shi Lei wrote:
Those functions that call virNetDevTapCreate don't need to
adding
'vnet%d' into ifname when it is empty, since virNetDevGenerateName
which is in virNetDevTapCreate can deal with it.
Signed-off-by: Shi Lei <shi_lei(a)massclouds.com>
---
src/bhyve/bhyve_command.c | 1 -
src/qemu/qemu_interface.c | 12 ------------
2 files changed, 13 deletions(-)
diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c
index 4cf98c0e..92b31a6e 100644
--- a/src/bhyve/bhyve_command.c
+++ b/src/bhyve/bhyve_command.c
@@ -83,7 +83,6 @@ bhyveBuildNetArgStr(const virDomainDef *def,
STRPREFIX(net->ifname, VIR_NET_GENERATED_VNET_PREFIX) ||
strchr(net->ifname, '%')) {
VIR_FREE(net->ifname);
- net->ifname = g_strdup(VIR_NET_GENERATED_VNET_PREFIX "%d");
}
Actually
1) this entire clause can be eliminated - from the start if the "if ("
to the "}" just before my comment - it is a remnant of code added all
the way back in 2007 (commit a8977b62ba), but we have been doing the
same clearing of "vnetN" generated names in the parser itself since
commit 282d135d (2008), and this was made more complete/consistent in
commit 77f72a861 (August 2019) which gives more details of the history
(in case you're interested, which you probably aren't, and shouldn't be
:-)). This is in Patch 2 of the patchset linked above.
2) Hmm. But I just noticed that in the case of the virNetDevCreateTap()
used by FreeBSD/OpenBSD, we actually had forgotten to switch it to use
virNetDevGenerateName() in your patches! I've remedied that in Patch 1
of the new patchset I linked above).
if (!dryRun) {
diff --git a/src/qemu/qemu_interface.c b/src/qemu/qemu_interface.c
index 197c0aa2..87cfb8fc 100644
--- a/src/qemu/qemu_interface.c
+++ b/src/qemu/qemu_interface.c
@@ -413,7 +413,6 @@ qemuInterfaceEthernetConnect(virDomainDefPtr def,
virMacAddr tapmac;
int ret = -1;
unsigned int tap_create_flags = VIR_NETDEV_TAP_CREATE_IFUP;
- bool template_ifname = false;
This bool was added by commit 2b1f67d418 from 2009, and is used to clear
out the autogenerated ifname in case of failure. We do want to preserve
that behavior (in order to avoid some unwanted "cleaning up what isn't
there" during failure), so while we can get rid of everything about
adding VIR_NET_GENERATED_VNET_PREFIX + "%d", we need to remember that we
generated this name, and clear it during a failure exit from the
function (see Patch 3 of my new patchset)
g_autoptr(virQEMUDriverConfig) cfg =
virQEMUDriverGetConfig(driver);
const char *tunpath = "/dev/net/tun";
const char *auditdev = tunpath;
@@ -459,9 +458,6 @@ qemuInterfaceEthernetConnect(virDomainDefPtr def,
STRPREFIX(net->ifname, VIR_NET_GENERATED_VNET_PREFIX) ||
strchr(net->ifname, '%')) {
VIR_FREE(net->ifname);
- net->ifname = g_strdup(VIR_NET_GENERATED_VNET_PREFIX "%d");
- /* avoid exposing vnet%d in getXMLDesc or error outputs */
- template_ifname = true;
}
if (virNetDevTapCreate(&net->ifname, tunpath, tapfd, tapfdSize,
tap_create_flags) < 0) {
@@ -512,8 +508,6 @@ qemuInterfaceEthernetConnect(virDomainDefPtr def,
virDomainAuditNetDevice(def, net, auditdev, false);
for (i = 0; i < tapfdSize && tapfd[i] >= 0; i++)
VIR_FORCE_CLOSE(tapfd[i]);
- if (template_ifname)
- VIR_FREE(net->ifname);
}
return ret;
@@ -541,7 +535,6 @@ qemuInterfaceBridgeConnect(virDomainDefPtr def,
const char *brname;
int ret = -1;
unsigned int tap_create_flags = VIR_NETDEV_TAP_CREATE_IFUP;
- bool template_ifname = false;
g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
const char *tunpath = "/dev/net/tun";
@@ -563,9 +556,6 @@ qemuInterfaceBridgeConnect(virDomainDefPtr def,
STRPREFIX(net->ifname, VIR_NET_GENERATED_VNET_PREFIX) ||
strchr(net->ifname, '%')) {
VIR_FREE(net->ifname);
- net->ifname = g_strdup(VIR_NET_GENERATED_VNET_PREFIX "%d");
- /* avoid exposing vnet%d in getXMLDesc or error outputs */
- template_ifname = true;
}
if (qemuInterfaceIsVnetCompatModel(net))
@@ -630,8 +620,6 @@ qemuInterfaceBridgeConnect(virDomainDefPtr def,
size_t i;
for (i = 0; i < *tapfdSize && tapfd[i] >= 0; i++)
VIR_FORCE_CLOSE(tapfd[i]);
- if (template_ifname)
- VIR_FREE(net->ifname);
}
return ret;