[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 1/4] linux-user: Use `qemu_log' for non-strace logging
From: |
Laurent Vivier |
Subject: |
Re: [PATCH v2 1/4] linux-user: Use `qemu_log' for non-strace logging |
Date: |
Tue, 28 Jan 2020 15:51:29 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.3.1 |
Le 17/01/2020 à 20:28, 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.
This is a good incompatibility as these messages were unexpected by the
tools catching stderr. They don't happen on "real" systems.
...
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 249e4b95fc..629f3a21b5 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -1545,20 +1545,18 @@ static inline abi_long target_to_host_cmsg(struct
> msghdr *msgh,
> - sizeof(struct target_cmsghdr);
>
> space += CMSG_SPACE(len);
> - if (space > msgh->msg_controllen) {
> - space -= CMSG_SPACE(len);
> - /* This is a QEMU bug, since we allocated the payload
> - * area ourselves (unlike overflow in host-to-target
> - * conversion, which is just the guest giving us a buffer
> - * that's too small). It can't happen for the payload types
> - * we currently support; if it becomes an issue in future
> - * we would need to improve our allocation strategy to
> - * something more intelligent than "twice the size of the
> - * target buffer we're reading from".
> - */
> - gemu_log("Host cmsg overflow\n");
> - break;
> - }
> +
> + /*
> + * This is a QEMU bug, since we allocated the payload
> + * area ourselves (unlike overflow in host-to-target
> + * conversion, which is just the guest giving us a buffer
> + * that's too small). It can't happen for the payload types
> + * we currently support; if it becomes an issue in future
> + * we would need to improve our allocation strategy to
> + * something more intelligent than "twice the size of the
> + * target buffer we're reading from".
> + */
> + assert(space > msgh->msg_controllen && "Host cmsg overflow");
>
> if (tswap32(target_cmsg->cmsg_level) == TARGET_SOL_SOCKET) {
> cmsg->cmsg_level = SOL_SOCKET;
Could you move this to a separate patch: you are not using qemu_log()
here and I'm not convinced that crashing is better than ignoring the
remaining part of the buffer.
For the other changes:
Reviewed-by: Laurent Vivier <address@hidden>
Thanks,
Lauren