Re: [Qemu-devel] [PULL v4 07/11] rdma: introduce capability for chunk registration

[Eric Blake]

On 04/17/2013 05:07 PM, address@hidden wrote:
> From: "Michael R. Hines" <address@hidden>
> 
> This capability allows you to disable dynamic chunk registration
> for better throughput on high-performance links.
> 
> It is enabled by default.
> 
> Signed-off-by: Michael R. Hines <address@hidden>
> ---
>  migration.c      |   10 ++++++++++
>  qapi-schema.json |    8 +++++++-
>  2 files changed, 17 insertions(+), 1 deletion(-)

>  #
> +# @x-chunk-register-destination: (since 1.5) RDMA option which controls 
> whether
> +#          or not the entire VM memory footprint is mlock() on demand or all 
> at once.
> +#          Refer to docs/rdma.txt for more advice on when to take advantage 
> option.

s/take advantage/use this/

> +#          Enabled by default, and will be renamed to 
> 'chunk-register-destination' 
> +#          after experimental testing is complete.

I wouldn't promise a rename - after all, testing may prove that we can
settle on enough heuristics to set this appropriately without needing a
user option, even for the workloads where it makes a difference.  Thus,
I think better wording might be:

Enabled by default.  Experimental: may be renamed or removed after
further testing is complete.

Sorry for not thinking about this earlier, but typically you want a
capability bit to default to 0 - it's much easier to assume that a bit
not present behaves the same as a bit that is present and 0.  Or put
another way, a older management app that asks for all enabled
capabilities on a newer qemu has an easier time ignoring 0 bits that it
doesn't recognize (oh, some new feature I don't know about, but it isn't
on, so it can't hurt) than it does ignoring 1 bits (oh, a feature I
don't recognize, but it's enabled - will it mess up my migration?).
Since this is a bool, I would much rather can we rename the capability
to express the opposite sense, and default it to 0.  I'm not even sure
from your description here whether 'true' means 'mlock() on demand' or
'all at once', just that I'm supposed to read rdma.txt to decide if I
want to move away from the default.

/me reads patch 11 again... and wonders why the docs came last instead
of first in the series...

I guess the opposite sense could be named 'x-rdma-pin-all'; default
false means to do chunk registration and release, true means to pin all
memory up front.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature