As discussed in https://gitlab.com/libvirt/libvirt/-/issues/757. I'm not quite sure about using a long-only-option, since that's a first in virsh. But OTOH this option seems too insignificant to also use one of the precious short option characters for it. Please check and let me know what you think. Thanks! Jens Schmidt (1): virsh: Add option '--no-pkttyagent' NEWS.rst | 4 ++++ docs/manpages/virsh.rst | 8 ++++++++ tools/virsh.c | 13 ++++++++++++- tools/vsh.h | 1 + 4 files changed, 25 insertions(+), 1 deletion(-) -- 2.39.5 In scripts repeated execution of virsh can result in a lot of journal noise when pkttyagent gets registered with polkitd each time. Resolves: https://gitlab.com/libvirt/libvirt/-/issues/757 --- NEWS.rst | 4 ++++ docs/manpages/virsh.rst | 8 ++++++++ tools/virsh.c | 13 ++++++++++++- tools/vsh.h | 1 + 4 files changed, 25 insertions(+), 1 deletion(-) diff --git a/NEWS.rst b/NEWS.rst index XXXXXXX..XXXXXXX 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -XXX,XX +XXX,XX @@ v11.4.0 (unreleased) * **New features** + * virsh: Add option ``--no-pkttyagent`` + + That option suppresses registration of pkttyagent with polkitd. + * **Improvements** * **Bug fixes** diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst index XXXXXXX..XXXXXXX 100644 --- a/docs/manpages/virsh.rst +++ b/docs/manpages/virsh.rst @@ -XXX,XX +XXX,XX @@ Output elapsed time information for each command. +- ``--no-pkttyagent`` + +Do not register ``pkttyagent`` as authentication agent with the +polkit system daemon, even if ``virsh`` has been started from a +terminal. + + + - ``-v``, ``--version[=short]`` Ignore all other arguments, and prints the version of the libvirt library diff --git a/tools/virsh.c b/tools/virsh.c index XXXXXXX..XXXXXXX 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -XXX,XX +XXX,XX @@ #include <config.h> #include "virsh.h" +#include <assert.h> #include <unistd.h> #include <getopt.h> #include <sys/time.h> @@ -XXX,XX +XXX,XX @@ virshConnect(vshControl *ctl, const char *uri, bool readonly) keepalive_forced = true; } - if (virPolkitAgentAvailable() && + if (!ctl->no_pkttyagent && + virPolkitAgentAvailable() && !(pkagent = virPolkitAgentCreate())) virResetLastError(); @@ -XXX,XX +XXX,XX @@ virshUsage(void) " -q | --quiet quiet mode\n" " -r | --readonly connect readonly\n" " -t | --timing print timing information\n" + " --no-pkttyagent suppress registration of pkttyagent\n" " -v short version\n" " -V long version\n" " --version[=TYPE] version, TYPE is short or long (default short)\n" @@ -XXX,XX +XXX,XX @@ virshParseArgv(vshControl *ctl, int argc, char **argv) { "quiet", no_argument, NULL, 'q' }, { "readonly", no_argument, NULL, 'r' }, { "timing", no_argument, NULL, 't' }, + { "no-pkttyagent", no_argument, NULL, 0 }, { "version", optional_argument, NULL, 'v' }, { NULL, 0, NULL, 0 }, }; @@ -XXX,XX +XXX,XX @@ virshParseArgv(vshControl *ctl, int argc, char **argv) case 'V': virshShowVersion(ctl); exit(EXIT_SUCCESS); + case 0: + if (STREQ(opt[longindex].name, "no-pkttyagent")) { + ctl->no_pkttyagent = true; + break; + } else { + assert(false); + } case ':': for (i = 0; opt[i].name != NULL; i++) { if (opt[i].val == optopt) diff --git a/tools/vsh.h b/tools/vsh.h index XXXXXXX..XXXXXXX 100644 --- a/tools/vsh.h +++ b/tools/vsh.h @@ -XXX,XX +XXX,XX @@ struct _vshControl { bool imode; /* interactive mode? */ bool quiet; /* quiet mode */ bool timing; /* print timing info? */ + bool no_pkttyagent; /* suppress registration of pkttyagent? */ int debug; /* print debug messages? */ char *logfile; /* log file name */ int log_fd; /* log file descriptor */ -- 2.39.5 This patchset should address all of your comments. Where "you" means Peter Krempa, I hope that the "--in-reply-to" does its job as I would expect it would do. Just to be sure, this is in reply to thread [1], namely to the message with ID <aBnHTttMLaNd3E3U@angien.pipo.sk>. Some minor nits I came across while implementing this, and you are totally free to a) ignore them, b) fix them yourself, or c) ask me to fix them in a separate patch: - You actually do use assertions, namely in vsh.c. This is where I got the idea from that using them in virsh.c, too, would be OK. (Well, I guess option c) from above isn't something I'd want to do for that nit...) - In docs/manpages/virsh.rst, you use plain paragraphs for the documentation of options --connect and --debug and bulleted items for all other options. Thanks for your support and for maintaining libvirt in general! [1]: https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/Y5JHSUVDZGHSAPZD32ROD5C4SLCVITLE/#7PD43EDB7JUBLYVOMTBN3Z624FENZYYC Jens Schmidt (2): virsh: Add option '--no-pkttyagent' NEWS: Mention new option '--no-pkttyagent' NEWS.rst | 4 ++++ docs/manpages/virsh.rst | 8 ++++++++ tools/virsh.c | 13 ++++++++++++- tools/vsh.h | 1 + 4 files changed, 25 insertions(+), 1 deletion(-) -- 2.39.5 From: Jens Schmidt <farblos@vodafonemail.de> In scripts repeated execution of virsh can result in a lot of journal noise when pkttyagent gets registered with polkitd each time. Closes: https://gitlab.com/libvirt/libvirt/-/issues/757 Signed-off-by: Jens Schmidt <farblos@vodafonemail.de> Reviewed-by: Peter Krempa <pkrempa@redhat.com> --- docs/manpages/virsh.rst | 8 ++++++++ tools/virsh.c | 13 ++++++++++++- tools/vsh.h | 1 + 3 files changed, 21 insertions(+), 1 deletion(-) diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst index XXXXXXX..XXXXXXX 100644 --- a/docs/manpages/virsh.rst +++ b/docs/manpages/virsh.rst @@ -XXX,XX +XXX,XX @@ Output elapsed time information for each command. +- ``--no-pkttyagent`` + +Do not register ``pkttyagent`` as authentication agent with the +polkit system daemon, even if ``virsh`` has been started from a +terminal. + + + - ``-v``, ``--version[=short]`` Ignore all other arguments, and prints the version of the libvirt library diff --git a/tools/virsh.c b/tools/virsh.c index XXXXXXX..XXXXXXX 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -XXX,XX +XXX,XX @@ virshConnect(vshControl *ctl, const char *uri, bool readonly) keepalive_forced = true; } - if (virPolkitAgentAvailable() && + if (!ctl->no_pkttyagent && + virPolkitAgentAvailable() && !(pkagent = virPolkitAgentCreate())) virResetLastError(); @@ -XXX,XX +XXX,XX @@ virshUsage(void) " -q | --quiet quiet mode\n" " -r | --readonly connect readonly\n" " -t | --timing print timing information\n" + " --no-pkttyagent suppress registration of pkttyagent\n" " -v short version\n" " -V long version\n" " --version[=TYPE] version, TYPE is short or long (default short)\n" @@ -XXX,XX +XXX,XX @@ virshParseArgv(vshControl *ctl, int argc, char **argv) { "quiet", no_argument, NULL, 'q' }, { "readonly", no_argument, NULL, 'r' }, { "timing", no_argument, NULL, 't' }, + { "no-pkttyagent", no_argument, NULL, 0 }, { "version", optional_argument, NULL, 'v' }, { NULL, 0, NULL, 0 }, }; @@ -XXX,XX +XXX,XX @@ virshParseArgv(vshControl *ctl, int argc, char **argv) case 'V': virshShowVersion(ctl); exit(EXIT_SUCCESS); + case 0: + if (STREQ(opt[longindex].name, "no-pkttyagent")) { + ctl->no_pkttyagent = true; + break; + } else { + vshError(ctl, "%s", _("unknown option")); + exit(EXIT_FAILURE); + } case ':': for (i = 0; opt[i].name != NULL; i++) { if (opt[i].val == optopt) diff --git a/tools/vsh.h b/tools/vsh.h index XXXXXXX..XXXXXXX 100644 --- a/tools/vsh.h +++ b/tools/vsh.h @@ -XXX,XX +XXX,XX @@ struct _vshControl { bool imode; /* interactive mode? */ bool quiet; /* quiet mode */ bool timing; /* print timing info? */ + bool no_pkttyagent; /* suppress registration of pkttyagent? */ int debug; /* print debug messages? */ char *logfile; /* log file name */ int log_fd; /* log file descriptor */ -- 2.39.5 From: Jens Schmidt <farblos@vodafonemail.de> Signed-off-by: Jens Schmidt <farblos@vodafonemail.de> Reviewed-by: Peter Krempa <pkrempa@redhat.com> --- NEWS.rst | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/NEWS.rst b/NEWS.rst index XXXXXXX..XXXXXXX 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -XXX,XX +XXX,XX @@ v11.4.0 (unreleased) * **New features** + * virsh: Add option ``--no-pkttyagent`` + + That option suppresses registration of pkttyagent with polkitd. + * **Improvements** * **Bug fixes** -- 2.39.5