On Thu, May 12, 2022 at 14:07:00 -0500, Jonathon Jongsma wrote:
> 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@xxxxxxxxxx>
> > ---
> > 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'.
This should not be the case:
netpriv->slirpfd is initialized in qemuSlirpStart, which is called
from qemuExtDevicesStart which in turn is called from
qemuProcessLaunch. Now qemuProcessLaunch calls the command line
formatting machinery with standalone=false.
Now the other code path which allows control of the 'standalone'
parameter is via qemuProcessCreatePretendCmdBuild, which actually
skips the call to qemuExtDevicesStart, thus the qemuFDPass object
will not be filled.
Now I think I'll actually remove the 'standalone' flag altogether as the
only thing that it does is that the 'vhost' fds are not formatted, but
in any case where we format those we already do use FD passing for the
TAP device file descriptors so it won't make the commandline magically
usable without libvirt anyways.
