[PATCH 6/7] util: Check for pkttyagent availability properly

[Martin Kletzander]

On Fri, Dec 10, 2021 at 05:48:03PM -0700, Jim Fehlig wrote:
>Hi Martin!
>
>I recently received a bug report (sorry, not public) about simple operations
>like 'virsh list' hanging when invoked with an internal test tool. I found this
>commit to be the culprit.
>
>On 11/20/21 16:10, Martin Kletzander wrote:
>> It does not need a tty to work, it opens its controlling terminal for user
>> interaction and with this patch even crazy things like this work:
>>
>>    echo 'list --name' | virsh -q >/dev/null
>
>FYI, your crazy thing worked for me without this commit :-).
>

With a polkit access driver, without a polkit agent already running, and
without a polkit rule allowing access without authentication?

>> Signed-off-by: Martin Kletzander <mkletzan at redhat.com>
>> ---
>>   src/util/virpolkit.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/util/virpolkit.c b/src/util/virpolkit.c
>> index 63bb8133a8aa..7156adc10c0a 100644
>> --- a/src/util/virpolkit.c
>> +++ b/src/util/virpolkit.c
>> @@ -180,9 +180,9 @@ virPolkitAgentCreate(void)
>>       int outfd = STDOUT_FILENO;
>>       int errfd = STDERR_FILENO;
>>
>> -    if (!isatty(STDIN_FILENO)) {
>
>With the test tool invoking virsh, isatty fails
>
>> +    if (!virPolkitAgentAvailable()) {
>
>but virPolkitAgentAvailable succeeds. pkttyagent is then needlessly spawned with
>no one to talk to.
>

Well, pkttyagent does not use stdin at all.  It checks the controlling
terminal and then opens that to be used for both stdout and stdin.
That's why I tried to make it work.

>I haven't been able to cook up a simple reproducer. Not sure if it helps, but
>here's a pstree view of the internal test tool
>
>sshd(15736)---bash(15739)---perl(17717) \
>---runtest(17722)---test.sh(17727) \
>---virsh(17728)-+-pkttyagent(17730)-+-{gdbus}(17732)
>                 |                   |-{gmain}(17731)
>                 |                   `-{pkttyagent}(17733)
>                 `-{vshEventLoop}(17729)
>
>I'm not familiar with the test tool but have cc'd Julie, who might be able to
>answer any questions about it.
>

The thing is that I got our test suite failing because we had the
opposite problem.  Meson is running tests in a new session without a
controlling terminal, however with stdin normally accessible.  Virsh
tries to run it and it fails every time.

I guess there is no harm in also checking if stdin is a tty, I'll send a
patch for that.  Thanks for letting me know!

>Regards,
>Jim
>
>>           virReportError(VIR_ERR_SYSTEM_ERROR, "%s",
>> -                       _("Cannot start polkit text agent without a tty"));
>> +                       _("polkit text authentication agent unavailable"));
>>           goto error;
>>       }
>>
>>
>
-------------- 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/20211211/d97e0102/attachment-0001.sig>