[PATCH common 0/2] cmd: run: do not leave the command behind on a caller's timeout

[Kefu Chai <k.chai@proxmox.com>]

Hi,

a colleague ran 'pvesm alloc' on an RBD pool whose placement groups
never became active. After 60 seconds the storage lock gave up and got
released, but 'rbd create' just kept running. As far as I can tell,
the lock's alarm handler dies inside run(), and run() only kills its
command on its own timeout or when a callback dies.

Patch 2 kills and reaps the command after run's eval if it's still
around. While looking into this I also noticed that run() cancels the
caller's alarm when it's done, so on shared LVM 'lvcreate' can run
without the lock's limit once 'vgs' ran before it. Patch 1 tries to
fix that.

My first attempt was a RAII-style guard that stops the child in
DESTROY. It worked, but once I let it send TERM first and wait a bit,
it needed signal masking and a few other things, which felt like too
much. Since run's body is wrapped in an eval that catches every die
anyway, a check after the eval seemed to do the same job with a lot
less code, so I went with that.

Things I didn't touch, as they behave like this today already:
- only the direct child gets killed, so commands behind a shell or
  pipe might keep running
- a run timeout longer than the caller's alarm still delays it
- with noerr, a die from the caller's handler still gets swallowed

Both patches add tests to test/cmd-test.pl, and I tried the 'pvesm
alloc' case on a three node test cluster. I might well have missed
something, so any feedback is welcome, including whether the guard
would be the nicer way after all.

Kefu Chai (2):
  cmd: run: do not cancel the caller's alarm
  cmd: run: kill the command when run dies

 src/PVE/Cmd.pm   | 74 +++++++++++++++++++++---------------------------
 test/cmd-test.pl | 68 +++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 100 insertions(+), 42 deletions(-)

-- 
2.47.3