On Tue, Aug 14, 2018 at 12:13 PM Bjoern Walk <bwalk(a)linux.ibm.com> wrote:
Christian Ehrhardt <christian.ehrhardt(a)canonical.com>
[2018-08-14,
11:27AM +0200]:
> diff --git a/src/util/virprocess.c b/src/util/virprocess.c
> index ecea27a2d4..46360cc051 100644
> --- a/src/util/virprocess.c
> +++ b/src/util/virprocess.c
> @@ -341,15 +341,19 @@ int virProcessKill(pid_t pid, int sig)
> * Returns 0 if it was killed gracefully, 1 if it
> * was killed forcibly, -1 if it is still alive,
> * or another error occurred.
> + *
> + * Callers can proide an extra delay to wait longer
> + * than the default.
> */
> int
> -virProcessKillPainfully(pid_t pid, bool force)
> +virProcessKillPainfullyDelay(pid_t pid, bool force, unsigned int
extradelay)
> {
> size_t i;
> int ret = -1;
> + unsigned int delay = 75 + (extradelay*5);
> const char *signame = "TERM";
>
> - VIR_DEBUG("vpid=%lld force=%d", (long long)pid, force);
> + VIR_DEBUG("vpid=%lld force=%d delay=%u", (long long)pid, force,
pid);
^
This probably should be delay, right?
Absolutely correct, bad old copy-pasta from the beginning of this change.
Thanks for spotting it.
Example is with 12 Devices passed through and force, which means with both
patches applied:
- raise base value to wait after kill by 30 seconds (force is set) - second
patch
- request 12*2 seconds extra for the devices - this patch
That should be 200+(12*2*5)=320 and that matches what I see int he log:
2018-08-16 13:04:51.999+0000: 61243: debug :
virProcessKillPainfullyDelay:359 : vpid=60939 force=1 delay=320
I'm not pushing a V2 just for the change, but I have it queued with the
fixed variable.
Any other feedback on this or the sibling patch?
Especially since those aren't my usual apparmor changes I'd really want
some more reviews/acks before considering a push.
--
Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd