Re: [libvirt] [PATCH 3/5] macvtap support for libvirt -- qemu support

Please respond to veillard On Mon, Feb 08, 2010 at 02:37:18PM -0500, Stefan Berger wrote: > This part adds support for qemu making a macvtap tap device available > via file descriptor passed to qemu command line. This also attempts to > tear down the macvtap device when a VM terminates. This includes

> for attachment and detachment to/from running VM. > > Signed-off-by: Stefan Berger <stefanb(a)us.ibm.com&gt; [...] > > int > +qemudPhysIfaceConnect(virConnectPtr conn, > + virDomainNetDefPtr net, > + char *linkdev, > + int brmode) > +{ > + int rc; > +#if defined(WITH_MACVTAP) > + char *res_ifname = NULL; > + int hasBusyDev = 0; > + > + delMacvtapByMACAddress(conn, net->mac, &hasBusyDev); > + > + if (hasBusyDev) { > + virReportSystemError(NULL, errno, "%s", > + _("A macvtap with the same MAC address is in use")); > + return -1; > + } > + > + rc = openMacvtapTap(conn, net->ifname, net->mac, linkdev, brmode, > + &res_ifname); > + if (rc > 0) { since openMacvtapTap seems to return an fd, rc == 0 sounds a correct possible return, or am I missing something ?

> + VIR_FREE(net->ifname); > + net->ifname = res_ifname; > + } > +#else > + (void)net; > + (void)linkdev; > + (void)brmode; > + qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, > + "%s", _("No support for macvtap device")); > + rc = -1; > +#endif > + return rc; > +} > + Since qemudPhysIfaceConnect can return -1, 0 or > 0 all with different semantic, I think a comment describing the function and return value is in order.

> +int > qemudNetworkIfaceConnect(virConnectPtr conn, > struct qemud_driver *driver, > virDomainNetDefPtr net, > @@ -2520,6 +2558,7 @@ qemuBuildHostNetStr(virConnectPtr conn, > switch (net->type) { > case VIR_DOMAIN_NET_TYPE_NETWORK: > case VIR_DOMAIN_NET_TYPE_BRIDGE: > + case VIR_DOMAIN_NET_TYPE_DIRECT: > virBufferAddLit(&buf, "tap"); > virBufferVSprintf(&buf, "%cfd=%s", type_sep, tapfd); > type_sep = ','; Otherwise looks fine, ACK once resolved :-)

Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit

daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/