Re: [Qemu-devel] [PATCHv2] block: add native support for NFS

[Stefan Hajnoczi]

On Fri, Dec 20, 2013 at 1:53 PM, Peter Lieven <address@hidden> wrote:
> On 20.12.2013 13:19, Stefan Hajnoczi wrote:
>>
>> On Fri, Dec 20, 2013 at 10:48:41AM +0100, Peter Lieven wrote:
>>>
>>> On 17.12.2013 17:47, Stefan Hajnoczi wrote:
>>>>
>>>> On Tue, Dec 17, 2013 at 10:15:25AM +0100, Peter Lieven wrote:
>>>>> +    /* set to -ENOTSUP since bdrv_allocated_file_size is only used
>>>>> +     * in qemu-img open. So we can use the cached value for allocate
>>>>> +     * filesize obtained from fstat at open time */
>>>>> +    client->allocated_file_size = -ENOTSUP;
>>>>
>>>> Can you implement this fully?  By stubbing it out like this we won't be
>>>> able to call get_allocated_file_size() at runtime in the future without
>>>> updating the nfs block driver code.  It's just an fstat call, shouldn't
>>>> be too hard to implement properly :).
>>>
>>> It seems I have to leave it as is currently. bdrv_get_allocated_file_size
>>> is not in a coroutine context. I get coroutine yields to no one.
>>
>> Create a coroutine and pump the event loop until it has reached
>> completion:
>>
>> co = qemu_coroutine_create(my_coroutine_fn, ...);
>> qemu_coroutine_enter(co, foo);
>> while (!complete) {
>>      qemu_aio_wait();
>> }
>>
>> See block.c for similar examples.
>
> Wouldn't it make sense to make this modification to
> bdrv_get_allocated_file_size in
> block.c rather than in client/nfs.c and in the future potentially other
> drivers?
>
> If yes, I would ask you to take v3 of the NFS protocol patch and I promise
> to send
> a follow up early next year to make this modification to block.c and change
> block/nfs.c
> and other implementations to be a coroutine_fn.

.bdrv_get_allocated_file_size() implementations in other block drivers
are synchronous.  Making the block driver interface use coroutines
would be wrong unless all the block drivers were updated to use
coroutines too.

Can you just call nfs_fstat() (the sync libnfs interface)?

Stefan