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

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.