[libvirt] [PATCH] virsh: Remove --flags from nodesuspend

Peter Krempa pkrempa at redhat.com
Thu Oct 25 10:08:13 UTC 2012


On 10/25/12 11:03, Michal Privoznik wrote:
> On 25.10.2012 10:22, Jiri Denemark wrote:
>> We always expose individual bits from flags as separate options rather
>> than exposing a raw flags options. Since virNodeSuspendForDuration does
>> not currently support any flags, the only way of using this --flags
>> options that would not fail is "--flags 0", which is equivalent to
>> omitting the option. Thus it is highly unlikely anyone would actually be
>> using it and removing it should be safe.
>> ---
>>   tools/virsh-host.c | 10 +---------
>>   tools/virsh.pod    |  2 +-
>>   2 files changed, 2 insertions(+), 10 deletions(-)
>>
>> diff --git a/tools/virsh-host.c b/tools/virsh-host.c
>> index 0f9b3f3..2ea24ac 100644
>> --- a/tools/virsh-host.c
>> +++ b/tools/virsh-host.c
>> @@ -506,7 +506,6 @@ static const vshCmdOptDef opts_node_suspend[] = {
>>       {"target", VSH_OT_DATA, VSH_OFLAG_REQ, N_("mem(Suspend-to-RAM), "
>>                                                  "disk(Suspend-to-Disk), hybrid(Hybrid-Suspend)")},
>>       {"duration", VSH_OT_INT, VSH_OFLAG_REQ, N_("Suspend duration in seconds, at least 60")},
>> -    {"flags", VSH_OT_INT, VSH_OFLAG_NONE, N_("Suspend flags, 0 for default")},
>>       {NULL, 0, 0, NULL}
>>   };
>
> While I agree that this design is broken I don't think we can do this.
> Okay, for now we only support 0; but what if in the future we invent a
> new flag? With current virsh one is able to use it however with your
> patch he isn't.
>
> Therefore I'd rather see slightly different approach. Like we do for
> other broken options/arguments in virsh - hide it, don't mention it
> anywhere but keep the code.

I agree with this approach.

Peter




More information about the libvir-list mailing list