Re: [PATCH v3] qemu: set default vhost-user ifname

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



I think I half assumed this would have been pushed already, but seeing
as it wasn't...

On 12/22/2016 04:45 AM, Michal Privoznik wrote:
> Based on work of Mehdi Abaakouk <sileht@xxxxxxxxxx>.
> 
> When parsing vhost-user interface XML and no ifname is found we
> can try to fill it in in post parse callback. The way this works
> is we try to make up interface name from given socket path and
> then ask openvswitch wether it knows the interface.

s/wether/whether

> 
> Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
> ---
> 
> diff to v2:
> - All the review points from v2 worked in
> 
>  src/libvirt_private.syms                           |  1 +
>  src/qemu/qemu_domain.c                             | 21 +++++---
>  src/util/virnetdevopenvswitch.c                    | 58 ++++++++++++++++++++++
>  src/util/virnetdevopenvswitch.h                    |  4 ++
>  tests/Makefile.am                                  |  7 +++
>  tests/qemuxml2argvmock.c                           |  8 +++
>  tests/qemuxml2xmlmock.c                            | 33 ++++++++++++
>  .../qemuxml2xmlout-net-vhostuser.xml               |  2 +
>  tests/qemuxml2xmltest.c                            |  2 +-
>  9 files changed, 129 insertions(+), 7 deletions(-)
>  create mode 100644 tests/qemuxml2xmlmock.c
> 


> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 2d23e462d..9e4e8f83f 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -2057,6 +2057,7 @@ virNetDevMidonetUnbindPort;
>  # util/virnetdevopenvswitch.h
>  virNetDevOpenvswitchAddPort;
>  virNetDevOpenvswitchGetMigrateData;
> +virNetDevOpenvswitchGetVhostuserIfname;
>  virNetDevOpenvswitchInterfaceStats;
>  virNetDevOpenvswitchRemovePort;
>  virNetDevOpenvswitchSetMigrateData;
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index acfa69589..eed5745b6 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -41,6 +41,7 @@
>  #include "domain_addr.h"
>  #include "domain_event.h"
>  #include "virtime.h"
> +#include "virnetdevopenvswitch.h"
>  #include "virstoragefile.h"
>  #include "virstring.h"
>  #include "virthreadjob.h"
> @@ -3028,12 +3029,20 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev,
>                                            def->emulator);
>      }
>  
> -    if (dev->type == VIR_DOMAIN_DEVICE_NET &&
> -        dev->data.net->type != VIR_DOMAIN_NET_TYPE_HOSTDEV &&
> -        !dev->data.net->model) {
> -        if (VIR_STRDUP(dev->data.net->model,
> -                       qemuDomainDefaultNetModel(def, qemuCaps)) < 0)
> -            goto cleanup;
> +    if (dev->type == VIR_DOMAIN_DEVICE_NET) {
> +        if (dev->data.net->type != VIR_DOMAIN_NET_TYPE_HOSTDEV &&
> +            !dev->data.net->model) {
> +            if (VIR_STRDUP(dev->data.net->model,
> +                           qemuDomainDefaultNetModel(def, qemuCaps)) < 0)
> +                goto cleanup;
> +        }
> +        if (dev->data.net->type == VIR_DOMAIN_NET_TYPE_VHOSTUSER &&
> +            !dev->data.net->ifname) {
> +            if (virNetDevOpenvswitchGetVhostuserIfname(
> +                   dev->data.net->data.vhostuser->data.nix.path,
> +                   &dev->data.net->ifname) < 0)
> +                goto cleanup;
> +        }
>      }
>  
>      /* set default disk types and drivers */
> diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c
> index f003b3b13..d93653317 100644
> --- a/src/util/virnetdevopenvswitch.c
> +++ b/src/util/virnetdevopenvswitch.c
> @@ -377,3 +377,61 @@ virNetDevOpenvswitchInterfaceStats(const char *ifname,
>      virCommandFree(cmd);
>      return ret;
>  }
> +
> +/**
> + * virNetDevOpenvswitchVhostuserGetIfname:
> + * @path: the path of the unix socket
> + * @ifname: the retrieved name of the interface
> + *
> + * Retreives the ovs ifname from vhostuser unix socket path.
> + *
> + * Returns: 1 if interface is an openvswitch interface,
> + *          0 if it is not, but no other error occurred,
> + *         -1 otherwise.
> + */
> +int
> +virNetDevOpenvswitchGetVhostuserIfname(const char *path,
> +                                       char **ifname)
> +{
> +    virCommandPtr cmd = NULL;
> +    char *tmpIfname = NULL;
> +    char **tokens = NULL;
> +    size_t ntokens = 0;
> +    int status;
> +    int ret = -1;
> +
> +    /* Openvswitch vhostuser path are hardcoded to
> +     * /<runstatedir>/openvswitch/<ifname>
> +     * for example: /var/run/openvswitch/dpdkvhostuser0
> +     *
> +     * so we pick the filename and check it's a openvswitch interface
> +     */
> +    if ((tokens = virStringSplitCount(path, "/", 0, &ntokens))) {

Seems like a lot of extra work just to pull off the last element of the
path. Why not something using strrchr?

ACK in principal - I think you can decide whether strrchr would be more
useful since it seems you can guarantee that @path has at least a '/' in
it...   Probably wouldn't need to strdup below either...

John

> +        if (VIR_STRDUP(tmpIfname, tokens[ntokens - 1]) < 0)
> +            goto cleanup;
> +    }
> +
> +    if (!tmpIfname) {
> +        ret = 0;
> +        goto cleanup;
> +    }
> +
> +    cmd = virCommandNewArgList(OVSVSCTL, "--timeout=5", "get", "Interface",
> +                               tmpIfname, "name", NULL);
> +    if (virCommandRun(cmd, &status) < 0 ||
> +        status) {
> +        /* it's not a openvswitch vhostuser interface. */
> +        ret = 0;
> +        goto cleanup;
> +    }
> +
> +    *ifname = tmpIfname;
> +    tmpIfname = NULL;
> +    ret = 1;
> +
> + cleanup:
> +    virStringListFreeCount(tokens, ntokens);
> +    virCommandFree(cmd);
> +    VIR_FREE(tmpIfname);
> +    return ret;
> +}
> diff --git a/src/util/virnetdevopenvswitch.h b/src/util/virnetdevopenvswitch.h
> index 0f9e1dfa6..8f5faf14e 100644
> --- a/src/util/virnetdevopenvswitch.h
> +++ b/src/util/virnetdevopenvswitch.h
> @@ -52,4 +52,8 @@ int virNetDevOpenvswitchInterfaceStats(const char *ifname,
>                                         virDomainInterfaceStatsPtr stats)
>      ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK;
>  
> +int virNetDevOpenvswitchGetVhostuserIfname(const char *path,
> +                                           char **ifname)
> +    ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
> +
>  #endif /* __VIR_NETDEV_OPENVSWITCH_H__ */
> diff --git a/tests/Makefile.am b/tests/Makefile.am
> index ecd04e895..c8584fea6 100644
> --- a/tests/Makefile.am
> +++ b/tests/Makefile.am
> @@ -414,6 +414,7 @@ if WITH_QEMU
>  test_libraries += libqemumonitortestutils.la \
>  		libqemutestdriver.la \
>  		qemuxml2argvmock.la \
> +		qemuxml2xmlmock.la \
>  		qemucaps2xmlmock.la \
>  		qemucapsprobemock.la \
>  		$(NULL)
> @@ -571,6 +572,12 @@ qemuxml2argvmock_la_CFLAGS = $(AM_CFLAGS)
>  qemuxml2argvmock_la_LDFLAGS = $(MOCKLIBS_LDFLAGS)
>  qemuxml2argvmock_la_LIBADD = $(MOCKLIBS_LIBS)
>  
> +qemuxml2xmlmock_la_SOURCES = \
> +	qemuxml2xmlmock.c
> +qemuxml2xmlmock_la_CFLAGS = $(AM_CFLAGS)
> +qemuxml2xmlmock_la_LDFLAGS = $(MOCKLIBS_LDFLAGS)
> +qemuxml2xmlmock_la_LIBADD = $(MOCKLIBS_LIBS)
> +
>  qemuxml2xmltest_SOURCES = \
>  	qemuxml2xmltest.c testutilsqemu.c testutilsqemu.h \
>  	testutils.c testutils.h
> diff --git a/tests/qemuxml2argvmock.c b/tests/qemuxml2argvmock.c
> index c501b59ac..177b24e0a 100644
> --- a/tests/qemuxml2argvmock.c
> +++ b/tests/qemuxml2argvmock.c
> @@ -28,6 +28,7 @@
>  #include "virnetdev.h"
>  #include "virnetdevip.h"
>  #include "virnetdevtap.h"
> +#include "virnetdevopenvswitch.h"
>  #include "virnuma.h"
>  #include "virrandom.h"
>  #include "virscsi.h"
> @@ -180,3 +181,10 @@ virCryptoGenerateRandom(size_t nbytes)
>  
>      return buf;
>  }
> +
> +int
> +virNetDevOpenvswitchGetVhostuserIfname(const char *path ATTRIBUTE_UNUSED,
> +                                       char **ifname)
> +{
> +    return VIR_STRDUP(*ifname, "vhost-user0");
> +}
> diff --git a/tests/qemuxml2xmlmock.c b/tests/qemuxml2xmlmock.c
> new file mode 100644
> index 000000000..0d3e6f2bd
> --- /dev/null
> +++ b/tests/qemuxml2xmlmock.c
> @@ -0,0 +1,33 @@
> +/*
> + * Copyright (C) 2016 Red Hat, Inc.
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library.  If not, see
> + * <http://www.gnu.org/licenses/>.
> + *
> + * Author: Michal Privoznik <mprivozn@xxxxxxxxxx>
> + */
> +
> +#include <config.h>
> +
> +#include "virnetdevopenvswitch.h"
> +#include "virstring.h"
> +
> +#define VIR_FROM_THIS VIR_FROM_NONE
> +
> +int
> +virNetDevOpenvswitchGetVhostuserIfname(const char *path ATTRIBUTE_UNUSED,
> +                                       char **ifname)
> +{
> +    return VIR_STRDUP(*ifname, "vhost-user0");
> +}
> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-net-vhostuser.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-net-vhostuser.xml
> index ac1d7e110..2bdcac358 100644
> --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-net-vhostuser.xml
> +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-net-vhostuser.xml
> @@ -30,12 +30,14 @@
>      <interface type='vhostuser'>
>        <mac address='52:54:00:ee:96:6b'/>
>        <source type='unix' path='/tmp/vhost0.sock' mode='server'/>
> +      <target dev='vhost-user0'/>
>        <model type='virtio'/>
>        <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
>      </interface>
>      <interface type='vhostuser'>
>        <mac address='52:54:00:ee:96:6c'/>
>        <source type='unix' path='/tmp/vhost1.sock' mode='client'/>
> +      <target dev='vhost-user0'/>
>        <model type='virtio'/>
>        <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/>
>      </interface>
> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
> index ddd17cb1e..30931f581 100644
> --- a/tests/qemuxml2xmltest.c
> +++ b/tests/qemuxml2xmltest.c
> @@ -1031,7 +1031,7 @@ mymain(void)
>      return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
>  }
>  
> -VIRT_TEST_MAIN(mymain)
> +VIRT_TEST_MAIN_PRELOAD(mymain, abs_builddir "/.libs/qemuxml2xmlmock.so")
>  
>  #else
>  
> 

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]
  Powered by Linux