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
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list