[libvirt] [PATCH v2 14/23] qemu: add slirp helper unit

Michal Privoznik mprivozn at redhat.com
Fri Sep 6 11:36:35 UTC 2019


On 8/8/19 4:55 PM, marcandre.lureau at redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau at redhat.com>
> 
> The unit provides the functions associated with a slirp-helper:
> - probing / checking capabilities
> - opening the socketpair
> - starting / stoping the helper
> - registering for dbus-vmstate migration
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau at redhat.com>
> ---
>   src/qemu/Makefile.inc.am |   2 +
>   src/qemu/qemu_domain.h   |   4 +
>   src/qemu/qemu_slirp.c    | 448 +++++++++++++++++++++++++++++++++++++++
>   src/qemu/qemu_slirp.h    |  81 +++++++
>   4 files changed, 535 insertions(+)
>   create mode 100644 src/qemu/qemu_slirp.c
>   create mode 100644 src/qemu/qemu_slirp.h
> 
> diff --git a/src/qemu/Makefile.inc.am b/src/qemu/Makefile.inc.am
> index 94dd0e56ff..505b418fa5 100644
> --- a/src/qemu/Makefile.inc.am
> +++ b/src/qemu/Makefile.inc.am
> @@ -58,6 +58,8 @@ QEMU_DRIVER_SOURCES = \
>   	qemu/qemu_security.h \
>   	qemu/qemu_qapi.c \
>   	qemu/qemu_qapi.h \
> +	qemu/qemu_slirp.c \
> +	qemu/qemu_slirp.h \
>   	qemu/qemu_tpm.c \
>   	qemu/qemu_tpm.h \
>   	$(NULL)
> diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
> index 560b01d80a..7293c87d7c 100644
> --- a/src/qemu/qemu_domain.h
> +++ b/src/qemu/qemu_domain.h
> @@ -509,6 +509,10 @@ struct _qemuDomainGraphicsPrivate {
>   };
>   
>   
> +typedef struct _qemuSlirp qemuSlirp;
> +typedef struct _qemuSlirp *qemuSlirpPtr;
> +
> +

I understand why you need this here, but it feels wrong. This must go 
into qemu_slirp.h and then from qemu_domain.h you include qemu_slirp.h. 
But that creates a circular dependency via qemu_process.h. But since you 
don'y really need qemuProcessIncomingDefPtr in qemu_slirp.h you only 
need to know if there's an incoming migration the qemuSlirpStart() 
argument can be turned into a boolean type and thus you qemu_process.h 
include can be dropped and the circular include is gone.

>   #define QEMU_DOMAIN_NETWORK_PRIVATE(dev)                \
>       ((qemuDomainNetworkPrivatePtr) (dev)->privateData)
>   
> diff --git a/src/qemu/qemu_slirp.c b/src/qemu/qemu_slirp.c
> new file mode 100644
> index 0000000000..36370b6be0
> --- /dev/null
> +++ b/src/qemu/qemu_slirp.c
> @@ -0,0 +1,448 @@
> +/*
> + * qemu_slirp.c: QEMU Slirp support
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library.  If not, see
> + * <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <config.h>
> +
> +#include "qemu_dbus.h"
> +#include "qemu_extdevice.h"
> +#include "qemu_security.h"
> +#include "qemu_slirp.h"
> +#include "viralloc.h"
> +#include "virenum.h"
> +#include "virerror.h"
> +#include "virjson.h"
> +#include "virlog.h"
> +#include "virpidfile.h"
> +#include "virstring.h"
> +#include "virtime.h"
> +
> +#define VIR_FROM_THIS VIR_FROM_NONE
> +
> +VIR_LOG_INIT("qemu.slirp");
> +
> +VIR_ENUM_IMPL(qemuSlirpFeature,
> +              QEMU_SLIRP_FEATURE_LAST,
> +              "",
> +              "ipv4",
> +              "ipv6",
> +              "tftp",
> +              "dbus-address",
> +              "dbus-p2p",
> +              "migrate",
> +              "restrict",
> +              "exit-with-parent",
> +);
> +
> +
> +void
> +qemuSlirpFree(qemuSlirpPtr slirp)
> +{

A free function must accept NULL.

> +    VIR_FORCE_CLOSE(slirp->fd[0]);
> +    VIR_FORCE_CLOSE(slirp->fd[1]);
> +    virBitmapFree(slirp->features);
> +    VIR_FREE(slirp);
> +}
> +
> +
> +void
> +qemuSlirpSetFeature(qemuSlirpPtr slirp,
> +                    qemuSlirpFeature feature)
> +{
> +    ignore_value(virBitmapSetBit(slirp->features, feature));
> +}
> +
> +
> +bool
> +qemuSlirpHasFeature(const qemuSlirpPtr slirp,
> +                    qemuSlirpFeature feature)
> +{
> +    return virBitmapIsBitSet(slirp->features, feature);
> +}
> +
> +
> +qemuSlirpPtr
> +qemuSlirpNew(void)
> +{
> +    qemuSlirpPtr slirp = NULL;
> +
> +    if (VIR_ALLOC(slirp) < 0)
> +        return NULL;
> +
> +    slirp->pid = (pid_t)-1;
> +    slirp->fd[0] = slirp->fd[1] = -1;
> +    slirp->features = virBitmapNew(QEMU_SLIRP_FEATURE_LAST);

Need to check if this hasn't failed.

> +
> +    return slirp;
> +}
> +
> +
> +qemuSlirpPtr
> +qemuSlirpNewForHelper(const char *helper)
> +{
> +    VIR_AUTOPTR(qemuSlirp) slirp = NULL;
> +    VIR_AUTOPTR(virCommand) cmd = NULL;
> +    VIR_AUTOFREE(char *) output = NULL;
> +    VIR_AUTOPTR(virJSONValue) doc = NULL;
> +    virJSONValuePtr featuresJSON;
> +    size_t i, nfeatures;
> +
> +    if (!helper)
> +        return NULL;
> +
> +    slirp = qemuSlirpNew();
> +    if (!slirp) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       _("Failed to allocate slirp for '%s'"), helper);
> +        return NULL;
> +    }
> +
> +    cmd = virCommandNewArgList(helper, "--print-capabilities", NULL);
> +    virCommandSetOutputBuffer(cmd, &output);
> +    if (virCommandRun(cmd, NULL) < 0)
> +        return NULL;
> +

So this will run slirp-helper as root:root to query caps. I guess it's 
okay, as long as we don't run the actual slirp-helper that we start 
together with qemu as root:root.

> +    if (!(doc = virJSONValueFromString(output)) ||
> +        !(featuresJSON = virJSONValueObjectGetArray(doc, "features"))) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       _("unable to parse json capabilities '%s'"),
> +                       helper);
> +        return NULL;
> +    }
> +
> +    nfeatures = virJSONValueArraySize(featuresJSON);
> +    for (i = 0; i < nfeatures; i++) {
> +        virJSONValuePtr item = virJSONValueArrayGet(featuresJSON, i);
> +        const char *tmpStr = virJSONValueGetString(item);
> +        int tmp;
> +
> +        if ((tmp = qemuSlirpFeatureTypeFromString(tmpStr)) <= 0) {
> +            VIR_WARN("unknown slirp feature %s", tmpStr);
> +            continue;
> +        }
> +
> +        qemuSlirpSetFeature(slirp, tmp);
> +    }
> +
> +    VIR_RETURN_PTR(slirp);
> +}
> +
> +
> +static char *
> +qemuSlirpCreatePidFilename(const char *stateDir,
> +                           const char *shortName,
> +                           const char *alias)
> +{
> +    VIR_AUTOFREE(char *) name = NULL;
> +
> +    if (virAsprintf(&name, "%s-%s-slirp", shortName, alias) < 0)
> +        return NULL;
> +
> +    return virPidFileBuildPath(stateDir, name);

Since you're passing @stateDir as an argument, it's easy to miss that 
you're passing cfg->stateDir instead of cfg->slirpStateDir later in the 
code. So I suggest you drop @stateDir in favor of @cfg and use 
cfg->slirpStateDir directly.
Also, @shortName can be replaced with vm->def so that we don't have to 
worry about getting the short name wrong.

> +}
> +
> +
> +static int
> +qemuSlirpGetPid(const char *binPath,
> +                const char *stateDir,
> +                const char *shortName,
> +                const char *alias,
> +                pid_t *pid)
> +{
> +    VIR_AUTOFREE(char *) pidfile = qemuSlirpCreatePidFilename(stateDir, shortName, alias);
> +    if (!pidfile)
> +        return -ENOMEM;
> +
> +    return virPidFileReadPathIfAlive(pidfile, pid, binPath);
> +}

This function is prettty useless. In the only place wher it's used we 
already have @pidfile constructed so why not use 
virPidFileReadPathIfAlive() directly? Which turns out is also not 
correct, but I'll get to that.

> +
> +
> +int
> +qemuSlirpOpen(qemuSlirpPtr slirp,
> +              virQEMUDriverPtr driver,
> +              virDomainDefPtr def)
> +{
> +    int rc, pair[2] = { -1, -1 };
> +
> +    if (qemuSecuritySetSocketLabel(driver->securityManager, def) < 0)
> +        goto error;
> +
> +    rc = socketpair(AF_UNIX, SOCK_DGRAM, 0, pair);
> +
> +    if (qemuSecurityClearSocketLabel(driver->securityManager, def) < 0)
> +        goto error;
> +
> +    if (rc < 0) {
> +        virReportSystemError(errno, "%s", _("failed to create socketpair"));
> +        goto error;
> +    }
> +
> +    slirp->fd[0] = pair[0];
> +    slirp->fd[1] = pair[1];
> +
> +    return 0;
> +
> +error:

s/^e/ e/

> +    VIR_FORCE_CLOSE(pair[0]);
> +    VIR_FORCE_CLOSE(pair[1]);
> +    return -1;
> +}
> +
> +
> +int
> +qemuSlirpGetFD(qemuSlirpPtr slirp)
> +{
> +    int fd = slirp->fd[0];
> +    slirp->fd[0] = -1;
> +    return fd;
> +}
> +
> +
> +static char *
> +qemuSlirpGetDBusVMStateId(virDomainNetDefPtr net)
> +{
> +    char macstr[VIR_MAC_STRING_BUFLEN] = "";
> +    char *id = NULL;
> +
> +    /* can't use alias, because it's not stable across restarts */
> +    if (virAsprintf(&id, "slirp-%s", virMacAddrFormat(&net->mac, macstr)) < 0)
> +        return NULL;
> +
> +    return id;
> +}
> +
> +
> +static char *
> +qemuSlirpGetDBusAddress(const char *stateDir,
> +                        const char *shortName,
> +                        const char *alias)
> +{
> +    char *name = NULL;
> +
> +    if (virAsprintf(&name, "unix:path=%s/%s-%s-slirp", stateDir, shortName, alias) < 0)
> +        return NULL;
> +
> +    return name;

The same comment applies here as for qemuSlirpCreatePidFilename().
Moreover, we are going to need the path a few more times and the 
prefixed version ("unix:path=...") only once. So this should be renamed 
to qemuSlirpGetDBusPath() and then virAsprintf() can be used again to 
construct the prefixed version.

> +}
> +
> +
> +void
> +qemuSlirpStop(qemuSlirpPtr slirp,
> +              virDomainObjPtr vm,
> +              virQEMUDriverPtr driver,
> +              virDomainNetDefPtr net,
> +              bool hot)
> +{
> +    virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);

This grabs a reference (virObjectRef()) so in order to use returns 
below, this needs to be VIR_AUTOUNREF().

> +    VIR_AUTOFREE(char *) pidfile = NULL;
> +    VIR_AUTOFREE(char *) shortName = virDomainDefGetShortName(vm->def);
> +    VIR_AUTOFREE(char *) id = qemuSlirpGetDBusVMStateId(net);
> +    virErrorPtr orig_err;
> +    int rc;
> +    pid_t pid;
> +
> +    if (!(pidfile = qemuSlirpCreatePidFilename(
> +              cfg->stateDir, shortName, net->info.alias))) {
> +        VIR_WARN("Unable to construct slirp pidfile path");
> +        return;
> +    }
> +
> +    qemuDBusVMStateRemove(driver, vm, id, hot);

@id can be NULL here (we are still checking for OOM errors).

> +
> +    rc = qemuSlirpGetPid(cfg->slirpHelperName,
> +                         cfg->stateDir, shortName, net->info.alias, &pid);

Here you can use virPidFileReadPathIfAlive() directly, sicne you already 
have @pidfile constructed.

> +    if (rc == 0 && pid != (pid_t)-1) {
> +        char ebuf[1024];
> +
> +        VIR_DEBUG("Killing slirp process %lld", (long long)pid);
> +        if (virProcessKill(pid, SIGKILL) < 0 && errno != ESRCH)

virProcessKillPainfully() to ensure the process is really killed.

> +            VIR_ERROR(_("Failed to kill process %lld: %s"),
> +                      (long long)pid,
> +                      virStrerror(errno, ebuf, sizeof(ebuf)));
> +    }
> +
> +    virErrorPreserveLast(&orig_err);
> +    if (virPidFileForceCleanupPath(pidfile) < 0) {

This doesn't do what you think it does. I'll get to that a few lines 
below, but pidfile is not locked, therefore this doesn't unlink the 
pidfile and returns with success.

> +        VIR_WARN("Unable to kill slirp process");
> +    } else {
> +        if (unlink(pidfile) < 0 &&

Which means, we can call this directly.

> +            errno != ENOENT) {
> +            virReportSystemError(errno,
> +                                 _("Unable to remove stale pidfile %s"),
> +                                 pidfile);
> +        }
> +    }


The dbus socket is also worth removing at this point.

> +    virErrorRestore(&orig_err);
> +    slirp->pid = 0;
> +}
> +
> +
> +int
> +qemuSlirpStart(qemuSlirpPtr slirp,
> +               virDomainObjPtr vm,
> +               virQEMUDriverPtr driver,
> +               virDomainNetDefPtr net,
> +               bool hotplug,
> +               qemuProcessIncomingDefPtr incoming)
> +{
> +    virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
> +    VIR_AUTOPTR(virCommand) cmd = NULL;
> +    VIR_AUTOFREE(char *) cmdstr = NULL;
> +    VIR_AUTOFREE(char *) addr = NULL;
> +    VIR_AUTOFREE(char *) pidfile = NULL;
> +    VIR_AUTOFREE(char *) dbus_addr = NULL;
> +    VIR_AUTOFREE(char *) id = NULL;
> +    VIR_AUTOFREE(char *) shortName = virDomainDefGetShortName(vm->def);
> +    size_t i;
> +    virTimeBackOffVar timebackoff;
> +    const unsigned long long timeout = 500 * 1000; /* ms */
> +    const char *dbus_path = NULL;
> +    pid_t pid;
> +    int cmdret = 0, exitstatus = 0;
> +
> +    if (incoming &&
> +        !qemuSlirpHasFeature(slirp, QEMU_SLIRP_FEATURE_MIGRATE)) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("The slirp-helper doesn't support migration"));
> +    }
> +
> +    if (!(pidfile = qemuSlirpCreatePidFilename(
> +              cfg->stateDir, shortName, net->info.alias)))

Ehm, s/stateDir/slirpStateDir/. But since I'm suggesting to pass @cfg 
directly, this change is bogus then.

> +        return -1;
> +
> +    if (!(cmd = virCommandNew(cfg->slirpHelperName)))
> +        return -1;
> +
> +    virCommandClearCaps(cmd);
> +    virCommandSetPidFile(cmd, pidfile);
> +    virCommandDaemonize(cmd);
> +
> +    virCommandAddArgFormat(cmd, "--fd=%d", slirp->fd[1]);
> +    virCommandPassFD(cmd, slirp->fd[1],
> +                     VIR_COMMAND_PASS_FD_CLOSE_PARENT);
> +    slirp->fd[1] = -1;
> +
> +    for (i = 0; i < net->guestIP.nips; i++) {
> +        const virNetDevIPAddr *ip = net->guestIP.ips[i];
> +        const char *opt = "";
> +
> +        if (!(addr = virSocketAddrFormat(&ip->address)))
> +            return -1;
> +
> +        if (VIR_SOCKET_ADDR_IS_FAMILY(&ip->address, AF_INET))
> +            opt = "--net";
> +        if (VIR_SOCKET_ADDR_IS_FAMILY(&ip->address, AF_INET6))
> +            opt = "--prefix-ipv6";
> +
> +        virCommandAddArgFormat(cmd, "%s=%s", opt, addr);
> +        VIR_FREE(addr);
> +
> +        if (ip->prefix) {
> +            if (VIR_SOCKET_ADDR_IS_FAMILY(&ip->address, AF_INET)) {
> +                virSocketAddr netmask;
> +                VIR_AUTOFREE(char *) netmaskStr = NULL;
> +
> +                if (virSocketAddrPrefixToNetmask(ip->prefix, &netmask, AF_INET) < 0) {
> +                    virReportError(VIR_ERR_INTERNAL_ERROR,
> +                                   _("Failed to translate prefix %d to netmask"),
> +                                   ip->prefix);
> +                    return -1;
> +                }
> +                if (!(netmaskStr = virSocketAddrFormat(&netmask)))
> +                    return -1;
> +                virCommandAddArgFormat(cmd, "--mask=%s", netmaskStr);
> +            }
> +            if (VIR_SOCKET_ADDR_IS_FAMILY(&ip->address, AF_INET6))
> +                virCommandAddArgFormat(cmd, "--prefix-length-ipv6=%u", ip->prefix);
> +        }
> +    }
> +
> +    if (qemuSlirpHasFeature(slirp,
> +                            QEMU_SLIRP_FEATURE_DBUS_P2P)) {
> +        id = qemuSlirpGetDBusVMStateId(net);
> +        dbus_addr = qemuSlirpGetDBusAddress(cfg->stateDir,
> +                                            shortName, net->info.alias);
> +        if (!id || !dbus_addr) {
> +            return -1;
> +        }
> +
> +        dbus_path = dbus_addr + strlen("unix:path=");

If we'd call qemuSlirpGetDBusPath() here, then we can avoid this string 
arithmetic and construct dbus_addr using an virAsprintf() call.

> +
> +        if (unlink(dbus_path) < 0 && errno != ENOENT) {
> +            virReportSystemError(errno, _("Unable to unlink %s"), dbus_path);
> +            return -1;
> +        }
> +
> +        virCommandAddArgFormat(cmd, "--dbus-id=%s", id);
> +
> +        virCommandAddArgFormat(cmd, "--dbus-p2p=%s", dbus_addr);
> +
> +        if (incoming &&
> +            qemuSlirpHasFeature(slirp, QEMU_SLIRP_FEATURE_MIGRATE))
> +            virCommandAddArg(cmd, "--dbus-incoming");
> +    }
> +
> +    if (qemuSlirpHasFeature(slirp,
> +                            QEMU_SLIRP_FEATURE_EXIT_WITH_PARENT)) {
> +        virCommandAddArg(cmd, "--exit-with-parent");
> +    }
> +
> +    if (qemuExtDeviceLogCommand(driver, vm, cmd, "slirp") < 0)
> +        return -1;
> +
> +    if (qemuSecurityCommandRun(driver, vm, cmd,
> +                               &exitstatus, &cmdret) < 0)
> +        return -1;
> +
> +    if (cmdret < 0 || exitstatus != 0) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       _("Could not start 'slirp'. exitstatus: %d"), exitstatus);
> +        return -1;
> +    }
> +
> +    /* check that the helper has written its pid into the file */
> +    if (virTimeBackOffStart(&timebackoff, 1, timeout) < 0)
> +        return -1;
> +    while (virTimeBackOffWait(&timebackoff)) {
> +        int rc = qemuSlirpGetPid(cfg->slirpHelperName,
> +                                 cfg->stateDir, shortName, net->info.alias, &pid);
> +        if (rc < 0)
> +            continue;
> +        if (rc == 0 && pid == (pid_t)-1)
> +            return -1;
> +        break;
> +    }

While this works, it doesn't do what you think it does. Because of the 
virCommandSetPidFile() call above, the pidfile is created and written to 
before virCommandRun() (hidden under qemuSecurityCommandRun() here) 
returns. So we don't have to wait for the pidfile. This is different to 
how pr-helper process is spawned (which is where you get the inspiration 
from). The pr-helper is passed '-f $pidfile' to, which makes the 
pr-helper to create the pidfile (and lock it). So in that case, the 
pidfile might not exist upon return from virCommandRun() and thus there 
has to be a wait loop.

Also, virCommandSetPidFile() does not cause the pidfile to be locked. 
This means, in the qemuSlirpStop() we have to use 
virPidFileReadPathIfAlive() which checks if the /proc/$pid/exe points to 
the expected binary to avoid us killing a different process with the 
same PID after PID wrap. If the pidfile was locked then we could use 
virPidFileForceCleanupPath() - just like pr-helper code does.

> +
> +    if (dbus_path) {
> +        while (virTimeBackOffWait(&timebackoff)) {
> +            if (!virFileExists(dbus_path))
> +                continue;

What if slirp-helper dies meanwhile? We need to 'ping' it periodically 
here to check it's still alive (virProcessKill(pid, 0)).

Also, we might set up errfd and read from it because I bet slirp-helper 
will produce some sensible error message on its error output just before 
quitting.

> +            break;
> +        }
> +    }
> +
> +    if (qemuSlirpHasFeature(slirp, QEMU_SLIRP_FEATURE_MIGRATE)) {
> +        if (qemuDBusVMStateAdd(driver, vm, id, dbus_addr, hotplug) < 0) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR,
> +                           _("Failed to register slirp migration"));
> +            return -1;

I don't think that we can just return here. At this point, the 
slirp-helper is up & running so we need to kill it and remove its pid 
file and dbus socket.

> +        }
> +    }
> +
> +    slirp->pid = pid;
> +    return 0;
> +}
> diff --git a/src/qemu/qemu_slirp.h b/src/qemu/qemu_slirp.h
> new file mode 100644
> index 0000000000..f44edce543
> --- /dev/null
> +++ b/src/qemu/qemu_slirp.h
> @@ -0,0 +1,81 @@
> +/*
> + * qemu_slirp.h: QEMU Slirp support
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library.  If not, see
> + * <http://www.gnu.org/licenses/>.
> + */
> +
> +#pragma once
> +
> +#include "qemu_conf.h"
> +#include "qemu_domain.h"
> +#include "qemu_process.h"
> +#include "vircommand.h"
> +

No need to include qemu_domain.h or vircommand.h.

> +typedef enum {
> +    QEMU_SLIRP_FEATURE_NONE = 0,
> +    QEMU_SLIRP_FEATURE_IPV4,
> +    QEMU_SLIRP_FEATURE_IPV6,
> +    QEMU_SLIRP_FEATURE_TFTP,
> +    QEMU_SLIRP_FEATURE_DBUS_ADDRESS,
> +    QEMU_SLIRP_FEATURE_DBUS_P2P,
> +    QEMU_SLIRP_FEATURE_MIGRATE,
> +    QEMU_SLIRP_FEATURE_RESTRICT,
> +    QEMU_SLIRP_FEATURE_EXIT_WITH_PARENT,
> +
> +    QEMU_SLIRP_FEATURE_LAST,
> +} qemuSlirpFeature;
> +
> +VIR_ENUM_DECL(qemuSlirpFeature);
> +
> +
> +struct _qemuSlirp {
> +    int fd[2];
> +    virBitmapPtr features;
> +    pid_t pid;
> +};
> +
> +
> +qemuSlirpPtr qemuSlirpNew(void);
> +
> +qemuSlirpPtr qemuSlirpNewForHelper(const char *helper);
> +
> +void qemuSlirpFree(qemuSlirpPtr slirp);
> +
> +void qemuSlirpSetFeature(qemuSlirpPtr slirp,
> +                         qemuSlirpFeature feature);
> +
> +bool qemuSlirpHasFeature(const qemuSlirpPtr slirp,
> +                         qemuSlirpFeature feature);
> +
> +int qemuSlirpOpen(qemuSlirpPtr slirp,
> +                  virQEMUDriverPtr driver,
> +                  virDomainDefPtr def);
> +
> +int qemuSlirpStart(qemuSlirpPtr slirp,
> +                   virDomainObjPtr vm,
> +                   virQEMUDriverPtr driver,
> +                   virDomainNetDefPtr net,
> +                   bool hot,
> +                   qemuProcessIncomingDefPtr incoming);

If this is turned to bool, then you can drop qemu_process.h include too.

> +
> +void qemuSlirpStop(qemuSlirpPtr slirp,
> +                   virDomainObjPtr vm,
> +                   virQEMUDriverPtr driver,
> +                   virDomainNetDefPtr net,
> +                   bool hot);
> +
> +int qemuSlirpGetFD(qemuSlirpPtr slirp);
> +
> +VIR_DEFINE_AUTOPTR_FUNC(qemuSlirp, qemuSlirpFree);
> 

With all that addressed:

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

Michal




More information about the libvir-list mailing list