On 09/20/2012 10:10 AM, Kevin Wolf wrote:
> Am 18.09.2012 20:53, schrieb Jeff Cody:
>> 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.
>>
>> Signed-off-by: Jeff Cody <jc...@redhat.com>
>
>> +static int raw_reopen_prepare(BDRVReopenState *state,
>> + BlockReopenQueue *queue, Error **errp)
>> +{
>> + BDRVRawState *s;
>> + BDRVRawReopenState *raw_s;
>> + int ret = 0;
>> +
>> + assert(state != NULL);
>> + assert(state->bs != NULL);
>> +
>> + s = state->bs->opaque;
>> +
>> + state->opaque = g_malloc0(sizeof(BDRVRawReopenState));
>> + raw_s = state->opaque;
>> + raw_s->use_aio = s->use_aio;
>> + raw_s->aio_ctx = s->aio_ctx;
>
> You can immediately set s->aio_ctx instead of going through
> BDRVRawReopenState with it. It seems to be valid to have it present
> while use_aio = 0. If it wasn't valid, you'd have to free the context
> when reopening without Linux AIO.
>
Good catch, thanks.
>> +
>> + raw_parse_flags(state->flags, &raw_s->open_flags);
>> + raw_set_aio(&raw_s->aio_ctx, &raw_s->use_aio, state->flags);
>
> At least you're consistently omitting the error check. :-)
>
Thanks, fixed.
I guess I am an optimist at heart :)
>> +
>> + raw_s->fd = -1;
>> +
>> + 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 */
>
> Hm, still not addressed?
>
No. I mentioned this in the cover letter for v2. I'd rather see the
qemu fcntl wrapper happen as a separate series, and then come back and
update this, if that is OK. I'm afraid changes to qemu_open or
qemu_dup_flags would delay getting this series in.
Although Eric is right, I do need to ifdef the F_DUPFD_CLOEXEC.
> Kevin
>
