[PATCH] util: Don't spawn pkttyagent when stdin is not a tty

[Martin Kletzander]

On Fri, Dec 17, 2021 at 09:33:53AM -0700, Jim Fehlig wrote:
>On 12/17/21 9:25 AM, Jim Fehlig wrote:
>> On 12/17/21 7:01 AM, Michal Prívozník wrote:
>>> On 12/11/21 14:23, Martin Kletzander wrote:
>>>> My idea was that running pkttyagent unconditionally, modulo checks that
>>>> pkttyagent itself does to make sure it does not fail, is not going to be an
>>>> issue turned out to be wrong.  Adding back the original check for stdin being a
>>>> tty helps in some testing scenarios as reported by Jim Fehlig and does not
>>>> really cause any issues.  I originally wanted it in because it also made
>>>> pkttyagent auth work with redirected input into virsh (with a connection that
>>>> requires polkit authentication and without a session-wide polkit tty agent,
>>>> basically making pkttyagent necessary to succeed).  But anyone running virsh
>>>> like that is asking for problems already anyway =)
>>>>
>>>> Signed-off-by: Martin Kletzander <mkletzan at redhat.com>
>>>> ---
>>>>   src/util/virpolkit.c | 3 +++
>>>>   1 file changed, 3 insertions(+)
>>>
>>> In case you want to push this:
>>>
>>> Reviewed-by: Michal Privoznik <mprivozn at redhat.com>
>>
>> And
>>
>> Tested-by: Jim Fehlig <jfehlig at suse.com>
>>
>>>
>>> But I'd really like to get to the bottom of this. Why does sleep()
>>> suspends for that long and why isn't polkit-agent killed?
>>
>> I got sidetracked before I could dig deeper, however I did notice the test
>> process and all children were in a stopped state per /proc/<pid>/status. I sent
>> SIGCONT to the processes and the test successfully completed. I'm baffled why
>> these processes become stopped when pkttyagent is in the picture and work fine
>> otherwise.
>

That's interesting.  So let's assume it is stopped because of a signal.
By default it can be one of:

SIGSTOP      P1990      Stop    Stop process
SIGTSTP      P1990      Stop    Stop typed at terminal
SIGTTIN      P1990      Stop    Terminal input for background process
SIGTTOU      P1990      Stop    Terminal output for background process

as those are the only ones that have default action set to Stop.
Looking at pkttyagent sources it does not look like it is sending its
parent any signals (looked for getppid and kill calls) and I presume
your test suite will not do that either, why would it.  So let's say we
can eliminate the first two.

What we're left with is SIGTTIN and SIGTTOU which should be sent to us
if we are trying to do I/O on the terminal.  If pkttyagent was to ask
for anything then it would be done on the controlling terminal even if
standard input, output, and error output are redirected.  But that (the
agent doing I/O on the terminal) should only happen if it is really
needed (not if there's an error for example, that goes to stderr).  And
if that happened then it should not be sent to the whole process group,
should it?

I would suggest you install a handler in virsh and get some debug output
for some of the signals (sigaction with SA_SIGINFO as siginfo_t hlready
has enough information to further debug the issue, mainly the caller.
Unfortunately that would not help for SIGSTOP as that cannot be used for
sigaction(2), but maybe we could at least get somewhere?  There's still
hope!

>I forgot to mention, it's likely a problem in the test framework code, which is
>apparently unmaintained by still used by SUSE QA. "I got sidetracked" is a nice
>way to say I didn't have a lot of motivation to read through a gob of old,
>unfamiliar perl code :-).
>

But libvirt-tck is still maintained O:-) =D

Anyway, looking at kill() and SIGSTOP references in that code might at
least give a clue to whether it is even possible.

>Jim
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20211217/f0f7cd46/attachment-0001.sig>