Re: [Qemu-devel] [PATCH 3/7] block: raw-posix image file reopen

[Kevin Wolf]

Am 05.09.2012 18:43, schrieb Jeff Cody:
>>> +    }
>>> +
>>> +    int fcntl_flags = O_APPEND | O_ASYNC | O_NONBLOCK;
>>> +#ifdef O_NOATIME
>>> +    fcntl_flags |= O_NOATIME;
>>> +#endif
>>> +    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);
>>> +        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);
>>> +        if (raw_s->fd == -1) {
>>> +            ret = -1;
>>> +        }
>>
>> Ignoring this part for now, with qemu_dup_flags() it's going to look a
>> bit different. In particular, I'm hoping that we don't get a second
>> fcntl_flags enumeration here, but can just fall back to qemu_open()
>> whenever qemu_dup_flags() fails.
> 
> That will require modification to qemu_dup_flags()... I believe
> qemu_dup_flags() silently filters out fcntl incompatible flags.
> 
> Maybe it would be best to create a small helper function in osdep.c, that
> fetches the fcntl_flags.  Then qemu_dup_flags() and this function would
> use the same helper to fetch fcntl_flags.  The results of that would
> determine if we call qemu_dup_flags() or qemu_open().
> 
> Although, I do think it makes sense to always try qemu_open() if
> qemu_dup_flags() fails for some reason.

If we can modify qemu_dup_flags() to fail if it can't provide the right
set of flags, then I think we should do it - and I think we can. Even
for the existing cases with fd passing it shouldn't break anything, but
only add an additional safety check.

And if touching the function motivates Corey to write some fd passing
test cases so that you can't break it, even better. ;-)

Kevin