RE: [Xen-devel] [PATCH] libxl: use named options for tsc_mode

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Thu, 2011-11-10 at 17:29 +0000, Dan Magenheimer wrote:
> > From: Ian Campbell [mailto:ian.campbell@xxxxxxxxxx]
> > Subject: [Xen-devel] [PATCH] libxl: use named options for tsc_mode
> > 
> > # HG changeset patch
> > # User Ian Campbell <ian.campbell@xxxxxxxxxx>
> > # Date 1320922479 0
> > # Node ID bc79b560aafa1e4dc42af00e6a326dc651b5636a
> > # Parent  460b507e15f864dd6712f5040e36538d6e076ae4
> > libxl: use named options for tsc_mode.
> > 
> > It seems that this knob is expoerted from the hypervisor as a raw
> > integer (no symbolic names) documented in xen/include/asm-x86. Propagating 
> > that
> > all the way to the end user is hardly friendly (it's bad enough in the
> > hypercall interface).
> 
> Thanks for looking at this!
> 
> > Add an enum at the libxl level with a hopefully descriptive set of names.
> > Deprecate the use of an integer in xl cfg files.
> 
> We (Oracle) already have shipped cfg files that use the integer
> so would prefer that the deprecation message be removed.  IMHO, to
> the vast majority of users, the symbolic names will be gibberish
> anyway and are likely to be misspelled.  For knowledgeable folk,
> they are nice though, so maybe just support both?

For my part I can never remember which number is which so I always have
to go look, whereas "always_emulate" etc is generally descriptive of
what I'm looking for. IMHO magic numbers are generally a pretty terrible
interface.

I could change the message to inform the user which specific option
corresponds to the number which they used. Would that help?

More generally we need to have mechanisms by which we can evolve the
functionality provided by xl, guiding users towards new and improved
options for what they are trying to achieve while allowing us to
eventually remove deprecated options and therefore combat the inevitable
growth of compatibility cruft. I don't really mind what that mechanism
is but a message explaining what the new option is seems like a simple
solution. Perhaps it could be dropped to "INFO" rather than "WARNING"?

>  
> > +   * `"always_emulate"`: guest rdtsc/p always emulated at 1GHz (kernel
> > +     and user).
> 
> Many guests will "appear" to run at a slower "bogomips" so a word
> or two more here may keep some from panicking.  Maybe:
> 
> guest rdtsc/p always emulated and the virtual TSC will appear to
> increment (kernel and user) at a fixed 1GHz rate, regardless of
> the PCPU HZ rate or power state; this will NOT affect underlying
> CPU performance

Presumably the actual act of emulation has a performance impact?

How about:

guest rdtsc/p always emulated and the virtual TSC will appear to
increment (kernel and user) at a fixed 1GHz rate, regardless of the PCPU
HZ rate or power state; Although there is an overhead associated with
emulation this will NOT affect underlying CPU performance.

?

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel