qemu-devel
[Top][All Lists]
Advanced

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

Re: [Qemu-devel] [Qemu-block] [PATCHv2] block/nfs: cache allocated files


From: Stefan Hajnoczi
Subject: Re: [Qemu-devel] [Qemu-block] [PATCHv2] block/nfs: cache allocated filesize for read-only files
Date: Thu, 3 Sep 2015 18:05:54 +0100
User-agent: Mutt/1.5.23 (2014-03-12)

On Wed, Aug 26, 2015 at 03:14:41PM -0400, Jeff Cody wrote:
> On Wed, Aug 26, 2015 at 08:49:06PM +0200, Peter Lieven wrote:
> > Am 26.08.2015 um 17:31 schrieb Jeff Cody:
> > > On Mon, Aug 24, 2015 at 10:13:16PM +0200, Max Reitz wrote:
> > >> On 24.08.2015 21:34, Peter Lieven wrote:
> > >>> Am 24.08.2015 um 20:39 schrieb Max Reitz:
> > >>>> On 24.08.2015 10:06, 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>
> > >>>>> ---
> > >>>>> v1->v2: update cache on reopen_prepare [Max]
> > >>>>>
> > >>>>>  block/nfs.c | 35 +++++++++++++++++++++++++++++++++++
> > >>>>>  1 file changed, 35 insertions(+)
> > >>>> Reviewed-by: Max Reitz <address@hidden>
> > >>>>
> > >>>> I hope you're ready for the "Stale actual-size value with
> > >>>> cache=direct,read-only=on,format=raw files on NFS" reports. :-)
> > >>> actually a good point, maybe the cache should only be used if
> > >>>
> > >>> !(bs->open_flags & BDRV_O_NOCACHE)
> > >> Good enough a point to fix it? ;-)
> > >>
> > >> Max
> > >>
> > > It seems more inline with expected behavior, to add the cache checking
> > > in before using the size cache.  Would you be opposed to a v3 with
> > > this check added in?
> > 
> > Of course, will send it tomorrow.
> > 
> > >
> > > One other concern I have is similar to a concern Max raised earlier -
> > > about an external program modifying the raw image, while QEMU has it
> > > opened r/o.  In particular, I wonder about an NFS server making an
> > > image either sparse / non-sparse.  If it was exported read-only, it
> > > may be a valid assumption that this could be done safely, as it would
> > > not change the reported file size or contents, just the allocated size
> > > on disk.
> > 
> > This might be a use case. But if I allow caching the allocated filesize
> > might not always be correct. This is even the case on a NFS share mounted
> > through the kernel where some attributes a cached for some time.
> > 
> > Anyway, would it hurt here if the actual filesize was too small?
> > In fact it was incorrect since libnfs support was added :-)
> > 
> 
> Yeah, I'm not sure what harm it would cause in practice.  It is a
> fairly edge use case to begin with, and a relatively benign side
> affect (especially since you added reopen() support).
> 
> With the cache flag checking, I am comfortable adding my r-b.

I don't remember QEMU's behavior on an LVM volume (which can be resized
underneath QEMU but isn't expected to grow or shrink under normal
operation).

The same semantics should probably be used here.

Stefan



reply via email to

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