All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: Corey Bryant <coreyb@linux.vnet.ibm.com>
Cc: kwolf@redhat.com, aliguori@us.ibm.com,
	stefanha@linux.vnet.ibm.com, libvir-list@redhat.com,
	qemu-devel@nongnu.org, lcapitulino@redhat.com
Subject: Re: [Qemu-devel] [PATCH v5 4/6] block: Convert open calls to qemu_open
Date: Wed, 25 Jul 2012 13:22:20 -0600	[thread overview]
Message-ID: <5010476C.9050405@redhat.com> (raw)
In-Reply-To: <1343048885-1701-5-git-send-email-coreyb@linux.vnet.ibm.com>

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

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   eblake@redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


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

  reply	other threads:[~2012-07-25 19:22 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-07-23 13:07 [Qemu-devel] [PATCH v5 0/6] file descriptor passing using fd sets Corey Bryant
2012-07-23 13:08 ` [Qemu-devel] [PATCH v5 1/6] qemu-char: Add MSG_CMSG_CLOEXEC flag to recvmsg Corey Bryant
2012-07-23 22:50   ` Eric Blake
2012-07-24  2:19     ` Corey Bryant
2012-07-23 13:08 ` [Qemu-devel] [PATCH v5 2/6] qapi: Introduce add-fd, remove-fd, query-fdsets Corey Bryant
2012-07-25 18:16   ` Eric Blake
2012-07-26  2:55     ` Corey Bryant
2012-07-23 13:08 ` [Qemu-devel] [PATCH v5 3/6] monitor: Clean up fd sets on monitor disconnect Corey Bryant
2012-07-23 13:08 ` [Qemu-devel] [PATCH v5 4/6] block: Convert open calls to qemu_open Corey Bryant
2012-07-25 19:22   ` Eric Blake [this message]
2012-07-26  3:11     ` Corey Bryant
2012-07-23 13:08 ` [Qemu-devel] [PATCH v5 5/6] block: Convert close calls to qemu_close Corey Bryant
2012-07-23 13:08 ` [Qemu-devel] [PATCH v5 6/6] block: Enable qemu_open/close to work with fd sets Corey Bryant
2012-07-23 13:14   ` Corey Bryant
2012-08-02 22:21     ` Corey Bryant
2012-08-06  9:15       ` Kevin Wolf
2012-08-06 13:32         ` Corey Bryant
2012-08-06 13:51           ` Kevin Wolf
2012-08-06 14:15             ` Corey Bryant
2012-08-07 16:43               ` Corey Bryant
2012-07-24 12:07   ` Kevin Wolf
2012-07-25  3:41     ` Corey Bryant
2012-07-25  8:22       ` Kevin Wolf
2012-07-25 19:25         ` Eric Blake
2012-07-26  3:21           ` Corey Bryant
2012-07-26 13:13             ` Eric Blake
2012-07-26 13:16               ` Kevin Wolf
2012-07-27  4:07                 ` Corey Bryant
2012-07-25 19:43   ` Eric Blake
2012-07-26  3:57     ` Corey Bryant
2012-07-26  9:07       ` Kevin Wolf
2012-07-27  3:59         ` Corey Bryant
2012-07-27  4:03         ` Corey Bryant
2012-08-02 15:08       ` Corey Bryant
2012-07-24 12:09 ` [Qemu-devel] [PATCH v5 0/6] file descriptor passing using " Kevin Wolf
2012-07-25  3:42   ` Corey Bryant

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5010476C.9050405@redhat.com \
    --to=eblake@redhat.com \
    --cc=aliguori@us.ibm.com \
    --cc=coreyb@linux.vnet.ibm.com \
    --cc=kwolf@redhat.com \
    --cc=lcapitulino@redhat.com \
    --cc=libvir-list@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@linux.vnet.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.