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

[Don Slutz <dslutz@verizon.com>]

[-- 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