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

[Michal Privoznik]

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