Re: [PATCH v3 1/4] linux-user: Use `qemu_log' for non-strace logging

[Laurent Vivier]

Le 04/02/2020 à 03:54, Josh Kunz a écrit :
> Since most calls to `gemu_log` are actually logging unimplemented features,
> this change replaces most non-strace calls to `gemu_log` with calls to
> `qemu_log_mask(LOG_UNIMP, ...)`.  This allows the user to easily log to
> a file, and to mask out these log messages if they desire.
> 
> Note: This change is slightly backwards incompatible, since now these
> "unimplemented" log messages will not be logged by default.
> 
> Signed-off-by: Josh Kunz <address@hidden>
> ---
>  linux-user/arm/cpu_loop.c |  5 ++--
>  linux-user/fd-trans.c     | 55 +++++++++++++++++++++++++--------------
>  linux-user/syscall.c      | 35 ++++++++++++++++---------
>  linux-user/vm86.c         |  3 ++-
>  4 files changed, 62 insertions(+), 36 deletions(-)
> 
> diff --git a/linux-user/arm/cpu_loop.c b/linux-user/arm/cpu_loop.c
> index 1fae90c6df..cf618daa1c 100644
> --- a/linux-user/arm/cpu_loop.c
> +++ b/linux-user/arm/cpu_loop.c
> @@ -349,8 +349,9 @@ void cpu_loop(CPUARMState *env)
>                              env->regs[0] = cpu_get_tls(env);
>                              break;
>                          default:
> -                            gemu_log("qemu: Unsupported ARM syscall: 0x%x\n",
> -                                     n);
> +                            qemu_log_mask(LOG_UNIMP,
> +                                          "qemu: Unsupported ARM syscall: 
> 0x%x\n",
> +                                          n);
>                              env->regs[0] = -TARGET_ENOSYS;
>                              break;
>                          }
> diff --git a/linux-user/fd-trans.c b/linux-user/fd-trans.c
> index 9b92386abf..c0687c52e6 100644
> --- a/linux-user/fd-trans.c
> +++ b/linux-user/fd-trans.c
> @@ -514,7 +514,8 @@ static abi_long host_to_target_data_bridge_nlattr(struct 
> nlattr *nlattr,
>          u32[1] = tswap32(u32[1]); /* optmask */
>          break;
>      default:
> -        gemu_log("Unknown QEMU_IFLA_BR type %d\n", nlattr->nla_type);
> +        qemu_log_mask(LOG_UNIMP, "Unknown QEMU_IFLA_BR type %d\n",
> +                      nlattr->nla_type);
>          break;
>      }
>      return 0;
> @@ -577,7 +578,8 @@ static abi_long 
> host_to_target_slave_data_bridge_nlattr(struct nlattr *nlattr,
>      case QEMU_IFLA_BRPORT_BRIDGE_ID:
>          break;
>      default:
> -        gemu_log("Unknown QEMU_IFLA_BRPORT type %d\n", nlattr->nla_type);
> +        qemu_log_mask(LOG_UNIMP, "Unknown QEMU_IFLA_BRPORT type %d\n",
> +                      nlattr->nla_type);
>          break;
>      }
>      return 0;
> @@ -605,7 +607,8 @@ static abi_long host_to_target_data_tun_nlattr(struct 
> nlattr *nlattr,
>          *u32 = tswap32(*u32);
>          break;
>      default:
> -        gemu_log("Unknown QEMU_IFLA_TUN type %d\n", nlattr->nla_type);
> +        qemu_log_mask(LOG_UNIMP, "Unknown QEMU_IFLA_TUN type %d\n",
> +                      nlattr->nla_type);
>          break;
>      }
>      return 0;
> @@ -652,7 +655,8 @@ static abi_long 
> host_to_target_data_linkinfo_nlattr(struct nlattr *nlattr,
>                                                    NULL,
>                                                  
> host_to_target_data_tun_nlattr);
>          } else {
> -            gemu_log("Unknown QEMU_IFLA_INFO_KIND %s\n", li_context->name);
> +            qemu_log_mask(LOG_UNIMP, "Unknown QEMU_IFLA_INFO_KIND %s\n",
> +                          li_context->name);
>          }
>          break;
>      case QEMU_IFLA_INFO_SLAVE_DATA:
> @@ -663,12 +667,13 @@ static abi_long 
> host_to_target_data_linkinfo_nlattr(struct nlattr *nlattr,
>                                                    NULL,
>                                         
> host_to_target_slave_data_bridge_nlattr);
>          } else {
> -            gemu_log("Unknown QEMU_IFLA_INFO_SLAVE_KIND %s\n",
> +            qemu_log_mask(LOG_UNIMP, "Unknown QEMU_IFLA_INFO_SLAVE_KIND 
> %s\n",
>                       li_context->slave_name);
>          }
>          break;
>      default:
> -        gemu_log("Unknown host QEMU_IFLA_INFO type: %d\n", nlattr->nla_type);
> +        qemu_log_mask(LOG_UNIMP, "Unknown host QEMU_IFLA_INFO type: %d\n",
> +                      nlattr->nla_type);
>          break;
>      }
>  
> @@ -690,7 +695,8 @@ static abi_long host_to_target_data_inet_nlattr(struct 
> nlattr *nlattr,
>          }
>          break;
>      default:
> -        gemu_log("Unknown host AF_INET type: %d\n", nlattr->nla_type);
> +        qemu_log_mask(LOG_UNIMP, "Unknown host AF_INET type: %d\n",
> +                      nlattr->nla_type);
>      }
>      return 0;
>  }
> @@ -741,7 +747,8 @@ static abi_long host_to_target_data_inet6_nlattr(struct 
> nlattr *nlattr,
>          }
>          break;
>      default:
> -        gemu_log("Unknown host AF_INET6 type: %d\n", nlattr->nla_type);
> +        qemu_log_mask(LOG_UNIMP, "Unknown host AF_INET6 type: %d\n",
> +                      nlattr->nla_type);
>      }
>      return 0;
>  }
> @@ -759,7 +766,8 @@ static abi_long host_to_target_data_spec_nlattr(struct 
> nlattr *nlattr,
>                                                NULL,
>                                               
> host_to_target_data_inet6_nlattr);
>      default:
> -        gemu_log("Unknown host AF_SPEC type: %d\n", nlattr->nla_type);
> +        qemu_log_mask(LOG_UNIMP, "Unknown host AF_SPEC type: %d\n",
> +                      nlattr->nla_type);
>          break;
>      }
>      return 0;
> @@ -780,7 +788,8 @@ static abi_long host_to_target_data_xdp_nlattr(struct 
> nlattr *nlattr,
>          *u32 = tswap32(*u32);
>          break;
>      default:
> -        gemu_log("Unknown host XDP type: %d\n", nlattr->nla_type);
> +        qemu_log_mask(
> +            LOG_UNIMP, "Unknown host XDP type: %d\n", nlattr->nla_type);
>          break;
>      }
>      return 0;
> @@ -920,7 +929,8 @@ static abi_long host_to_target_data_link_rtattr(struct 
> rtattr *rtattr)
>                                                NULL,
>                                                  
> host_to_target_data_xdp_nlattr);
>      default:
> -        gemu_log("Unknown host QEMU_IFLA type: %d\n", rtattr->rta_type);
> +        qemu_log_mask(LOG_UNIMP, "Unknown host QEMU_IFLA type: %d\n",
> +                      rtattr->rta_type);
>          break;
>      }
>      return 0;
> @@ -954,7 +964,8 @@ static abi_long host_to_target_data_addr_rtattr(struct 
> rtattr *rtattr)
>          ci->tstamp = tswap32(ci->tstamp);
>          break;
>      default:
> -        gemu_log("Unknown host IFA type: %d\n", rtattr->rta_type);
> +        qemu_log_mask(
> +            LOG_UNIMP, "Unknown host IFA type: %d\n", rtattr->rta_type);
>          break;
>      }
>      return 0;
> @@ -996,7 +1007,8 @@ static abi_long host_to_target_data_route_rtattr(struct 
> rtattr *rtattr)
>  #endif
>          break;
>      default:
> -        gemu_log("Unknown host RTA type: %d\n", rtattr->rta_type);
> +        qemu_log_mask(
> +            LOG_UNIMP, "Unknown host RTA type: %d\n", rtattr->rta_type);
>          break;
>      }
>      return 0;
> @@ -1111,7 +1123,8 @@ static abi_long target_to_host_data_link_rtattr(struct 
> rtattr *rtattr)
>  {
>      switch (rtattr->rta_type) {
>      default:
> -        gemu_log("Unknown target QEMU_IFLA type: %d\n", rtattr->rta_type);
> +        qemu_log_mask(LOG_UNIMP, "Unknown target QEMU_IFLA type: %d\n",
> +                      rtattr->rta_type);
>          break;
>      }
>      return 0;
> @@ -1125,7 +1138,8 @@ static abi_long target_to_host_data_addr_rtattr(struct 
> rtattr *rtattr)
>      case IFA_ADDRESS:
>          break;
>      default:
> -        gemu_log("Unknown target IFA type: %d\n", rtattr->rta_type);
> +        qemu_log_mask(LOG_UNIMP, "Unknown target IFA type: %d\n",
> +                      rtattr->rta_type);
>          break;
>      }
>      return 0;
> @@ -1147,7 +1161,8 @@ static abi_long target_to_host_data_route_rtattr(struct 
> rtattr *rtattr)
>          *u32 = tswap32(*u32);
>          break;
>      default:
> -        gemu_log("Unknown target RTA type: %d\n", rtattr->rta_type);
> +        qemu_log_mask(LOG_UNIMP, "Unknown target RTA type: %d\n",
> +                      rtattr->rta_type);
>          break;
>      }
>      return 0;
> @@ -1232,8 +1247,8 @@ static abi_long host_to_target_data_audit(struct 
> nlmsghdr *nlh)
>  {
>      switch (nlh->nlmsg_type) {
>      default:
> -        gemu_log("Unknown host audit message type %d\n",
> -                 nlh->nlmsg_type);
> +        qemu_log_mask(LOG_UNIMP, "Unknown host audit message type %d\n",
> +                      nlh->nlmsg_type);
>          return -TARGET_EINVAL;
>      }
>      return 0;
> @@ -1253,8 +1268,8 @@ static abi_long target_to_host_data_audit(struct 
> nlmsghdr *nlh)
>      case AUDIT_FIRST_USER_MSG2 ... AUDIT_LAST_USER_MSG2:
>          break;
>      default:
> -        gemu_log("Unknown target audit message type %d\n",
> -                 nlh->nlmsg_type);
> +        qemu_log_mask(LOG_UNIMP, "Unknown target audit message type %d\n",
> +                      nlh->nlmsg_type);
>          return -TARGET_EINVAL;
>      }
>  
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index d60142f069..c48c2a9a55 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -1560,7 +1560,11 @@ static inline abi_long target_to_host_cmsg(struct 
> msghdr *msgh,
>               * something more intelligent than "twice the size of the
>               * target buffer we're reading from".
>               */
> -            gemu_log("Host cmsg overflow\n");
> +            qemu_log_mask(LOG_UNIMP,
> +                          ("Unsupported ancillary data %d/%d: "
> +                           "unhandled msg size\n"),
> +                          tswap32(target_cmsg->cmsg_level),
> +                          tswap32(target_cmsg->cmsg_type));
>              break;
>          }
>  
> @@ -1590,8 +1594,8 @@ static inline abi_long target_to_host_cmsg(struct 
> msghdr *msgh,
>              __get_user(cred->uid, &target_cred->uid);
>              __get_user(cred->gid, &target_cred->gid);
>          } else {
> -            gemu_log("Unsupported ancillary data: %d/%d\n",
> -                                        cmsg->cmsg_level, cmsg->cmsg_type);
> +            qemu_log_mask(LOG_UNIMP, "Unsupported ancillary data: %d/%d\n",
> +                          cmsg->cmsg_level, cmsg->cmsg_type);
>              memcpy(data, target_data, len);
>          }
>  
> @@ -1812,8 +1816,8 @@ static inline abi_long host_to_target_cmsg(struct 
> target_msghdr *target_msgh,
>  
>          default:
>          unimplemented:
> -            gemu_log("Unsupported ancillary data: %d/%d\n",
> -                                        cmsg->cmsg_level, cmsg->cmsg_type);
> +            qemu_log_mask(LOG_UNIMP, "Unsupported ancillary data: %d/%d\n",
> +                          cmsg->cmsg_level, cmsg->cmsg_type);
>              memcpy(target_data, data, MIN(len, tgt_len));
>              if (tgt_len > len) {
>                  memset(target_data + len, 0, tgt_len - len);
> @@ -2288,7 +2292,8 @@ set_timeout:
>  #endif /* SOL_NETLINK */
>      default:
>      unimplemented:
> -        gemu_log("Unsupported setsockopt level=%d optname=%d\n", level, 
> optname);
> +        qemu_log_mask(LOG_UNIMP, "Unsupported setsockopt level=%d 
> optname=%d\n",
> +                      level, optname);
>          ret = -TARGET_ENOPROTOOPT;
>      }
>      return ret;
> @@ -2641,8 +2646,9 @@ static abi_long do_getsockopt(int sockfd, int level, 
> int optname,
>  #endif /* SOL_NETLINK */
>      default:
>      unimplemented:
> -        gemu_log("getsockopt level=%d optname=%d not yet supported\n",
> -                 level, optname);
> +        qemu_log_mask(LOG_UNIMP,
> +                      "getsockopt level=%d optname=%d not yet supported\n",
> +                      level, optname);
>          ret = -TARGET_EOPNOTSUPP;
>          break;
>      }
> @@ -3397,7 +3403,7 @@ static abi_long do_socketcall(int num, abi_ulong vptr)
>      case TARGET_SYS_SENDMMSG: /* sockfd, msgvec, vlen, flags */
>          return do_sendrecvmmsg(a[0], a[1], a[2], a[3], 1);
>      default:
> -        gemu_log("Unsupported socketcall: %d\n", num);
> +        qemu_log_mask(LOG_UNIMP, "Unsupported socketcall: %d\n", num);
>          return -TARGET_EINVAL;
>      }
>  }
> @@ -4308,7 +4314,8 @@ static abi_long do_ipc(CPUArchState *cpu_env,
>          ret = do_shmctl(first, second, ptr);
>          break;
>      default:
> -     gemu_log("Unsupported ipc call: %d (version %d)\n", call, version);
> +        qemu_log_mask(LOG_UNIMP, "Unsupported ipc call: %d (version %d)\n",
> +                      call, version);
>       ret = -TARGET_ENOSYS;
>       break;
>      }
> @@ -5156,7 +5163,8 @@ static abi_long do_ioctl(int fd, int cmd, abi_long arg)
>      ie = ioctl_entries;
>      for(;;) {
>          if (ie->target_cmd == 0) {
> -            gemu_log("Unsupported ioctl: cmd=0x%04lx\n", (long)cmd);
> +            qemu_log_mask(
> +                LOG_UNIMP, "Unsupported ioctl: cmd=0x%04lx\n", (long)cmd);
>              return -TARGET_ENOSYS;
>          }
>          if (ie->target_cmd == cmd)
> @@ -5224,8 +5232,9 @@ static abi_long do_ioctl(int fd, int cmd, abi_long arg)
>          }
>          break;
>      default:
> -        gemu_log("Unsupported ioctl type: cmd=0x%04lx type=%d\n",
> -                 (long)cmd, arg_type[0]);
> +        qemu_log_mask(LOG_UNIMP,
> +                      "Unsupported ioctl type: cmd=0x%04lx type=%d\n",
> +                      (long)cmd, arg_type[0]);
>          ret = -TARGET_ENOSYS;
>          break;
>      }
> diff --git a/linux-user/vm86.c b/linux-user/vm86.c
> index 2fa7a89edc..4412522c4c 100644
> --- a/linux-user/vm86.c
> +++ b/linux-user/vm86.c
> @@ -402,7 +402,8 @@ int do_vm86(CPUX86State *env, long subfunction, abi_ulong 
> vm86_addr)
>      case TARGET_VM86_FREE_IRQ:
>      case TARGET_VM86_GET_IRQ_BITS:
>      case TARGET_VM86_GET_AND_RESET_IRQ:
> -        gemu_log("qemu: unsupported vm86 subfunction (%ld)\n", subfunction);
> +        qemu_log_mask(LOG_UNIMP, "qemu: unsupported vm86 subfunction 
> (%ld)\n",
> +                      subfunction);
>          ret = -TARGET_EINVAL;
>          goto out;
>      case TARGET_VM86_PLUS_INSTALL_CHECK:
> 

Reviewed-by: Laurent Vivier <address@hidden>