[PATCH] libxl: use libxl_fd_set_{cloexec, nonblock} helpers

[PATCH] libxl: use libxl_fd_set_{cloexec, nonblock} helpers

[Jan Beulich]

[-- Attachment #1: Type: text/plain, Size: 2653 bytes --]

... instead of open-coding them or not using them at all. This in
particular fixes a build (and presumably also runtime) problem on old
enough libc due to the recent introduction of a use of O_CLOEXEC. The
other two changes are only of cleanup kind.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -977,7 +977,7 @@ void libxl__xc_domain_restore_done(libxl
     libxl_ctx *ctx = libxl__gc_owner(gc);
     char **vments = NULL, **localents = NULL;
     struct timeval start_time;
-    int i, esave, flags;
+    int i, esave;
 
     /* convenience aliases */
     const uint32_t domid = dcs->guest_domid;
@@ -1046,15 +1046,10 @@ out:
 
     esave = errno;
 
-    flags = fcntl(fd, F_GETFL);
-    if (flags == -1) {
-        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "unable to get flags on restore fd");
-    } else {
-        flags &= ~O_NONBLOCK;
-        if (fcntl(fd, F_SETFL, flags) == -1)
-            LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "unable to put restore fd"
+    ret = libxl_fd_set_nonblock(ctx, fd, 0);
+    if (ret)
+        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "unable to put restore fd"
                          " back to blocking mode");
-    }
 
     errno = esave;
     domcreate_rebuild_done(egc, dcs, ret);
--- a/tools/libxl/libxl_qmp.c
+++ b/tools/libxl/libxl_qmp.c
@@ -358,19 +358,14 @@ static int qmp_open(libxl__qmp_handler *
                     int timeout)
 {
     int ret;
-    int flags = 0;
     int i = 0;
 
     qmp->qmp_fd = socket(AF_UNIX, SOCK_STREAM, 0);
     if (qmp->qmp_fd < 0) {
         return -1;
     }
-    if ((flags = fcntl(qmp->qmp_fd, F_GETFL)) == -1) {
-        flags = 0;
-    }
-    if (fcntl(qmp->qmp_fd, F_SETFL, flags | O_NONBLOCK) == -1) {
-        return -1;
-    }
+    ret = libxl_fd_set_nonblock(qmp->ctx, qmp->qmp_fd, 1);
+    if (ret) return -1;
     ret = libxl_fd_set_cloexec(qmp->ctx, qmp->qmp_fd, 1);
     if (ret) return -1;
 
--- a/tools/libxl/libxl_utils.c
+++ b/tools/libxl/libxl_utils.c
@@ -1047,11 +1047,17 @@ int libxl__random_bytes(libxl__gc *gc, u
     int fd;
     int ret;
 
-    fd = open(dev, O_RDONLY | O_CLOEXEC);
+    fd = open(dev, O_RDONLY);
     if (fd < 0) {
         LOGE(ERROR, "failed to open \"%s\"", dev);
         return ERROR_FAIL;
     }
+    ret = libxl_fd_set_cloexec(CTX, fd, 1);
+    if (ret) {
+        close(fd);
+        LOGE(ERROR, "failed to set close-on-exec on handle to \"%s\"", dev);
+        return ERROR_FAIL;
+    }
 
     ret = libxl_read_exactly(CTX, fd, buf, len, dev, NULL);
 




[-- Attachment #2: libxl-use-cloexec-nonblock-helpers.patch --]
[-- Type: text/plain, Size: 2701 bytes --]

libxl: use libxl_fd_set_{cloexec,nonblock} helpers

... instead of open-coding them or not using them at all. This in
particular fixes a build (and presumably also runtime) problem on old
enough libc due to the recent introduction of a use of O_CLOEXEC. The
other two changes are only of cleanup kind.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -977,7 +977,7 @@ void libxl__xc_domain_restore_done(libxl
     libxl_ctx *ctx = libxl__gc_owner(gc);
     char **vments = NULL, **localents = NULL;
     struct timeval start_time;
-    int i, esave, flags;
+    int i, esave;
 
     /* convenience aliases */
     const uint32_t domid = dcs->guest_domid;
@@ -1046,15 +1046,10 @@ out:
 
     esave = errno;
 
-    flags = fcntl(fd, F_GETFL);
-    if (flags == -1) {
-        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "unable to get flags on restore fd");
-    } else {
-        flags &= ~O_NONBLOCK;
-        if (fcntl(fd, F_SETFL, flags) == -1)
-            LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "unable to put restore fd"
+    ret = libxl_fd_set_nonblock(ctx, fd, 0);
+    if (ret)
+        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "unable to put restore fd"
                          " back to blocking mode");
-    }
 
     errno = esave;
     domcreate_rebuild_done(egc, dcs, ret);
--- a/tools/libxl/libxl_qmp.c
+++ b/tools/libxl/libxl_qmp.c
@@ -358,19 +358,14 @@ static int qmp_open(libxl__qmp_handler *
                     int timeout)
 {
     int ret;
-    int flags = 0;
     int i = 0;
 
     qmp->qmp_fd = socket(AF_UNIX, SOCK_STREAM, 0);
     if (qmp->qmp_fd < 0) {
         return -1;
     }
-    if ((flags = fcntl(qmp->qmp_fd, F_GETFL)) == -1) {
-        flags = 0;
-    }
-    if (fcntl(qmp->qmp_fd, F_SETFL, flags | O_NONBLOCK) == -1) {
-        return -1;
-    }
+    ret = libxl_fd_set_nonblock(qmp->ctx, qmp->qmp_fd, 1);
+    if (ret) return -1;
     ret = libxl_fd_set_cloexec(qmp->ctx, qmp->qmp_fd, 1);
     if (ret) return -1;
 
--- a/tools/libxl/libxl_utils.c
+++ b/tools/libxl/libxl_utils.c
@@ -1047,11 +1047,17 @@ int libxl__random_bytes(libxl__gc *gc, u
     int fd;
     int ret;
 
-    fd = open(dev, O_RDONLY | O_CLOEXEC);
+    fd = open(dev, O_RDONLY);
     if (fd < 0) {
         LOGE(ERROR, "failed to open \"%s\"", dev);
         return ERROR_FAIL;
     }
+    ret = libxl_fd_set_cloexec(CTX, fd, 1);
+    if (ret) {
+        close(fd);
+        LOGE(ERROR, "failed to set close-on-exec on handle to \"%s\"", dev);
+        return ERROR_FAIL;
+    }
 
     ret = libxl_read_exactly(CTX, fd, buf, len, dev, NULL);
 

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH] libxl: use libxl_fd_set_{cloexec, nonblock} helpers

[Ian Jackson]

Jan Beulich writes ("[PATCH] libxl: use libxl_fd_set_{cloexec,nonblock} helpers"):
> ... instead of open-coding them or not using them at all. This in
> particular fixes a build (and presumably also runtime) problem on old
> enough libc due to the recent introduction of a use of O_CLOEXEC. The
> other two changes are only of cleanup kind.

I think most of libxl should probably be using libxl__carefd* not
setting cloexec.  This is necessary whenever we need cloexec for
correctness rather than just out of tidiness.  (Search for carefd in
libxl_internal.h.)

> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> --- a/tools/libxl/libxl_create.c
> +++ b/tools/libxl/libxl_create.c
> @@ -977,7 +977,7 @@ void libxl__xc_domain_restore_done(libxl
>      libxl_ctx *ctx = libxl__gc_owner(gc);
...
> +    ret = libxl_fd_set_nonblock(ctx, fd, 0);
> +    if (ret)
> +        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "unable to put restore fd"
>                           " back to blocking mode");

You do not need to log when libxl_fd_set_nonblock fails - it logs
already.

> --- a/tools/libxl/libxl_qmp.c
> +++ b/tools/libxl/libxl_qmp.c
> @@ -358,19 +358,14 @@ static int qmp_open(libxl__qmp_handler *
>                      int timeout)
...
>      qmp->qmp_fd = socket(AF_UNIX, SOCK_STREAM, 0);
...
> +    ret = libxl_fd_set_nonblock(qmp->ctx, qmp->qmp_fd, 1);
> +    if (ret) return -1;

This change is correct.

>      ret = libxl_fd_set_cloexec(qmp->ctx, qmp->qmp_fd, 1);
>      if (ret) return -1;

But this (which you haven't touched) needs to be done with
libxl__carefd* instead.  (Your patch is not unacceptable due to you
not making this change, on the basis that we should accept
improvements even if they don't leave the code perfect.)

> --- a/tools/libxl/libxl_utils.c
> +++ b/tools/libxl/libxl_utils.c
> @@ -1047,11 +1047,17 @@ int libxl__random_bytes(libxl__gc *gc, u
...
> -    fd = open(dev, O_RDONLY | O_CLOEXEC);
> +    fd = open(dev, O_RDONLY);

Firstly, I don't understand why we care about cloexec on this fd, but
it's harmless to do so.

> +    ret = libxl_fd_set_cloexec(CTX, fd, 1);
> +    if (ret) {
> +        close(fd);
> +        LOGE(ERROR, "failed to set close-on-exec on handle to \"%s\"", dev);
> +        return ERROR_FAIL;
> +    }

Again, no need to log.


Thanks,
Ian.

Re: [PATCH] libxl: use libxl_fd_set_{cloexec, nonblock} helpers

[Jan Beulich]

>>> On 25.07.14 at 19:07, <Ian.Jackson@eu.citrix.com> wrote:
> Jan Beulich writes ("[PATCH] libxl: use libxl_fd_set_{cloexec,nonblock} 
>> --- a/tools/libxl/libxl_qmp.c
>> +++ b/tools/libxl/libxl_qmp.c
>> @@ -358,19 +358,14 @@ static int qmp_open(libxl__qmp_handler *
>>                      int timeout)
> ...
>>      qmp->qmp_fd = socket(AF_UNIX, SOCK_STREAM, 0);
> ...
>> +    ret = libxl_fd_set_nonblock(qmp->ctx, qmp->qmp_fd, 1);
>> +    if (ret) return -1;
> 
> This change is correct.
> 
>>      ret = libxl_fd_set_cloexec(qmp->ctx, qmp->qmp_fd, 1);
>>      if (ret) return -1;
> 
> But this (which you haven't touched) needs to be done with
> libxl__carefd* instead.  (Your patch is not unacceptable due to you
> not making this change, on the basis that we should accept
> improvements even if they don't leave the code perfect.)

And indeed I'd like to leave that conversion to you or someone
else more familiar with the libxl concepts.

>> --- a/tools/libxl/libxl_utils.c
>> +++ b/tools/libxl/libxl_utils.c
>> @@ -1047,11 +1047,17 @@ int libxl__random_bytes(libxl__gc *gc, u
> ...
>> -    fd = open(dev, O_RDONLY | O_CLOEXEC);
>> +    fd = open(dev, O_RDONLY);
> 
> Firstly, I don't understand why we care about cloexec on this fd, but
> it's harmless to do so.

And of course I'd be perfectly fine with a one liner just dropping it
(all I really care about is that the file builds correctly in all cases).
The question of why cloexec is needed/wanted here I'll pass on to
author and committer, but I'd guess this is just to avoid
proliferation of file handles into clones.

Jan

[PATCH v2] libxl: use libxl_fd_set_{cloexec, nonblock} helpers

[Jan Beulich]

[-- Attachment #1: Type: text/plain, Size: 2887 bytes --]

... instead of open-coding them or not using them at all. This in
particular fixes a build (and presumably also runtime) problem on old
enough libc due to the recent introduction of a use of O_CLOEXEC. The
other two changes are only of cleanup kind.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
v2: Drop unnecessary logging (and result value corruption in the
    libxl__xc_domain_restore_done() case).

--- 2014-07-25.orig/tools/libxl/libxl_create.c	2014-07-25 10:41:10.000000000 +0200
+++ 2014-07-25/tools/libxl/libxl_create.c	2014-07-28 09:23:18.000000000 +0200
@@ -977,7 +977,7 @@ void libxl__xc_domain_restore_done(libxl
     libxl_ctx *ctx = libxl__gc_owner(gc);
     char **vments = NULL, **localents = NULL;
     struct timeval start_time;
-    int i, esave, flags;
+    int i, esave;
 
     /* convenience aliases */
     const uint32_t domid = dcs->guest_domid;
@@ -1045,17 +1045,7 @@ out:
     }
 
     esave = errno;
-
-    flags = fcntl(fd, F_GETFL);
-    if (flags == -1) {
-        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "unable to get flags on restore fd");
-    } else {
-        flags &= ~O_NONBLOCK;
-        if (fcntl(fd, F_SETFL, flags) == -1)
-            LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "unable to put restore fd"
-                         " back to blocking mode");
-    }
-
+    libxl_fd_set_nonblock(ctx, fd, 0);
     errno = esave;
     domcreate_rebuild_done(egc, dcs, ret);
 }
--- 2014-07-25.orig/tools/libxl/libxl_qmp.c	2014-07-25 10:41:10.000000000 +0200
+++ 2014-07-25/tools/libxl/libxl_qmp.c	2014-07-25 16:17:40.000000000 +0200
@@ -358,19 +358,14 @@ static int qmp_open(libxl__qmp_handler *
                     int timeout)
 {
     int ret;
-    int flags = 0;
     int i = 0;
 
     qmp->qmp_fd = socket(AF_UNIX, SOCK_STREAM, 0);
     if (qmp->qmp_fd < 0) {
         return -1;
     }
-    if ((flags = fcntl(qmp->qmp_fd, F_GETFL)) == -1) {
-        flags = 0;
-    }
-    if (fcntl(qmp->qmp_fd, F_SETFL, flags | O_NONBLOCK) == -1) {
-        return -1;
-    }
+    ret = libxl_fd_set_nonblock(qmp->ctx, qmp->qmp_fd, 1);
+    if (ret) return -1;
     ret = libxl_fd_set_cloexec(qmp->ctx, qmp->qmp_fd, 1);
     if (ret) return -1;
 
--- 2014-07-25.orig/tools/libxl/libxl_utils.c	2014-07-25 10:41:10.000000000 +0200
+++ 2014-07-25/tools/libxl/libxl_utils.c	2014-07-28 09:24:38.000000000 +0200
@@ -1047,11 +1047,16 @@ int libxl__random_bytes(libxl__gc *gc, u
     int fd;
     int ret;
 
-    fd = open(dev, O_RDONLY | O_CLOEXEC);
+    fd = open(dev, O_RDONLY);
     if (fd < 0) {
         LOGE(ERROR, "failed to open \"%s\"", dev);
         return ERROR_FAIL;
     }
+    ret = libxl_fd_set_cloexec(CTX, fd, 1);
+    if (ret) {
+        close(fd);
+        return ERROR_FAIL;
+    }
 
     ret = libxl_read_exactly(CTX, fd, buf, len, dev, NULL);
 




[-- Attachment #2: libxl-use-cloexec-nonblock-helpers.patch --]
[-- Type: text/plain, Size: 2935 bytes --]

libxl: use libxl_fd_set_{cloexec,nonblock} helpers

... instead of open-coding them or not using them at all. This in
particular fixes a build (and presumably also runtime) problem on old
enough libc due to the recent introduction of a use of O_CLOEXEC. The
other two changes are only of cleanup kind.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
v2: Drop unnecessary logging (and result value corruption in the
    libxl__xc_domain_restore_done() case).

--- 2014-07-25.orig/tools/libxl/libxl_create.c	2014-07-25 10:41:10.000000000 +0200
+++ 2014-07-25/tools/libxl/libxl_create.c	2014-07-28 09:23:18.000000000 +0200
@@ -977,7 +977,7 @@ void libxl__xc_domain_restore_done(libxl
     libxl_ctx *ctx = libxl__gc_owner(gc);
     char **vments = NULL, **localents = NULL;
     struct timeval start_time;
-    int i, esave, flags;
+    int i, esave;
 
     /* convenience aliases */
     const uint32_t domid = dcs->guest_domid;
@@ -1045,17 +1045,7 @@ out:
     }
 
     esave = errno;
-
-    flags = fcntl(fd, F_GETFL);
-    if (flags == -1) {
-        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "unable to get flags on restore fd");
-    } else {
-        flags &= ~O_NONBLOCK;
-        if (fcntl(fd, F_SETFL, flags) == -1)
-            LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "unable to put restore fd"
-                         " back to blocking mode");
-    }
-
+    libxl_fd_set_nonblock(ctx, fd, 0);
     errno = esave;
     domcreate_rebuild_done(egc, dcs, ret);
 }
--- 2014-07-25.orig/tools/libxl/libxl_qmp.c	2014-07-25 10:41:10.000000000 +0200
+++ 2014-07-25/tools/libxl/libxl_qmp.c	2014-07-25 16:17:40.000000000 +0200
@@ -358,19 +358,14 @@ static int qmp_open(libxl__qmp_handler *
                     int timeout)
 {
     int ret;
-    int flags = 0;
     int i = 0;
 
     qmp->qmp_fd = socket(AF_UNIX, SOCK_STREAM, 0);
     if (qmp->qmp_fd < 0) {
         return -1;
     }
-    if ((flags = fcntl(qmp->qmp_fd, F_GETFL)) == -1) {
-        flags = 0;
-    }
-    if (fcntl(qmp->qmp_fd, F_SETFL, flags | O_NONBLOCK) == -1) {
-        return -1;
-    }
+    ret = libxl_fd_set_nonblock(qmp->ctx, qmp->qmp_fd, 1);
+    if (ret) return -1;
     ret = libxl_fd_set_cloexec(qmp->ctx, qmp->qmp_fd, 1);
     if (ret) return -1;
 
--- 2014-07-25.orig/tools/libxl/libxl_utils.c	2014-07-25 10:41:10.000000000 +0200
+++ 2014-07-25/tools/libxl/libxl_utils.c	2014-07-28 09:24:38.000000000 +0200
@@ -1047,11 +1047,16 @@ int libxl__random_bytes(libxl__gc *gc, u
     int fd;
     int ret;
 
-    fd = open(dev, O_RDONLY | O_CLOEXEC);
+    fd = open(dev, O_RDONLY);
     if (fd < 0) {
         LOGE(ERROR, "failed to open \"%s\"", dev);
         return ERROR_FAIL;
     }
+    ret = libxl_fd_set_cloexec(CTX, fd, 1);
+    if (ret) {
+        close(fd);
+        return ERROR_FAIL;
+    }
 
     ret = libxl_read_exactly(CTX, fd, buf, len, dev, NULL);
 

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH] libxl: use libxl_fd_set_{cloexec, nonblock} helpers

[David Vrabel]

On 28/07/14 07:32, Jan Beulich wrote:
>>>> On 25.07.14 at 19:07, <Ian.Jackson@eu.citrix.com> wrote:
>> Jan Beulich writes ("[PATCH] libxl: use libxl_fd_set_{cloexec,nonblock} 
>>> --- a/tools/libxl/libxl_qmp.c
>>> +++ b/tools/libxl/libxl_qmp.c
>>> @@ -358,19 +358,14 @@ static int qmp_open(libxl__qmp_handler *
>>>                      int timeout)
>> ...
>>>      qmp->qmp_fd = socket(AF_UNIX, SOCK_STREAM, 0);
>> ...
>>> +    ret = libxl_fd_set_nonblock(qmp->ctx, qmp->qmp_fd, 1);
>>> +    if (ret) return -1;
>>
>> This change is correct.
>>
>>>      ret = libxl_fd_set_cloexec(qmp->ctx, qmp->qmp_fd, 1);
>>>      if (ret) return -1;
>>
>> But this (which you haven't touched) needs to be done with
>> libxl__carefd* instead.  (Your patch is not unacceptable due to you
>> not making this change, on the basis that we should accept
>> improvements even if they don't leave the code perfect.)
> 
> And indeed I'd like to leave that conversion to you or someone
> else more familiar with the libxl concepts.
> 
>>> --- a/tools/libxl/libxl_utils.c
>>> +++ b/tools/libxl/libxl_utils.c
>>> @@ -1047,11 +1047,17 @@ int libxl__random_bytes(libxl__gc *gc, u
>> ...
>>> -    fd = open(dev, O_RDONLY | O_CLOEXEC);
>>> +    fd = open(dev, O_RDONLY);
>>
>> Firstly, I don't understand why we care about cloexec on this fd, but
>> it's harmless to do so.
> 
> And of course I'd be perfectly fine with a one liner just dropping it
> (all I really care about is that the file builds correctly in all cases).
> The question of why cloexec is needed/wanted here I'll pass on to
> author and committer, but I'd guess this is just to avoid
> proliferation of file handles into clones.

Surely it should be standard practice for a library to always set
CLOEXEC on any files it opens, to minimize the side effects the library
call has?

And if CLOEXEC is required then setting it at open time is the only safe
way.

David

Re: [PATCH] libxl: use libxl_fd_set_{cloexec, nonblock} helpers

[Ian Campbell]

On Mon, 2014-07-28 at 15:51 +0100, David Vrabel wrote:
>  
> > And of course I'd be perfectly fine with a one liner just dropping it
> > (all I really care about is that the file builds correctly in all cases).
> > The question of why cloexec is needed/wanted here I'll pass on to
> > author and committer, but I'd guess this is just to avoid
> > proliferation of file handles into clones.
> 
> Surely it should be standard practice for a library to always set
> CLOEXEC on any files it opens, to minimize the side effects the library
> call has?
> 
> And if CLOEXEC is required then setting it at open time is the only safe
> way.

Jan's problem (according to the commit log) is that O_CLOEXEC isn't
universally available yet.

Fortunately libxl has the carefd infrastructure which offers an
alternative.

Ian.

Re: [PATCH v2] libxl: use libxl_fd_set_{cloexec, nonblock} helpers

[Ian Jackson]

Jan Beulich writes ("[PATCH v2] libxl: use libxl_fd_set_{cloexec,nonblock} helpers"):
> ... instead of open-coding them or not using them at all. This in
> particular fixes a build (and presumably also runtime) problem on old
> enough libc due to the recent introduction of a use of O_CLOEXEC. The
> other two changes are only of cleanup kind.

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

Re: [PATCH] libxl: use libxl_fd_set_{cloexec, nonblock} helpers

[Don Slutz]

[-- Attachment #1: Type: text/plain, Size: 2391 bytes --]

On 07/28/14 10:51, David Vrabel wrote:
> On 28/07/14 07:32, Jan Beulich wrote:
>>>>> On 25.07.14 at 19:07, <Ian.Jackson@eu.citrix.com> wrote:
>>> Jan Beulich writes ("[PATCH] libxl: use libxl_fd_set_{cloexec,nonblock}
>>>> --- a/tools/libxl/libxl_qmp.c
>>>> +++ b/tools/libxl/libxl_qmp.c
>>>> @@ -358,19 +358,14 @@ static int qmp_open(libxl__qmp_handler *
>>>>                       int timeout)
>>> ...
>>>> --- a/tools/libxl/libxl_utils.c
>>>> +++ b/tools/libxl/libxl_utils.c
>>>> @@ -1047,11 +1047,17 @@ int libxl__random_bytes(libxl__gc *gc, u
>>> ...
>>>> -    fd = open(dev, O_RDONLY | O_CLOEXEC);
>>>> +    fd = open(dev, O_RDONLY);
>>> Firstly, I don't understand why we care about cloexec on this fd, but
>>> it's harmless to do so.
>> And of course I'd be perfectly fine with a one liner just dropping it
>> (all I really care about is that the file builds correctly in all cases).
>> The question of why cloexec is needed/wanted here I'll pass on to
>> author and committer, but I'd guess this is just to avoid
>> proliferation of file handles into clones.
> Surely it should be standard practice for a library to always set
> CLOEXEC on any files it opens, to minimize the side effects the library
> call has?
>
> And if CLOEXEC is required then setting it at open time is the only safe
> way.
>
> David

Since this is an open, read, close; CLOEXEC is not needed at all. And it 
breaks
the build on RHEL5.  So I did a quick one liner.

 From 46e776a2b770857d869ef54c418924af043274d0 Mon Sep 17 00:00:00 2001
From: Don Slutz <dslutz@verizon.com>
Date: Thu, 31 Jul 2014 01:26:58 +0000
Subject: [PATCH] Make RHEL5 build again

Signed-off-by: Don Slutz <dslutz@verizon.com>
---
  tools/libxl/libxl_utils.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
index 1fdf5ea..2649778 100644
--- a/tools/libxl/libxl_utils.c
+++ b/tools/libxl/libxl_utils.c
@@ -1047,7 +1047,7 @@ int libxl__random_bytes(libxl__gc *gc, uint8_t 
*buf, size_t len)
      int fd;
      int ret;

-    fd = open(dev, O_RDONLY | O_CLOEXEC);
+    fd = open(dev, O_RDONLY);
      if (fd < 0) {
          LOGE(ERROR, "failed to open \"%s\"", dev);
          return ERROR_FAIL;
-- 
1.8.2.1

    -Don Slutz



> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel


[-- Attachment #2: 0001-Make-RHEL5-build-again.patch --]
[-- Type: text/x-patch, Size: 783 bytes --]

>From 46e776a2b770857d869ef54c418924af043274d0 Mon Sep 17 00:00:00 2001
From: Don Slutz <dslutz@verizon.com>
Date: Thu, 31 Jul 2014 01:26:58 +0000
Subject: [PATCH] Make RHEL5 build again

Signed-off-by: Don Slutz <dslutz@verizon.com>
---
 tools/libxl/libxl_utils.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
index 1fdf5ea..2649778 100644
--- a/tools/libxl/libxl_utils.c
+++ b/tools/libxl/libxl_utils.c
@@ -1047,7 +1047,7 @@ int libxl__random_bytes(libxl__gc *gc, uint8_t *buf, size_t len)
     int fd;
     int ret;
 
-    fd = open(dev, O_RDONLY | O_CLOEXEC);
+    fd = open(dev, O_RDONLY);
     if (fd < 0) {
         LOGE(ERROR, "failed to open \"%s\"", dev);
         return ERROR_FAIL;
-- 
1.8.2.1


[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel