[libvirt] [PATCH v2 11/12] virsh: add helper for returning event name string

[Lin Ma]

On 05/11/2018 05:03 PM, Michal Privoznik wrote:
> On 05/11/2018 10:22 AM, Lin Ma wrote:
>>
>> On 05/10/2018 05:17 PM, Michal Privoznik wrote:
>>> On 05/08/2018 04:20 PM, Lin Ma wrote:
>>>> Signed-off-by: Lin Ma <lma at suse.com>
>>>> ---
>>>>    tools/virsh-util.c | 60
>>>> ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>    tools/virsh-util.h |  3 +++
>>>>    2 files changed, 63 insertions(+)
>>>>
>>>> diff --git a/tools/virsh-util.c b/tools/virsh-util.c
>>>> index 44be3ad64b..686f9aef98 100644
>>>> --- a/tools/virsh-util.c
>>>> +++ b/tools/virsh-util.c
>>>> @@ -285,3 +285,63 @@ virshDomainGetXML(vshControl *ctl,
>>>>          return ret;
>>>>    }
>>>> +
>>>> +
>>>> +const char *
>>>> +virshDomainEventGetName(int event)
>>>> +{
>>>> +    switch ((int)event) {
>>>> +    case VIR_DOMAIN_EVENT_ID_LIFECYCLE:
>>>> +        return "lifecycle";
>>>> +    case VIR_DOMAIN_EVENT_ID_REBOOT:
>>>> +        return "reboot";
>>>> +    case VIR_DOMAIN_EVENT_ID_RTC_CHANGE:
>>>> +        return "rtc-change";
>>>> +    case VIR_DOMAIN_EVENT_ID_WATCHDOG:
>>>> +        return "watchdog";
>>>> +    case VIR_DOMAIN_EVENT_ID_IO_ERROR:
>>>> +        return "io-error";
>>>> +    case VIR_DOMAIN_EVENT_ID_GRAPHICS:
>>>> +        return "graphics";
>>>> +    case VIR_DOMAIN_EVENT_ID_IO_ERROR_REASON:
>>>> +        return "io-error-reason";
>>>> +    case VIR_DOMAIN_EVENT_ID_CONTROL_ERROR:
>>>> +        return "control-error";
>>>> +    case VIR_DOMAIN_EVENT_ID_BLOCK_JOB:
>>>> +        return "block-job";
>>>> +    case VIR_DOMAIN_EVENT_ID_DISK_CHANGE:
>>>> +        return "disk-change";
>>>> +    case VIR_DOMAIN_EVENT_ID_TRAY_CHANGE:
>>>> +        return "tray-change";
>>>> +    case VIR_DOMAIN_EVENT_ID_PMWAKEUP:
>>>> +        return "pm-wakeup";
>>>> +    case VIR_DOMAIN_EVENT_ID_PMSUSPEND:
>>>> +        return "pm-suspend";
>>>> +    case VIR_DOMAIN_EVENT_ID_BALLOON_CHANGE:
>>>> +        return "balloon-change";
>>>> +    case VIR_DOMAIN_EVENT_ID_PMSUSPEND_DISK:
>>>> +        return "pm-suspend-disk";
>>>> +    case VIR_DOMAIN_EVENT_ID_DEVICE_REMOVED:
>>>> +        return "device-removed";
>>>> +    case VIR_DOMAIN_EVENT_ID_BLOCK_JOB_2:
>>>> +        return "block-job-2";
>>>> +    case VIR_DOMAIN_EVENT_ID_TUNABLE:
>>>> +        return "tunable";
>>>> +    case VIR_DOMAIN_EVENT_ID_AGENT_LIFECYCLE:
>>>> +        return "agent-lifecycle";
>>>> +    case VIR_DOMAIN_EVENT_ID_DEVICE_ADDED:
>>>> +        return "device-added";
>>>> +    case VIR_DOMAIN_EVENT_ID_MIGRATION_ITERATION:
>>>> +        return "migration-iteration";
>>>> +    case VIR_DOMAIN_EVENT_ID_JOB_COMPLETED:
>>>> +        return "job-completed";
>>>> +    case VIR_DOMAIN_EVENT_ID_DEVICE_REMOVAL_FAILED:
>>>> +        return "device-removal-failed";
>>>> +    case VIR_DOMAIN_EVENT_ID_METADATA_CHANGE:
>>>> +        return "metadata-change";
>>>> +    case VIR_DOMAIN_EVENT_ID_BLOCK_THRESHOLD:
>>>> +        return "block-threshold";
>>>> +    default:
>>>> +        return NULL;
>>>> +    }
>>> Or simply:
>>>
>>> const char *
>>> virshDomainEventGetName(int event)
>>> {
>>>       if (event < 0 || event >= VIR_DOMAIN_EVENT_ID_LAST)
>>>           return NULL;
>>>
>>>       return vshEventCallbacks[event].name;
>>> }
>>>
>>>
>>> I really want to avoid having two places defining strings (one for
>>> command parse code, the other for completions).
>> For using vshEventCallbacks in tools/virsh-util.c, It seems that we need to
>> move lots of code which related to vshEventCallbacks to virsh-util.h or
>> somewhere else.
>> Say those event callback functions and corresponding structures and macros.
>>
>> I'm not sure what is the reasonable way to make it happen, May I have
>> your idea?
> Well, the virshDomainEventGetName() function doesn't necessarily have to
> live in virsh-utils.c. And now that I'm looking into other sources,
> maybe this function can be renamed to virshDomainEventToString()? To
> match the other functions like virshSecretEventToString() for instance.
>
>
>>
>> (btw, there are other 'vshEventCallbacks' array which defining in
>> virsh-secret.c,
>> virsh-pool.c and virsh-nodedev.c.we only move the vshEventCallbacks and
>> related
>> data structures from virsh-domain.c?)
> We can have virshSecretEventToString(), virshPoolEventToString(), ...
> exposed and used in completers.
The function names about *EventToString are already occupied, So I'd like
to use the original name virshDomainEventGetName for the time being.

Because the function virshDomainEventGetName only be used for virsh, I think
I don't need to export it.
I prefer to put the implmentation of function virshDomainEventGetName into
virsh-domain.c and declare it in virsh-domain.h, Then virsh-completer.c 
includes
virsh-domain.h for using virshDomainEventGetName in next patch, What do 
you think?

Thanks,
Lin