From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([208.118.235.92]:42608)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <kwolf@redhat.com>) id 1T7SXs-0000Xu-Gx
	for qemu-devel@nongnu.org; Fri, 31 Aug 2012 10:50:09 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <kwolf@redhat.com>) id 1T7SXn-00025x-8P
	for qemu-devel@nongnu.org; Fri, 31 Aug 2012 10:50:04 -0400
Received: from mx1.redhat.com ([209.132.183.28]:46849)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <kwolf@redhat.com>) id 1T7SXm-00025j-Ui
	for qemu-devel@nongnu.org; Fri, 31 Aug 2012 10:49:59 -0400
Message-ID: <5040CF0E.10408@redhat.com>
Date: Fri, 31 Aug 2012 16:49:50 +0200
From: Kevin Wolf <kwolf@redhat.com>
MIME-Version: 1.0
References: <cover.1346352124.git.jcody@redhat.com>
	<7ad1da59f3ef71e4c99c083a0675508ca9279c53.1346352124.git.jcody@redhat.com>
	<503FE5FC.1040002@redhat.com> <5040CD69.5060801@redhat.com>
In-Reply-To: <5040CD69.5060801@redhat.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH 3/7] block: raw-posix image file reopen
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/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: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: jcody@redhat.com
Cc: stefanha@gmail.com, pbonzini@redhat.com, Eric Blake <eblake@redhat.com>, qemu-devel@nongnu.org, supriyak@linux.vnet.ibm.com

Am 31.08.2012 16:42, schrieb Jeff Cody:
> On 08/30/2012 06:15 PM, Eric Blake wrote:
>> On 08/30/2012 11:47 AM, Jeff Cody wrote:
>>> This is derived from the Supriya Kannery's reopen patches.
>>>
>>> This contains the raw-posix driver changes for the bdrv_reopen_*
>>> functions.  All changes are staged into a temporary scratch buffer
>>> during the prepare() stage, and copied over to the live structure
>>> during commit().  Upon abort(), all changes are abandoned, and the
>>> live structures are unmodified.
>>>
>>> The _prepare() will create an extra fd - either by means of a dup,
>>> if possible, or opening a new fd if not (for instance, access
>>> control changes).  Upon _commit(), the original fd is closed and
>>> the new fd is used.  Upon _abort(), the duplicate/new fd is closed.
>>>
>>
>>> +    if ((raw_s->open_flags & ~fcntl_flags) == (s->open_flags & ~fcntl_flags)) {
>>> +        /* dup the original fd */
>>> +        /* TODO: use qemu fcntl wrapper */
>>> +        raw_s->fd = fcntl(s->fd, F_DUPFD_CLOEXEC, 0);
>>
>> I assume this TODO has to be fixed to allow compilation on systems that
>> lack F_DUPFD_CLOEXEC.
> 
> Yes, either that or add the logic here.

Would qemu_dup_flags() from osdep.c be the right thing here? It was
introduces with Corey's fd passing series.

>>> +        if (raw_s->fd == -1) {
>>> +            ret = -1;
>>> +            goto error;
>>> +        }
>>> +        ret = fcntl_setfl(raw_s->fd, raw_s->open_flags);
>>> +    } else {
>>> +        raw_s->fd = qemu_open(state->bs->filename, raw_s->open_flags, 0644);
>>
>> Is raw_s->open_flags every going to contain O_CREAT, or is the 0644 mode
>> argument spurious?
> 
> Thanks, you are right, it is spurious.  The raw_s->open_flags are
> explicitly set via raw_parse_flags(), so we know it will never contain
> O_CREAT.

We can probably assert it.

Kevin