[libvirt] [PATCH] test_driver: implement virDomainSendProcessSignal

[Ilias Stamatis]

On Fri, Jun 14, 2019 at 10:07 AM Erik Skultety <eskultet at redhat.com> wrote:
>
> On Thu, Jun 13, 2019 at 02:20:22PM +0200, Ilias Stamatis wrote:
> > On Thu, Jun 13, 2019 at 10:20 AM Erik Skultety <eskultet at redhat.com> wrote:
> > >
> > > On Tue, Jun 04, 2019 at 03:17:43PM +0200, Ilias Stamatis wrote:
> > > > Only succeed when @pid_value is 1, since according to the docs this is
> > >
> > > Why do we need to restrict ourselves for @pid 1 in the test driver? This
> > > restriction exists in LXC for a reason, but why in test driver?
> > > a) it's only a check with @pid not being actually used
> > > b) each guest OS will have their own limit in /proc/sys/kernel/pid_max for the
> > > max number of PID so restricting it from the top doesn't make sense
> > > c) - at pid is used for process groups, so again, this will be handled within the
> > > guest OS in reality
> >
> > To my understanding if there's no process with such pid in the guest,
> > this API will fail. If we succeed for any pid, that would mean that
> > the test domain has 2^8 processes running and I wasn't sure if we
> > wanted that. But yes, as I wrote as a "comment" in the initial patch
> > this could as well make sense within test driver's scope so it's fine
> > to remove the check.
>
> Well, 2^8 isn't that bad is it? That's realistic, but long long is 64bit, so
> 2^63-1 that's a lot of PIDs, so you're right, it would feel weird, on the
> other hand, looking at it from the perspective of an app developer, requiring
> pid==1 is restrictive, if you allow any pid, you give them flexibility - PID
> being filled from a dynamic data structure, so eventually, they're just simply
> switch test driver for the real API, but that is my perspective, I don't want
> to force it to upstream, we can wait for another opinion, one can say that if
> they want to test with dynamic PIDs, use the real API.
>
> Erik

Of course I meant 2^64, but I had 8 bytes in my mind so I got confused.

This scenario you describe makes sense, even though currently only the
LXC driver implements this API.

So in conclusion I would say that the possible options are a) only
succeed for pid 1 b) succeed for ANY pid c) succeed for any pid up to
a pid_limit.

All 3 options make sense to me. Maybe I would be a bit more in favor
of the second one since choosing a pid limit would be a bit arbitrary.

Ilias