8:43 a.m.
On 01/18/2017 02:21 PM, John Ferlan wrote:
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(a)sileht.net>.
>
> 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(a)redhat.com>
> ---
>
> 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...
Ah, yeah. Fixed and pushed. Thanks.
Michal