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

[Daniel P. Berrange]

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.

Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|