[libvirt] [PATCH v3 4/4] FreeBSD: implement virNetDevTapCreate() and virNetDevTapDelete().

[Eric Blake]

On 01/26/2013 08:13 AM, Roman Bogorodskiy wrote:
> ---
>  src/util/virnetdevtap.c | 114 ++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 111 insertions(+), 3 deletions(-)
> 
> diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c
> index a884de1..36994fc 100644
> --- a/src/util/virnetdevtap.c
> +++ b/src/util/virnetdevtap.c
> @@ -103,7 +103,7 @@ virNetDevProbeVnetHdr(int tapfd)
>  #endif
>  
>  
> -#ifdef TUNSETIFF
> +#if defined(TUNSETIFF)

Pointless churn.

> @@ -230,7 +230,115 @@ cleanup:
>      VIR_FORCE_CLOSE(fd);
>      return ret;
>  }
> -#else /* ! TUNSETIFF */
> +#elif defined(__FreeBSD__)

Again, what is the REAL feature that we are probing here, instead of
hard-coding things to an OS probe?

> +int virNetDevTapCreate(char **ifname,
> +                       int *tapfd,
> +                       unsigned int flags ATTRIBUTE_UNUSED)
> +{
> +    int s;
> +    struct ifreq ifr;
> +    int ret = -1;
> +    char *newifname = NULL;
> +
> +    /* As FreeBSD determines interface type by name,
> +     * we have to create 'tap' interface first and
> +     * then rename it to 'vnet'
> +     */
> +    if ((s = virNetDevSetupControl("tap", &ifr)) < 0)
> +        return -1;
> +
> +    if (ioctl(s, SIOCIFCREATE2, &ifr) < 0) {

Is the existence of SIOCIFCREATE2 the right feature to be probing for?

> +        virReportSystemError(errno, "%s",
> +                             _("Unable to create tap device"));
> +        goto cleanup;
> +    }
> +
> +    /* In case we were given exact interface name (e.g. 'vnetN'),
> +     * we just rename to it. If we have format string like
> +     * 'vnet%d', we need to find the first available name that
> +     * matches this pattern
> +     */
> +    if (strstr(*ifname, "%d") != NULL) {
> +        int i;
> +        for (i = 0; i <= IF_MAXUNIT; i++) {
> +            virBuffer newname = VIR_BUFFER_INITIALIZER;
> +            virBufferAsprintf(&newname, *ifname, i);
> +
> +            if (virBufferError(&newname)) {
> +                virBufferFreeAndReset(&newname);
> +                virReportOOMError();
> +                goto cleanup;
> +            }

virBuffer is useful for concatenation.  But your use case is one-shot
formatting; in which case I'd use virAsprintf instead of virBufferAsprintf:

char *newname;
if (virAsprintf(&newname, *ifname, i) < 0) {
    virReportOOMError();
    goto cleanup;
}

> +
> +            if (virNetDevExists(virBufferCurrentContent(&newname)) == 0) {
> +                newifname = strdup(virBufferContentAndReset(&newname));
> +                break;
> +            }

at which point, you don't have to strdup(); newname is already the right
allocated string.

> +        }
> +        if (newifname) {
> +            VIR_FREE(*ifname);
> +            *ifname = newifname;
> +        } else {
> +            virReportError(VIR_ERR_INTERNAL_ERROR,
> +                           _("Failed to generate new name for interface %s"),
> +                           ifr.ifr_name);
> +            goto cleanup;
> +        }
> +    }

> +
> +int virNetDevTapDelete(const char *ifname)

int
virNetDevTapDelete(const char *ifname)

> +
> +#else /* !defined(__FreeBSD__) */

This comment isn't quite right; it is more like:

#else /* !TUNSETIFF && !__FreeBSD__ */

or whatever feature check we actually use.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 621 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20130204/c6f1d77d/attachment-0001.sig>