[PATCH 34/37] qemu: slirp: Pass FDs to qemu via qemuFDPass in the network private data

[Jonathon Jongsma]

On 5/10/22 10:20 AM, Peter Krempa wrote:
> Populate the 'slirpfd' qemuFDPass structure inside the private data for
> passing the fd to qemu rather than using out-of-band variables.
> 
> Signed-off-by: Peter Krempa <pkrempa at redhat.com>
> ---
>   src/qemu/qemu_command.c | 27 +++++++++------------------
>   src/qemu/qemu_command.h |  3 +--
>   src/qemu/qemu_hotplug.c |  9 +++------
>   src/qemu/qemu_slirp.c   |  8 +++++++-
>   4 files changed, 20 insertions(+), 27 deletions(-)
> 
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 92b91b0a52..e48b59abbb 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -4186,8 +4186,7 @@ qemuBuildNicDevProps(virDomainDef *def,
> 
> 
>   virJSONValue *
> -qemuBuildHostNetProps(virDomainNetDef *net,
> -                      const char *slirpfd)
> +qemuBuildHostNetProps(virDomainNetDef *net)
>   {
>       virDomainNetType netType = virDomainNetGetActualType(net);
>       size_t i;
> @@ -4302,9 +4301,11 @@ qemuBuildHostNetProps(virDomainNetDef *net,
>           break;
> 
>       case VIR_DOMAIN_NET_TYPE_USER:
> -        if (slirpfd) {
> -            if (virJSONValueObjectAdd(&netprops, "s:type", "socket", NULL) < 0 ||
> -                virJSONValueObjectAppendString(netprops, "fd", slirpfd) < 0)
> +        if (netpriv->slirpfd) {
> +            if (virJSONValueObjectAdd(&netprops,
> +                                      "s:type", "socket",
> +                                      "s:fd", qemuFDPassGetPath(netpriv->slirpfd),
> +                                      NULL) < 0)
>                   return NULL;
>           } else {
>               if (virJSONValueObjectAdd(&netprops, "s:type", "user", NULL) < 0)
> @@ -8760,11 +8761,9 @@ qemuBuildInterfaceCommandLine(virQEMUDriver *driver,
>       int ret = -1;
>       g_autoptr(virJSONValue) nicprops = NULL;
>       g_autofree char *nic = NULL;
> -    g_autofree char *slirpfdName = NULL;
>       virDomainNetType actualType = virDomainNetGetActualType(net);
>       const virNetDevBandwidth *actualBandwidth;
>       bool requireNicdev = false;
> -    qemuSlirp *slirp;
>       g_autoptr(virJSONValue) hostnetprops = NULL;
>       qemuDomainNetworkPrivate *netpriv = QEMU_DOMAIN_NETWORK_PRIVATE(net);
>       GSList *n;
> @@ -8890,14 +8889,6 @@ qemuBuildInterfaceCommandLine(virQEMUDriver *driver,
>           virNetDevSetMTU(net->ifname, net->mtu) < 0)
>           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);
> -        slirpfdName = g_strdup_printf("%d", slirpfd);
> -    }
> -
>       for (n = netpriv->tapfds; n; n = n->next) {
>           if (qemuFDPassTransferCommand(n->data, cmd) < 0)
>               return -1;
> @@ -8908,11 +8899,11 @@ qemuBuildInterfaceCommandLine(virQEMUDriver *driver,
>               return -1;
>       }
> 
> -    if (qemuFDPassTransferCommand(netpriv->vdpafd, cmd) < 0)
> +    if (qemuFDPassTransferCommand(netpriv->slirpfd, cmd) < 0 ||
> +        qemuFDPassTransferCommand(netpriv->vdpafd, cmd) < 0)
>           return -1;


If I'm reading this correctly, there's a small behavior change here when 
'standalone' is true. Previously we were not passing the slirpfd in that 
case, but now it looks like we will to pass it regardless of the value 
of 'standalone'.

Reviewed-by: Jonathon Jongsma <jjongsma at redhat.com>


> 
> -    if (!(hostnetprops = qemuBuildHostNetProps(net,
> -                                               slirpfdName)))
> +    if (!(hostnetprops = qemuBuildHostNetProps(net)))
>           goto cleanup;
> 
>       if (qemuBuildNetdevCommandlineFromJSON(cmd, hostnetprops, qemuCaps) < 0)
> diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
> index 9bb63d9598..84877b3d90 100644
> --- a/src/qemu/qemu_command.h
> +++ b/src/qemu/qemu_command.h
> @@ -86,8 +86,7 @@ virJSONValue *
>   qemuBuildChannelGuestfwdNetdevProps(virDomainChrDef *chr);
> 
>   virJSONValue *
> -qemuBuildHostNetProps(virDomainNetDef *net,
> -                      const char *slirpfd);
> +qemuBuildHostNetProps(virDomainNetDef *net);
> 
>   int
>   qemuBuildInterfaceConnect(virDomainObj *vm,
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 8f5e971570..3f368551e3 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -1318,9 +1318,6 @@ qemuDomainAttachNetDevice(virQEMUDriver *driver,
>                                  "%s", _("Failed to start slirp"));
>                   goto cleanup;
>               }
> -
> -            slirpfd = qemuSlirpGetFD(slirp);
> -            slirpfdName = g_strdup_printf("slirpfd-%s", net->info.alias);
>           }
>           break;
> 
> @@ -1376,8 +1373,7 @@ qemuDomainAttachNetDevice(virQEMUDriver *driver,
>           virNetDevSetMTU(net->ifname, net->mtu) < 0)
>           goto cleanup;
> 
> -    if (!(netprops = qemuBuildHostNetProps(net,
> -                                           slirpfdName)))
> +    if (!(netprops = qemuBuildHostNetProps(net)))
>           goto cleanup;
> 
>       qemuDomainObjEnterMonitor(driver, vm);
> @@ -1396,7 +1392,8 @@ qemuDomainAttachNetDevice(virQEMUDriver *driver,
>           }
>       }
> 
> -    if (qemuFDPassTransferMonitor(netpriv->vdpafd, priv->mon) < 0) {
> +    if (qemuFDPassTransferMonitor(netpriv->slirpfd, priv->mon) < 0 ||
> +        qemuFDPassTransferMonitor(netpriv->vdpafd, priv->mon) < 0) {
>           qemuDomainObjExitMonitor(vm);
>           goto cleanup;
>       }
> diff --git a/src/qemu/qemu_slirp.c b/src/qemu/qemu_slirp.c
> index 136e4b29d2..b62934f292 100644
> --- a/src/qemu/qemu_slirp.c
> +++ b/src/qemu/qemu_slirp.c
> @@ -251,7 +251,8 @@ qemuSlirpStart(virDomainObj *vm,
>   {
>       qemuDomainObjPrivate *priv = vm->privateData;
>       virQEMUDriver *driver = priv->driver;
> -    qemuSlirp *slirp = QEMU_DOMAIN_NETWORK_PRIVATE(net)->slirp;
> +    qemuDomainNetworkPrivate *netpriv = QEMU_DOMAIN_NETWORK_PRIVATE(net);
> +    qemuSlirp *slirp = netpriv->slirp;
>       g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
>       g_autoptr(virCommand) cmd = NULL;
>       g_autofree char *pidfile = NULL;
> @@ -262,6 +263,7 @@ qemuSlirpStart(virDomainObj *vm,
>       int cmdret = 0;
>       VIR_AUTOCLOSE errfd = -1;
>       bool killDBusDaemon = false;
> +    g_autofree char *fdname = g_strdup_printf("slirpfd-%s", net->info.alias);
> 
>       if (!slirp)
>           return 0;
> @@ -359,6 +361,10 @@ qemuSlirpStart(virDomainObj *vm,
> 
>       slirp->pid = pid;
> 
> +    netpriv->slirpfd = qemuFDPassNewDirect(fdname, priv);
> +
> +    qemuFDPassAddFD(netpriv->slirpfd, &slirp->fd[0], NULL);
> +
>       return 0;
> 
>    error: