Re: [PULL 5/5] qemu_set_log_filename: filename argument may be NULL
On 2/12/20 2:45 AM, Alex Bennée wrote: > > Richard Henderson writes: > >> On 1/30/20 1:38 PM, Stefan Hajnoczi wrote: >>> From: Salvador Fandino >>> >>> NULL is a valid log filename used to indicate we want to use stderr >>> but qemu_set_log_filename (which is called by bsd-user/main.c) was not >>> handling it correctly. >>> >>> That also made redundant a couple of NULL checks in calling code which >>> have been removed. >>> >>> Signed-off-by: Salvador Fandino >>> Message-Id: <20200123193626.19956-1-salva...@qindel.com> >>> Signed-off-by: Stefan Hajnoczi >>> --- >>> trace/control.c | 4 +--- >>> util/log.c | 28 >>> vl.c| 5 + >>> 3 files changed, 18 insertions(+), 19 deletions(-) >> >> This patch has broken -D for *-linux-user. >> After e144a605a, all logging goes to stderr. > > I posted: > > Subject: [PATCH] tracing: only allow -trace to override -D if set > Date: Tue, 11 Feb 2020 11:10:54 + > Message-Id: <2020021054.27538-1-alex.ben...@linaro.org> > > as a fix which partially reverted this. Thanks, that fixes it for me. r~
Re: [PULL 5/5] qemu_set_log_filename: filename argument may be NULL
Richard Henderson writes:
> On 1/30/20 1:38 PM, Stefan Hajnoczi wrote:
>> From: Salvador Fandino
>>
>> NULL is a valid log filename used to indicate we want to use stderr
>> but qemu_set_log_filename (which is called by bsd-user/main.c) was not
>> handling it correctly.
>>
>> That also made redundant a couple of NULL checks in calling code which
>> have been removed.
>>
>> Signed-off-by: Salvador Fandino
>> Message-Id: <20200123193626.19956-1-salva...@qindel.com>
>> Signed-off-by: Stefan Hajnoczi
>> ---
>> trace/control.c | 4 +---
>> util/log.c | 28
>> vl.c| 5 +
>> 3 files changed, 18 insertions(+), 19 deletions(-)
>
> This patch has broken -D for *-linux-user.
> After e144a605a, all logging goes to stderr.
I posted:
Subject: [PATCH] tracing: only allow -trace to override -D if set
Date: Tue, 11 Feb 2020 11:10:54 +
Message-Id: <2020021054.27538-1-alex.ben...@linaro.org>
as a fix which partially reverted this.
>
>> +if (filename) {
>> +char *pidstr = strstr(filename, "%");
>> +if (pidstr) {
>
> Also, the indentation is off.
>
>
> r~
--
Alex Bennée
Re: [PULL 5/5] qemu_set_log_filename: filename argument may be NULL
On 1/30/20 1:38 PM, Stefan Hajnoczi wrote:
> From: Salvador Fandino
>
> NULL is a valid log filename used to indicate we want to use stderr
> but qemu_set_log_filename (which is called by bsd-user/main.c) was not
> handling it correctly.
>
> That also made redundant a couple of NULL checks in calling code which
> have been removed.
>
> Signed-off-by: Salvador Fandino
> Message-Id: <20200123193626.19956-1-salva...@qindel.com>
> Signed-off-by: Stefan Hajnoczi
> ---
> trace/control.c | 4 +---
> util/log.c | 28
> vl.c| 5 +
> 3 files changed, 18 insertions(+), 19 deletions(-)
This patch has broken -D for *-linux-user.
After e144a605a, all logging goes to stderr.
> +if (filename) {
> +char *pidstr = strstr(filename, "%");
> +if (pidstr) {
Also, the indentation is off.
r~
[PULL 5/5] qemu_set_log_filename: filename argument may be NULL
From: Salvador Fandino
NULL is a valid log filename used to indicate we want to use stderr
but qemu_set_log_filename (which is called by bsd-user/main.c) was not
handling it correctly.
That also made redundant a couple of NULL checks in calling code which
have been removed.
Signed-off-by: Salvador Fandino
Message-Id: <20200123193626.19956-1-salva...@qindel.com>
Signed-off-by: Stefan Hajnoczi
---
trace/control.c | 4 +---
util/log.c | 28
vl.c| 5 +
3 files changed, 18 insertions(+), 19 deletions(-)
diff --git a/trace/control.c b/trace/control.c
index 0fb8124160..6c775e68eb 100644
--- a/trace/control.c
+++ b/trace/control.c
@@ -229,9 +229,7 @@ void trace_init_file(const char *file)
/* If both the simple and the log backends are enabled, "--trace file"
* only applies to the simple backend; use "-D" for the log backend.
*/
-if (file) {
-qemu_set_log_filename(file, _fatal);
-}
+qemu_set_log_filename(file, _fatal);
#else
if (file) {
fprintf(stderr, "error: --trace file=...: "
diff --git a/util/log.c b/util/log.c
index 867264da8d..47f2827397 100644
--- a/util/log.c
+++ b/util/log.c
@@ -148,25 +148,29 @@ void qemu_log_needs_buffers(void)
* Allow the user to include %d in their logfile which will be
* substituted with the current PID. This is useful for debugging many
* nested linux-user tasks but will result in lots of logs.
+ *
+ * filename may be NULL. In that case, log output is sent to stderr
*/
void qemu_set_log_filename(const char *filename, Error **errp)
{
-char *pidstr;
g_free(logfilename);
logfilename = NULL;
-pidstr = strstr(filename, "%");
-if (pidstr) {
-/* We only accept one %d, no other format strings */
-if (pidstr[1] != 'd' || strchr(pidstr + 2, '%')) {
-error_setg(errp, "Bad logfile format: %s", filename);
-return;
-} else {
-logfilename = g_strdup_printf(filename, getpid());
-}
-} else {
-logfilename = g_strdup(filename);
+if (filename) {
+char *pidstr = strstr(filename, "%");
+if (pidstr) {
+/* We only accept one %d, no other format strings */
+if (pidstr[1] != 'd' || strchr(pidstr + 2, '%')) {
+error_setg(errp, "Bad logfile format: %s", filename);
+return;
+} else {
+logfilename = g_strdup_printf(filename, getpid());
+}
+} else {
+logfilename = g_strdup(filename);
+}
}
+
qemu_log_close();
qemu_set_log(qemu_loglevel);
}
diff --git a/vl.c b/vl.c
index 24951b51a9..7dcb0879c4 100644
--- a/vl.c
+++ b/vl.c
@@ -3903,10 +3903,7 @@ int main(int argc, char **argv, char **envp)
/* Open the logfile at this point and set the log mask if necessary.
*/
-if (log_file) {
-qemu_set_log_filename(log_file, _fatal);
-}
-
+qemu_set_log_filename(log_file, _fatal);
if (log_mask) {
int mask;
mask = qemu_str_to_log_mask(log_mask);
--
2.24.1
