qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] qio: fix command spawn RDONLY/WRONLY


From: Marc-André Lureau
Subject: Re: [PATCH] qio: fix command spawn RDONLY/WRONLY
Date: Thu, 1 Sep 2022 14:47:17 +0400

Hi

On Thu, Sep 1, 2022 at 2:32 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
On Thu, Sep 01, 2022 at 02:11:20PM +0400, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> The in/out handling is inverted, although nothing seemed to notice that yet.

On the contrary, it is correct, and the unit tests validate this.

> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  io/channel-command.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/io/channel-command.c b/io/channel-command.c
> index 9f2f4a1793..ed17b44f74 100644
> --- a/io/channel-command.c
> +++ b/io/channel-command.c
> @@ -79,10 +79,10 @@ qio_channel_command_new_spawn(const char *const argv[],
>      flags = flags & O_ACCMODE;

>      if (flags == O_RDONLY) {
> -        stdinnull = true;
> +        stdoutnull = true;
>      }
>      if (flags == O_WRONLY) {
> -        stdoutnull = true;
> +        stdinnull = true;
>      }

This change breaks the unit tests.


Does it really test it then? we are talking about test-io-channel-command ? It works before and after for me. Other tests as well.

The confusion is because there are two parties involves. The 'flags'
variable is from the POV of the parent process, while stdinnull/stdoutnull
are from the POV of the child process.

IOW, if the parent process is reading from the child (O_RDONLY),
then the child needs a stdout to write to the parent, but not
any stdin to read from the parent, hence we set stdin to /dev/null
in the child.

Ok, thanks for the clarification!


--
Marc-André Lureau

reply via email to

[Prev in Thread] Current Thread [Next in Thread]