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@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...
Ah, yeah. Fixed and pushed. Thanks.
Michal
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
