Re: [libvirt] [PATCH v5 4/6] block: Convert open calls to qemu_open
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
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
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