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

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.

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?

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.