[libvirt] [PATCH v2 20/23] qemu-command: use -net socket, fd= with slirp-helper

[Michal Privoznik]

On 8/8/19 4:55 PM, marcandre.lureau at redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau at redhat.com>
> 
> If a slirp-helper is associated with a network interface (after
> probing & preparing succesfully), pass the socket fd to QEMU and use
> "-net socket,fd=".
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau at redhat.com>
> ---
>   src/qemu/qemu_command.c | 35 +++++++++++++++++++++++++++++------
>   src/qemu/qemu_command.h |  3 ++-
>   src/qemu/qemu_hotplug.c |  4 +++-
>   3 files changed, 34 insertions(+), 8 deletions(-)
> 
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 4357aa2fe1..90e61a336e 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -28,6 +28,7 @@
>   #include "qemu_alias.h"
>   #include "qemu_security.h"
>   #include "qemu_dbus.h"
> +#include "qemu_slirp.h"
>   #include "qemu_block.h"
>   #include "cpu/cpu.h"
>   #include "dirname.h"
> @@ -4008,7 +4009,8 @@ qemuBuildHostNetStr(virDomainNetDefPtr net,
>                       char **tapfd,
>                       size_t tapfdSize,
>                       char **vhostfd,
> -                    size_t vhostfdSize)
> +                    size_t vhostfdSize,
> +                    const char *slirpfd)
>   {
>       bool is_tap = false;
>       virBuffer buf = VIR_BUFFER_INITIALIZER;
> @@ -4079,6 +4081,12 @@ qemuBuildHostNetStr(virDomainNetDefPtr net,
>           break;
>   
>       case VIR_DOMAIN_NET_TYPE_USER:
> +        if (slirpfd) {
> +            virBufferAsprintf(&buf, "socket,fd=%s,",
> +                              slirpfd);
> +            break;
> +        }

No, please don't put a break at random places within 'case' bodies.

> +
>           virBufferAddLit(&buf, "user,");
>           for (i = 0; i < net->guestIP.nips; i++) {
>               const virNetDevIPAddr *ip = net->guestIP.ips[i];
> @@ -8634,10 +8642,10 @@ qemuInterfaceVhostuserConnect(virQEMUDriverPtr driver,
>   
>   static int
>   qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver,
> +                              virDomainObjPtr vm,
>                                 virLogManagerPtr logManager,
>                                 virSecurityManagerPtr secManager,
>                                 virCommandPtr cmd,
> -                              virDomainDefPtr def,
>                                 virDomainNetDefPtr net,
>                                 virQEMUCapsPtr qemuCaps,
>                                 unsigned int bootindex,
> @@ -8646,6 +8654,7 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver,
>                                 size_t *nnicindexes,
>                                 int **nicindexes)
>   {
> +    virDomainDefPtr def = vm->def;
>       int ret = -1;
>       char *nic = NULL;
>       char *host = NULL;
> @@ -8656,9 +8665,11 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver,
>       size_t vhostfdSize = 0;
>       char **tapfdName = NULL;
>       char **vhostfdName = NULL;
> +    VIR_AUTOFREE(char *) slirpfdName = NULL;
>       virDomainNetType actualType = virDomainNetGetActualType(net);
>       virNetDevBandwidthPtr actualBandwidth;
>       bool requireNicdev = false;
> +    qemuSlirpPtr slirp;
>       size_t i;
>   
>   
> @@ -8884,6 +8895,16 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver,
>               goto cleanup;
>       }
>   
> +    slirp = QEMU_DOMAIN_NETWORK_PRIVATE(net)->slirp;
> +    if (slirp && !standalone) {
> +        int slirpfd = qemuSlirpGetFD(slirp);
> +        virCommandPassFD(cmd, slirpfd,
> +                         VIR_COMMAND_PASS_FD_CLOSE_PARENT);
> +        if (virAsprintf(&slirpfdName, "%d", slirpfd) < 0)
> +            goto cleanup;
> +    }
> +
> +
>       for (i = 0; i < tapfdSize; i++) {
>           if (qemuSecuritySetTapFDLabel(driver->securityManager,
>                                         def, tapfd[i]) < 0)
> @@ -8908,7 +8929,8 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver,
>   
>       if (!(host = qemuBuildHostNetStr(net, driver,
>                                        tapfdName, tapfdSize,
> -                                     vhostfdName, vhostfdSize)))
> +                                     vhostfdName, vhostfdSize,
> +                                     slirpfdName)))
>           goto cleanup;
>       virCommandAddArgList(cmd, "-netdev", host, NULL);
>   
> @@ -8976,10 +8998,10 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver,
>    */
>   static int
>   qemuBuildNetCommandLine(virQEMUDriverPtr driver,
> +                        virDomainObjPtr vm,
>                           virLogManagerPtr logManager,
>                           virSecurityManagerPtr secManager,
>                           virCommandPtr cmd,
> -                        virDomainDefPtr def,
>                           virQEMUCapsPtr qemuCaps,
>                           virNetDevVPortProfileOp vmop,
>                           bool standalone,
> @@ -8990,6 +9012,7 @@ qemuBuildNetCommandLine(virQEMUDriverPtr driver,
>       size_t i;
>       int last_good_net = -1;
>       virErrorPtr originalError = NULL;
> +    virDomainDefPtr def = vm->def;
>   
>       if (def->nnets) {
>           unsigned int bootNet = 0;
> @@ -9005,7 +9028,7 @@ qemuBuildNetCommandLine(virQEMUDriverPtr driver,
>           for (i = 0; i < def->nnets; i++) {
>               virDomainNetDefPtr net = def->nets[i];
>   
> -            if (qemuBuildInterfaceCommandLine(driver, logManager, secManager, cmd, def, net,
> +            if (qemuBuildInterfaceCommandLine(driver, vm, logManager, secManager, cmd, net,
>                                                 qemuCaps, bootNet, vmop,
>                                                 standalone, nnicindexes,
>                                                 nicindexes) < 0)
> @@ -10815,7 +10838,7 @@ qemuBuildCommandLine(virQEMUDriverPtr driver,
>       if (qemuBuildFilesystemCommandLine(cmd, def, qemuCaps) < 0)
>           goto error;
>   
> -    if (qemuBuildNetCommandLine(driver, logManager, secManager, cmd, def,
> +    if (qemuBuildNetCommandLine(driver, vm, logManager, secManager, cmd,
>                                   qemuCaps, vmop, standalone,
>                                   nnicindexes, nicindexes, &bootHostdevNet) < 0)
>           goto error;
> diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
> index 3a957c52fc..6faa6f2b18 100644
> --- a/src/qemu/qemu_command.h
> +++ b/src/qemu/qemu_command.h
> @@ -90,7 +90,8 @@ char *qemuBuildHostNetStr(virDomainNetDefPtr net,
>                             char **tapfd,
>                             size_t tapfdSize,
>                             char **vhostfd,
> -                          size_t vhostfdSize);
> +                          size_t vhostfdSize,
> +                          const char *slirpfd);
>   
>   /* Current, best practice */
>   char *qemuBuildNicDevStr(virDomainDefPtr def,
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 028921fb47..43c3f0755b 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -1135,6 +1135,7 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
>       qemuDomainObjPrivatePtr priv = vm->privateData;
>       virDomainDeviceDef dev = { VIR_DOMAIN_DEVICE_NET, { .net = net } };
>       virErrorPtr originalError = NULL;
> +    VIR_AUTOFREE(char *) slirpfdName = NULL;
>       char **tapfdName = NULL;
>       int *tapfd = NULL;
>       size_t tapfdSize = 0;
> @@ -1373,7 +1374,8 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
>   
>       if (!(netstr = qemuBuildHostNetStr(net, driver,
>                                          tapfdName, tapfdSize,
> -                                       vhostfdName, vhostfdSize)))
> +                                       vhostfdName, vhostfdSize,
> +                                       slirpfdName)))

This looks like a spurious change. Just pass NULL and introduce the 
variable once you need it.

Reviewed-by: Michal Privoznik <mprivozn at redhat.com>

Michal