Re: [Qemu-block] [RFC 2/2] block/file-posix: verify page cache is not used

[Stefan Hajnoczi]

On Fri, Apr 20, 2018 at 08:25:13AM +0200, Kevin Wolf wrote:
> Am 20.04.2018 um 05:02 hat Stefan Hajnoczi geschrieben:
> > On Thu, Apr 19, 2018 at 10:05:47AM +0100, Dr. David Alan Gilbert wrote:
> > > * Stefan Hajnoczi (address@hidden) wrote:
> > > > This commit is for debugging only.  Do not merge it.
> > > > 
> > > > mincore(2) checks whether pages are resident.  Use it to verify that
> > > > page cache has been dropped.
> > > > 
> > > > You can trigger a verification failure by mmapping the image file from
> > > > another process and loading a byte from a page so that it becomes
> > > > resident.  bdrv_co_invalidate_cache() will fail while the process is
> > > > alive.
> > > 
> > > It doesn't seem a bad diagnostic to keep in (with a switch to activate)
> > > for when we're faced with some weird corruption on some weird storage
> > > system.
> > 
> > Okay.  It's very slow to mmap an entire image file and query mincore(2)
> > so it needs to be off by default.
> 
> Also, having it enabled breaks localhost migration at least on tmpfs
> (which was what I tried out first).
> 
> I wonder if the kernel would add some way to query whether the "advice"
> was actually acted upon if we asked. Either with a new function that
> returns an error if not everything is dropped (basically
> .bdrv_invalidate_cache on the kernel level), or a function that just
> queries if any page is allocated (or maybe the address of the first
> allocated page in a given range) without having to use mincore() and
> iterating over all the pages in userspace.

I'm trying to figure out how to expose the optional mincore check on the
command-line/QMP:

1. Add a check_consistency bool argument to bdrv_invalidate_cache*().
   Add command-line/QMP option to -incoming and migrate_incoming.  This
   is messy and won't be easy to access for libvirt users.

2. Add a BlockdevOptionsFile *check-cache-consistency bool field.  This
   is specified at .bdrv_open() time.  It can be changed at runtime with
   .bdrv_reopen*().

3. Add a 'blockdev-check-cache-consistency' QMP command that calls a new
   .bdrv_check_cache_consistency() callback that is implemented by
   file-posix.c.  The problem is users might issue this command after
   I/O has resumed and pages have become resident again.  It only makes
   sense if the guest is still paused.  Probably a bad interface...

Have I missed a good way to expose this optional check functionality?

Which approach do you prefer?  I'm leaning towards #2.

Stefan

signature.asc
Description: PGP signature