Re: [PATCH] command: enforce fd vs. buffer considerations

[Eric Blake <eblake@xxxxxxxxxx>]

On 12/07/2010 02:59 PM, Matthias Bolte wrote:
> 2010/12/7 Eric Blake <eblake@xxxxxxxxxx>:
>> * docs/internals/command.html.in: Better documentation of buffer
>> vs. fd considerations.
>> * src/util/command.c (virCommandRunAsync): Reject raw execution
>> with string io.
>> (virCommandRun): Reject execution with user-specified fds not
>> visiting a regular file.
>> +    if (async_io ? (!(cmd->flags & VIR_EXEC_DAEMON) || string_io)
>> +        : ((cmd->flags & VIR_EXEC_DAEMON) && string_io)) {
>> +        virCommandError(VIR_ERR_INTERNAL_ERROR, "%s",
>> +                        _("invalid use of command API"));
>> +        return -1;
>> +    }
> 
>> +    /* Buffer management can only be requested via virCommandRun.  */
>> +    if ((cmd->inbuf && cmd->infd == -1) ||
>> +        (cmd->outbuf && cmd->outfdptr != &cmd->outfd) ||
>> +        (cmd->errbuf && cmd->errfdptr != &cmd->errfd)) {
>> +        virCommandError(VIR_ERR_INTERNAL_ERROR, "%s",
>> +                        _("invalid use of command API"));
>> +        return -1;
>> +    }
> 
> Maybe make the error message about invalid use distinct from each
> other. That would allow to tell based on the error message what type
> of invalid usage has happened.
> 
> ACK.

I'm squashing this in before pushing:

diff --git i/src/util/command.c w/src/util/command.c
index d1d8f6d..3dfd33d 100644
--- i/src/util/command.c
+++ w/src/util/command.c
@@ -902,11 +902,18 @@ virCommandRun(virCommandPtr cmd, int *exitstatus)
         (*cmd->errfdptr == -1 ||
          fstat(*cmd->errfdptr, &st) < 0 || !S_ISREG(st.st_mode)))
         async_io = true;
-    if (async_io ? (!(cmd->flags & VIR_EXEC_DAEMON) || string_io)
-        : ((cmd->flags & VIR_EXEC_DAEMON) && string_io)) {
-        virCommandError(VIR_ERR_INTERNAL_ERROR, "%s",
-                        _("invalid use of command API"));
-        return -1;
+    if (async_io) {
+        if (!(cmd->flags & VIR_EXEC_DAEMON) || string_io) {
+            virCommandError(VIR_ERR_INTERNAL_ERROR, "%s",
+                            _("cannot mix caller fds with blocking
execution"));
+            return -1;
+        }
+    } else {
+        if ((cmd->flags & VIR_EXEC_DAEMON) && string_io) {
+            virCommandError(VIR_ERR_INTERNAL_ERROR, "%s",
+                            _("cannot mix string I/O with daemon"));
+            return -1;
+        }
     }

     /* If we have an input buffer, we need
@@ -1032,7 +1039,7 @@ virCommandRunAsync(virCommandPtr cmd, pid_t *pid)
         (cmd->outbuf && cmd->outfdptr != &cmd->outfd) ||
         (cmd->errbuf && cmd->errfdptr != &cmd->errfd)) {
         virCommandError(VIR_ERR_INTERNAL_ERROR, "%s",
-                        _("invalid use of command API"));
+                        _("cannot mix string I/O with asynchronous
command"));
         return -1;
     }


-- 
Eric Blake   eblake@xxxxxxxxxx    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

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