From 54ee5b3da00cdbee836a06a7e6eb319c895f2a57 Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Sun, 17 Apr 2022 11:29:41 -0700 Subject: util/log: Drop manual log buffering MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This buffering was introduced during the Paleozoic: 9fa3e853531. There has never been an explanation as to why we may not allow glibc to allocate the file buffer itself. We certainly have many other uses of mmap and malloc during user-only startup, so presumably whatever the issue was, it has been fixed during the preceeding 18 years. Reviewed-by: Alex Bennée Signed-off-by: Richard Henderson Message-Id: <20220417183019.755276-2-richard.henderson@linaro.org> --- linux-user/main.c | 1 - 1 file changed, 1 deletion(-) (limited to 'linux-user/main.c') diff --git a/linux-user/main.c b/linux-user/main.c index fbc9bcfd5f..2b06350688 100644 --- a/linux-user/main.c +++ b/linux-user/main.c @@ -679,7 +679,6 @@ int main(int argc, char **argv, char **envp) log_mask = last_log_mask | (enable_strace ? LOG_STRACE : 0); if (log_mask) { - qemu_log_needs_buffers(); qemu_set_log(log_mask); } -- cgit 1.4.1 From c5955f4ff4689b7a04cf0a1109fa97ce885876b4 Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Sun, 17 Apr 2022 11:29:44 -0700 Subject: util/log: Pass Error pointer to qemu_set_log MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Do not force exit within qemu_set_log; return bool and pass an Error value back up the stack as per usual. Reviewed-by: Alex Bennée Signed-off-by: Richard Henderson Message-Id: <20220417183019.755276-5-richard.henderson@linaro.org> --- bsd-user/main.c | 2 +- include/qemu/log.h | 2 +- linux-user/main.c | 2 +- monitor/misc.c | 6 +++++- qemu-img.c | 2 +- qemu-io.c | 2 +- qemu-nbd.c | 2 +- scsi/qemu-pr-helper.c | 2 +- softmmu/vl.c | 4 ++-- storage-daemon/qemu-storage-daemon.c | 2 +- tests/unit/test-logging.c | 2 +- util/log.c | 12 ++++++------ 12 files changed, 22 insertions(+), 18 deletions(-) (limited to 'linux-user/main.c') diff --git a/bsd-user/main.c b/bsd-user/main.c index 08f43af235..0c8aa1ae90 100644 --- a/bsd-user/main.c +++ b/bsd-user/main.c @@ -414,7 +414,7 @@ int main(int argc, char **argv) qemu_print_log_usage(stdout); exit(1); } - qemu_set_log(mask); + qemu_set_log(mask, &error_fatal); } if (optind >= argc) { diff --git a/include/qemu/log.h b/include/qemu/log.h index fabfbe41da..0b892f5e90 100644 --- a/include/qemu/log.h +++ b/include/qemu/log.h @@ -147,7 +147,7 @@ typedef struct QEMULogItem { extern const QEMULogItem qemu_log_items[]; -void qemu_set_log(int log_flags); +bool qemu_set_log(int log_flags, Error **errp); bool qemu_set_log_filename(const char *filename, Error **errp); void qemu_set_dfilter_ranges(const char *ranges, Error **errp); bool qemu_log_in_addr_range(uint64_t addr); diff --git a/linux-user/main.c b/linux-user/main.c index 2b06350688..6aed4929ab 100644 --- a/linux-user/main.c +++ b/linux-user/main.c @@ -679,7 +679,7 @@ int main(int argc, char **argv, char **envp) log_mask = last_log_mask | (enable_strace ? LOG_STRACE : 0); if (log_mask) { - qemu_set_log(log_mask); + qemu_set_log(log_mask, &error_fatal); } if (!trace_init_backends()) { diff --git a/monitor/misc.c b/monitor/misc.c index 4b68fcd346..33372b93cc 100644 --- a/monitor/misc.c +++ b/monitor/misc.c @@ -428,6 +428,7 @@ static void hmp_log(Monitor *mon, const QDict *qdict) { int mask; const char *items = qdict_get_str(qdict, "items"); + Error *err = NULL; if (!strcmp(items, "none")) { mask = 0; @@ -438,7 +439,10 @@ static void hmp_log(Monitor *mon, const QDict *qdict) return; } } - qemu_set_log(mask); + + if (!qemu_set_log(mask, &err)) { + error_report_err(err); + } } static void hmp_singlestep(Monitor *mon, const QDict *qdict) diff --git a/qemu-img.c b/qemu-img.c index 116e058675..ef3224a9d4 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -5443,7 +5443,7 @@ int main(int argc, char **argv) exit(1); } trace_init_file(); - qemu_set_log(LOG_TRACE); + qemu_set_log(LOG_TRACE, &error_fatal); /* find the command */ for (cmd = img_cmds; cmd->name != NULL; cmd++) { diff --git a/qemu-io.c b/qemu-io.c index eb8afc8b41..38321a27a3 100644 --- a/qemu-io.c +++ b/qemu-io.c @@ -634,7 +634,7 @@ int main(int argc, char **argv) exit(1); } trace_init_file(); - qemu_set_log(LOG_TRACE); + qemu_set_log(LOG_TRACE, &error_fatal); /* initialize commands */ qemuio_add_command(&quit_cmd); diff --git a/qemu-nbd.c b/qemu-nbd.c index 713e7557a9..bf9c5fedce 100644 --- a/qemu-nbd.c +++ b/qemu-nbd.c @@ -804,7 +804,7 @@ int main(int argc, char **argv) exit(1); } trace_init_file(); - qemu_set_log(LOG_TRACE); + qemu_set_log(LOG_TRACE, &error_fatal); socket_activation = check_socket_activation(); if (socket_activation == 0) { diff --git a/scsi/qemu-pr-helper.c b/scsi/qemu-pr-helper.c index f281daeced..1f2a84c534 100644 --- a/scsi/qemu-pr-helper.c +++ b/scsi/qemu-pr-helper.c @@ -1001,7 +1001,7 @@ int main(int argc, char **argv) exit(EXIT_FAILURE); } trace_init_file(); - qemu_set_log(LOG_TRACE); + qemu_set_log(LOG_TRACE, &error_fatal); #ifdef CONFIG_MPATH dm_init(); diff --git a/softmmu/vl.c b/softmmu/vl.c index 46aba6a039..fe9de2f896 100644 --- a/softmmu/vl.c +++ b/softmmu/vl.c @@ -2570,9 +2570,9 @@ static void qemu_process_early_options(void) qemu_print_log_usage(stdout); exit(1); } - qemu_set_log(mask); + qemu_set_log(mask, &error_fatal); } else { - qemu_set_log(0); + qemu_set_log(0, &error_fatal); } qemu_add_default_firmwarepath(); diff --git a/storage-daemon/qemu-storage-daemon.c b/storage-daemon/qemu-storage-daemon.c index eb72407257..1398f0443d 100644 --- a/storage-daemon/qemu-storage-daemon.c +++ b/storage-daemon/qemu-storage-daemon.c @@ -392,7 +392,7 @@ int main(int argc, char *argv[]) if (!trace_init_backends()) { return EXIT_FAILURE; } - qemu_set_log(LOG_TRACE); + qemu_set_log(LOG_TRACE, &error_fatal); qemu_init_main_loop(&error_fatal); process_options(argc, argv, false); diff --git a/tests/unit/test-logging.c b/tests/unit/test-logging.c index 05703a4e51..d071400ebc 100644 --- a/tests/unit/test-logging.c +++ b/tests/unit/test-logging.c @@ -122,7 +122,7 @@ static void test_logfile_write(gconstpointer data) * In cases where a logging backend other than log is used, * this is needed. */ - qemu_set_log(CPU_LOG_TB_OUT_ASM); + qemu_set_log(CPU_LOG_TB_OUT_ASM, &error_abort); file_path = g_build_filename(dir, "qemu_test_log_write0.log", NULL); file_path1 = g_build_filename(dir, "qemu_test_log_write1.log", NULL); diff --git a/util/log.c b/util/log.c index a838686a18..bab4d29ecb 100644 --- a/util/log.c +++ b/util/log.c @@ -73,7 +73,7 @@ static void qemu_logfile_free(QemuLogFile *logfile) } /* enable or disable low levels log */ -void qemu_set_log(int log_flags) +bool qemu_set_log(int log_flags, Error **errp) { bool need_to_open_file = false; QemuLogFile *logfile; @@ -103,9 +103,9 @@ void qemu_set_log(int log_flags) if (logfilename) { logfile->fd = fopen(logfilename, log_append ? "a" : "w"); if (!logfile->fd) { - g_free(logfile); - perror(logfilename); - _exit(1); + error_setg_errno(errp, errno, "Error opening logfile %s", + logfilename); + return false; } /* In case we are a daemon redirect stderr to logfile */ if (is_daemonized()) { @@ -129,6 +129,7 @@ void qemu_set_log(int log_flags) log_append = 1; qatomic_rcu_set(&qemu_logfile, logfile); } + return true; } /* @@ -159,8 +160,7 @@ bool qemu_set_log_filename(const char *filename, Error **errp) } qemu_log_close(); - qemu_set_log(qemu_loglevel); - return true; + return qemu_set_log(qemu_loglevel, errp); } /* Returns true if addr is in our debug filter or no filter defined -- cgit 1.4.1 From 93756fdcf6f3f6482bd9f22d8189b6c4ee09303e Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Sun, 17 Apr 2022 11:30:02 -0700 Subject: linux-user: Expand log_page_dump inline MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We have extra stuff to log at the same time. Hoist the qemu_log_lock/unlock to the caller and use fprintf. Reviewed-by: Alex Bennée Signed-off-by: Richard Henderson Message-Id: <20220417183019.755276-23-richard.henderson@linaro.org> --- include/exec/log.h | 15 --------------- linux-user/main.c | 45 ++++++++++++++++++++++++++++++--------------- linux-user/mmap.c | 7 ++++++- 3 files changed, 36 insertions(+), 31 deletions(-) (limited to 'linux-user/main.c') diff --git a/include/exec/log.h b/include/exec/log.h index 06ab9841ee..ad0a40cfeb 100644 --- a/include/exec/log.h +++ b/include/exec/log.h @@ -42,19 +42,4 @@ static inline void log_cpu_state_mask(int mask, CPUState *cpu, int flags) } } -#ifdef NEED_CPU_H -#if defined(CONFIG_USER_ONLY) -/* page_dump() output to the log file: */ -static inline void log_page_dump(const char *operation) -{ - FILE *logfile = qemu_log_trylock(); - if (logfile) { - fprintf(logfile, "page layout changed following %s\n", operation); - page_dump(logfile); - } - qemu_log_unlock(logfile); -} -#endif -#endif - #endif diff --git a/linux-user/main.c b/linux-user/main.c index 6aed4929ab..d263b2a669 100644 --- a/linux-user/main.c +++ b/linux-user/main.c @@ -857,21 +857,36 @@ int main(int argc, char **argv, char **envp) g_free(target_environ); if (qemu_loglevel_mask(CPU_LOG_PAGE)) { - qemu_log("guest_base %p\n", (void *)guest_base); - log_page_dump("binary load"); - - qemu_log("start_brk 0x" TARGET_ABI_FMT_lx "\n", info->start_brk); - qemu_log("end_code 0x" TARGET_ABI_FMT_lx "\n", info->end_code); - qemu_log("start_code 0x" TARGET_ABI_FMT_lx "\n", info->start_code); - qemu_log("start_data 0x" TARGET_ABI_FMT_lx "\n", info->start_data); - qemu_log("end_data 0x" TARGET_ABI_FMT_lx "\n", info->end_data); - qemu_log("start_stack 0x" TARGET_ABI_FMT_lx "\n", info->start_stack); - qemu_log("brk 0x" TARGET_ABI_FMT_lx "\n", info->brk); - qemu_log("entry 0x" TARGET_ABI_FMT_lx "\n", info->entry); - qemu_log("argv_start 0x" TARGET_ABI_FMT_lx "\n", info->arg_start); - qemu_log("env_start 0x" TARGET_ABI_FMT_lx "\n", - info->arg_end + (abi_ulong)sizeof(abi_ulong)); - qemu_log("auxv_start 0x" TARGET_ABI_FMT_lx "\n", info->saved_auxv); + FILE *f = qemu_log_trylock(); + if (f) { + fprintf(f, "guest_base %p\n", (void *)guest_base); + fprintf(f, "page layout changed following binary load\n"); + page_dump(f); + + fprintf(f, "start_brk 0x" TARGET_ABI_FMT_lx "\n", + info->start_brk); + fprintf(f, "end_code 0x" TARGET_ABI_FMT_lx "\n", + info->end_code); + fprintf(f, "start_code 0x" TARGET_ABI_FMT_lx "\n", + info->start_code); + fprintf(f, "start_data 0x" TARGET_ABI_FMT_lx "\n", + info->start_data); + fprintf(f, "end_data 0x" TARGET_ABI_FMT_lx "\n", + info->end_data); + fprintf(f, "start_stack 0x" TARGET_ABI_FMT_lx "\n", + info->start_stack); + fprintf(f, "brk 0x" TARGET_ABI_FMT_lx "\n", + info->brk); + fprintf(f, "entry 0x" TARGET_ABI_FMT_lx "\n", + info->entry); + fprintf(f, "argv_start 0x" TARGET_ABI_FMT_lx "\n", + info->arg_start); + fprintf(f, "env_start 0x" TARGET_ABI_FMT_lx "\n", + info->arg_end + (abi_ulong)sizeof(abi_ulong)); + fprintf(f, "auxv_start 0x" TARGET_ABI_FMT_lx "\n", + info->saved_auxv); + qemu_log_unlock(f); + } } target_set_brk(info->brk); diff --git a/linux-user/mmap.c b/linux-user/mmap.c index a861f1e1d1..48e1373796 100644 --- a/linux-user/mmap.c +++ b/linux-user/mmap.c @@ -630,7 +630,12 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int target_prot, the_end: trace_target_mmap_complete(start); if (qemu_loglevel_mask(CPU_LOG_PAGE)) { - log_page_dump(__func__); + FILE *f = qemu_log_trylock(); + if (f) { + fprintf(f, "page layout changed following mmap\n"); + page_dump(f); + qemu_log_unlock(f); + } } tb_invalidate_phys_range(start, start + len); mmap_unlock(); -- cgit 1.4.1 From b410253f9fcb8d940469a20e732f51f6e43b1265 Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Sun, 17 Apr 2022 11:30:09 -0700 Subject: linux-user: Use qemu_set_log_filename_flags MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Perform all logfile setup in one step. Reviewed-by: Alex Bennée Signed-off-by: Richard Henderson Message-Id: <20220417183019.755276-30-richard.henderson@linaro.org> --- linux-user/main.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) (limited to 'linux-user/main.c') diff --git a/linux-user/main.c b/linux-user/main.c index d263b2a669..0297ae8321 100644 --- a/linux-user/main.c +++ b/linux-user/main.c @@ -85,6 +85,7 @@ static bool enable_strace; * Used to support command line arguments overriding environment variables. */ static int last_log_mask; +static const char *last_log_filename; /* * When running 32-on-64 we should make sure we can fit all of the possible @@ -257,7 +258,7 @@ static void handle_arg_dfilter(const char *arg) static void handle_arg_log_filename(const char *arg) { - qemu_set_log_filename(arg, &error_fatal); + last_log_filename = arg; } static void handle_arg_set_env(const char *arg) @@ -643,7 +644,6 @@ int main(int argc, char **argv, char **envp) int i; int ret; int execfd; - int log_mask; unsigned long max_reserved_va; bool preserve_argv0; @@ -677,10 +677,9 @@ int main(int argc, char **argv, char **envp) optind = parse_args(argc, argv); - log_mask = last_log_mask | (enable_strace ? LOG_STRACE : 0); - if (log_mask) { - qemu_set_log(log_mask, &error_fatal); - } + qemu_set_log_filename_flags(last_log_filename, + last_log_mask | (enable_strace * LOG_STRACE), + &error_fatal); if (!trace_init_backends()) { exit(1); -- cgit 1.4.1