Message ID | BANLkTimkBR7ZNOGaUO1gDmq2haBO41ciZ0F+5tdXoSdUgTxB=g@mail.gmail.com |
---|---|
State | New |
Headers | show |
Thanks, applied. On Wed, Jun 8, 2011 at 5:32 AM, Matthew Fernandez <matthew.fernandez@gmail.com> wrote: > Add command line support for logging to a location other than /tmp/qemu.log. > > With logging enabled (command line option -d), the log is written to > the hard-coded path /tmp/qemu.log. This patch adds support for writing > the log to a different location by passing the -D option. > > Signed-off-by: Matthew Fernandez <matthew.fernandez@gmail.com> > ---- > > Thanks Kevin and Blue Swirl for the feedback. Amended patch is below. > On 7 June 2011 18:49, Kevin Wolf <kwolf@redhat.com> wrote: >> Am 05.06.2011 02:46, schrieb Matthew Fernandez: >>> So, to clarify, all text above the '----' is included as the commit >>> message and the sign off line should be in this section. All text >>> below the '----' before the first diff is treated as comments that are >>> not to be committed. Is this correct? If so I'll send through an >>> ammended patch with these changes. >> >> Yes, this is correct. >> >> Kevin >> >>> >>> On 4 June 2011 20:18, Blue Swirl <blauwirbel@gmail.com> wrote: >>>> On Tue, May 31, 2011 at 9:20 AM, Matthew Fernandez >>>> <matthew.fernandez@gmail.com> wrote: >>>>> Hi, >>>>> >>>>> The included patch adds command line support for logging to a location >>>>> other than /tmp/qemu.log. The diff is relative to commit >>>>> 2eb9f241824d000fcd90bd7f4b49e40b88e62975. Please let me know if >>>>> anything needs to be cleaned up or changed. Anthony, I'm not sure who >>>>> should be responsible for reviewing/accepting this, but I've CCed you >>>>> as it touches vl.c. >>>> >>>> The patch looks OK, however the above text (which git-am would use as >>>> the commit message) needs to be changed. How about something like >>>> this: >>>> Add command line support for logging to a location other than /tmp/qemu.log. >>>> >>>> If you want to add comments which should not go to the commit message, >>>> please put them below the line with '----'. >>>> >>>>> Thanks, >>>>> Matthew >>>>> >>>>> ---- >>>>> Signed-off-by: Matthew Fernandez <matthew.fernandez@gmail.com> >>>> >>>> Also this needs to be above the '----' line. > > > > diff --git a/bsd-user/main.c b/bsd-user/main.c > index 0c3fca1..0af8a7e 100644 > --- a/bsd-user/main.c > +++ b/bsd-user/main.c > @@ -690,7 +690,8 @@ static void usage(void) > "-bsd type select emulated BSD type > FreeBSD/NetBSD/OpenBSD (default)\n" > "\n" > "Debug options:\n" > - "-d options activate log (logfile=%s)\n" > + "-d options activate log (default logfile=%s)\n" > + "-D logfile override default logfile location\n" > "-p pagesize set the host page size to 'pagesize'\n" > "-singlestep always run in singlestep mode\n" > "-strace log system calls\n" > @@ -731,6 +732,8 @@ int main(int argc, char **argv) > { > const char *filename; > const char *cpu_model; > + const char *log_file = DEBUG_LOGFILE; > + const char *log_mask = NULL; > struct target_pt_regs regs1, *regs = ®s1; > struct image_info info1, *info = &info1; > TaskState ts1, *ts = &ts1; > @@ -745,9 +748,6 @@ int main(int argc, char **argv) > if (argc <= 1) > usage(); > > - /* init debug */ > - cpu_set_log_filename(DEBUG_LOGFILE); > - > if ((envlist = envlist_create()) == NULL) { > (void) fprintf(stderr, "Unable to allocate envlist\n"); > exit(1); > @@ -775,22 +775,15 @@ int main(int argc, char **argv) > if (!strcmp(r, "-")) { > break; > } else if (!strcmp(r, "d")) { > - int mask; > - const CPULogItem *item; > - > - if (optind >= argc) > + if (optind >= argc) { > break; > - > - r = argv[optind++]; > - mask = cpu_str_to_log_mask(r); > - if (!mask) { > - printf("Log items (comma separated):\n"); > - for(item = cpu_log_items; item->mask != 0; item++) { > - printf("%-10s %s\n", item->name, item->help); > - } > - exit(1); > } > - cpu_set_log(mask); > + log_mask = argv[optind++]; > + } else if (!strcmp(r, "D")) { > + if (optind >= argc) { > + break; > + } > + log_file = argv[optind++]; > } else if (!strcmp(r, "E")) { > r = argv[optind++]; > if (envlist_setenv(envlist, r) != 0) > @@ -867,6 +860,23 @@ int main(int argc, char **argv) > usage(); > filename = argv[optind]; > > + /* init debug */ > + cpu_set_log_filename(log_file); > + if (log_mask) { > + int mask; > + const CPULogItem *item; > + > + mask = cpu_str_to_log_mask(r); > + if (!mask) { > + printf("Log items (comma separated):\n"); > + for (item = cpu_log_items; item->mask != 0; item++) { > + printf("%-10s %s\n", item->name, item->help); > + } > + exit(1); > + } > + cpu_set_log(mask); > + } > + > /* Zero out regs */ > memset(regs, 0, sizeof(struct target_pt_regs)); > > diff --git a/cpus.c b/cpus.c > index 1fc34b7..17e96b5 100644 > --- a/cpus.c > +++ b/cpus.c > @@ -1142,6 +1142,11 @@ void set_cpu_log(const char *optarg) > cpu_set_log(mask); > } > > +void set_cpu_log_filename(const char *optarg) > +{ > + cpu_set_log_filename(optarg); > +} > + > /* Return the virtual CPU time, based on the instruction counter. */ > int64_t cpu_get_icount(void) > { > diff --git a/cpus.h b/cpus.h > index 6fdeb0d..f42b54e 100644 > --- a/cpus.h > +++ b/cpus.h > @@ -19,6 +19,7 @@ void vm_state_notify(int running, int reason); > bool cpu_exec_all(void); > void set_numa_modes(void); > void set_cpu_log(const char *optarg); > +void set_cpu_log_filename(const char *optarg); > void list_cpus(FILE *f, fprintf_function cpu_fprintf, const char *optarg); > > #endif > diff --git a/darwin-user/main.c b/darwin-user/main.c > index 175e12f..a6dc859 100644 > --- a/darwin-user/main.c > +++ b/darwin-user/main.c > @@ -738,6 +738,8 @@ TaskState *first_task_state; > int main(int argc, char **argv) > { > const char *filename; > + const char *log_file = DEBUG_LOGFILE; > + const char *log_mask = NULL; > struct target_pt_regs regs1, *regs = ®s1; > TaskState ts1, *ts = &ts1; > CPUState *env; > @@ -749,9 +751,6 @@ int main(int argc, char **argv) > if (argc <= 1) > usage(); > > - /* init debug */ > - cpu_set_log_filename(DEBUG_LOGFILE); > - > optind = 1; > for(;;) { > if (optind >= argc) > @@ -764,22 +763,15 @@ int main(int argc, char **argv) > if (!strcmp(r, "-")) { > break; > } else if (!strcmp(r, "d")) { > - int mask; > - CPULogItem *item; > - > - if (optind >= argc) > - break; > - > - r = argv[optind++]; > - mask = cpu_str_to_log_mask(r); > - if (!mask) { > - printf("Log items (comma separated):\n"); > - for(item = cpu_log_items; item->mask != 0; item++) { > - printf("%-10s %s\n", item->name, item->help); > - } > - exit(1); > + if (optind >= argc) { > + break; > } > - cpu_set_log(mask); > + log_mask = argv[optind++]; > + } else if (!strcmp(r, "D")) { > + if (optind >= argc) { > + break; > + } > + log_file = argv[optind++]; > } else if (!strcmp(r, "s")) { > r = argv[optind++]; > stack_size = strtol(r, (char **)&r, 0); > @@ -821,6 +813,23 @@ int main(int argc, char **argv) > usage(); > filename = argv[optind]; > > + /* init debug */ > + cpu_set_log_filename(log_file); > + if (log_mask) { > + int mask; > + CPULogItem *item; > + > + mask = cpu_str_to_log_mask(r); > + if (!mask) { > + printf("Log items (comma separated):\n"); > + for (item = cpu_log_items; item->mask != 0; item++) { > + printf("%-10s %s\n", item->name, item->help); > + } > + exit(1); > + } > + cpu_set_log(mask); > + } > + > /* Zero out regs */ > memset(regs, 0, sizeof(struct target_pt_regs)); > > diff --git a/linux-user/main.c b/linux-user/main.c > index 088def3..23c229c 100644 > --- a/linux-user/main.c > +++ b/linux-user/main.c > @@ -2848,6 +2848,8 @@ int main(int argc, char **argv, char **envp) > { > const char *filename; > const char *cpu_model; > + const char *log_file = DEBUG_LOGFILE; > + const char *log_mask = NULL; > struct target_pt_regs regs1, *regs = ®s1; > struct image_info info1, *info = &info1; > struct linux_binprm bprm; > @@ -2869,9 +2871,6 @@ int main(int argc, char **argv, char **envp) > > qemu_cache_utils_init(envp); > > - /* init debug */ > - cpu_set_log_filename(DEBUG_LOGFILE); > - > if ((envlist = envlist_create()) == NULL) { > (void) fprintf(stderr, "Unable to allocate envlist\n"); > exit(1); > @@ -2910,22 +2909,15 @@ int main(int argc, char **argv, char **envp) > if (!strcmp(r, "-")) { > break; > } else if (!strcmp(r, "d")) { > - int mask; > - const CPULogItem *item; > - > - if (optind >= argc) > + if (optind >= argc) { > break; > - > - r = argv[optind++]; > - mask = cpu_str_to_log_mask(r); > - if (!mask) { > - printf("Log items (comma separated):\n"); > - for(item = cpu_log_items; item->mask != 0; item++) { > - printf("%-10s %s\n", item->name, item->help); > - } > - exit(1); > } > - cpu_set_log(mask); > + log_mask = argv[optind++]; > + } else if (!strcmp(r, "D")) { > + if (optind >= argc) { > + break; > + } > + log_file = argv[optind++]; > } else if (!strcmp(r, "E")) { > r = argv[optind++]; > if (envlist_setenv(envlist, r) != 0) > @@ -3038,6 +3030,23 @@ int main(int argc, char **argv, char **envp) > filename = argv[optind]; > exec_path = argv[optind]; > > + /* init debug */ > + cpu_set_log_filename(log_file); > + if (log_mask) { > + int mask; > + const CPULogItem *item; > + > + mask = cpu_str_to_log_mask(r); > + if (!mask) { > + printf("Log items (comma separated):\n"); > + for (item = cpu_log_items; item->mask != 0; item++) { > + printf("%-10s %s\n", item->name, item->help); > + } > + exit(1); > + } > + cpu_set_log(mask); > + } > + > /* Zero out regs */ > memset(regs, 0, sizeof(struct target_pt_regs)); > > diff --git a/qemu-options.hx b/qemu-options.hx > index 82e085a..05513e8 100644 > --- a/qemu-options.hx > +++ b/qemu-options.hx > @@ -1991,6 +1991,15 @@ STEXI > Output log in /tmp/qemu.log > ETEXI > > +DEF("D", HAS_ARG, QEMU_OPTION_D, \ > + "-D logfile output log to logfile (instead of the default > /tmp/qemu.log)\n", > + QEMU_ARCH_ALL) > +STEXI > +@item -D > +@findex -D > +Output log in logfile instead of /tmp/qemu.log > +ETEXI > + > DEF("hdachs", HAS_ARG, QEMU_OPTION_hdachs, \ > "-hdachs c,h,s[,t]\n" \ > " force hard disk 0 physical geometry and the > optional BIOS\n" \ > diff --git a/vl.c b/vl.c > index b362871..e459f64 100644 > --- a/vl.c > +++ b/vl.c > @@ -2060,6 +2060,8 @@ int main(int argc, char **argv, char **envp) > #endif > int defconfig = 1; > const char *trace_file = NULL; > + const char *log_mask = NULL; > + const char *log_file = NULL; > > atexit(qemu_run_exit_notifiers); > error_set_progname(argv[0]); > @@ -2434,7 +2436,10 @@ int main(int argc, char **argv, char **envp) > break; > #endif > case QEMU_OPTION_d: > - set_cpu_log(optarg); > + log_mask = optarg; > + break; > + case QEMU_OPTION_D: > + log_file = optarg; > break; > case QEMU_OPTION_s: > gdbstub_dev = "tcp::" DEFAULT_GDBSTUB_PORT; > @@ -2900,6 +2905,18 @@ int main(int argc, char **argv, char **envp) > } > loc_set_none(); > > + /* Open the logfile at this point, if necessary. We can't open the logfile > + * when encountering either of the logging options (-d or -D) because the > + * other one may be encountered later on the command line, changing the > + * location or level of logging. > + */ > + if (log_mask) { > + if (log_file) { > + set_cpu_log_filename(log_file); > + } > + set_cpu_log(log_mask); > + } > + > if (!st_init(trace_file)) { > fprintf(stderr, "warning: unable to initialize simple trace > backend\n"); > } >
On Wed, Jun 08, 2011 at 12:32:40PM +1000, Matthew Fernandez wrote: > Add command line support for logging to a location other than /tmp/qemu.log. > > With logging enabled (command line option -d), the log is written to > the hard-coded path /tmp/qemu.log. This patch adds support for writing > the log to a different location by passing the -D option. > > Signed-off-by: Matthew Fernandez <matthew.fernandez@gmail.com> Hi, This patch broke -d for all *-user targets AFAICT. r is passed to cpu_str_to_log_mask(r) instead of log_mask. Cheers > } else if (!strcmp(r, "d")) { > - int mask; > - const CPULogItem *item; > - > - if (optind >= argc) > + if (optind >= argc) { > break; > - > - r = argv[optind++]; > - mask = cpu_str_to_log_mask(r); > - if (!mask) { > - printf("Log items (comma separated):\n"); > - for(item = cpu_log_items; item->mask != 0; item++) { > - printf("%-10s %s\n", item->name, item->help); > - } > - exit(1); > } > - cpu_set_log(mask); > + log_mask = argv[optind++]; > + } else if (!strcmp(r, "D")) { > + if (optind >= argc) { > + break; > + } > + log_file = argv[optind++]; > } else if (!strcmp(r, "E")) { > r = argv[optind++]; > if (envlist_setenv(envlist, r) != 0) > @@ -867,6 +860,23 @@ int main(int argc, char **argv) > usage(); > filename = argv[optind]; > > + /* init debug */ > + cpu_set_log_filename(log_file); > + if (log_mask) { > + int mask; > + const CPULogItem *item; > + > + mask = cpu_str_to_log_mask(r);
diff --git a/bsd-user/main.c b/bsd-user/main.c index 0c3fca1..0af8a7e 100644 --- a/bsd-user/main.c +++ b/bsd-user/main.c @@ -690,7 +690,8 @@ static void usage(void) "-bsd type select emulated BSD type FreeBSD/NetBSD/OpenBSD (default)\n" "\n" "Debug options:\n" - "-d options activate log (logfile=%s)\n" + "-d options activate log (default logfile=%s)\n" + "-D logfile override default logfile location\n" "-p pagesize set the host page size to 'pagesize'\n" "-singlestep always run in singlestep mode\n" "-strace log system calls\n" @@ -731,6 +732,8 @@ int main(int argc, char **argv) { const char *filename; const char *cpu_model; + const char *log_file = DEBUG_LOGFILE; + const char *log_mask = NULL; struct target_pt_regs regs1, *regs = ®s1; struct image_info info1, *info = &info1; TaskState ts1, *ts = &ts1; @@ -745,9 +748,6 @@ int main(int argc, char **argv) if (argc <= 1) usage(); - /* init debug */ - cpu_set_log_filename(DEBUG_LOGFILE); - if ((envlist = envlist_create()) == NULL) { (void) fprintf(stderr, "Unable to allocate envlist\n"); exit(1); @@ -775,22 +775,15 @@ int main(int argc, char **argv) if (!strcmp(r, "-")) { break; } else if (!strcmp(r, "d")) { - int mask; - const CPULogItem *item; - - if (optind >= argc) + if (optind >= argc) { break; - - r = argv[optind++]; - mask = cpu_str_to_log_mask(r); - if (!mask) { - printf("Log items (comma separated):\n"); - for(item = cpu_log_items; item->mask != 0; item++) { - printf("%-10s %s\n", item->name, item->help); - } - exit(1); } - cpu_set_log(mask); + log_mask = argv[optind++]; + } else if (!strcmp(r, "D")) { + if (optind >= argc) { + break; + } + log_file = argv[optind++]; } else if (!strcmp(r, "E")) { r = argv[optind++]; if (envlist_setenv(envlist, r) != 0) @@ -867,6 +860,23 @@ int main(int argc, char **argv) usage(); filename = argv[optind]; + /* init debug */ + cpu_set_log_filename(log_file); + if (log_mask) { + int mask; + const CPULogItem *item; + + mask = cpu_str_to_log_mask(r); + if (!mask) { + printf("Log items (comma separated):\n"); + for (item = cpu_log_items; item->mask != 0; item++) { + printf("%-10s %s\n", item->name, item->help); + } + exit(1); + } + cpu_set_log(mask); + } + /* Zero out regs */ memset(regs, 0, sizeof(struct target_pt_regs)); diff --git a/cpus.c b/cpus.c index 1fc34b7..17e96b5 100644 --- a/cpus.c +++ b/cpus.c @@ -1142,6 +1142,11 @@ void set_cpu_log(const char *optarg) cpu_set_log(mask); } +void set_cpu_log_filename(const char *optarg) +{ + cpu_set_log_filename(optarg); +} + /* Return the virtual CPU time, based on the instruction counter. */ int64_t cpu_get_icount(void) { diff --git a/cpus.h b/cpus.h index 6fdeb0d..f42b54e 100644 --- a/cpus.h +++ b/cpus.h @@ -19,6 +19,7 @@ void vm_state_notify(int running, int reason); bool cpu_exec_all(void); void set_numa_modes(void); void set_cpu_log(const char *optarg); +void set_cpu_log_filename(const char *optarg); void list_cpus(FILE *f, fprintf_function cpu_fprintf, const char *optarg); #endif diff --git a/darwin-user/main.c b/darwin-user/main.c index 175e12f..a6dc859 100644 --- a/darwin-user/main.c +++ b/darwin-user/main.c @@ -738,6 +738,8 @@ TaskState *first_task_state; int main(int argc, char **argv) { const char *filename; + const char *log_file = DEBUG_LOGFILE; + const char *log_mask = NULL; struct target_pt_regs regs1, *regs = ®s1; TaskState ts1, *ts = &ts1; CPUState *env; @@ -749,9 +751,6 @@ int main(int argc, char **argv) if (argc <= 1) usage(); - /* init debug */ - cpu_set_log_filename(DEBUG_LOGFILE); - optind = 1; for(;;) { if (optind >= argc) @@ -764,22 +763,15 @@ int main(int argc, char **argv) if (!strcmp(r, "-")) { break; } else if (!strcmp(r, "d")) { - int mask; - CPULogItem *item; - - if (optind >= argc) - break; - - r = argv[optind++]; - mask = cpu_str_to_log_mask(r); - if (!mask) { - printf("Log items (comma separated):\n"); - for(item = cpu_log_items; item->mask != 0; item++) { - printf("%-10s %s\n", item->name, item->help); - } - exit(1); + if (optind >= argc) { + break; } - cpu_set_log(mask); + log_mask = argv[optind++]; + } else if (!strcmp(r, "D")) { + if (optind >= argc) { + break; + } + log_file = argv[optind++]; } else if (!strcmp(r, "s")) { r = argv[optind++]; stack_size = strtol(r, (char **)&r, 0); @@ -821,6 +813,23 @@ int main(int argc, char **argv) usage(); filename = argv[optind]; + /* init debug */ + cpu_set_log_filename(log_file); + if (log_mask) { + int mask; + CPULogItem *item; + + mask = cpu_str_to_log_mask(r); + if (!mask) { + printf("Log items (comma separated):\n"); + for (item = cpu_log_items; item->mask != 0; item++) { + printf("%-10s %s\n", item->name, item->help); + } + exit(1); + } + cpu_set_log(mask); + } + /* Zero out regs */ memset(regs, 0, sizeof(struct target_pt_regs)); diff --git a/linux-user/main.c b/linux-user/main.c index 088def3..23c229c 100644 --- a/linux-user/main.c +++ b/linux-user/main.c @@ -2848,6 +2848,8 @@ int main(int argc, char **argv, char **envp) { const char *filename; const char *cpu_model; + const char *log_file = DEBUG_LOGFILE; + const char *log_mask = NULL; struct target_pt_regs regs1, *regs = ®s1; struct image_info info1, *info = &info1; struct linux_binprm bprm; @@ -2869,9 +2871,6 @@ int main(int argc, char **argv, char **envp) qemu_cache_utils_init(envp); - /* init debug */ - cpu_set_log_filename(DEBUG_LOGFILE); - if ((envlist = envlist_create()) == NULL) { (void) fprintf(stderr, "Unable to allocate envlist\n"); exit(1); @@ -2910,22 +2909,15 @@ int main(int argc, char **argv, char **envp) if (!strcmp(r, "-")) { break; } else if (!strcmp(r, "d")) { - int mask; - const CPULogItem *item; - - if (optind >= argc) + if (optind >= argc) { break; - - r = argv[optind++]; - mask = cpu_str_to_log_mask(r); - if (!mask) { - printf("Log items (comma separated):\n"); - for(item = cpu_log_items; item->mask != 0; item++) { - printf("%-10s %s\n", item->name, item->help); - } - exit(1); } - cpu_set_log(mask); + log_mask = argv[optind++]; + } else if (!strcmp(r, "D")) { + if (optind >= argc) { + break; + } + log_file = argv[optind++]; } else if (!strcmp(r, "E")) { r = argv[optind++]; if (envlist_setenv(envlist, r) != 0) @@ -3038,6 +3030,23 @@ int main(int argc, char **argv, char **envp) filename = argv[optind]; exec_path = argv[optind]; + /* init debug */ + cpu_set_log_filename(log_file); + if (log_mask) { + int mask; + const CPULogItem *item; + + mask = cpu_str_to_log_mask(r); + if (!mask) { + printf("Log items (comma separated):\n"); + for (item = cpu_log_items; item->mask != 0; item++) { + printf("%-10s %s\n", item->name, item->help); + } + exit(1); + } + cpu_set_log(mask); + } + /* Zero out regs */ memset(regs, 0, sizeof(struct target_pt_regs)); diff --git a/qemu-options.hx b/qemu-options.hx index 82e085a..05513e8 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -1991,6 +1991,15 @@ STEXI Output log in /tmp/qemu.log ETEXI +DEF("D", HAS_ARG, QEMU_OPTION_D, \ + "-D logfile output log to logfile (instead of the default /tmp/qemu.log)\n", + QEMU_ARCH_ALL) +STEXI +@item -D +@findex -D +Output log in logfile instead of /tmp/qemu.log +ETEXI + DEF("hdachs", HAS_ARG, QEMU_OPTION_hdachs, \ "-hdachs c,h,s[,t]\n" \ " force hard disk 0 physical geometry and the optional BIOS\n" \ diff --git a/vl.c b/vl.c index b362871..e459f64 100644 --- a/vl.c +++ b/vl.c @@ -2060,6 +2060,8 @@ int main(int argc, char **argv, char **envp) #endif int defconfig = 1; const char *trace_file = NULL; + const char *log_mask = NULL; + const char *log_file = NULL; atexit(qemu_run_exit_notifiers); error_set_progname(argv[0]); @@ -2434,7 +2436,10 @@ int main(int argc, char **argv, char **envp) break; #endif case QEMU_OPTION_d: - set_cpu_log(optarg); + log_mask = optarg; + break; + case QEMU_OPTION_D: + log_file = optarg; break; case QEMU_OPTION_s: gdbstub_dev = "tcp::" DEFAULT_GDBSTUB_PORT; @@ -2900,6 +2905,18 @@ int main(int argc, char **argv, char **envp) } loc_set_none(); + /* Open the logfile at this point, if necessary. We can't open the logfile + * when encountering either of the logging options (-d or -D) because the + * other one may be encountered later on the command line, changing the + * location or level of logging. + */ + if (log_mask) { + if (log_file) { + set_cpu_log_filename(log_file); + } + set_cpu_log(log_mask); + } + if (!st_init(trace_file)) { fprintf(stderr, "warning: unable to initialize simple trace backend\n");