Libguestfs - Mar 2022 - [p2v PATCH 09/10] rely on Linux for killing nbdkit, when the parent thread exits

So this patch unfortunately isn't correct for a few reasons:

(1) Non-Linux/BSD OSes don't support a concept like
--exit-with-parent.  nbdkit will exit with an error if you try to use
this feature on such OSes.  You can test if --exit-with-parent is
supported before trying to use it by:

  nbdkit --exit-with-parent --version

which will exit with an error if the feature is not supported.

However this isn't really a major issue for us.  The bigger one is:

(2) --exit-with-parent is not reliable.  It's best thought of as a
best-effort, fallback attempt to kill nbdkit.  It requires that nbdkit
issues a prctl(2), so any time that the parent (virt-p2v) dies before
nbdkit issues the prctl, nbdkit will not get cleaned up.

So I would say to implement this you'll have to:

 - Check if ?system ("nbdkit --exit-with-parent --version")? returns
0.

 - If so, add the --exit-with-parent flag (otherwise, don't).

 - Keep the PID tracking / kill code that already exists.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v

On 03/31/22 15:11, Richard W.M. Jones wrote:
> So this patch unfortunately isn't correct for a few reasons:
> 
> (1) Non-Linux/BSD OSes don't support a concept like
> --exit-with-parent.  nbdkit will exit with an error if you try to use
> this feature on such OSes.  You can test if --exit-with-parent is
> supported before trying to use it by:
> 
>   nbdkit --exit-with-parent --version
> 
> which will exit with an error if the feature is not supported.
> 
> However this isn't really a major issue for us.  The bigger one is:
> 
> (2) --exit-with-parent is not reliable.  It's best thought of as a
> best-effort, fallback attempt to kill nbdkit.  It requires that nbdkit
> issues a prctl(2), so any time that the parent (virt-p2v) dies before
> nbdkit issues the prctl, nbdkit will not get cleaned up.
> 
> So I would say to implement this you'll have to:
> 
>  - Check if ?system ("nbdkit --exit-with-parent --version")?
returns 0.
> 
>  - If so, add the --exit-with-parent flag (otherwise, don't).
> 
>  - Keep the PID tracking / kill code that already exists.
I've been completely prepared to just drop this patch. In fact, I
started writing the question "what does --exit-with-parent gain us?"
in
response to your
<https://listman.redhat.com/archives/libguestfs/2022-March/028475.html>.
But then as I progressed with my counter-arguments, I just figured I'd
pose my question as a patch.

And yes, that's the crux: if we need to keep the PID tracking / killing
/ reaping (and I agree that we do!), then what does
"--exit-with-parent"
buy us? I don't think it improves anything. So if I can't remove the PID
tracking (and I agree I shouldn't), I'd just forget about
"--exit-with-parent" and drop this patch altogether.

Thanks!
Laszlo