qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Peter Lieven
Subject: Re: [Qemu-devel] [PATCHv2] block: add native support for NFS
Date: Fri, 20 Dec 2013 15:07:10 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.2.0

On 20.12.2013 14:57, Stefan Hajnoczi wrote:
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.
I can do that. I think its not too complicated because all those
implementations do not rely on callbacks. It should be possible
to just rename the existing implemenations to lets say
.bdrv_co_get_allocated_file_size and call them inside a coroutine.

Can you just call nfs_fstat() (the sync libnfs interface)?
I can only do that if its guaranteed that no other requests are in flight
otherwise it will mess up.

Peter




reply via email to

[Prev in Thread] Current Thread [Next in Thread]