Re: [libvirt] [PATCH v5 4/6] block: Convert open calls to qemu_open

2012-07-25 Thread Eric Blake
On 07/23/2012 07:08 AM, Corey Bryant wrote:
 This patch converts all block layer open calls to qemu_open.
 
 Note that this adds the O_CLOEXEC flag to the changed open paths
 when the O_CLOEXEC macro is defined.

Is it actually adding O_CLOEXEC, or just the ability to use O_CLOEXEC?

Or is the actual change that the end result is that the fd now has
FD_CLOEXEC set unconditionally, whether by O_CLOEXEC (which the caller
need not pass) or by fcntl()?

 +++ b/block/raw-posix.c
 @@ -572,8 +572,8 @@ static int raw_create(const char *filename, 
 QEMUOptionParameter *options)
  options++;
  }
  
 -fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY,
 -  0644);
 +fd = qemu_open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY,
 +   0644);

After all, I don't see O_CLOEXEC used here.

  if (fd  0) {
  result = -errno;
  } else {
 @@ -846,7 +846,7 @@ static int hdev_open(BlockDriverState *bs, const char 
 *filename, int flags)
  if ( bsdPath[ 0 ] != '\0' ) {
  strcat(bsdPath,s0);
  /* some CDs don't have a partition 0 */
 -fd = open(bsdPath, O_RDONLY | O_BINARY | O_LARGEFILE);
 +fd = qemu_open(bsdPath, O_RDONLY | O_BINARY | O_LARGEFILE);

Also, I still stand by my earlier claim that we don't need O_LARGEFILE
here (we should already be configuring for 64-bit off_t by default),
although cleaning that up is probably worth an independent commit.


 +++ b/block/vdi.c
 @@ -653,8 +653,9 @@ static int vdi_create(const char *filename, 
 QEMUOptionParameter *options)
  options++;
  }
  
 -fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY | 
 O_LARGEFILE,
 -  0644);
 +fd = qemu_open(filename,
 +   O_WRONLY | O_CREAT | O_TRUNC | O_BINARY | O_LARGEFILE,
 +   0644);

Another pointless O_LARGEFILE, and so forth.

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



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v5 4/6] block: Convert open calls to qemu_open

2012-07-25 Thread Corey Bryant



On 07/25/2012 03:22 PM, Eric Blake wrote:

On 07/23/2012 07:08 AM, Corey Bryant wrote:

This patch converts all block layer open calls to qemu_open.

Note that this adds the O_CLOEXEC flag to the changed open paths
when the O_CLOEXEC macro is defined.


Is it actually adding O_CLOEXEC, or just the ability to use O_CLOEXEC?


It is adding O_CLOEXEC in qemu_open() on the open() call (as long as it 
is defined).




Or is the actual change that the end result is that the fd now has
FD_CLOEXEC set unconditionally, whether by O_CLOEXEC (which the caller
need not pass) or by fcntl()?



The statement in the commit message isn't referring to anything dealing 
with qemu_dup(). The statement is specifically talking about the open() 
call in qemu_open(), and the point is to alert folks that old open() 
calls are now being routed through qemu_open() and likely are using the 
O_CLOEXEC flag, which is new behavior.



+++ b/block/raw-posix.c
@@ -572,8 +572,8 @@ static int raw_create(const char *filename, 
QEMUOptionParameter *options)
  options++;
  }

-fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY,
-  0644);
+fd = qemu_open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY,
+   0644);


After all, I don't see O_CLOEXEC used here.



That's right.  qemu_open() adds O_CLOEXEC on the open() call.


  if (fd  0) {
  result = -errno;
  } else {
@@ -846,7 +846,7 @@ static int hdev_open(BlockDriverState *bs, const char 
*filename, int flags)
  if ( bsdPath[ 0 ] != '\0' ) {
  strcat(bsdPath,s0);
  /* some CDs don't have a partition 0 */
-fd = open(bsdPath, O_RDONLY | O_BINARY | O_LARGEFILE);
+fd = qemu_open(bsdPath, O_RDONLY | O_BINARY | O_LARGEFILE);


Also, I still stand by my earlier claim that we don't need O_LARGEFILE
here (we should already be configuring for 64-bit off_t by default),
although cleaning that up is probably worth an independent commit.



I have a note to get rid of O_LARGEFILE as a separate follow-on patch.




+++ b/block/vdi.c
@@ -653,8 +653,9 @@ static int vdi_create(const char *filename, 
QEMUOptionParameter *options)
  options++;
  }

-fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY | O_LARGEFILE,
-  0644);
+fd = qemu_open(filename,
+   O_WRONLY | O_CREAT | O_TRUNC | O_BINARY | O_LARGEFILE,
+   0644);


Another pointless O_LARGEFILE, and so forth.



Yep.

--
Regards,
Corey


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


[libvirt] [PATCH v5 4/6] block: Convert open calls to qemu_open

2012-07-23 Thread Corey Bryant
This patch converts all block layer open calls to qemu_open.

Note that this adds the O_CLOEXEC flag to the changed open paths
when the O_CLOEXEC macro is defined.

Signed-off-by: Corey Bryant cor...@linux.vnet.ibm.com
---
v2:
 -Convert calls to qemu_open instead of file_open (kw...@redhat.com)
 -Mention introduction of O_CLOEXEC (kw...@redhat.com)

v3-v5:
 -No changes
---
 block/raw-posix.c |   18 +-
 block/raw-win32.c |4 ++--
 block/vdi.c   |5 +++--
 block/vmdk.c  |   21 +
 block/vpc.c   |2 +-
 block/vvfat.c |4 ++--
 6 files changed, 26 insertions(+), 28 deletions(-)

diff --git a/block/raw-posix.c b/block/raw-posix.c
index 0dce089..7408a42 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -572,8 +572,8 @@ static int raw_create(const char *filename, 
QEMUOptionParameter *options)
 options++;
 }
 
-fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY,
-  0644);
+fd = qemu_open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY,
+   0644);
 if (fd  0) {
 result = -errno;
 } else {
@@ -846,7 +846,7 @@ static int hdev_open(BlockDriverState *bs, const char 
*filename, int flags)
 if ( bsdPath[ 0 ] != '\0' ) {
 strcat(bsdPath,s0);
 /* some CDs don't have a partition 0 */
-fd = open(bsdPath, O_RDONLY | O_BINARY | O_LARGEFILE);
+fd = qemu_open(bsdPath, O_RDONLY | O_BINARY | O_LARGEFILE);
 if (fd  0) {
 bsdPath[strlen(bsdPath)-1] = '1';
 } else {
@@ -903,7 +903,7 @@ static int fd_open(BlockDriverState *bs)
 #endif
 return -EIO;
 }
-s-fd = open(bs-filename, s-open_flags  ~O_NONBLOCK);
+s-fd = qemu_open(bs-filename, s-open_flags  ~O_NONBLOCK);
 if (s-fd  0) {
 s-fd_error_time = get_clock();
 s-fd_got_error = 1;
@@ -977,7 +977,7 @@ static int hdev_create(const char *filename, 
QEMUOptionParameter *options)
 options++;
 }
 
-fd = open(filename, O_WRONLY | O_BINARY);
+fd = qemu_open(filename, O_WRONLY | O_BINARY);
 if (fd  0)
 return -errno;
 
@@ -1055,7 +1055,7 @@ static int floppy_probe_device(const char *filename)
 if (strstart(filename, /dev/fd, NULL))
 prio = 50;
 
-fd = open(filename, O_RDONLY | O_NONBLOCK);
+fd = qemu_open(filename, O_RDONLY | O_NONBLOCK);
 if (fd  0) {
 goto out;
 }
@@ -1108,7 +1108,7 @@ static void floppy_eject(BlockDriverState *bs, bool 
eject_flag)
 close(s-fd);
 s-fd = -1;
 }
-fd = open(bs-filename, s-open_flags | O_NONBLOCK);
+fd = qemu_open(bs-filename, s-open_flags | O_NONBLOCK);
 if (fd = 0) {
 if (ioctl(fd, FDEJECT, 0)  0)
 perror(FDEJECT);
@@ -1158,7 +1158,7 @@ static int cdrom_probe_device(const char *filename)
 int prio = 0;
 struct stat st;
 
-fd = open(filename, O_RDONLY | O_NONBLOCK);
+fd = qemu_open(filename, O_RDONLY | O_NONBLOCK);
 if (fd  0) {
 goto out;
 }
@@ -1282,7 +1282,7 @@ static int cdrom_reopen(BlockDriverState *bs)
  */
 if (s-fd = 0)
 close(s-fd);
-fd = open(bs-filename, s-open_flags, 0644);
+fd = qemu_open(bs-filename, s-open_flags, 0644);
 if (fd  0) {
 s-fd = -1;
 return -EIO;
diff --git a/block/raw-win32.c b/block/raw-win32.c
index e4b0b75..8d7838d 100644
--- a/block/raw-win32.c
+++ b/block/raw-win32.c
@@ -255,8 +255,8 @@ static int raw_create(const char *filename, 
QEMUOptionParameter *options)
 options++;
 }
 
-fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY,
-  0644);
+fd = qemu_open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY,
+   0644);
 if (fd  0)
 return -EIO;
 set_sparse(fd);
diff --git a/block/vdi.c b/block/vdi.c
index 57325d6..c4f1529 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -653,8 +653,9 @@ static int vdi_create(const char *filename, 
QEMUOptionParameter *options)
 options++;
 }
 
-fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY | O_LARGEFILE,
-  0644);
+fd = qemu_open(filename,
+   O_WRONLY | O_CREAT | O_TRUNC | O_BINARY | O_LARGEFILE,
+   0644);
 if (fd  0) {
 return -errno;
 }
diff --git a/block/vmdk.c b/block/vmdk.c
index 18e9b4c..557dc1b 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1161,10 +1161,9 @@ static int vmdk_create_extent(const char *filename, 
int64_t filesize,
 VMDK4Header header;
 uint32_t tmp, magic, grains, gd_size, gt_size, gt_count;
 
-fd = open(
-filename,
-O_WRONLY | O_CREAT | O_TRUNC | O_BINARY | O_LARGEFILE,
-0644);
+fd = qemu_open(filename,
+   O_WRONLY | O_CREAT | O_TRUNC | O_BINARY | O_LARGEFILE,
+   0644);
 if (fd  0) {
 return -errno;
 }
@@ -1484,15 +1483,13