Daniel Veillard <veillard@redhat.com> wrote on 02/11/2010 05:12:07 AM:
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
support
for attachment and detachment to/from running VM.
Signed-off-by: Stefan Berger <stefanb@us.ibm.com> [...]
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 ?
I'll fix this. Should be rc >= 0, with 0 as a possible filedescriptor.
+ 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.
Added description of function in upcoming patch.
+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 :-)
Will repost. Stefan
Daniel
-- Daniel Veillard | libxml Gnome XML XSLT toolkit
daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/