On Fri, Dec 20, 2013 at 3:07 PM, Peter Lieven <address@hidden> wrote:
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.
No, that would be wrong because coroutine functions should not block.
The point of coroutines is that if they cannot proceed they must yield
so the event loop regains control. If you simply rename the function
to _co_ then they will block the event loop and not be true coroutine
functions.
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.
How will it mess up?