[libvirt] [PATCH v3 1/2] process: wait longer on kill per assigned Hostdev

Christian Ehrhardt christian.ehrhardt at canonical.com
Thu Aug 16 13:09:59 UTC 2018


On Tue, Aug 14, 2018 at 12:13 PM Bjoern Walk <bwalk at linux.ibm.com> wrote:

> Christian Ehrhardt <christian.ehrhardt at 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20180816/1e6d5807/attachment-0001.htm>


More information about the libvir-list mailing list