From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from [140.186.70.92] (port=43685 helo=eggs.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1Q2jq3-0002m7-OV
	for qemu-devel@nongnu.org; Thu, 24 Mar 2011 08:40:32 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <kwolf@redhat.com>) id 1Q2jq2-0001rn-KI
	for qemu-devel@nongnu.org; Thu, 24 Mar 2011 08:40:31 -0400
Received: from mx1.redhat.com ([209.132.183.28]:33065)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <kwolf@redhat.com>) id 1Q2jq2-0001rN-8R
	for qemu-devel@nongnu.org; Thu, 24 Mar 2011 08:40:30 -0400
Message-ID: <4D8B3C3A.3030407@redhat.com>
Date: Thu, 24 Mar 2011 13:42:34 +0100
From: Kevin Wolf <kwolf@redhat.com>
MIME-Version: 1.0
Subject: Re: [Qemu-devel] Re: [PATCH 3/3] raw-posix: Re-open host CD-ROM after
	media change
References: <1300902055-25850-1-git-send-email-stefanha@linux.vnet.ibm.com>	<1300902055-25850-4-git-send-email-stefanha@linux.vnet.ibm.com>	<m3ei5xlgzs.fsf@neno.mitica>
	<AANLkTin5rN8gYv9S5aDXUTFGfCJZujMmzy_XK1=Sg8fh@mail.gmail.com>
In-Reply-To: <AANLkTin5rN8gYv9S5aDXUTFGfCJZujMmzy_XK1=Sg8fh@mail.gmail.com>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
List-Id: qemu-devel.nongnu.org
List-Unsubscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Stefan Hajnoczi <stefanha@gmail.com>
Cc: Anthony Liguori <aliguori@us.ibm.com>, Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>, quintela@redhat.com, qemu-devel@nongnu.org, Ryan Harper <ryanh@us.ibm.com>, Amit Shah <amit.shah@redhat.com>

Am 23.03.2011 21:50, schrieb Stefan Hajnoczi:
> On Wed, Mar 23, 2011 at 8:27 PM, Juan Quintela <quintela@redhat.com> wrote:
>> Stefan Hajnoczi <stefanha@linux.vnet.ibm.com> wrote:
>>> +
>>> +    if (s->fd == -1) {
>>> +        s->fd = qemu_open(bs->filename, s->open_flags, 0644);
>>
>> Everything else on that file uses plain "open" not "qemu_open".
>> diference is basically that qemu_open() adds flag O_CLOEXEC.
>>
>> I don't know if this one should be vanilla open or the other ones
>> qemu_open().
>>
>> What do you think?
> 
> raw_open_common() uses qemu_open().  That's why I used it.

And I think it's correct. There's no reason not to set O_CLOEXEC here.
Maybe some of the open() users need to be fixed.

>>> +        if (s->fd < 0) {
>>> +            return 0;
>>> +        }
>>> +    }
>>> +
>>> +    ret = (ioctl(s->fd, CDROM_DRIVE_STATUS, CDSL_CURRENT) == CDS_DISC_OK);
>>
>> parens are not needed around ==.
> 
> Yes, if you want I'll remove them.  I just did it for readability.

I like them.

Kevin