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 :| -- Libvir-list mailing list Libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list