Re: [Qemu-devel] [PATCH] block/nfs: cache allocated filesize for read-only files

[Max Reitz]

On 2015-08-21 at 11:11, Peter Lieven wrote:
> Am 21.08.2015 um 18:46 schrieb Max Reitz:
> > On 2015-08-21 at 00:49, Peter Lieven wrote:
> > > If the file is readonly its not expected to grow so
> > > save the blocking call to nfs_fstat_async and use
> > > the value saved at connection time. Also important
> > > the monitor (and thus the main loop) will not hang
> > > if block device info is queried and the NFS share
> > > is unresponsive.
> > >
> > > Signed-off-by: Peter Lieven <address@hidden>
> > > ---
> > >    block/nfs.c | 6 ++++++
> > >    1 file changed, 6 insertions(+)
> >
> > First, I don't like the idea of this patch very much, but since I've never used 
> > qemu's native NFS client, it's not up to me to decide whether it's worth it.
>
> I am trying to solve that a stale NFS Server with a CDROM ISO on it can hang 
> Qemus main loop. One of the things that happens is that
> you query "info block" in hmp or "query-block" via QMP and indirectly call 
> bdrv_get_allocated_file_size and bang, Qemu hangs. Also I don't
> know if its worth to issue an RPC call for each executing of info block.

OK.

> > When it comes to breaking this, what comes to mind first is some external 
> > program opening the image read-write outside of qemu and writing to it. Maybe 
> > that's a case we generally don't want, but maybe that's something some people 
> > do on purpose, knowing what they're doing (with raw images), you never know.
>
> I would consider this bad behaviour. However, allocated file size shouldn't 
> matter for raw images.

I don't know about NFS, but for other file systems it does.

$ ./qemu-img create -f raw test.raw 1G
$ ./qemu-io -c 'write 1023M 1M' test.raw
$ ./qemu-img info test.raw
...
virtual size: 1.0G (1073741824 bytes)
disk size: 1.0M

I do consider it bad behavior, too, but with raw images it should 
actually be valid for some use cases.

> If you resize the image from external you have to call bdrv_truncate anyway to 
> make Qemu aware
> of that change.
>
> > Other than that, there's reopening. As far as I'm aware, qemu can reopen a R/W 
> > image read-only, and if that happens, st_blocks may be stale.
>
> Thats a valid point. But it can be solved be implementing .bdrv_reopen_prepare 
> and update st_blocks there.

Right. Since the allocated size is not that important of an information, 
generally, I think I'd be fine with breaking the use case of writing to 
an image with an external tool while qemu is using that image read-only, 
as long as reopening works.

> Thanks for you thoughts,

Thanks for your patch. :-)

Max