Re: [Qemu-devel] [PATCH] Command line support for altering the log file location

[Blue Swirl]

Thanks, applied.

On Wed, Jun 8, 2011 at 5:32 AM, Matthew Fernandez
<address@hidden> 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 <address@hidden>
> ----
>
> Thanks Kevin and Blue Swirl for the feedback. Amended patch is below.
> On 7 June 2011 18:49, Kevin Wolf <address@hidden> 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 <address@hidden> wrote:
>>>> On Tue, May 31, 2011 at 9:20 AM, Matthew Fernandez
>>>> <address@hidden> 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 <address@hidden>
>>>>
>>>> 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 = &regs1;
>     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 = &regs1;
>     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 = &regs1;
>     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
> address@hidden -D
> address@hidden -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");
>     }
>