NuKVM/virt-manager
3
0
Fork 0
mirror of https://github.com/virt-manager/virt-manager.git synced 2025-01-08 07:03:02 -06:00
Code Issues Packages Projects Releases Wiki Activity

virt-manager: Switch to --no-fork by default

Browse Source 
The reason we fork by default, is to force ssh to invoke
ssh-askpass when a password is required, rather than prompt on
 a terminal no one is looking at. There's a more thorough
explanation here:
https://github.com/virt-manager/virt-manager/issues/731

With SSH_ASKPASS_REQUIRE=force, we now have a way to force ssh
to use askpass in the above scenario, when ssh and libvirt are new
enough.

The default forking behavior has caused maintenance pain in the
past, and is currently causing issues on macos:
https://github.com/virt-manager/virt-manager/issues/620

Let's flip the default to `--no-fork`. The VIRT_MANAGER_DEFAULT_FORK
env variable is there as an escape hatch incase I really miscalculated.

I don't expect many people are depending on use of askpass either
way, or if they are, they are launching virt-manager from their
desktop and not a terminal, which already gives us the correct
behavior AFAICT>

My suspicion is barely anyone will notice, which is why
I'm ok with changing this now, despite the libvirt support being
brand new.

If this doesn't raise any issues, then we can eventually drop
the forking behavior all together.

Signed-off-by: Cole Robinson <crobinso@redhat.com>
This commit is contained in:
Cole Robinson 2024-10-10 10:52:20 -04:00 committed by Pavel Hrdina
parent 028fdf2c35
commit 408a39a777
2 changed files with 9 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
man/virt-manager.rst
View File

@ -108,14 +108,11 @@ manual ``--connect`` URI. But it supports ``--connect`` URI as well:
VIRT-MANAGER, SSH, AND FORKING VIRT-MANAGER, SSH, AND FORKING
============================== ==============================
On startup virt-manager will detach from the running Historically, on startup virt-manager would detach from the running
terminal and fork into the background. This is to force any usage of terminal and fork into the background. This was to force any usage of
ssh to call ssh-askpass when it needs a password, rather than silently ssh to call ssh-askpass when it needed a password, rather than silently
asking on a terminal the user probably isn't watching. asking on a terminal the user probably isn't watching.
Users can opt out of this forking behavior with ``--no-fork``, or
by setting the ``VIRT_MANAGER_DEFAULT_FORK=no`` environment variable.
openssh 8.4p1 released in Sep 2020 added the SSH_ASKPASS_REQUIRE openssh 8.4p1 released in Sep 2020 added the SSH_ASKPASS_REQUIRE
environment variable that saves us from having to do the fork dance. environment variable that saves us from having to do the fork dance.
https://man.openbsd.org/ssh.1#SSH_ASKPASS_REQUIRE https://man.openbsd.org/ssh.1#SSH_ASKPASS_REQUIRE
@ -124,9 +121,10 @@ virt-manager now sets SSH_ASKPASS_REQUIRE=force.
However to get this to work with libvirt ssh connections, you'll need However to get this to work with libvirt ssh connections, you'll need
libvirt 10.8.0 released in October 1st 2024. libvirt 10.8.0 released in October 1st 2024.
In the future, virt-manager will likely stop forking by default. virt-manager no longer forks by defaults.
You can future proof request forking with ``--fork`` or by
settings the ``VIRT_MANAGER_DEFAULT_FORK=yes`` environment variable. You can get the old forking behavior with the ``--fork`` option,
or by setting the ``VIRT_MANAGER_DEFAULT_FORK=yes`` environment variable.
However if you find you need forking for a usecase other than temporarily However if you find you need forking for a usecase other than temporarily
working around libvirt version issues, please let the virt-manager developers working around libvirt version issues, please let the virt-manager developers

4
virtManager/virtmanager.py
View File

@ -135,8 +135,8 @@ def do_we_fork(options):
if val: if val:
log.warning("Unknown %s=%s, expected 'yes' or 'no'", key, val) log.warning("Unknown %s=%s, expected 'yes' or 'no'", key, val)
# Default is `--fork` # Default is `--no-fork`
return True return False
def parse_commandline(): def parse_commandline():