[libvirt] [PATCH] virsh: fix memtune's help message for swap_hard_limit

[KAMEZAWA Hiroyuki]

On Tue, 15 Mar 2011 14:30:02 +0000
"Daniel P. Berrange" <berrange at redhat.com> wrote:

> On Mon, Mar 14, 2011 at 09:24:58PM -0600, Eric Blake wrote:
> > On 03/09/2011 01:13 AM, KAMEZAWA Hiroyuki wrote:
> > > +++ b/docs/schemas/domain.rng
> > > @@ -341,7 +341,7 @@
> > >            </optional>
> > >            <!-- Maximum swap area the VM can use -->
> > >            <optional>
> > > -            <element name="swap_hard_limit">
> > > +            <element name="memswap_hard_limit">
> > >                <ref name="memoryKB"/>
> > >              </element>
> > >            </optional>
> > 
> > This needs fixing.  We need to support both old and new styles, since
> > we've had a release that used the old name.  That means we need to add a
> > new test, rather than modifying an existing test, to show that we can
> > parse both styles when reading XML, but that we generate the new style
> > when writing.
> 
> Adding back-compat hacks are only reasonable if we have already had
> an accident & mistakenly changed public facing XML/API. Given, that
> we're not in that situation, we should simply not make the proposed
> changes to the XML/API, rather than changing it & adding compat hacks.
> 
> > 
> > Maybe danpb or DV will have some further comments on whether it is wise
> > to deprecate XML like this; it's unpleasant business to think about
> > backwards compatibility.  We might be stuck with just documenting that
> > the choice of name was unfortunate to avoid the hassle of back-compat;
> > but I hope it doesn't come to that.
> 
> 
> > > diff --git a/tools/virsh.c b/tools/virsh.c
> > > index 14c6d6b..b790248 100644
> > > --- a/tools/virsh.c
> > > +++ b/tools/virsh.c
> > > @@ -3034,8 +3034,8 @@ static const vshCmdOptDef opts_memtune[] = {
> > >       N_("Max memory in kilobytes")},
> > >      {"soft-limit", VSH_OT_INT, VSH_OFLAG_NONE,
> > >       N_("Memory during contention in kilobytes")},
> > > -    {"swap-hard-limit", VSH_OT_INT, VSH_OFLAG_NONE,
> > > -     N_("Max swap in kilobytes")},
> > > +    {"memswap-hard-limit", VSH_OT_INT, VSH_OFLAG_NONE,
> > > +     N_("Max memory+swap in kilobytes")},
> > 
> > This name change might break existing clients of virsh.  Is there any
> > way to support the older name?  Maybe we need to add some more framework
> > into virsh option parsing to allow option aliases that the parser
> > recognizes but which don't get documented, so that someone still doing
> > --swap-hard-limit will work?
> 
> Agreed, we shouldn't change this either.
> 
> > I guess I need more convincing that this rename is worth the hassle, or
> > whether we are stuck with the smaller but less controversial patch of
> > documenting that the name isn't optimal for what it really represents.
> 
> Agreed, we should simply improve docs for the existing names to clarify
> their meaning.
> 

Ok, I'll stop.
-Kame