On 08/31/2012 10:49 AM, Kevin Wolf wrote:
> 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.
I think so - that is the one I was thinking about. It would just need
to be made non-static.
>
>>>> + 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.
>
OK