Re: [Qemu-block] [PATCH v2] file-posix: add drop-cache=on|off option

[Eric Blake]

On 3/1/19 10:09 AM, Stefan Hajnoczi wrote:
> Commit dd577a26ff03b6829721b1ffbbf9e7c411b72378 ("block/file-posix:
> implement bdrv_co_invalidate_cache() on Linux") introduced page cache
> invalidation so that cache.direct=off live migration is safe on Linux.
> 
> The invalidation takes a significant amount of time when the file is
> large and present in the page cache.  Normally this is not the case for
> cross-host live migration but it can happen when migrating between QEMU
> processes on the same host.
> 
> On same-host migration we don't need to invalidate pages for correctness
> anyway, so an option to skip page cache invalidation is useful.  I
> investigated optimizing invalidation and detecting same-host migration,
> but both are hard to achieve so a user-visible option will suffice.
> 
> Suggested-by: Neil Skrypuch <address@hidden>
> Tested-by: Neil Skrypuch <address@hidden>
> Reviewed-by: Stefano Garzarella <address@hidden>
> Reviewed-by: Eric Blake <address@hidden>
> Signed-off-by: Stefan Hajnoczi <address@hidden>
> ---
> v2:
>  * Remove outdated comment about libvirt feature detection [danpb]

Question - if we used qapi's 'if':COND to only declare the field on
platforms where we know at compile time that we can support it, would
that be enough for libvirt to introspect that if the field exists then
migration is safe, without having to rely on an query-qemu-features command?

> +++ b/qapi/block-core.json
> @@ -2807,6 +2807,10 @@
>  # @locking:     whether to enable file locking. If set to 'auto', only enable
>  #               when Open File Descriptor (OFD) locking API is available
>  #               (default: auto, since 2.10)
> +# @drop-cache:  invalidate page cache during live migration.  This prevents
> +#               stale data on the migration destination with 
> cache.direct=off.
> +#               Currently only supported on Linux hosts.
> +#               (default: on, since: 4.0)
>  # @x-check-cache-dropped: whether to check that page cache was dropped on 
> live
>  #                         migration.  May cause noticeable delays if the 
> image
>  #                         file is large, do not use in production.
> @@ -2819,6 +2823,7 @@
>              '*pr-manager': 'str',
>              '*locking': 'OnOffAuto',
>              '*aio': 'BlockdevAioOptions',
> +            '*drop-cache': 'bool',
>              '*x-check-cache-dropped': 'bool' } }

In other words, now that we can use 'if' to hide features that aren't
supported based on compile-time knowledge, shouldn't we use that to make
the doc comment "only supported on Linux hosts" introspectible?

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org