From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52349) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Vu0ib-0006SP-94 for qemu-devel@nongnu.org; Fri, 20 Dec 2013 09:06:26 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Vu0iV-00024a-N9 for qemu-devel@nongnu.org; Fri, 20 Dec 2013 09:06:21 -0500 Received: from mx.ipv6.kamp.de ([2a02:248:0:51::16]:39029 helo=mx01.kamp.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Vu0iV-00024R-Ch for qemu-devel@nongnu.org; Fri, 20 Dec 2013 09:06:15 -0500 Message-ID: <52B44F0E.5070403@kamp.de> Date: Fri, 20 Dec 2013 15:07:10 +0100 From: Peter Lieven MIME-Version: 1.0 References: <1387271725-17060-1-git-send-email-pl@kamp.de> <20131217164730.GD2708@stefanha-thinkpad.redhat.com> <52B41279.8060304@kamp.de> <20131220121935.GA5905@stefanha-thinkpad.redhat.com> <52B43DC7.90007@kamp.de> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCHv2] block: add native support for NFS List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: Kevin Wolf , Paolo Bonzini , Fam Zheng , qemu-devel , Stefan Hajnoczi On 20.12.2013 14:57, Stefan Hajnoczi wrote: > On Fri, Dec 20, 2013 at 1:53 PM, Peter Lieven 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