[PATCH] virfile: Report error when changing pipe size fails

Michal Privoznik posted 1 patch 2 years, 7 months ago
Test syntax-check passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/81c66d46b66511b31c90b3fcb865f6444367c2d6.1648467968.git.mprivozn@redhat.com
src/util/virfile.c | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
[PATCH] virfile: Report error when changing pipe size fails
Posted by Michal Privoznik 2 years, 7 months ago
When changing the size of pipe that virFileWrapperFdNew() creates
we start at 1MiB and if that fails because it's above the system
wide limit we get EPERM and continue with half of the size.

However, we might get another error in which case we should
report proper system error and return failure from
virFileWrapperFdNew().

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/util/virfile.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/src/util/virfile.c b/src/util/virfile.c
index 12b359d550..130b0fbace 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -217,7 +217,7 @@ struct _virFileWrapperFd {
  *
  * OS note: only for linux, on other OS this is a no-op.
  */
-static void
+static int
 virFileWrapperSetPipeSize(int fd)
 {
     int sz;
@@ -230,21 +230,24 @@ virFileWrapperSetPipeSize(int fd)
             continue; /* retry with half the size */
         }
         if (rv < 0) {
-            break;
+            virReportSystemError(errno, "%s",
+                                 _("unable to set pipe size"));
+            return -1;
         }
         VIR_DEBUG("fd %d pipe size adjusted to %d", fd, sz);
-        return;
+        return 0;
     }
 
     VIR_WARN("unable to set pipe size, data transfer might be slow: %s",
              g_strerror(errno));
+    return 0;
 }
 
 # else /* !__linux__ */
-static void
+static int
 virFileWrapperSetPipeSize(int fd G_GNUC_UNUSED)
 {
-    return;
+    return 0;
 }
 # endif /* !__linux__ */
 
@@ -323,6 +326,9 @@ virFileWrapperFdNew(int *fd, const char *name, unsigned int flags)
     if (virPipe(pipefd) < 0)
         goto error;
 
+    if (virFileWrapperSetPipeSize(pipefd[output]) < 0)
+        goto error;
+
     if (!(iohelper_path = virFileFindResource("libvirt_iohelper",
                                               abs_top_builddir "/src",
                                               LIBEXECDIR)))
@@ -330,8 +336,6 @@ virFileWrapperFdNew(int *fd, const char *name, unsigned int flags)
 
     ret->cmd = virCommandNewArgList(iohelper_path, name, NULL);
 
-    virFileWrapperSetPipeSize(pipefd[output]);
-
     if (output) {
         virCommandSetInputFD(ret->cmd, pipefd[0]);
         virCommandSetOutputFD(ret->cmd, fd);
-- 
2.34.1
Re: [PATCH] virfile: Report error when changing pipe size fails
Posted by Daniel P. Berrangé 2 years, 7 months ago
On Mon, Mar 28, 2022 at 01:46:08PM +0200, Michal Privoznik wrote:
> When changing the size of pipe that virFileWrapperFdNew() creates
> we start at 1MiB and if that fails because it's above the system
> wide limit we get EPERM and continue with half of the size.
> 
> However, we might get another error in which case we should
> report proper system error and return failure from
> virFileWrapperFdNew().
> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  src/util/virfile.c | 18 +++++++++++-------
>  1 file changed, 11 insertions(+), 7 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With 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 :|

Re: [PATCH] virfile: Report error when changing pipe size fails
Posted by Claudio Fontana 2 years, 7 months ago
On 3/28/22 2:22 PM, Daniel P. Berrangé wrote:
> On Mon, Mar 28, 2022 at 01:46:08PM +0200, Michal Privoznik wrote:
>> When changing the size of pipe that virFileWrapperFdNew() creates
>> we start at 1MiB and if that fails because it's above the system
>> wide limit we get EPERM and continue with half of the size.
>>
>> However, we might get another error in which case we should
>> report proper system error and return failure from
>> virFileWrapperFdNew().
>>
>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>> ---
>>  src/util/virfile.c | 18 +++++++++++-------
>>  1 file changed, 11 insertions(+), 7 deletions(-)
> 
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> 
> 
> With regards,
> Daniel
> 

testing it right now, thanks!
Re: [PATCH] virfile: Report error when changing pipe size fails
Posted by Claudio Fontana 2 years, 7 months ago
On 3/28/22 2:24 PM, Claudio Fontana wrote:
> On 3/28/22 2:22 PM, Daniel P. Berrangé wrote:
>> On Mon, Mar 28, 2022 at 01:46:08PM +0200, Michal Privoznik wrote:
>>> When changing the size of pipe that virFileWrapperFdNew() creates
>>> we start at 1MiB and if that fails because it's above the system
>>> wide limit we get EPERM and continue with half of the size.
>>>
>>> However, we might get another error in which case we should
>>> report proper system error and return failure from
>>> virFileWrapperFdNew().
>>>
>>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>>> ---
>>>  src/util/virfile.c | 18 +++++++++++-------
>>>  1 file changed, 11 insertions(+), 7 deletions(-)
>>
>> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
>>
>>
>> With regards,
>> Daniel
>>
> 
> testing it right now, thanks!
> 

Tested-by: Claudio Fontana <cfontana@suse.de>