qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Michal Privoznik <mprivozn@redhat.com>
To: qemu-devel@nongnu.org
Cc: kkostiuk@redhat.com, michael.roth@amd.com, jtomko@redhat.com
Subject: [PATCH v2 3/4] qga: Don't daemonize before channel is initialized
Date: Thu,  5 Dec 2024 17:18:47 +0100	[thread overview]
Message-ID: <45717b2498a83e91479a57be7832c3868ac9d135.1733414906.git.mprivozn@redhat.com> (raw)
In-Reply-To: <cover.1733414906.git.mprivozn@redhat.com>

If the agent is set to daemonize but for whatever reason fails to
init the channel, the error message is lost. Worse, the agent
daemonizes needlessly and returns success. For instance:

  # qemu-ga -m virtio-serial \
            -p /dev/nonexistent_device \
            -f /run/qemu-ga.pid \
            -t /run \
            -d
  # echo $?
  0

This makes it needlessly hard for init scripts to detect a
failure in qemu-ga startup. Though, they shouldn't pass '-d' in
the first place.

Let's open the channel first and only after that become a daemon.

Related bug: https://bugs.gentoo.org/810628

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
---
 qga/main.c | 24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/qga/main.c b/qga/main.c
index 68ea7f275a..35f061b5ea 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -1430,7 +1430,6 @@ static GAState *initialize_agent(GAConfig *config, int socket_activation)
         if (config->daemonize) {
             /* delay opening/locking of pidfile till filesystems are unfrozen */
             s->deferred_options.pid_filepath = config->pid_filepath;
-            become_daemon(NULL);
         }
         if (config->log_filepath) {
             /* delay opening the log file till filesystems are unfrozen */
@@ -1438,9 +1437,6 @@ static GAState *initialize_agent(GAConfig *config, int socket_activation)
         }
         ga_disable_logging(s);
     } else {
-        if (config->daemonize) {
-            become_daemon(config->pid_filepath);
-        }
         if (config->log_filepath) {
             FILE *log_file = ga_open_logfile(config->log_filepath);
             if (!log_file) {
@@ -1487,6 +1483,20 @@ static GAState *initialize_agent(GAConfig *config, int socket_activation)
 
     ga_apply_command_filters(s);
 
+    if (!channel_init(s, s->config->method, s->config->channel_path,
+                      s->socket_activation ? FIRST_SOCKET_ACTIVATION_FD : -1)) {
+        g_critical("failed to initialize guest agent channel");
+        return NULL;
+    }
+
+    if (config->daemonize) {
+        if (ga_is_frozen(s)) {
+            become_daemon(NULL);
+        } else {
+            become_daemon(config->pid_filepath);
+        }
+    }
+
     ga_state = s;
     return s;
 }
@@ -1513,8 +1523,9 @@ static void cleanup_agent(GAState *s)
 
 static int run_agent_once(GAState *s)
 {
-    if (!channel_init(s, s->config->method, s->config->channel_path,
-                      s->socket_activation ? FIRST_SOCKET_ACTIVATION_FD : -1)) {
+    if (!s->channel &&
+        channel_init(s, s->config->method, s->config->channel_path,
+                     s->socket_activation ? FIRST_SOCKET_ACTIVATION_FD : -1)) {
         g_critical("failed to initialize guest agent channel");
         return EXIT_FAILURE;
     }
@@ -1523,6 +1534,7 @@ static int run_agent_once(GAState *s)
 
     if (s->channel) {
         ga_channel_free(s->channel);
+        s->channel = NULL;
     }
 
     return EXIT_SUCCESS;
-- 
2.45.2



  parent reply	other threads:[~2024-12-05 16:21 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-12-05 16:18 [PATCH v2 0/4] qga: Open channel before going daemon Michal Privoznik
2024-12-05 16:18 ` [PATCH v2 1/4] qga: Don't access global variable in run_agent_once() Michal Privoznik
2024-12-18 12:05   ` Konstantin Kostiuk
2024-12-05 16:18 ` [PATCH v2 2/4] qga: Invert logic on return value in main() Michal Privoznik
2024-12-05 16:18 ` Michal Privoznik [this message]
2024-12-05 16:18 ` [PATCH v2 4/4] qga: Make run_agent() and run_agent_once() return no value Michal Privoznik
2024-12-16 15:52   ` Konstantin Kostiuk
     [not found]     ` <CAPMcbCoibsbysQLj_LRmuJUZ=ZM2ATP03dbo5zWzPxyyFfycMA@mail.gmail.com>
2025-01-07 13:09       ` Michal Prívozník

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=45717b2498a83e91479a57be7832c3868ac9d135.1733414906.git.mprivozn@redhat.com \
    --to=mprivozn@redhat.com \
    --cc=jtomko@redhat.com \
    --cc=kkostiuk@redhat.com \
    --cc=michael.roth@amd.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).