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

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(a)suse.com&gt; >> --- >>   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?

(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?)