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 ([email protected]) 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
