Re: [PATCH 6/7] util: Check for pkttyagent availability properly
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(a)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;
> }
>
>
Attachments:
- signature.asc (application/pgp-signature — 833 bytes)