Mark a virCommand's inpipe (write-end of pipe) as non-blocking so that it
will never block when we were to try to write too many bytes to it while
it doesn't have the capacity to hold them.

Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
---
 src/util/vircommand.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/src/util/vircommand.c b/src/util/vircommand.c
index 3ac51c77a8..b10bba5b32 100644
--- a/src/util/vircommand.c
+++ b/src/util/vircommand.c
@@ -2535,6 +2535,13 @@ virCommandRunAsync(virCommandPtr cmd, pid_t *pid)
         }
         cmd->infd = infd[0];
         cmd->inpipe = infd[1];
+        if (fcntl(cmd->inpipe, F_SETFL, O_NONBLOCK) < 0) {
+            virReportSystemError(errno, "%s",
+                                 _("fcntl failed to set O_NONBLOCK"));
+            cmd->has_error = -1;
+            ret = -1;
+            goto cleanup;
+        }
     } else if ((cmd->inbuf && cmd->infd == -1) ||
                (cmd->outbuf && cmd->outfdptr != &cmd->outfd) ||
                (cmd->errbuf && cmd->errfdptr != &cmd->errfd)) {
-- 
2.20.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Thu, Jul 11, 2019 at 03:41:44PM -0400, Stefan Berger wrote:
> Mark a virCommand's inpipe (write-end of pipe) as non-blocking so that it
> will never block when we were to try to write too many bytes to it while
> it doesn't have the capacity to hold them.

So this is the right way todo it.  Why did you have the previous patch 15
at all given you've used O_NONBLOCK here ?

> 
> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
> ---
>  src/util/vircommand.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/src/util/vircommand.c b/src/util/vircommand.c
> index 3ac51c77a8..b10bba5b32 100644
> --- a/src/util/vircommand.c
> +++ b/src/util/vircommand.c
> @@ -2535,6 +2535,13 @@ virCommandRunAsync(virCommandPtr cmd, pid_t *pid)
>          }
>          cmd->infd = infd[0];
>          cmd->inpipe = infd[1];
> +        if (fcntl(cmd->inpipe, F_SETFL, O_NONBLOCK) < 0) {
> +            virReportSystemError(errno, "%s",
> +                                 _("fcntl failed to set O_NONBLOCK"));
> +            cmd->has_error = -1;
> +            ret = -1;
> +            goto cleanup;
> +        }
>      } else if ((cmd->inbuf && cmd->infd == -1) ||
>                 (cmd->outbuf && cmd->outfdptr != &cmd->outfd) ||
>                 (cmd->errbuf && cmd->errfdptr != &cmd->errfd)) {
> -- 
> 2.20.1
> 
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On 7/12/19 4:24 AM, Daniel P. Berrangé wrote:
> On Thu, Jul 11, 2019 at 03:41:44PM -0400, Stefan Berger wrote:
>> Mark a virCommand's inpipe (write-end of pipe) as non-blocking so that it
>> will never block when we were to try to write too many bytes to it while
>> it doesn't have the capacity to hold them.
> So this is the right way todo it.  Why did you have the previous patch 15
> at all given you've used O_NONBLOCK here ?


I thought that with a buffer bigger than the capacity of a pipe we 
wouldn't be able to make write progress but that's not the case but it 
fills up the pipe with whatever fits. I'll drop the MIN() stuff.


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list