[Qemu-devel] [PATCH v2 0/2] block: Add qemu_dup in osdep.c and use it

[Qemu-devel] [PATCH v2 0/2] block: Add qemu_dup in osdep.c and use it

[Fam Zheng]

v2: Fix patch 1 #else branch, and "r" => "ret". [Kevin]
    Add Kevin's r-b line in patch 2.

This is an independent tiny change extracted from the image locking series,
which can be processed separately.

Improved according to Kevin's suggestion in v6 image locking series.

Fam


Fam Zheng (2):
  osdep: Introduce qemu_dup
  raw-posix: Use qemu_dup

 block/raw-posix.c    | 10 +---------
 include/qemu/osdep.h |  3 +++
 util/osdep.c         | 23 +++++++++++++++--------
 3 files changed, 19 insertions(+), 17 deletions(-)

-- 
2.8.3

[Qemu-devel] [PATCH v2 1/2] osdep: Introduce qemu_dup

[Fam Zheng]

And use it in qemu_dup_flags.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 include/qemu/osdep.h |  3 +++
 util/osdep.c         | 23 +++++++++++++++--------
 2 files changed, 18 insertions(+), 8 deletions(-)

diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index e63da28..7361006 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -278,6 +278,9 @@ int qemu_madvise(void *addr, size_t len, int advice);
 
 int qemu_open(const char *name, int flags, ...);
 int qemu_close(int fd);
+#ifndef _WIN32
+int qemu_dup(int fd);
+#endif
 
 #if defined(__HAIKU__) && defined(__i386__)
 #define FMT_pid "%ld"
diff --git a/util/osdep.c b/util/osdep.c
index ff004e8..06fb1cf 100644
--- a/util/osdep.c
+++ b/util/osdep.c
@@ -83,14 +83,7 @@ static int qemu_dup_flags(int fd, int flags)
     int serrno;
     int dup_flags;
 
-#ifdef F_DUPFD_CLOEXEC
-    ret = fcntl(fd, F_DUPFD_CLOEXEC, 0);
-#else
-    ret = dup(fd);
-    if (ret != -1) {
-        qemu_set_cloexec(ret);
-    }
-#endif
+    ret = qemu_dup(fd);
     if (ret == -1) {
         goto fail;
     }
@@ -129,6 +122,20 @@ fail:
     return -1;
 }
 
+int qemu_dup(int fd)
+{
+    int ret;
+#ifdef F_DUPFD_CLOEXEC
+    ret = fcntl(fd, F_DUPFD_CLOEXEC, 0);
+#else
+    ret = dup(fd);
+    if (ret != -1) {
+        qemu_set_cloexec(ret);
+    }
+#endif
+    return ret;
+}
+
 static int qemu_parse_fdset(const char *param)
 {
     return qemu_parse_fd(param);
-- 
2.8.3

Re: [Qemu-devel] [PATCH v2 1/2] osdep: Introduce qemu_dup

[Eric Blake]

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

On 06/22/2016 06:53 AM, Fam Zheng wrote:
> And use it in qemu_dup_flags.
> 
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  include/qemu/osdep.h |  3 +++
>  util/osdep.c         | 23 +++++++++++++++--------
>  2 files changed, 18 insertions(+), 8 deletions(-)
> 

> +int qemu_dup(int fd)
> +{
> +    int ret;
> +#ifdef F_DUPFD_CLOEXEC
> +    ret = fcntl(fd, F_DUPFD_CLOEXEC, 0);
> +#else
> +    ret = dup(fd);
> +    if (ret != -1) {
> +        qemu_set_cloexec(ret);
> +    }

Is it any more efficient to try and use dup3(fd, 0, O_CLOEXEC), or are
we assuming that F_DUPFD_CLOEXEC and dup3() are only likely to both be
present or absent?

Otherwise,
Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH v2 1/2] osdep: Introduce qemu_dup

[Eric Blake]

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

On 06/22/2016 09:21 AM, Eric Blake wrote:
> On 06/22/2016 06:53 AM, Fam Zheng wrote:
>> And use it in qemu_dup_flags.
>>
>> Signed-off-by: Fam Zheng <famz@redhat.com>
>> ---
>>  include/qemu/osdep.h |  3 +++
>>  util/osdep.c         | 23 +++++++++++++++--------
>>  2 files changed, 18 insertions(+), 8 deletions(-)
>>
> 
>> +int qemu_dup(int fd)
>> +{
>> +    int ret;
>> +#ifdef F_DUPFD_CLOEXEC
>> +    ret = fcntl(fd, F_DUPFD_CLOEXEC, 0);
>> +#else
>> +    ret = dup(fd);
>> +    if (ret != -1) {
>> +        qemu_set_cloexec(ret);
>> +    }
> 
> Is it any more efficient to try and use dup3(fd, 0, O_CLOEXEC), or are
> we assuming that F_DUPFD_CLOEXEC and dup3() are only likely to both be
> present or absent?

Scratch that. dup3() has different semantics than dup()/fcntl(F_DUPFD)
(overwrite destination fd vs. next available fd).  So no change to your
code after all.

> 
> Otherwise,
> Reviewed-by: Eric Blake <eblake@redhat.com>
> 

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

[Qemu-devel] [PATCH v2 2/2] raw-posix: Use qemu_dup

[Fam Zheng]

Signed-off-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
---
 block/raw-posix.c | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/block/raw-posix.c b/block/raw-posix.c
index bef7a67..c8ef64b 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -639,15 +639,7 @@ static int raw_reopen_prepare(BDRVReopenState *state,
 
     if ((raw_s->open_flags & ~fcntl_flags) == (s->open_flags & ~fcntl_flags)) {
         /* dup the original fd */
-        /* TODO: use qemu fcntl wrapper */
-#ifdef F_DUPFD_CLOEXEC
-        raw_s->fd = fcntl(s->fd, F_DUPFD_CLOEXEC, 0);
-#else
-        raw_s->fd = dup(s->fd);
-        if (raw_s->fd != -1) {
-            qemu_set_cloexec(raw_s->fd);
-        }
-#endif
+        raw_s->fd = qemu_dup(s->fd);
         if (raw_s->fd >= 0) {
             ret = fcntl_setfl(raw_s->fd, raw_s->open_flags);
             if (ret) {
-- 
2.8.3

Re: [Qemu-devel] [Qemu-block] [PATCH v2 0/2] block: Add qemu_dup in osdep.c and use it

[John Snow]

On 06/22/2016 08:53 AM, Fam Zheng wrote:
> v2: Fix patch 1 #else branch, and "r" => "ret". [Kevin]
>     Add Kevin's r-b line in patch 2.
> 
> This is an independent tiny change extracted from the image locking series,
> which can be processed separately.
> 
> Improved according to Kevin's suggestion in v6 image locking series.
> 
> Fam
> 
> 
> Fam Zheng (2):
>   osdep: Introduce qemu_dup
>   raw-posix: Use qemu_dup
> 
>  block/raw-posix.c    | 10 +---------
>  include/qemu/osdep.h |  3 +++
>  util/osdep.c         | 23 +++++++++++++++--------
>  3 files changed, 19 insertions(+), 17 deletions(-)
> 

You submitted two versions of this, I think this is the right one.

Reviewed-by: John Snow <jsnow@redhat.com>

Re: [Qemu-devel] [Qemu-block] [PATCH v2 0/2] block: Add qemu_dup in osdep.c and use it

[Kevin Wolf]

Am 07.07.2016 um 23:14 hat John Snow geschrieben:
> On 06/22/2016 08:53 AM, Fam Zheng wrote:
> > v2: Fix patch 1 #else branch, and "r" => "ret". [Kevin]
> >     Add Kevin's r-b line in patch 2.
> > 
> > This is an independent tiny change extracted from the image locking series,
> > which can be processed separately.
> > 
> > Improved according to Kevin's suggestion in v6 image locking series.
> > 
> > Fam
> > 
> > 
> > Fam Zheng (2):
> >   osdep: Introduce qemu_dup
> >   raw-posix: Use qemu_dup
> > 
> >  block/raw-posix.c    | 10 +---------
> >  include/qemu/osdep.h |  3 +++
> >  util/osdep.c         | 23 +++++++++++++++--------
> >  3 files changed, 19 insertions(+), 17 deletions(-)
> > 
> 
> You submitted two versions of this, I think this is the right one.
> 
> Reviewed-by: John Snow <jsnow@redhat.com>

Oops, completely forgot about this one. Thanks for reminding, I've
applied it now.

Kevin