qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Daniel P. Berrangé" <berrange@redhat.com>
To: qemu-devel@nongnu.org
Cc: "Paolo Bonzini" <pbonzini@redhat.com>,
	"Daniel P. Berrangé" <berrange@redhat.com>,
	"Marc-André Lureau" <marcandre.lureau@redhat.com>
Subject: [PATCH] char: fix logging when chardev write fails
Date: Mon, 14 Sep 2020 11:31:09 +0100	[thread overview]
Message-ID: <20200914103109.1327875-1-berrange@redhat.com> (raw)

The qemu_chr_write_buffer() method sends data to the chardev backend for
writing, and then also writes to the log file. In case the chardev
backend only writes part of the data buffer, we need to make sure we
only log the same subset. qemu_chr_write_buffer() will be invoked again
later to write the rest of the buffer.

In the case the chardev backend returns an error though, no further
attempts to likely to be made to write the data. We must therefore write
the entire buffer to the log immediately.

An example where this is important is with the socket backend. This will
return -1 for all writes if no client is currently connected. We still
wish to write data to the log file when no client is present though.
This used to work because the chardev would return "len" to pretend it
had written all data when no client is connected, but this changed to
return an error in

  commit 271094474b65de1ad7aaf729938de3d9b9d0d36f
  Author: Dima Stepanov <dimastep@yandex-team.ru>
  Date: Thu May 28 12:11:18 2020 +0300

    char-socket: return -1 in case of disconnect during tcp_chr_write

and this broke the logging, resulting in all data being discarded when
no client is present.

Fixes: https://bugs.launchpad.net/qemu/+bug/1893691
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 chardev/char.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/chardev/char.c b/chardev/char.c
index 77e7ec814f..e30cd025f5 100644
--- a/chardev/char.c
+++ b/chardev/char.c
@@ -138,7 +138,20 @@ static int qemu_chr_write_buffer(Chardev *s,
         }
     }
     if (*offset > 0) {
+        /*
+         * If some data was written by backend, we should
+         * only log what was actually written. This method
+         * may be invoked again to write the remaining
+         * method, thus we'll log the remainder at that time.
+         */
         qemu_chr_write_log(s, buf, *offset);
+    } else if (res < 0) {
+        /*
+         * If a fatal error was reported by the backend,
+         * assume this method won't be invoked again with
+         * this buffer, so log it all right away.
+         */
+        qemu_chr_write_log(s, buf, len);
     }
     qemu_mutex_unlock(&s->chr_write_lock);
 
-- 
2.26.2



             reply	other threads:[~2020-09-14 10:33 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-14 10:31 Daniel P. Berrangé [this message]
2020-09-14 10:40 ` [PATCH] char: fix logging when chardev write fails Marc-André Lureau
2020-09-22  9:37 ` Paolo Bonzini

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=20200914103109.1327875-1-berrange@redhat.com \
    --to=berrange@redhat.com \
    --cc=marcandre.lureau@redhat.com \
    --cc=pbonzini@redhat.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).