[libvirt] [PATCH v6 10/19] utils: Implement function to pass a buffer to send via a fd to virCommand

Stefan Berger posted 19 patches 6 years, 6 months ago
There is a newer version of this series
[libvirt] [PATCH v6 10/19] utils: Implement function to pass a buffer to send via a fd to virCommand
Posted by Stefan Berger 6 years, 6 months ago
Implement virCommandSetSendBuffer() that allows the caller to pass a
file descriptor and buffer to virCommand. virCommand will write the
buffer into the file descriptor. That file descriptor could be the
write end of a pipe or one of the file descriptors of a socketpair.
The other file descriptor should be passed to the launched process to
read the data from.

Only implement the function to allocate memory for send buffers
and to free them later on.

Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
---
 src/libvirt_private.syms |  1 +
 src/util/vircommand.c    | 76 ++++++++++++++++++++++++++++++++++++++++
 src/util/vircommand.h    |  5 +++
 3 files changed, 82 insertions(+)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index cf80ea3e44..e6249caa80 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1725,6 +1725,7 @@ virCommandSetOutputFD;
 virCommandSetPidFile;
 virCommandSetPreExecHook;
 virCommandSetSELinuxLabel;
+virCommandSetSendBuffer;
 virCommandSetUID;
 virCommandSetUmask;
 virCommandSetWorkingDirectory;
diff --git a/src/util/vircommand.c b/src/util/vircommand.c
index e10ca3eb7c..5dee730826 100644
--- a/src/util/vircommand.c
+++ b/src/util/vircommand.c
@@ -76,6 +76,16 @@ struct _virCommandFD {
     unsigned int flags;
 };
 
+typedef struct _virCommandSendBuffer virCommandSendBuffer;
+typedef virCommandSendBuffer *virCommandSendBufferPtr;
+
+struct _virCommandSendBuffer {
+    int fd;
+    unsigned char *buffer;
+    size_t buflen;
+    off_t offset;
+};
+
 struct _virCommand {
     int has_error; /* ENOMEM on allocation failure, -1 for anything else.  */
 
@@ -135,6 +145,9 @@ struct _virCommand {
     char *appArmorProfile;
 #endif
     int mask;
+
+    virCommandSendBufferPtr sendBuffers;
+    size_t numSendBuffers;
 };
 
 /* See virCommandSetDryRun for description for this variable */
@@ -1728,6 +1741,67 @@ virCommandSetWorkingDirectory(virCommandPtr cmd, const char *pwd)
 }
 
 
+static int
+virCommandGetNumSendBuffers(virCommandPtr cmd)
+{
+    return cmd->numSendBuffers;
+}
+
+
+static void
+virCommandFreeSendBuffers(virCommandPtr cmd)
+{
+    size_t i;
+
+    for (i = 0; i < virCommandGetNumSendBuffers(cmd); i++) {
+        VIR_FORCE_CLOSE(cmd->sendBuffers[i].fd);
+        VIR_FREE(cmd->sendBuffers[i].buffer);
+    }
+    VIR_FREE(cmd->sendBuffers);
+}
+
+
+/**
+ * virCommandSetSendBuffer
+ * @cmd: the command to modify
+ *
+ * Pass a buffer to virCommand that will be written into the
+ * given file descriptor. The buffer will be freed automatically
+ * and the file descriptor closed.
+ */
+int
+virCommandSetSendBuffer(virCommandPtr cmd,
+                        int fd,
+                        unsigned char *buffer, size_t buflen)
+{
+    size_t i = virCommandGetNumSendBuffers(cmd);
+
+    if (!cmd || cmd->has_error)
+        return -1;
+
+    if (fcntl(fd, F_SETFL, O_NONBLOCK) < 0) {
+        virReportSystemError(errno, "%s",
+                             _("fcntl failed to set O_NONBLOCK"));
+        cmd->has_error = errno;
+        return -1;
+    }
+
+    if (VIR_REALLOC_N(cmd->sendBuffers, i + 1) < 0) {
+        cmd->has_error = ENOMEM;
+        return -1;
+    }
+
+    cmd->sendBuffers[i].fd = fd;
+    cmd->sendBuffers[i].buffer = buffer;
+    cmd->sendBuffers[i].buflen = buflen;
+    cmd->sendBuffers[i].offset = 0;
+
+    cmd->numSendBuffers++;
+
+    return 0;
+}
+
+
 /**
  * virCommandSetInputBuffer:
  * @cmd: the command to modify
@@ -2867,6 +2941,8 @@ virCommandFree(virCommandPtr cmd)
     VIR_FREE(cmd->appArmorProfile);
 #endif
 
+    virCommandFreeSendBuffers(cmd);
+
     VIR_FREE(cmd);
 }
 
diff --git a/src/util/vircommand.h b/src/util/vircommand.h
index 2a9ee5cdc7..c2abc7b2c3 100644
--- a/src/util/vircommand.h
+++ b/src/util/vircommand.h
@@ -146,6 +146,11 @@ void virCommandAddArgList(virCommandPtr cmd,
 void virCommandSetWorkingDirectory(virCommandPtr cmd,
                                    const char *pwd) ATTRIBUTE_NONNULL(2);
 
+int virCommandSetSendBuffer(virCommandPtr cmd,
+                            int fd,
+                            unsigned char *buffer, size_t buflen)
+    ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3);
+
 void virCommandSetInputBuffer(virCommandPtr cmd,
                               const char *inbuf) ATTRIBUTE_NONNULL(2);
 
-- 
2.20.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v6 10/19] utils: Implement function to pass a buffer to send via a fd to virCommand
Posted by Daniel P. Berrangé 6 years, 6 months ago
On Thu, Jul 25, 2019 at 10:30:24AM -0400, Stefan Berger wrote:
> Implement virCommandSetSendBuffer() that allows the caller to pass a
> file descriptor and buffer to virCommand. virCommand will write the
> buffer into the file descriptor. That file descriptor could be the
> write end of a pipe or one of the file descriptors of a socketpair.
> The other file descriptor should be passed to the launched process to
> read the data from.
> 
> Only implement the function to allocate memory for send buffers
> and to free them later on.
> 
> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  src/libvirt_private.syms |  1 +
>  src/util/vircommand.c    | 76 ++++++++++++++++++++++++++++++++++++++++
>  src/util/vircommand.h    |  5 +++
>  3 files changed, 82 insertions(+)
> 
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index cf80ea3e44..e6249caa80 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1725,6 +1725,7 @@ virCommandSetOutputFD;
>  virCommandSetPidFile;
>  virCommandSetPreExecHook;
>  virCommandSetSELinuxLabel;
> +virCommandSetSendBuffer;
>  virCommandSetUID;
>  virCommandSetUmask;
>  virCommandSetWorkingDirectory;
> diff --git a/src/util/vircommand.c b/src/util/vircommand.c
> index e10ca3eb7c..5dee730826 100644
> --- a/src/util/vircommand.c
> +++ b/src/util/vircommand.c
> @@ -76,6 +76,16 @@ struct _virCommandFD {
>      unsigned int flags;
>  };
>  
> +typedef struct _virCommandSendBuffer virCommandSendBuffer;
> +typedef virCommandSendBuffer *virCommandSendBufferPtr;
> +
> +struct _virCommandSendBuffer {
> +    int fd;
> +    unsigned char *buffer;
> +    size_t buflen;
> +    off_t offset;
> +};
> +
>  struct _virCommand {
>      int has_error; /* ENOMEM on allocation failure, -1 for anything else.  */
>  
> @@ -135,6 +145,9 @@ struct _virCommand {
>      char *appArmorProfile;
>  #endif
>      int mask;
> +
> +    virCommandSendBufferPtr sendBuffers;
> +    size_t numSendBuffers;
>  };
>  
>  /* See virCommandSetDryRun for description for this variable */
> @@ -1728,6 +1741,67 @@ virCommandSetWorkingDirectory(virCommandPtr cmd, const char *pwd)
>  }
>  
>  
> +static int
> +virCommandGetNumSendBuffers(virCommandPtr cmd)
> +{
> +    return cmd->numSendBuffers;
> +}
> +
> +
> +static void
> +virCommandFreeSendBuffers(virCommandPtr cmd)
> +{
> +    size_t i;
> +
> +    for (i = 0; i < virCommandGetNumSendBuffers(cmd); i++) {
> +        VIR_FORCE_CLOSE(cmd->sendBuffers[i].fd);
> +        VIR_FREE(cmd->sendBuffers[i].buffer);
> +    }
> +    VIR_FREE(cmd->sendBuffers);
> +}
> +
> +
> +/**
> + * virCommandSetSendBuffer
> + * @cmd: the command to modify
> + *
> + * Pass a buffer to virCommand that will be written into the
> + * given file descriptor. The buffer will be freed automatically
> + * and the file descriptor closed.
> + */
> +int
> +virCommandSetSendBuffer(virCommandPtr cmd,
> +                        int fd,
> +                        unsigned char *buffer, size_t buflen)
> +{
> +    size_t i = virCommandGetNumSendBuffers(cmd);
> +
> +    if (!cmd || cmd->has_error)
> +        return -1;
> +
> +    if (fcntl(fd, F_SETFL, O_NONBLOCK) < 0) {
> +        virReportSystemError(errno, "%s",
> +                             _("fcntl failed to set O_NONBLOCK"));
> +        cmd->has_error = errno;
> +        return -1;
> +    }

I hit a build failure on mingw with this change, because F_SETFL
doesn't exist.

None of the vircommand code actually runs on windows, so we just
need to stub this method out to immediately return -1 with
has_error set to ENOSUPP i guess.

Thre's another usage of O_NONBLOCK with same problem.


If you can fix this, I'm happy to push the series.

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
Re: [libvirt] [PATCH v6 10/19] utils: Implement function to pass a buffer to send via a fd to virCommand
Posted by Stefan Berger 6 years, 6 months ago
On 7/25/19 1:34 PM, Daniel P. Berrangé wrote:
> On Thu, Jul 25, 2019 at 10:30:24AM -0400, Stefan Berger wrote:
>> Implement virCommandSetSendBuffer() that allows the caller to pass a
>> file descriptor and buffer to virCommand. virCommand will write the
>> buffer into the file descriptor. That file descriptor could be the
>> write end of a pipe or one of the file descriptors of a socketpair.
>> The other file descriptor should be passed to the launched process to
>> read the data from.
>>
>> Only implement the function to allocate memory for send buffers
>> and to free them later on.
>>
>> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
>> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
>> ---
>>   src/libvirt_private.syms |  1 +
>>   src/util/vircommand.c    | 76 ++++++++++++++++++++++++++++++++++++++++
>>   src/util/vircommand.h    |  5 +++
>>   3 files changed, 82 insertions(+)
>>
>> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
>> index cf80ea3e44..e6249caa80 100644
>> --- a/src/libvirt_private.syms
>> +++ b/src/libvirt_private.syms
>> @@ -1725,6 +1725,7 @@ virCommandSetOutputFD;
>>   virCommandSetPidFile;
>>   virCommandSetPreExecHook;
>>   virCommandSetSELinuxLabel;
>> +virCommandSetSendBuffer;
>>   virCommandSetUID;
>>   virCommandSetUmask;
>>   virCommandSetWorkingDirectory;
>> diff --git a/src/util/vircommand.c b/src/util/vircommand.c
>> index e10ca3eb7c..5dee730826 100644
>> --- a/src/util/vircommand.c
>> +++ b/src/util/vircommand.c
>> @@ -76,6 +76,16 @@ struct _virCommandFD {
>>       unsigned int flags;
>>   };
>>   
>> +typedef struct _virCommandSendBuffer virCommandSendBuffer;
>> +typedef virCommandSendBuffer *virCommandSendBufferPtr;
>> +
>> +struct _virCommandSendBuffer {
>> +    int fd;
>> +    unsigned char *buffer;
>> +    size_t buflen;
>> +    off_t offset;
>> +};
>> +
>>   struct _virCommand {
>>       int has_error; /* ENOMEM on allocation failure, -1 for anything else.  */
>>   
>> @@ -135,6 +145,9 @@ struct _virCommand {
>>       char *appArmorProfile;
>>   #endif
>>       int mask;
>> +
>> +    virCommandSendBufferPtr sendBuffers;
>> +    size_t numSendBuffers;
>>   };
>>   
>>   /* See virCommandSetDryRun for description for this variable */
>> @@ -1728,6 +1741,67 @@ virCommandSetWorkingDirectory(virCommandPtr cmd, const char *pwd)
>>   }
>>   
>>   
>> +static int
>> +virCommandGetNumSendBuffers(virCommandPtr cmd)
>> +{
>> +    return cmd->numSendBuffers;
>> +}
>> +
>> +
>> +static void
>> +virCommandFreeSendBuffers(virCommandPtr cmd)
>> +{
>> +    size_t i;
>> +
>> +    for (i = 0; i < virCommandGetNumSendBuffers(cmd); i++) {
>> +        VIR_FORCE_CLOSE(cmd->sendBuffers[i].fd);
>> +        VIR_FREE(cmd->sendBuffers[i].buffer);
>> +    }
>> +    VIR_FREE(cmd->sendBuffers);
>> +}
>> +
>> +
>> +/**
>> + * virCommandSetSendBuffer
>> + * @cmd: the command to modify
>> + *
>> + * Pass a buffer to virCommand that will be written into the
>> + * given file descriptor. The buffer will be freed automatically
>> + * and the file descriptor closed.
>> + */
>> +int
>> +virCommandSetSendBuffer(virCommandPtr cmd,
>> +                        int fd,
>> +                        unsigned char *buffer, size_t buflen)
>> +{
>> +    size_t i = virCommandGetNumSendBuffers(cmd);
>> +
>> +    if (!cmd || cmd->has_error)
>> +        return -1;
>> +
>> +    if (fcntl(fd, F_SETFL, O_NONBLOCK) < 0) {
>> +        virReportSystemError(errno, "%s",
>> +                             _("fcntl failed to set O_NONBLOCK"));
>> +        cmd->has_error = errno;
>> +        return -1;
>> +    }
> I hit a build failure on mingw with this change, because F_SETFL
> doesn't exist.
>
> None of the vircommand code actually runs on windows, so we just
> need to stub this method out to immediately return -1 with
> has_error set to ENOSUPP i guess.

Fixed them with ENOTSUP. Will post v7 assuming you will add your R-b's 
then...


    Stefan

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