[PATCH v2 4/5] qemu_passt: Deduplicate passt killing code
Stefano Brivio
sbrivio at redhat.com
Thu Feb 16 16:07:08 UTC 2023
On Thu, 16 Feb 2023 14:32:51 +0100
Michal Privoznik <mprivozn at redhat.com> wrote:
> There are two places where we kill passt:
>
> 1) qemuPasstStop() - called transitively from qemuProcessStop(),
> 2) qemuPasstStart() - after failed start.
>
> Now, the code from 2) lack error preservation (so if there's
> another error during cleanup we might overwrite the original
> error). Therefore, move the internals of qemuPasstStop() into a
> separate function and call it from both places.
>
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
> src/qemu/qemu_passt.c | 23 +++++++++++++----------
> 1 file changed, 13 insertions(+), 10 deletions(-)
>
> diff --git a/src/qemu/qemu_passt.c b/src/qemu/qemu_passt.c
> index 881205449b..a4cc9e7166 100644
> --- a/src/qemu/qemu_passt.c
> +++ b/src/qemu/qemu_passt.c
> @@ -102,11 +102,9 @@ qemuPasstAddNetProps(virDomainObj *vm,
> }
>
>
> -void
> -qemuPasstStop(virDomainObj *vm,
> - virDomainNetDef *net)
> +static void
> +qemuPasstKill(const char *pidfile)
A minor comment, should you respin: I think it should be made clear that
this is not the expected/normal way in which passt will terminate --
here or in the next patch. Removing the PID file is nice, but that's
(usually) about it.
> {
> - g_autofree char *pidfile = qemuPasstCreatePidFilename(vm, net);
> virErrorPtr orig_err;
>
> virErrorPreserveLast(&orig_err);
> @@ -118,6 +116,16 @@ qemuPasstStop(virDomainObj *vm,
> }
>
>
> +void
> +qemuPasstStop(virDomainObj *vm,
> + virDomainNetDef *net)
> +{
> + g_autofree char *pidfile = qemuPasstCreatePidFilename(vm, net);
> +
> + qemuPasstKill(pidfile);
> +}
> +
> +
> int
> qemuPasstSetupCgroup(virDomainObj *vm,
> virDomainNetDef *net,
> @@ -147,7 +155,6 @@ qemuPasstStart(virDomainObj *vm,
> g_autofree char *errbuf = NULL;
> char macaddr[VIR_MAC_STRING_BUFLEN];
> size_t i;
> - pid_t pid = (pid_t) -1;
> int exitstatus = 0;
> int cmdret = 0;
>
> @@ -289,10 +296,6 @@ qemuPasstStart(virDomainObj *vm,
> return 0;
>
> error:
> - ignore_value(virPidFileReadPathIfLocked(pidfile, &pid));
> - if (pid != -1)
> - virProcessKillPainfully(pid, true);
> - unlink(pidfile);
> -
> + qemuPasstKill(pidfile);
...what takes care of terminating passt in case qemu doesn't start, now?
The fact that the process is in the cgroup, right?
> return -1;
> }
--
Stefano
More information about the libvir-list
mailing list