Re: [PATCH v2 4/5] qemu_passt: Deduplicate passt killing code

[Laine Stump <laine@redhat.com>]

On 2/16/23 8:32 AM, Michal Privoznik 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@redhat.com>

Reviewed-by: Laine Stump <laine@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)
>   {
> -    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);
>       return -1;
>   }