On Tue, Aug 18, 2009 at 01:38:35PM +0100, Mark McLoughlin wrote:
https://bugzilla.redhat.com/517371
Matt Booth points out that if you use a non-existent bridge name when
start a guest you get a weird error message:
Failed to add tap interface 'vnet%d' to bridge 'virbr0'
and dev='vnet%d' appears in the dumpxml output.
Fix that by not including 'vnet%d' in the error message and freeing the
'vnet%d' string if adding the tap device to the bridge fails.
* src/qemu_conf.c, src/uml_conf.c: fix qemudNetworkIfaceConnect()
and umlConnectTapDevice() to not expose 'vnet%d' to the user
---
src/qemu_conf.c | 25 +++++++++++++++++--------
src/uml_conf.c | 28 +++++++++++++++++++---------
2 files changed, 36 insertions(+), 17 deletions(-)
diff --git a/src/qemu_conf.c b/src/qemu_conf.c
index 082f107..bcdab11 100644
--- a/src/qemu_conf.c
+++ b/src/qemu_conf.c
@@ -1038,6 +1038,7 @@ qemudNetworkIfaceConnect(virConnectPtr conn,
int err;
int tapfd = -1;
int vnet_hdr = 0;
+ int template_ifname = 0;
if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK) {
virNetworkPtr network = virNetworkLookupByName(conn,
@@ -1059,6 +1060,14 @@ qemudNetworkIfaceConnect(virConnectPtr conn,
return -1;
}
+ char ebuf[1024];
+ if (!driver->brctl && (err = brInit(&driver->brctl))) {
+ qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ _("cannot initialize bridge support: %s"),
+ virStrerror(err, ebuf, sizeof ebuf));
+ return -1;
+ }
+
if (!net->ifname ||
STRPREFIX(net->ifname, "vnet") ||
strchr(net->ifname, '%')) {
@@ -1067,14 +1076,8 @@ qemudNetworkIfaceConnect(virConnectPtr conn,
virReportOOMError(conn);
return -1;
}
- }
-
- char ebuf[1024];
- if (!driver->brctl && (err = brInit(&driver->brctl))) {
- qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
- _("cannot initialize bridge support: %s"),
- virStrerror(err, ebuf, sizeof ebuf));
- return -1;
+ /* avoid exposing vnet%d in dumpxml or error outputs */
+ template_ifname = 1;
}
if (qemuCmdFlags & QEMUD_CMD_FLAG_VNET_HDR &&
@@ -1088,12 +1091,18 @@ qemudNetworkIfaceConnect(virConnectPtr conn,
qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
_("Failed to add tap interface to bridge. "
"%s is not a bridge device"), brname);
+ } else if (template_ifname) {
+ qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ _("Failed to add tap interface to bridge '%s'
: %s"),
+ brname, virStrerror(err, ebuf, sizeof ebuf));
} else {
qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
_("Failed to add tap interface '%s' "
"to bridge '%s' : %s"),
net->ifname, brname, virStrerror(err, ebuf, sizeof
ebuf));
}
Both the original and new error reporting code here is using the wrong
error code here. Any error that has an associated errno value should
use virReportSystemError(conn, errno, fmt, args).
+ if (template_ifname)
+ VIR_FREE(net->ifname);
return -1;
}
diff --git a/src/uml_conf.c b/src/uml_conf.c
index 4f756d4..a4c434f 100644
--- a/src/uml_conf.c
+++ b/src/uml_conf.c
@@ -104,17 +104,10 @@ umlConnectTapDevice(virConnectPtr conn,
virDomainNetDefPtr net,
const char *bridge)
{
+ brControl *brctl = NULL;
int tapfd = -1;
+ int template_ifname = 0;
int err;
- brControl *brctl = NULL;
-
- if (!net->ifname ||
- STRPREFIX(net->ifname, "vnet") ||
- strchr(net->ifname, '%')) {
- VIR_FREE(net->ifname);
- if (!(net->ifname = strdup("vnet%d")))
- goto no_memory;
- }
if ((err = brInit(&brctl))) {
char ebuf[1024];
@@ -124,6 +117,16 @@ umlConnectTapDevice(virConnectPtr conn,
goto error;
}
+ if (!net->ifname ||
+ STRPREFIX(net->ifname, "vnet") ||
+ strchr(net->ifname, '%')) {
+ VIR_FREE(net->ifname);
+ if (!(net->ifname = strdup("vnet%d")))
+ goto no_memory;
+ /* avoid exposing vnet%d in dumpxml or error outputs */
+ template_ifname = 1;
+ }
+
if ((err = brAddTap(brctl, bridge,
&net->ifname, BR_TAP_PERSIST, &tapfd))) {
if (errno == ENOTSUP) {
@@ -131,6 +134,11 @@ umlConnectTapDevice(virConnectPtr conn,
umlReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
_("Failed to add tap interface to bridge. "
"%s is not a bridge device"), bridge);
+ } else if (template_ifname) {
+ char ebuf[1024];
+ umlReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ _("Failed to add tap interface to bridge '%s' :
%s"),
+ bridge, virStrerror(err, ebuf, sizeof ebuf));
} else {
char ebuf[1024];
umlReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
@@ -138,6 +146,8 @@ umlConnectTapDevice(virConnectPtr conn,
"to bridge '%s' : %s"),
net->ifname, bridge, virStrerror(err, ebuf, sizeof
ebuf));
}
+ if (template_ifname)
+ VIR_FREE(net->ifname);
Same errno issue here
THe patch looks OK in general though
Daniel
--
|: Red Hat, Engineering, London -o-
http://people.redhat.com/berrange/ :|
|:
http://libvirt.org -o-
http://virt-manager.org -o-
http://ovirt.org :|
|:
http://autobuild.org -o-
http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|