[libvirt] [PATCH] Don't expose 'vnet%d' to the user
Daniel P. Berrange
berrange at redhat.com
Tue Aug 18 12:57:24 UTC 2009
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 :|
More information about the libvir-list
mailing list