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@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@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