[CRIU] Re: [PATCH 2/2] restorer: Take current loglevel into account when priting messages

Pavel Emelyanov xemul at parallels.com
Mon Sep 3 05:27:14 EDT 2012


On 09/03/2012 12:07 PM, Cyrill Gorcunov wrote:
> All other code in crtools do consider current logging
> level and filter out messages depending on it.
> 
> Except the restorer code. With this commit we teach
> restorer to honor logging levels.
> 
> The patch is a bit intrusive though.

Split it.

> Signed-off-by: Cyrill Gorcunov <gorcunov at openvz.org>
> ---
>  cr-restore.c           |    1 +
>  include/log.h          |    1 +
>  include/restorer-log.h |   32 +++++++++++---
>  include/restorer.h     |    1 +
>  include/util.h         |    6 +-
>  log.c                  |    5 ++
>  restorer-log.c         |  107 +++++++++++++++++++++++++++------------------
>  restorer.c             |  113 ++++++++++++++++++++++++------------------------
>  8 files changed, 158 insertions(+), 108 deletions(-)
> 
> diff --git a/cr-restore.c b/cr-restore.c
> index 343a0c8..43fea5c 100644
> --- a/cr-restore.c
> +++ b/cr-restore.c
> @@ -1347,6 +1347,7 @@ static int sigreturn_restore(pid_t pid, CoreEntry *core, struct list_head *tgt_v
> 
>         task_args->pid          = pid;
>         task_args->logfd        = log_get_fd();
> +       task_args->loglevel     = log_get_loglevel();
>         task_args->sigchld_act  = sigchld_act;
>         task_args->fd_pages     = fd_pages;
> 
> diff --git a/include/log.h b/include/log.h
> index 087d4ae..24d69dd 100644
> --- a/include/log.h
> +++ b/include/log.h
> @@ -11,6 +11,7 @@ extern void log_closedir(void);
>  extern int log_get_fd(void);
> 
>  extern void log_set_loglevel(unsigned int loglevel);
> +extern unsigned int log_get_loglevel(void);
> 
>  extern void print_on_level(unsigned int loglevel, const char *format, ...)
>         __attribute__ ((__format__ (__printf__, 2, 3)));
> diff --git a/include/restorer-log.h b/include/restorer-log.h
> index 7a2316f..eb0e726 100644
> --- a/include/restorer-log.h
> +++ b/include/restorer-log.h
> @@ -1,11 +1,31 @@
>  #ifndef RESTORER_LOG_H__
>  #define RESTORER_LOG_H__
> +
> +#include "log-levels.h"
> +
>  extern long vprint_num(char *buf, long num);
> 
> -extern void write_hex_n(unsigned long num);
> -extern void write_num_n(long num);
> -extern void write_num(long num);
> -extern void write_string_n(char *str);
> -extern void write_string(char *str);
> +extern void write_hex_n_on_level(unsigned int loglevel, unsigned long num);
> +extern void write_num_n_on_level(unsigned int loglevel, long num);
> +extern void write_num_on_level(unsigned int loglevel, long num);
> +extern void write_str_n_on_level(unsigned int loglevel, char *str);
> +extern void write_str_on_level(unsigned int loglevel, char *str);
> +
>  extern void restorer_set_logfd(int fd);
> -#endif
> +extern void restorer_set_loglevel(unsigned int loglevel);
> +
> +#define write_str_err(str)     write_str_on_level(LOG_ERROR, str)
> +#define write_str_n_err(str)   write_str_n_on_level(LOG_ERROR, str)
> +
> +#define        write_num_err(num)      write_num_on_level(LOG_ERROR, num)
> +#define write_num_n_err(num)   write_num_n_on_level(LOG_ERROR, num)
> +
> +#define write_str_info(str)    write_str_on_level(LOG_INFO, str)
> +#define write_str_n_info(str)  write_str_n_on_level(LOG_INFO, str)
> +
> +#define write_num_info(num)    write_num_on_level(LOG_INFO, num)
> +#define write_num_n_info(num)  write_num_n_on_level(LOG_INFO, num)
> +
> +#define write_hex_n_err(num)   write_hex_n_on_level(LOG_ERROR, num)
> +
> +#endif /* RESTORER_LOG_H__ */
> diff --git a/include/restorer.h b/include/restorer.h
> index d50a9c8..274459b 100644
> --- a/include/restorer.h
> +++ b/include/restorer.h
> @@ -77,6 +77,7 @@ struct task_restore_core_args {
>         int                             fd_exe_link;            /* opened self->exe file */
>         int                             fd_pages;               /* opened pages dump file */
>         int                             logfd;
> +       unsigned int                    loglevel;
>         bool                            restore_threads;        /* if to restore threads */
>         mutex_t                         rst_lock;
> 
> diff --git a/include/util.h b/include/util.h
> index 38a6fdb..155b0c2 100644
> --- a/include/util.h
> +++ b/include/util.h
> @@ -38,9 +38,9 @@
>  #define BUG_ON_HANDLER(condition)                                      \
>         do {                                                            \
>                 if ((condition)) {                                      \
> -                       write_string("BUG at " __FILE__ ": ");          \
> -                       write_num(__LINE__);                            \
> -                       write_string("\n");                             \
> +                       write_str_err("BUG at " __FILE__ ": ");         \
> +                       write_num_err(__LINE__);                        \
> +                       write_str_err("\n");                            \
>                         *(volatile unsigned long *)NULL = 0xdead0000 + __LINE__;        \
>                 }                                                       \
>         } while (0)
> diff --git a/log.c b/log.c
> index 8abdba9..d9a36f1 100644
> --- a/log.c
> +++ b/log.c
> @@ -130,6 +130,11 @@ void log_set_loglevel(unsigned int level)
>                 current_loglevel = level;
>  }
> 
> +unsigned int log_get_loglevel(void)
> +{
> +       return current_loglevel;
> +}
> +
>  void print_on_level(unsigned int loglevel, const char *format, ...)
>  {
>         va_list params;
> diff --git a/restorer-log.c b/restorer-log.c
> index bf99d9a..c52a364 100644
> --- a/restorer-log.c
> +++ b/restorer-log.c
> @@ -1,14 +1,7 @@
>  #include "restorer-log.h"
>  #include "syscall.h"
> 
> -static int logfd;
> -
> -void restorer_set_logfd(int fd)
> -{
> -       logfd = fd;
> -}
> -
> -#define add_ord(c)                     \
> +#define __add_ord(c)                   \
>         do {                            \
>                 if (c < 10)             \
>                         c += '0';       \
> @@ -16,34 +9,56 @@ void restorer_set_logfd(int fd)
>                         c += 'a' - 10;  \
>         } while (0)
> 
> -void write_string(char *str)
> +static int current_logfd;
> +static unsigned int current_loglevel;
> +
> +void restorer_set_logfd(int fd)
> +{
> +       current_logfd = fd;
> +}
> +
> +void restorer_set_loglevel(unsigned int loglevel)
> +{
> +       current_loglevel = loglevel;
> +}
> +
> +void write_str_on_level(unsigned int loglevel, char *str)
>  {
>         int len = 0;
> 
> +       if (loglevel > current_loglevel)
> +               return;
> +
>         while (str[len])
>                 len++;
> 
> -       sys_write(logfd, str, len);
> +       sys_write(current_logfd, str, len);
>  }
> 
> -void write_string_n(char *str)
> +void write_str_n_on_level(unsigned int loglevel, char *str)
>  {
>         char new_line = '\n';
> 
> -       write_string(str);
> -       sys_write(logfd, &new_line, 1);
> +       if (loglevel > current_loglevel)
> +               return;
> +
> +       write_str_on_level(loglevel, str);
> +       sys_write(current_logfd, &new_line, 1);
>  }
> 
> -void write_num(long num)
> +void write_num_on_level(unsigned int loglevel, long num)
>  {
>         unsigned long d = 1000000000000000000;
>         unsigned int started = 0;
>         unsigned int c;
> 
> +       if (loglevel > current_loglevel)
> +               return;
> +
>         if (num < 0) {
>                 num = -num;
>                 c = '-';
> -               sys_write(logfd, &c, 1);
> +               sys_write(current_logfd, &c, 1);
>         }
> 
>         while (d) {
> @@ -54,18 +69,46 @@ void write_num(long num)
>                         continue;
>                 if (!started)
>                         started = 1;
> -               add_ord(c);
> -               sys_write(logfd, &c, 1);
> +               __add_ord(c);
> +               sys_write(current_logfd, &c, 1);
> 
>         }
>  }
> 
> -void write_num_n(long num)
> +void write_num_n_on_level(unsigned int loglevel, long num)
> +{
> +       unsigned char c = '\n';
> +
> +       if (loglevel > current_loglevel)
> +               return;
> +
> +       write_num_on_level(loglevel, num);
> +       sys_write(current_logfd, &c, sizeof(c));
> +}
> +
> +void write_hex_n_on_level(unsigned int loglevel, unsigned long num)
>  {
> +       unsigned char *s = (unsigned char *)&num;
>         unsigned char c;
> -       write_num(num);
> +       int i;
> +
> +       if (loglevel > current_loglevel)
> +               return;
> +
> +       c = 'x';
> +       sys_write(current_logfd, &c, 1);
> +       for (i = sizeof(long)/sizeof(char) - 1; i >= 0; i--) {
> +               c = (s[i] & 0xf0) >> 4;
> +               __add_ord(c);
> +               sys_write(current_logfd, &c, 1);
> +
> +               c = (s[i] & 0x0f);
> +               __add_ord(c);
> +               sys_write(current_logfd, &c, 1);
> +       }
> +
>         c = '\n';
> -       sys_write(logfd, &c, sizeof(c));
> +       sys_write(current_logfd, &c, 1);
>  }
> 
>  long vprint_num(char *buf, long num)
> @@ -88,7 +131,7 @@ long vprint_num(char *buf, long num)
>                         continue;
>                 if (!started)
>                         started = 1;
> -               add_ord(c);
> +               __add_ord(c);
>                 buf[i++] = c;
> 
>         }
> @@ -97,25 +140,3 @@ long vprint_num(char *buf, long num)
> 
>         return i;
>  }
> -
> -void write_hex_n(unsigned long num)
> -{
> -       unsigned char *s = (unsigned char *)&num;
> -       unsigned char c;
> -       int i;
> -
> -       c = 'x';
> -       sys_write(logfd, &c, 1);
> -       for (i = sizeof(long)/sizeof(char) - 1; i >= 0; i--) {
> -               c = (s[i] & 0xf0) >> 4;
> -               add_ord(c);
> -               sys_write(logfd, &c, 1);
> -
> -               c = (s[i] & 0x0f);
> -               add_ord(c);
> -               sys_write(logfd, &c, 1);
> -       }
> -
> -       c = '\n';
> -       sys_write(logfd, &c, 1);
> -}
> diff --git a/restorer.c b/restorer.c
> index 478ddfc..53d6a24 100644
> --- a/restorer.c
> +++ b/restorer.c
> @@ -29,8 +29,8 @@
>         ({                                                              \
>                 long __ret = sys_prctl(opcode, val1, val2, val3, 0);    \
>                 if (__ret) {                                            \
> -                       write_num_n(__LINE__);                          \
> -                       write_num_n(__ret);                             \
> +                       write_num_n_err(__LINE__);                      \
> +                       write_num_n_err(__ret);                         \
>                 }                                                       \
>                 __ret;                                                  \
>         })
> @@ -39,12 +39,12 @@ static struct task_entries *task_entries;
> 
>  static void sigchld_handler(int signal, siginfo_t *siginfo, void *data)
>  {
> -       write_num(siginfo->si_pid);
> +       write_num_info(siginfo->si_pid);
>         if (siginfo->si_code & CLD_EXITED)
> -               write_string(" exited, status=");
> +               write_str_info(" exited, status=");
>         else if (siginfo->si_code & CLD_KILLED)
> -               write_string(" killed by signal ");
> -       write_num_n(siginfo->si_status);
> +               write_str_info(" killed by signal ");
> +       write_num_n_info(siginfo->si_status);
> 
>         futex_abort_and_wake(&task_entries->nr_in_progress);
>         /* sa_restorer may be unmaped, so we can't go back to userspace*/
> @@ -136,9 +136,9 @@ long __export_restore_thread(struct thread_restore_args *args)
>         int my_pid = sys_gettid();
> 
>         if (my_pid != args->pid) {
> -               write_num_n(__LINE__);
> -               write_num_n(my_pid);
> -               write_num_n(args->pid);
> +               write_num_n_err(__LINE__);
> +               write_num_n_err(my_pid);
> +               write_num_n_err(args->pid);
>                 goto core_restore_end;
>         }
> 
> @@ -146,9 +146,9 @@ long __export_restore_thread(struct thread_restore_args *args)
> 
>         if (args->has_futex) {
>                 if (sys_set_robust_list((void *)args->futex_rla, args->futex_rla_len)) {
> -                       write_num_n(__LINE__);
> -                       write_num_n(my_pid);
> -                       write_num_n(args->pid);
> +                       write_num_n_err(__LINE__);
> +                       write_num_n_err(my_pid);
> +                       write_num_n_err(args->pid);
>                         goto core_restore_end;
>                 }
>         }
> @@ -183,16 +183,16 @@ long __export_restore_thread(struct thread_restore_args *args)
>         fsgs_base = args->gpregs.fs_base;
>         ret = sys_arch_prctl(ARCH_SET_FS, fsgs_base);
>         if (ret) {
> -               write_num_n(__LINE__);
> -               write_num_n(ret);
> +               write_num_n_err(__LINE__);
> +               write_num_n_err(ret);
>                 goto core_restore_end;
>         }
> 
>         fsgs_base = args->gpregs.gs_base;
>         ret = sys_arch_prctl(ARCH_SET_GS, fsgs_base);
>         if (ret) {
> -               write_num_n(__LINE__);
> -               write_num_n(ret);
> +               write_num_n_err(__LINE__);
> +               write_num_n_err(ret);
>                 goto core_restore_end;
>         }
> 
> @@ -207,8 +207,8 @@ long __export_restore_thread(struct thread_restore_args *args)
>         restore_creds(NULL);
>         futex_dec_and_wake(&task_entries->nr_in_progress);
> 
> -       write_num(sys_gettid());
> -       write_string_n(": Restored");
> +       write_num_info(sys_gettid());
> +       write_str_n_info(": Restored");
> 
>         futex_wait_while(&task_entries->start, CR_STATE_RESTORE);
>         futex_dec_and_wake(&task_entries->nr_in_progress);
> @@ -223,8 +223,8 @@ long __export_restore_thread(struct thread_restore_args *args)
>                 : "r"(new_sp)
>                 : "rax","rsp","memory");
>  core_restore_end:
> -       write_num_n(__LINE__);
> -       write_num_n(sys_getpid());
> +       write_num_n_err(__LINE__);
> +       write_num_n_err(sys_getpid());
>         sys_exit_group(1);
>         return -1;
>  }
> @@ -233,7 +233,7 @@ static long restore_self_exe_late(struct task_restore_core_args *args)
>  {
>         int fd = args->fd_exe_link;
> 
> -       write_string("Restoring EXE\n");
> +       write_str_info("Restoring EXE\n");
>         sys_prctl_safe(PR_SET_MM, PR_SET_MM_EXE_FILE, fd, 0);
>         sys_close(fd);
> 
> @@ -305,6 +305,7 @@ long __export_restore_task(struct task_restore_core_args *args)
>         sys_sigaction(SIGCHLD, &act, NULL, sizeof(rt_sigset_t));
> 
>         restorer_set_logfd(args->logfd);
> +       restorer_set_loglevel(args->loglevel);
> 
>         for (vma_entry = args->self_vmas; vma_entry->start != 0; vma_entry++) {
>                 if (!vma_entry_is(vma_entry, VMA_AREA_REGULAR))
> @@ -318,7 +319,7 @@ long __export_restore_task(struct task_restore_core_args *args)
>                         vma_entry->start -= PAGE_SIZE;
> 
>                 if (sys_munmap((void *)vma_entry->start, vma_entry_len(vma_entry))) {
> -                       write_num_n(__LINE__);
> +                       write_num_n_err(__LINE__);
>                         goto core_restore_end;
>                 }
>         }
> @@ -336,14 +337,14 @@ long __export_restore_task(struct task_restore_core_args *args)
>                 va = restore_mapping(vma_entry);
> 
>                 if (va != vma_entry->start) {
> -                       write_num_n(__LINE__);
> -                       write_hex_n(vma_entry->start);
> -                       write_hex_n(vma_entry->end);
> -                       write_hex_n(vma_entry->prot);
> -                       write_hex_n(vma_entry->flags);
> -                       write_hex_n(vma_entry->fd);
> -                       write_hex_n(vma_entry->pgoff);
> -                       write_hex_n(va);
> +                       write_num_n_err(__LINE__);
> +                       write_hex_n_err(vma_entry->start);
> +                       write_hex_n_err(vma_entry->end);
> +                       write_hex_n_err(vma_entry->prot);
> +                       write_hex_n_err(vma_entry->flags);
> +                       write_hex_n_err(vma_entry->fd);
> +                       write_hex_n_err(vma_entry->pgoff);
> +                       write_hex_n_err(va);
>                         goto core_restore_end;
>                 }
>         }
> @@ -357,15 +358,15 @@ long __export_restore_task(struct task_restore_core_args *args)
>                         break;
> 
>                 if (ret != sizeof(va)) {
> -                       write_num_n(__LINE__);
> -                       write_num_n(ret);
> +                       write_num_n_err(__LINE__);
> +                       write_num_n_err(ret);
>                         goto core_restore_end;
>                 }
> 
>                 ret = sys_read(args->fd_pages, (void *)va, PAGE_SIZE);
>                 if (ret != PAGE_SIZE) {
> -                       write_num_n(__LINE__);
> -                       write_num_n(ret);
> +                       write_num_n_err(__LINE__);
> +                       write_num_n_err(ret);
>                         goto core_restore_end;
>                 }
>         }
> @@ -403,8 +404,8 @@ long __export_restore_task(struct task_restore_core_args *args)
> 
>         ret = sys_munmap(args->shmems, SHMEMS_SIZE);
>         if (ret < 0) {
> -               write_num_n(__LINE__);
> -               write_num_n(ret);
> +               write_num_n_err(__LINE__);
> +               write_num_n_err(ret);
>                 goto core_restore_end;
>         }
> 
> @@ -443,9 +444,9 @@ long __export_restore_task(struct task_restore_core_args *args)
> 
>         if (args->has_futex) {
>                 if (sys_set_robust_list((void *)args->futex_rla, args->futex_rla_len)) {
> -                       write_num_n(__LINE__);
> -                       write_num_n(my_pid);
> -                       write_num_n(args->pid);
> +                       write_num_n_err(__LINE__);
> +                       write_num_n_err(my_pid);
> +                       write_num_n_err(args->pid);
>                         goto core_restore_end;
>                 }
>         }
> @@ -486,16 +487,16 @@ long __export_restore_task(struct task_restore_core_args *args)
>         fsgs_base = args->gpregs.fs_base;
>         ret = sys_arch_prctl(ARCH_SET_FS, fsgs_base);
>         if (ret) {
> -               write_num_n(__LINE__);
> -               write_num_n(ret);
> +               write_num_n_err(__LINE__);
> +               write_num_n_err(ret);
>                 goto core_restore_end;
>         }
> 
>         fsgs_base = args->gpregs.gs_base;
>         ret = sys_arch_prctl(ARCH_SET_GS, fsgs_base);
>         if (ret) {
> -               write_num_n(__LINE__);
> -               write_num_n(ret);
> +               write_num_n_err(__LINE__);
> +               write_num_n_err(ret);
>                 goto core_restore_end;
>         }
> 
> @@ -532,15 +533,15 @@ long __export_restore_task(struct task_restore_core_args *args)
> 
>                 fd = sys_open(LAST_PID_PATH, O_RDWR, LAST_PID_PERM);
>                 if (fd < 0) {
> -                       write_num_n(__LINE__);
> -                       write_num_n(fd);
> +                       write_num_n_err(__LINE__);
> +                       write_num_n_err(fd);
>                         goto core_restore_end;
>                 }
> 
>                 ret = sys_flock(fd, LOCK_EX);
>                 if (ret) {
> -                       write_num_n(__LINE__);
> -                       write_num_n(ret);
> +                       write_num_n_err(__LINE__);
> +                       write_num_n_err(ret);
>                         goto core_restore_end;
>                 }
> 
> @@ -560,9 +561,9 @@ long __export_restore_task(struct task_restore_core_args *args)
>                         last_pid_len = vprint_num(last_pid_buf, thread_args[i].pid - 1);
>                         ret = sys_write(fd, last_pid_buf, last_pid_len - 1);
>                         if (ret < 0) {
> -                               write_num_n(__LINE__);
> -                               write_num_n(ret);
> -                               write_string_n(last_pid_buf);
> +                               write_num_n_err(__LINE__);
> +                               write_num_n_err(ret);
> +                               write_str_n_err(last_pid_buf);
>                                 goto core_restore_end;
>                         }
> 
> @@ -611,8 +612,8 @@ long __export_restore_task(struct task_restore_core_args *args)
> 
>                 ret = sys_flock(fd, LOCK_UN);
>                 if (ret) {
> -                       write_num_n(__LINE__);
> -                       write_num_n(ret);
> +                       write_num_n_err(__LINE__);
> +                       write_num_n_err(ret);
>                         goto core_restore_end;
>                 }
> 
> @@ -628,8 +629,8 @@ long __export_restore_task(struct task_restore_core_args *args)
> 
>         futex_dec_and_wake(&args->task_entries->nr_in_progress);
> 
> -       write_num(sys_getpid());
> -       write_string_n(": Restored");
> +       write_num_info(sys_getpid());
> +       write_str_n_info(": Restored");
> 
>         futex_wait_while(&args->task_entries->start, CR_STATE_RESTORE);
> 
> @@ -683,8 +684,8 @@ long __export_restore_task(struct task_restore_core_args *args)
>                 : "rax","rsp","memory");
> 
>  core_restore_end:
> -       write_num_n(__LINE__);
> -       write_num_n(sys_getpid());
> +       write_num_n_err(__LINE__);
> +       write_num_n_err(sys_getpid());
>         sys_exit_group(1);
>         return -1;
> 
> --
> 1.7.7.6
> 
> .
> 



More information about the CRIU mailing list