[libvirt] [PATCH 0/2] virsh cleanups
Followups written based on review to my 'virsh event' addition. Eric Blake (2): virsh: use more compact VIR_ENUM_IMPL virsh: kill over-engineered asprintf failure recovery tools/virsh-domain-monitor.c | 280 +++++++++++++++------------------- tools/virsh-domain.c | 347 ++++++++++++++++++------------------------- tools/virsh-network.c | 36 ++--- tools/virsh-pool.c | 139 ++++++----------- tools/virsh-volume.c | 104 ++++--------- 5 files changed, 358 insertions(+), 548 deletions(-) -- 1.8.5.3
Dan Berrange suggested that using VIR_ENUM_IMPL is more compact than open-coding switch statements, and still just as forceful at making us remember to update lists if we add enum values in the future. Make this change throughout virsh. Sure enough, doing this change caught that we missed at least VIR_STORAGE_VOL_NETDIR. * tools/virsh-domain-monitor.c (vshDomainIOErrorToString) (vshDomainControlStateToString, vshDomainStateToString) (vshDomainStateReasonToString): Change switch to enum lookup. (cmdDomControl, cmdDominfo): Update caller. * tools/virsh-domain.c (vshDomainVcpuStateToString) (vshDomainEventToString, vshDomainEventDetailToString): Change switch to enum lookup. (vshDomainBlockJobToString, vshDomainJobToString): New functions. (cmdVcpuinfo, cmdBlockJob, cmdDomjobinfo, cmdEvent): Update callers. * tools/virsh-network.c (vshNetworkEventToString): Change switch to enum lookup. * tools/virsh-pool.c (vshStoragePoolStateToString): New function. (cmdPoolList, cmdPoolInfo): Update callers. * tools/virsh-volume.c (vshVolumeTypeToString): Change switch to enum lookup. (cmdVolInfo, cmdVolList): Update callers. Signed-off-by: Eric Blake <eblake@redhat.com> --- tools/virsh-domain-monitor.c | 280 +++++++++++++++------------------- tools/virsh-domain.c | 347 ++++++++++++++++++------------------------- tools/virsh-network.c | 36 ++--- tools/virsh-pool.c | 66 +++----- tools/virsh-volume.c | 38 ++--- 5 files changed, 316 insertions(+), 451 deletions(-) diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c index 6291ca5..c67b833 100644 --- a/tools/virsh-domain-monitor.c +++ b/tools/virsh-domain-monitor.c @@ -1,7 +1,7 @@ /* * virsh-domain-monitor.c: Commands to monitor domain status * - * Copyright (C) 2005, 2007-2013 Red Hat, Inc. + * Copyright (C) 2005, 2007-2014 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -40,21 +40,18 @@ #include "virxml.h" #include "virstring.h" +VIR_ENUM_DECL(vshDomainIOError) +VIR_ENUM_IMPL(vshDomainIOError, + VIR_DOMAIN_DISK_ERROR_LAST, + N_("no error"), + N_("unspecified error"), + N_("no space")) + static const char * vshDomainIOErrorToString(int error) { - switch ((virDomainDiskErrorCode) error) { - case VIR_DOMAIN_DISK_ERROR_NONE: - return _("no error"); - case VIR_DOMAIN_DISK_ERROR_UNSPEC: - return _("unspecified error"); - case VIR_DOMAIN_DISK_ERROR_NO_SPACE: - return _("no space"); - case VIR_DOMAIN_DISK_ERROR_LAST: - ; - } - - return _("unknown error"); + const char *str = vshDomainIOErrorTypeToString(error); + return str ? _(str) : _("unknown error"); } /* extract description or title from domain xml */ @@ -116,181 +113,142 @@ cleanup: return desc; } +VIR_ENUM_DECL(vshDomainControlState) +VIR_ENUM_IMPL(vshDomainControlState, + VIR_DOMAIN_CONTROL_LAST, + N_("ok"), + N_("background job"), + N_("occupied"), + N_("error")) + static const char * vshDomainControlStateToString(int state) { - switch ((virDomainControlState) state) { - case VIR_DOMAIN_CONTROL_OK: - return N_("ok"); - case VIR_DOMAIN_CONTROL_JOB: - return N_("background job"); - case VIR_DOMAIN_CONTROL_OCCUPIED: - return N_("occupied"); - case VIR_DOMAIN_CONTROL_ERROR: - return N_("error"); - default: - ; - } - - return N_("unknown"); + const char *str = vshDomainControlStateTypeToString(state); + return str ? _(str) : _("unknown"); } +VIR_ENUM_DECL(vshDomainState) +VIR_ENUM_IMPL(vshDomainState, + VIR_DOMAIN_LAST, + N_("no state"), + N_("running"), + N_("idle"), + N_("paused"), + N_("in shutdown"), + N_("shut off"), + N_("crashed"), + N_("pmsuspended")) + static const char * vshDomainStateToString(int state) { - /* Can't use virDomainStateTypeToString, because we want to mark - * strings for translation. */ - switch ((virDomainState) state) { - case VIR_DOMAIN_RUNNING: - return N_("running"); - case VIR_DOMAIN_BLOCKED: - return N_("idle"); - case VIR_DOMAIN_PAUSED: - return N_("paused"); - case VIR_DOMAIN_SHUTDOWN: - return N_("in shutdown"); - case VIR_DOMAIN_SHUTOFF: - return N_("shut off"); - case VIR_DOMAIN_CRASHED: - return N_("crashed"); - case VIR_DOMAIN_PMSUSPENDED: - return N_("pmsuspended"); - case VIR_DOMAIN_NOSTATE: - case VIR_DOMAIN_LAST: - break; - } - return N_("no state"); /* = dom0 state */ + const char *str = vshDomainStateTypeToString(state); + return str ? _(str) : _("no state"); } +VIR_ENUM_DECL(vshDomainNostateReason) +VIR_ENUM_IMPL(vshDomainNostateReason, + VIR_DOMAIN_NOSTATE_LAST, + N_("unknown")) + +VIR_ENUM_DECL(vshDomainRunningReason) +VIR_ENUM_IMPL(vshDomainRunningReason, + VIR_DOMAIN_RUNNING_LAST, + N_("unkown"), + N_("booted"), + N_("migrated"), + N_("restored"), + N_("from snapshot"), + N_("unpaused"), + N_("migration canceled"), + N_("save canceled"), + N_("event wakeup"), + N_("crashed")) + +VIR_ENUM_DECL(vshDomainBlockedReason) +VIR_ENUM_IMPL(vshDomainBlockedReason, + VIR_DOMAIN_BLOCKED_LAST, + N_("unknown")) + +VIR_ENUM_DECL(vshDomainPausedReason) +VIR_ENUM_IMPL(vshDomainPausedReason, + VIR_DOMAIN_PAUSED_LAST, + N_("unknown"), + N_("user"), + N_("migrating"), + N_("saving"), + N_("dumping"), + N_("I/O error"), + N_("watchdog"), + N_("from snapshot"), + N_("shutting down"), + N_("creating snapshot"), + N_("crashed")) + +VIR_ENUM_DECL(vshDomainShutdownReason) +VIR_ENUM_IMPL(vshDomainShutdownReason, + VIR_DOMAIN_SHUTDOWN_LAST, + N_("unknown"), + N_("user")) + +VIR_ENUM_DECL(vshDomainShutoffReason) +VIR_ENUM_IMPL(vshDomainShutoffReason, + VIR_DOMAIN_SHUTOFF_LAST, + N_("unknown"), + N_("shutdown"), + N_("destroyed"), + N_("crashed"), + N_("migrated"), + N_("saved"), + N_("failed"), + N_("from snapshot")) + +VIR_ENUM_DECL(vshDomainCrashedReason) +VIR_ENUM_IMPL(vshDomainCrashedReason, + VIR_DOMAIN_CRASHED_LAST, + N_("unknown"), + N_("panicked")) + +VIR_ENUM_DECL(vshDomainPMSuspendedReason) +VIR_ENUM_IMPL(vshDomainPMSuspendedReason, + VIR_DOMAIN_PMSUSPENDED_LAST, + N_("unknown")) + static const char * vshDomainStateReasonToString(int state, int reason) { + const char *str; switch ((virDomainState) state) { case VIR_DOMAIN_NOSTATE: - switch ((virDomainNostateReason) reason) { - case VIR_DOMAIN_NOSTATE_UNKNOWN: - case VIR_DOMAIN_NOSTATE_LAST: - ; - } + str = vshDomainNostateReasonTypeToString(reason); break; - case VIR_DOMAIN_RUNNING: - switch ((virDomainRunningReason) reason) { - case VIR_DOMAIN_RUNNING_BOOTED: - return N_("booted"); - case VIR_DOMAIN_RUNNING_MIGRATED: - return N_("migrated"); - case VIR_DOMAIN_RUNNING_RESTORED: - return N_("restored"); - case VIR_DOMAIN_RUNNING_FROM_SNAPSHOT: - return N_("from snapshot"); - case VIR_DOMAIN_RUNNING_UNPAUSED: - return N_("unpaused"); - case VIR_DOMAIN_RUNNING_MIGRATION_CANCELED: - return N_("migration canceled"); - case VIR_DOMAIN_RUNNING_SAVE_CANCELED: - return N_("save canceled"); - case VIR_DOMAIN_RUNNING_WAKEUP: - return N_("event wakeup"); - case VIR_DOMAIN_RUNNING_CRASHED: - return N_("crashed"); - case VIR_DOMAIN_RUNNING_UNKNOWN: - case VIR_DOMAIN_RUNNING_LAST: - ; - } + str = vshDomainRunningReasonTypeToString(reason); break; - case VIR_DOMAIN_BLOCKED: - switch ((virDomainBlockedReason) reason) { - case VIR_DOMAIN_BLOCKED_UNKNOWN: - case VIR_DOMAIN_BLOCKED_LAST: - ; - } + str = vshDomainBlockedReasonTypeToString(reason); break; - case VIR_DOMAIN_PAUSED: - switch ((virDomainPausedReason) reason) { - case VIR_DOMAIN_PAUSED_USER: - return N_("user"); - case VIR_DOMAIN_PAUSED_MIGRATION: - return N_("migrating"); - case VIR_DOMAIN_PAUSED_SAVE: - return N_("saving"); - case VIR_DOMAIN_PAUSED_DUMP: - return N_("dumping"); - case VIR_DOMAIN_PAUSED_IOERROR: - return N_("I/O error"); - case VIR_DOMAIN_PAUSED_WATCHDOG: - return N_("watchdog"); - case VIR_DOMAIN_PAUSED_FROM_SNAPSHOT: - return N_("from snapshot"); - case VIR_DOMAIN_PAUSED_SHUTTING_DOWN: - return N_("shutting down"); - case VIR_DOMAIN_PAUSED_SNAPSHOT: - return N_("creating snapshot"); - case VIR_DOMAIN_PAUSED_CRASHED: - return N_("crashed"); - case VIR_DOMAIN_PAUSED_UNKNOWN: - case VIR_DOMAIN_PAUSED_LAST: - ; - } + str = vshDomainPausedReasonTypeToString(reason); break; - case VIR_DOMAIN_SHUTDOWN: - switch ((virDomainShutdownReason) reason) { - case VIR_DOMAIN_SHUTDOWN_USER: - return N_("user"); - case VIR_DOMAIN_SHUTDOWN_UNKNOWN: - case VIR_DOMAIN_SHUTDOWN_LAST: - ; - } + str = vshDomainShutdownReasonTypeToString(reason); break; - case VIR_DOMAIN_SHUTOFF: - switch ((virDomainShutoffReason) reason) { - case VIR_DOMAIN_SHUTOFF_SHUTDOWN: - return N_("shutdown"); - case VIR_DOMAIN_SHUTOFF_DESTROYED: - return N_("destroyed"); - case VIR_DOMAIN_SHUTOFF_CRASHED: - return N_("crashed"); - case VIR_DOMAIN_SHUTOFF_MIGRATED: - return N_("migrated"); - case VIR_DOMAIN_SHUTOFF_SAVED: - return N_("saved"); - case VIR_DOMAIN_SHUTOFF_FAILED: - return N_("failed"); - case VIR_DOMAIN_SHUTOFF_FROM_SNAPSHOT: - return N_("from snapshot"); - case VIR_DOMAIN_SHUTOFF_UNKNOWN: - case VIR_DOMAIN_SHUTOFF_LAST: - ; - } + str = vshDomainShutoffReasonTypeToString(reason); break; - case VIR_DOMAIN_CRASHED: - switch ((virDomainCrashedReason) reason) { - case VIR_DOMAIN_CRASHED_PANICKED: - return N_("panicked"); - case VIR_DOMAIN_CRASHED_UNKNOWN: - case VIR_DOMAIN_CRASHED_LAST: - ; - } + str = vshDomainCrashedReasonTypeToString(reason); break; - case VIR_DOMAIN_PMSUSPENDED: - switch ((virDomainPMSuspendedReason) reason) { - case VIR_DOMAIN_PMSUSPENDED_UNKNOWN: - case VIR_DOMAIN_PMSUSPENDED_LAST: - ; - } + str = vshDomainPMSuspendedReasonTypeToString(reason); break; - case VIR_DOMAIN_LAST: ; } - return N_("unknown"); + return str ? _(str) : _("unknown"); } /* @@ -854,11 +812,11 @@ cmdDomControl(vshControl *ctl, const vshCmd *cmd) if (info.state != VIR_DOMAIN_CONTROL_OK && info.state != VIR_DOMAIN_CONTROL_ERROR) { vshPrint(ctl, "%s (%0.3fs)\n", - _(vshDomainControlStateToString(info.state)), + vshDomainControlStateToString(info.state), info.stateTime / 1000.0); } else { vshPrint(ctl, "%s\n", - _(vshDomainControlStateToString(info.state))); + vshDomainControlStateToString(info.state)); } cleanup: @@ -1255,7 +1213,7 @@ cmdDominfo(vshControl *ctl, const vshCmd *cmd) if (virDomainGetInfo(dom, &info) == 0) { vshPrint(ctl, "%-15s %s\n", _("State:"), - _(vshDomainStateToString(info.state))); + vshDomainStateToString(info.state)); vshPrint(ctl, "%-15s %d\n", _("CPU(s):"), info.nrVirtCpu); @@ -1385,11 +1343,11 @@ cmdDomstate(vshControl *ctl, const vshCmd *cmd) if (showReason) { vshPrint(ctl, "%s (%s)\n", - _(vshDomainStateToString(state)), + vshDomainStateToString(state), vshDomainStateReasonToString(state, reason)); } else { vshPrint(ctl, "%s\n", - _(vshDomainStateToString(state))); + vshDomainStateToString(state)); } cleanup: @@ -1859,14 +1817,14 @@ cmdList(vshControl *ctl, const vshCmd *cmd) vshPrint(ctl, " %-5s %-30s %-10s %-20s\n", id_buf, virDomainGetName(dom), - state == -2 ? _("saved") : _(vshDomainStateToString(state)), + state == -2 ? _("saved") : vshDomainStateToString(state), title); VIR_FREE(title); } else { vshPrint(ctl, " %-5s %-30s %s\n", id_buf, virDomainGetName(dom), - state == -2 ? _("saved") : _(vshDomainStateToString(state))); + state == -2 ? _("saved") : vshDomainStateToString(state)); } } else if (optUUID) { if (virDomainGetUUIDString(dom, uuid) < 0) { diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 00ace11..74b52c9 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -111,20 +111,18 @@ vshCommandOptDomainBy(vshControl *ctl, const vshCmd *cmd, return dom; } +VIR_ENUM_DECL(vshDomainVcpuState) +VIR_ENUM_IMPL(vshDomainVcpuState, + VIR_VCPU_LAST, + N_("offline"), + N_("running"), + N_("blocked")) + static const char * vshDomainVcpuStateToString(int state) { - switch ((virVcpuState) state) { - case VIR_VCPU_OFFLINE: - return N_("offline"); - case VIR_VCPU_BLOCKED: - return N_("idle"); - case VIR_VCPU_RUNNING: - return N_("running"); - case VIR_VCPU_LAST: - break; - } - return N_("no state"); + const char *str = vshDomainVcpuStateTypeToString(state); + return str ? _(str) : _("no state"); } /* @@ -1960,12 +1958,26 @@ static const vshCmdOptDef opts_block_job[] = { {.name = NULL} }; +VIR_ENUM_DECL(vshDomainBlockJob) +VIR_ENUM_IMPL(vshDomainBlockJob, + VIR_DOMAIN_BLOCK_JOB_TYPE_LAST, + N_("Unknown job"), + N_("Block Pull"), + N_("Block Copy"), + N_("Block Commit")) + +static const char * +vshDomainBlockJobToString(int type) +{ + const char *str = vshDomainBlockJobTypeToString(type); + return str ? _(str) : _("Unknown job"); +} + static bool cmdBlockJob(vshControl *ctl, const vshCmd *cmd) { int mode; virDomainBlockJobInfo info; - const char *type; int ret; bool abortMode = (vshCommandOptBool(cmd, "abort") || vshCommandOptBool(cmd, "async") || @@ -1993,19 +2005,8 @@ cmdBlockJob(vshControl *ctl, const vshCmd *cmd) if (ret == 0 || mode != VSH_CMD_BLOCK_JOB_INFO) return true; - switch (info.type) { - case VIR_DOMAIN_BLOCK_JOB_TYPE_PULL: - type = _("Block Pull"); - break; - case VIR_DOMAIN_BLOCK_JOB_TYPE_COPY: - type = _("Block Copy"); - break; - default: - type = _("Unknown job"); - break; - } - - vshPrintJobProgress(type, info.end - info.cur, info.end); + vshPrintJobProgress(vshDomainBlockJobToString(info.type), + info.end - info.cur, info.end); if (info.bandwidth != 0) vshPrint(ctl, _(" Bandwidth limit: %lu MiB/s\n"), info.bandwidth); return true; @@ -5006,6 +5007,23 @@ static const vshCmdOptDef opts_domjobinfo[] = { {.name = NULL} }; +VIR_ENUM_DECL(vshDomainJob) +VIR_ENUM_IMPL(vshDomainJob, + VIR_DOMAIN_JOB_LAST, + N_("None"), + N_("Bounded"), + N_("Unbounded"), + N_("Completed"), + N_("Failed"), + N_("Cancelled")) + +static const char * +vshDomainJobToString(int type) +{ + const char *str = vshDomainJobTypeToString(type); + return str ? _(str) : _("unknown"); +} + static bool cmdDomjobinfo(vshControl *ctl, const vshCmd *cmd) { @@ -5068,26 +5086,18 @@ cmdDomjobinfo(vshControl *ctl, const vshCmd *cmd) if (rc < 0) goto cleanup; - vshPrint(ctl, "%-17s ", _("Job type:")); - switch (info.type) { - case VIR_DOMAIN_JOB_BOUNDED: - vshPrint(ctl, "%-12s\n", _("Bounded")); - break; - - case VIR_DOMAIN_JOB_UNBOUNDED: - vshPrint(ctl, "%-12s\n", _("Unbounded")); - break; - - case VIR_DOMAIN_JOB_NONE: - default: - vshPrint(ctl, "%-12s\n", _("None")); + vshPrint(ctl, "%-17s %-12s\n", _("Job type:"), + vshDomainJobToString(info.type)); + if (info.type != VIR_DOMAIN_JOB_BOUNDED && + info.type != VIR_DOMAIN_JOB_UNBOUNDED) { ret = true; goto cleanup; } vshPrint(ctl, "%-17s %-12llu ms\n", _("Time elapsed:"), info.timeElapsed); if (info.type == VIR_DOMAIN_JOB_BOUNDED) - vshPrint(ctl, "%-17s %-12llu ms\n", _("Time remaining:"), info.timeRemaining); + vshPrint(ctl, "%-17s %-12llu ms\n", _("Time remaining:"), + info.timeRemaining); if (info.dataTotal || info.dataRemaining || info.dataProcessed) { val = vshPrettyCapacity(info.dataProcessed, &unit); @@ -5504,7 +5514,7 @@ cmdVcpuinfo(vshControl *ctl, const vshCmd *cmd) vshPrint(ctl, "%-15s %d\n", _("VCPU:"), n); vshPrint(ctl, "%-15s %d\n", _("CPU:"), cpuinfo[n].cpu); vshPrint(ctl, "%-15s %s\n", _("State:"), - _(vshDomainVcpuStateToString(cpuinfo[n].state))); + vshDomainVcpuStateToString(cpuinfo[n].state)); if (cpuinfo[n].cpuTime != 0) { double cpuUsed = cpuinfo[n].cpuTime; @@ -10298,194 +10308,127 @@ cmdEdit(vshControl *ctl, const vshCmd *cmd) /* * "event" command */ +VIR_ENUM_DECL(vshDomainEvent) +VIR_ENUM_IMPL(vshDomainEvent, + VIR_DOMAIN_EVENT_LAST, + N_("Defined"), + N_("Undefined"), + N_("Started"), + N_("Suspended"), + N_("Resumed"), + N_("Stopped"), + N_("Shutdown"), + N_("PMSuspended"), + N_("Crashed")) + static const char * vshDomainEventToString(int event) { - const char *ret = _("unknown"); - switch ((virDomainEventType) event) { - case VIR_DOMAIN_EVENT_DEFINED: - ret = _("Defined"); - break; - case VIR_DOMAIN_EVENT_UNDEFINED: - ret = _("Undefined"); - break; - case VIR_DOMAIN_EVENT_STARTED: - ret = _("Started"); - break; - case VIR_DOMAIN_EVENT_SUSPENDED: - ret = _("Suspended"); - break; - case VIR_DOMAIN_EVENT_RESUMED: - ret = _("Resumed"); - break; - case VIR_DOMAIN_EVENT_STOPPED: - ret = _("Stopped"); - break; - case VIR_DOMAIN_EVENT_SHUTDOWN: - ret = _("Shutdown"); - break; - case VIR_DOMAIN_EVENT_PMSUSPENDED: - ret = _("PMSuspended"); - break; - case VIR_DOMAIN_EVENT_CRASHED: - ret = _("Crashed"); - break; - case VIR_DOMAIN_EVENT_LAST: - break; - } - return ret; + const char *str = vshDomainEventTypeToString(event); + return str ? _(str) : _("unknown"); } +VIR_ENUM_DECL(vshDomainEventDefined) +VIR_ENUM_IMPL(vshDomainEventDefined, + VIR_DOMAIN_EVENT_DEFINED_LAST, + N_("Added"), + N_("Updated")) + +VIR_ENUM_DECL(vshDomainEventUndefined) +VIR_ENUM_IMPL(vshDomainEventUndefined, + VIR_DOMAIN_EVENT_UNDEFINED_LAST, + N_("Removed")) + +VIR_ENUM_DECL(vshDomainEventStarted) +VIR_ENUM_IMPL(vshDomainEventStarted, + VIR_DOMAIN_EVENT_STARTED_LAST, + N_("Booted"), + N_("Migrated"), + N_("Restored"), + N_("Snapshot"), + N_("Event wakeup")) + +VIR_ENUM_DECL(vshDomainEventSuspended) +VIR_ENUM_IMPL(vshDomainEventSuspended, + VIR_DOMAIN_EVENT_SUSPENDED_LAST, + N_("Paused"), + N_("Migrated"), + N_("I/O Error"), + N_("Watchdog"), + N_("Restored"), + N_("Snapshot"), + N_("API error")) + +VIR_ENUM_DECL(vshDomainEventResumed) +VIR_ENUM_IMPL(vshDomainEventResumed, + VIR_DOMAIN_EVENT_RESUMED_LAST, + N_("Unpaused"), + N_("Migrated"), + N_("Snapshot")) + +VIR_ENUM_DECL(vshDomainEventStopped) +VIR_ENUM_IMPL(vshDomainEventStopped, + VIR_DOMAIN_EVENT_STOPPED_LAST, + N_("Shutdown"), + N_("Destroyed"), + N_("Crashed"), + N_("Migrated"), + N_("Saved"), + N_("Failed"), + N_("Snapshot")) + +VIR_ENUM_DECL(vshDomainEventShutdown) +VIR_ENUM_IMPL(vshDomainEventShutdown, + VIR_DOMAIN_EVENT_SHUTDOWN_LAST, + N_("Finished")) + +VIR_ENUM_DECL(vshDomainEventPMSuspended) +VIR_ENUM_IMPL(vshDomainEventPMSuspended, + VIR_DOMAIN_EVENT_PMSUSPENDED_LAST, + N_("Memory"), + N_("Disk")) + +VIR_ENUM_DECL(vshDomainEventCrashed) +VIR_ENUM_IMPL(vshDomainEventCrashed, + VIR_DOMAIN_EVENT_CRASHED_LAST, + N_("Panicked")) + static const char * vshDomainEventDetailToString(int event, int detail) { - const char *ret = _("unknown"); + const char *str; switch ((virDomainEventType) event) { case VIR_DOMAIN_EVENT_DEFINED: - switch ((virDomainEventDefinedDetailType) detail) { - case VIR_DOMAIN_EVENT_DEFINED_ADDED: - ret = _("Added"); - break; - case VIR_DOMAIN_EVENT_DEFINED_UPDATED: - ret = _("Updated"); - break; - case VIR_DOMAIN_EVENT_DEFINED_LAST: - break; - } + str = vshDomainEventDefinedTypeToString(detail); break; case VIR_DOMAIN_EVENT_UNDEFINED: - switch ((virDomainEventUndefinedDetailType) detail) { - case VIR_DOMAIN_EVENT_UNDEFINED_REMOVED: - ret = _("Removed"); - break; - case VIR_DOMAIN_EVENT_UNDEFINED_LAST: - break; - } + str = vshDomainEventUndefinedTypeToString(detail); break; case VIR_DOMAIN_EVENT_STARTED: - switch ((virDomainEventStartedDetailType) detail) { - case VIR_DOMAIN_EVENT_STARTED_BOOTED: - ret = _("Booted"); - break; - case VIR_DOMAIN_EVENT_STARTED_MIGRATED: - ret = _("Migrated"); - break; - case VIR_DOMAIN_EVENT_STARTED_RESTORED: - ret = _("Restored"); - break; - case VIR_DOMAIN_EVENT_STARTED_FROM_SNAPSHOT: - ret = _("Snapshot"); - break; - case VIR_DOMAIN_EVENT_STARTED_WAKEUP: - ret = _("Event wakeup"); - break; - case VIR_DOMAIN_EVENT_STARTED_LAST: - break; - } + str = vshDomainEventStartedTypeToString(detail); break; case VIR_DOMAIN_EVENT_SUSPENDED: - switch ((virDomainEventSuspendedDetailType) detail) { - case VIR_DOMAIN_EVENT_SUSPENDED_PAUSED: - ret = _("Paused"); - break; - case VIR_DOMAIN_EVENT_SUSPENDED_MIGRATED: - ret = _("Migrated"); - break; - case VIR_DOMAIN_EVENT_SUSPENDED_IOERROR: - ret = _("I/O Error"); - break; - case VIR_DOMAIN_EVENT_SUSPENDED_WATCHDOG: - ret = _("Watchdog"); - break; - case VIR_DOMAIN_EVENT_SUSPENDED_RESTORED: - ret = _("Restored"); - break; - case VIR_DOMAIN_EVENT_SUSPENDED_FROM_SNAPSHOT: - ret = _("Snapshot"); - break; - case VIR_DOMAIN_EVENT_SUSPENDED_API_ERROR: - ret = _("API error"); - break; - case VIR_DOMAIN_EVENT_SUSPENDED_LAST: - break; - } + str = vshDomainEventSuspendedTypeToString(detail); break; case VIR_DOMAIN_EVENT_RESUMED: - switch ((virDomainEventResumedDetailType) detail) { - case VIR_DOMAIN_EVENT_RESUMED_UNPAUSED: - ret = _("Unpaused"); - break; - case VIR_DOMAIN_EVENT_RESUMED_MIGRATED: - ret = _("Migrated"); - break; - case VIR_DOMAIN_EVENT_RESUMED_FROM_SNAPSHOT: - ret = _("Snapshot"); - break; - case VIR_DOMAIN_EVENT_RESUMED_LAST: - break; - } + str = vshDomainEventResumedTypeToString(detail); break; case VIR_DOMAIN_EVENT_STOPPED: - switch ((virDomainEventStoppedDetailType) detail) { - case VIR_DOMAIN_EVENT_STOPPED_SHUTDOWN: - ret = _("Shutdown"); - break; - case VIR_DOMAIN_EVENT_STOPPED_DESTROYED: - ret = _("Destroyed"); - break; - case VIR_DOMAIN_EVENT_STOPPED_CRASHED: - ret = _("Crashed"); - break; - case VIR_DOMAIN_EVENT_STOPPED_MIGRATED: - ret = _("Migrated"); - break; - case VIR_DOMAIN_EVENT_STOPPED_SAVED: - ret = _("Saved"); - break; - case VIR_DOMAIN_EVENT_STOPPED_FAILED: - ret = _("Failed"); - break; - case VIR_DOMAIN_EVENT_STOPPED_FROM_SNAPSHOT: - ret = _("Snapshot"); - break; - case VIR_DOMAIN_EVENT_STOPPED_LAST: - break; - } + str = vshDomainEventStoppedTypeToString(detail); break; case VIR_DOMAIN_EVENT_SHUTDOWN: - switch ((virDomainEventShutdownDetailType) detail) { - case VIR_DOMAIN_EVENT_SHUTDOWN_FINISHED: - ret = _("Finished"); - break; - case VIR_DOMAIN_EVENT_SHUTDOWN_LAST: - break; - } + str = vshDomainEventShutdownTypeToString(detail); break; case VIR_DOMAIN_EVENT_PMSUSPENDED: - switch ((virDomainEventPMSuspendedDetailType) detail) { - case VIR_DOMAIN_EVENT_PMSUSPENDED_MEMORY: - ret = _("Memory"); - break; - case VIR_DOMAIN_EVENT_PMSUSPENDED_DISK: - ret = _("Disk"); - break; - case VIR_DOMAIN_EVENT_PMSUSPENDED_LAST: - break; - } + str = vshDomainEventPMSuspendedTypeToString(detail); break; case VIR_DOMAIN_EVENT_CRASHED: - switch ((virDomainEventCrashedDetailType) detail) { - case VIR_DOMAIN_EVENT_CRASHED_PANICKED: - ret = _("Panicked"); - break; - case VIR_DOMAIN_EVENT_CRASHED_LAST: - break; - } + str = vshDomainEventCrashedTypeToString(detail); break; case VIR_DOMAIN_EVENT_LAST: break; } - return ret; + return str ? _(str) : _("unknown"); } struct vshDomEventData { @@ -10496,8 +10439,8 @@ struct vshDomEventData { typedef struct vshDomEventData vshDomEventData; /* FIXME: Support all callbacks, not just lifecycle */ -VIR_ENUM_DECL(vshDomainEvent) -VIR_ENUM_IMPL(vshDomainEvent, +VIR_ENUM_DECL(vshDomainEventId) +VIR_ENUM_IMPL(vshDomainEventId, /* VIR_DOMAIN_EVENT_ID_LAST, */ 1, "lifecycle") @@ -10569,7 +10512,7 @@ cmdEvent(vshControl *ctl, const vshCmd *cmd) size_t i; for (i = 0; i < 1 /* VIR_DOMAIN_EVENT_ID_LAST */; i++) - vshPrint(ctl, "%s\n", vshDomainEventTypeToString(i)); + vshPrint(ctl, "%s\n", vshDomainEventIdTypeToString(i)); return true; } @@ -10579,7 +10522,7 @@ cmdEvent(vshControl *ctl, const vshCmd *cmd) vshError(ctl, "%s", _("either --list or event type is required")); return false; } - if ((event = vshDomainEventTypeFromString(eventName) < 0)) { + if ((event = vshDomainEventIdTypeFromString(eventName) < 0)) { vshError(ctl, _("unknown event type %s"), eventName); return false; } diff --git a/tools/virsh-network.c b/tools/virsh-network.c index 4377391..22d21e7 100644 --- a/tools/virsh-network.c +++ b/tools/virsh-network.c @@ -1134,27 +1134,19 @@ cmdNetworkEdit(vshControl *ctl, const vshCmd *cmd) /* * "net-event" command */ +VIR_ENUM_DECL(vshNetworkEvent) +VIR_ENUM_IMPL(vshNetworkEvent, + VIR_NETWORK_EVENT_LAST, + N_("Defined"), + N_("Undefined"), + N_("Started"), + N_("Stopped")) + static const char * vshNetworkEventToString(int event) { - const char *ret = _("unknown"); - switch ((virNetworkEventLifecycleType) event) { - case VIR_NETWORK_EVENT_DEFINED: - ret = _("Defined"); - break; - case VIR_NETWORK_EVENT_UNDEFINED: - ret = _("Undefined"); - break; - case VIR_NETWORK_EVENT_STARTED: - ret = _("Started"); - break; - case VIR_NETWORK_EVENT_STOPPED: - ret = _("Stopped"); - break; - case VIR_NETWORK_EVENT_LAST: - break; - } - return ret; + const char *str = vshNetworkEventTypeToString(event); + return str ? _(str) : _("unknown"); } struct vshNetEventData { @@ -1164,8 +1156,8 @@ struct vshNetEventData { }; typedef struct vshNetEventData vshNetEventData; -VIR_ENUM_DECL(vshNetworkEvent) -VIR_ENUM_IMPL(vshNetworkEvent, +VIR_ENUM_DECL(vshNetworkEventId) +VIR_ENUM_IMPL(vshNetworkEventId, VIR_NETWORK_EVENT_ID_LAST, "lifecycle") @@ -1236,7 +1228,7 @@ cmdNetworkEvent(vshControl *ctl, const vshCmd *cmd) size_t i; for (i = 0; i < VIR_NETWORK_EVENT_ID_LAST; i++) - vshPrint(ctl, "%s\n", vshNetworkEventTypeToString(i)); + vshPrint(ctl, "%s\n", vshNetworkEventIdTypeToString(i)); return true; } @@ -1246,7 +1238,7 @@ cmdNetworkEvent(vshControl *ctl, const vshCmd *cmd) vshError(ctl, "%s", _("either --list or event type is required")); return false; } - if ((event = vshNetworkEventTypeFromString(eventName) < 0)) { + if ((event = vshNetworkEventIdTypeFromString(eventName) < 0)) { vshError(ctl, _("unknown event type %s"), eventName); return false; } diff --git a/tools/virsh-pool.c b/tools/virsh-pool.c index ac645ea..ec99564 100644 --- a/tools/virsh-pool.c +++ b/tools/virsh-pool.c @@ -1,7 +1,7 @@ /* * virsh-pool.c: Commands to manage storage pool * - * Copyright (C) 2005, 2007-2013 Red Hat, Inc. + * Copyright (C) 2005, 2007-2014 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -890,6 +890,24 @@ cleanup: return list; } + +VIR_ENUM_DECL(vshStoragePoolState) +VIR_ENUM_IMPL(vshStoragePoolState, + VIR_STORAGE_POOL_STATE_LAST, + N_("inactive"), + N_("building"), + N_("running"), + N_("degraded"), + N_("inaccessible")) + +static const char * +vshStoragePoolStateToString(int state) +{ + const char *str = vshStoragePoolStateTypeToString(state); + return str ? _(str) : _("unknown"); +} + + /* * "pool-list" command */ @@ -1093,25 +1111,9 @@ cmdPoolList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) } else { /* Decide which state string to display */ if (details) { - /* --details option was specified, we're using detailed state - * strings */ - switch (info.state) { - case VIR_STORAGE_POOL_INACTIVE: - poolInfoTexts[i].state = vshStrdup(ctl, _("inactive")); - break; - case VIR_STORAGE_POOL_BUILDING: - poolInfoTexts[i].state = vshStrdup(ctl, _("building")); - break; - case VIR_STORAGE_POOL_RUNNING: - poolInfoTexts[i].state = vshStrdup(ctl, _("running")); - break; - case VIR_STORAGE_POOL_DEGRADED: - poolInfoTexts[i].state = vshStrdup(ctl, _("degraded")); - break; - case VIR_STORAGE_POOL_INACCESSIBLE: - poolInfoTexts[i].state = vshStrdup(ctl, _("inaccessible")); - break; - } + const char *state = vshStoragePoolStateToString(info.state); + + poolInfoTexts[i].state = vshStrdup(ctl, state); /* Create the pool size related strings */ if (info.state == VIR_STORAGE_POOL_RUNNING || @@ -1525,28 +1527,8 @@ cmdPoolInfo(vshControl *ctl, const vshCmd *cmd) if (virStoragePoolGetInfo(pool, &info) == 0) { double val; const char *unit; - switch (info.state) { - case VIR_STORAGE_POOL_INACTIVE: - vshPrint(ctl, "%-15s %s\n", _("State:"), - _("inactive")); - break; - case VIR_STORAGE_POOL_BUILDING: - vshPrint(ctl, "%-15s %s\n", _("State:"), - _("building")); - break; - case VIR_STORAGE_POOL_RUNNING: - vshPrint(ctl, "%-15s %s\n", _("State:"), - _("running")); - break; - case VIR_STORAGE_POOL_DEGRADED: - vshPrint(ctl, "%-15s %s\n", _("State:"), - _("degraded")); - break; - case VIR_STORAGE_POOL_INACCESSIBLE: - vshPrint(ctl, "%-15s %s\n", _("State:"), - _("inaccessible")); - break; - } + vshPrint(ctl, "%-15s %s\n", _("State:"), + vshStoragePoolStateToString(info.state)); /* Check and display whether the pool is persistent or not */ persistent = virStoragePoolIsPersistent(pool); diff --git a/tools/virsh-volume.c b/tools/virsh-volume.c index 22b10d5..839a17c 100644 --- a/tools/virsh-volume.c +++ b/tools/virsh-volume.c @@ -1,7 +1,7 @@ /* * virsh-volume.c: Commands to manage storage volume * - * Copyright (C) 2005, 2007-2013 Red Hat, Inc. + * Copyright (C) 2005, 2007-2014 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -944,30 +944,20 @@ out: } +VIR_ENUM_DECL(vshStorageVol) +VIR_ENUM_IMPL(vshStorageVol, + VIR_STORAGE_VOL_LAST, + N_("file"), + N_("block"), + N_("dir"), + N_("network"), + N_("netdir")) + static const char * vshVolumeTypeToString(int type) { - switch ((virStorageVolType) type) { - case VIR_STORAGE_VOL_FILE: - return N_("file"); - - case VIR_STORAGE_VOL_BLOCK: - return N_("block"); - - case VIR_STORAGE_VOL_DIR: - return N_("dir"); - - case VIR_STORAGE_VOL_NETWORK: - return N_("network"); - - case VIR_STORAGE_VOL_NETDIR: - return N_("netdir"); - - case VIR_STORAGE_VOL_LAST: - break; - } - - return N_("unknown"); + const char *str = vshStorageVolTypeToString(type); + return str ? _(str) : _("unknown"); } @@ -1014,7 +1004,7 @@ cmdVolInfo(vshControl *ctl, const vshCmd *cmd) const char *unit; vshPrint(ctl, "%-15s %s\n", _("Type:"), - _(vshVolumeTypeToString(info.type))); + vshVolumeTypeToString(info.type)); val = vshPrettyCapacity(info.capacity, &unit); vshPrint(ctl, "%-15s %2.2lf %s\n", _("Capacity:"), val, unit); @@ -1390,7 +1380,7 @@ cmdVolList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) /* Volume type */ volInfoTexts[i].type = vshStrdup(ctl, - _(vshVolumeTypeToString(volumeInfo.type))); + vshVolumeTypeToString(volumeInfo.type)); /* Create the capacity output string */ val = vshPrettyCapacity(volumeInfo.capacity, &unit); -- 1.8.5.3
On 21.02.2014 23:59, Eric Blake wrote:
Dan Berrange suggested that using VIR_ENUM_IMPL is more compact than open-coding switch statements, and still just as forceful at making us remember to update lists if we add enum values in the future. Make this change throughout virsh.
Sure enough, doing this change caught that we missed at least VIR_STORAGE_VOL_NETDIR.
* tools/virsh-domain-monitor.c (vshDomainIOErrorToString) (vshDomainControlStateToString, vshDomainStateToString) (vshDomainStateReasonToString): Change switch to enum lookup. (cmdDomControl, cmdDominfo): Update caller. * tools/virsh-domain.c (vshDomainVcpuStateToString) (vshDomainEventToString, vshDomainEventDetailToString): Change switch to enum lookup. (vshDomainBlockJobToString, vshDomainJobToString): New functions. (cmdVcpuinfo, cmdBlockJob, cmdDomjobinfo, cmdEvent): Update callers. * tools/virsh-network.c (vshNetworkEventToString): Change switch to enum lookup. * tools/virsh-pool.c (vshStoragePoolStateToString): New function. (cmdPoolList, cmdPoolInfo): Update callers. * tools/virsh-volume.c (vshVolumeTypeToString): Change switch to enum lookup. (cmdVolInfo, cmdVolList): Update callers.
Signed-off-by: Eric Blake <eblake@redhat.com> --- tools/virsh-domain-monitor.c | 280 +++++++++++++++------------------- tools/virsh-domain.c | 347 ++++++++++++++++++------------------------- tools/virsh-network.c | 36 ++--- tools/virsh-pool.c | 66 +++----- tools/virsh-volume.c | 38 ++--- 5 files changed, 316 insertions(+), 451 deletions(-)
diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c index 6291ca5..c67b833 100644 --- a/tools/virsh-domain-monitor.c +++ b/tools/virsh-domain-monitor.c @@ -1,7 +1,7 @@ /* * virsh-domain-monitor.c: Commands to monitor domain status * - * Copyright (C) 2005, 2007-2013 Red Hat, Inc. + * Copyright (C) 2005, 2007-2014 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -40,21 +40,18 @@ #include "virxml.h" #include "virstring.h"
+VIR_ENUM_DECL(vshDomainIOError) +VIR_ENUM_IMPL(vshDomainIOError, + VIR_DOMAIN_DISK_ERROR_LAST, + N_("no error"), + N_("unspecified error"), + N_("no space")) + static const char * vshDomainIOErrorToString(int error) { - switch ((virDomainDiskErrorCode) error) { - case VIR_DOMAIN_DISK_ERROR_NONE: - return _("no error"); - case VIR_DOMAIN_DISK_ERROR_UNSPEC: - return _("unspecified error"); - case VIR_DOMAIN_DISK_ERROR_NO_SPACE: - return _("no space"); - case VIR_DOMAIN_DISK_ERROR_LAST: - ; - } - - return _("unknown error"); + const char *str = vshDomainIOErrorTypeToString(error); + return str ? _(str) : _("unknown error"); }
Why _(str) if str itself already contains translated message? Michal
On 02/24/2014 10:42 AM, Michal Privoznik wrote:
+VIR_ENUM_DECL(vshDomainIOError) +VIR_ENUM_IMPL(vshDomainIOError, + VIR_DOMAIN_DISK_ERROR_LAST, + N_("no error"), + N_("unspecified error"), + N_("no space")) +
- return _("unknown error"); + const char *str = vshDomainIOErrorTypeToString(error); + return str ? _(str) : _("unknown error"); }
Why _(str) if str itself already contains translated message?
str is NOT translated. C semantics forbid initializing something with a function call, and VIR_ENUM_IMPL is creating an initializer. N_() is a markup that is a no-op to the C compiler (so you aren't violating initializer rules) while still marking a string for translation for the purposes of xgettext scanning; we still have to actually translate the string at some point down the road. Hence, my call to _(str) - where we are translating a string, but where xgettext sees that it is a variable rather than a string literal and so has nothing it can stick in the .po file. You need both halves for translating strings that are stored in an initializer list. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 24.02.2014 18:49, Eric Blake wrote:
On 02/24/2014 10:42 AM, Michal Privoznik wrote:
+VIR_ENUM_DECL(vshDomainIOError) +VIR_ENUM_IMPL(vshDomainIOError, + VIR_DOMAIN_DISK_ERROR_LAST, + N_("no error"), + N_("unspecified error"), + N_("no space")) +
- return _("unknown error"); + const char *str = vshDomainIOErrorTypeToString(error); + return str ? _(str) : _("unknown error"); }
Why _(str) if str itself already contains translated message?
str is NOT translated. C semantics forbid initializing something with a function call, and VIR_ENUM_IMPL is creating an initializer. N_() is a markup that is a no-op to the C compiler (so you aren't violating initializer rules) while still marking a string for translation for the purposes of xgettext scanning; we still have to actually translate the string at some point down the road. Hence, my call to _(str) - where we are translating a string, but where xgettext sees that it is a variable rather than a string literal and so has nothing it can stick in the .po file. You need both halves for translating strings that are stored in an initializer list.
I see. ACK then. Michal
I noticed this while shortning switch statements via VIR_ENUM. Basically, the only ways virAsprintf can fail are if we pass a bogus format string (but we're not THAT bad) or if we run out of memory (but it already warns on our behalf in that case). Throw away the cruft that tries too hard to diagnose a printf failure. * tools/virsh-volume.c (cmdVolList): Simplify. * tools/virsh-pool.c (cmdPoolList): Likewise. Signed-off-by: Eric Blake <eblake@redhat.com> --- tools/virsh-pool.c | 73 +++++++++++++++++----------------------------------- tools/virsh-volume.c | 66 ++++++++++++++--------------------------------- 2 files changed, 42 insertions(+), 97 deletions(-) diff --git a/tools/virsh-pool.c b/tools/virsh-pool.c index ec99564..b21b682 100644 --- a/tools/virsh-pool.c +++ b/tools/virsh-pool.c @@ -961,9 +961,8 @@ static bool cmdPoolList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) { virStoragePoolInfo info; - int ret; size_t i; - bool functionReturn = false; + bool ret = false; size_t stringLength = 0, nameStrLength = 0; size_t autostartStrLength = 0, persistStrLength = 0; size_t stateStrLength = 0, capStrLength = 0; @@ -1121,29 +1120,20 @@ cmdPoolList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) double val; const char *unit; - /* Create the capacity output string */ val = vshPrettyCapacity(info.capacity, &unit); - ret = virAsprintf(&poolInfoTexts[i].capacity, - "%.2lf %s", val, unit); - if (ret < 0) - /* An error occurred creating the string, return */ - goto asprintf_failure; + if (virAsprintf(&poolInfoTexts[i].capacity, + "%.2lf %s", val, unit) < 0) + goto cleanup; - /* Create the allocation output string */ val = vshPrettyCapacity(info.allocation, &unit); - ret = virAsprintf(&poolInfoTexts[i].allocation, - "%.2lf %s", val, unit); - if (ret < 0) - /* An error occurred creating the string, return */ - goto asprintf_failure; + if (virAsprintf(&poolInfoTexts[i].allocation, + "%.2lf %s", val, unit) < 0) + goto cleanup; - /* Create the available space output string */ val = vshPrettyCapacity(info.available, &unit); - ret = virAsprintf(&poolInfoTexts[i].available, - "%.2lf %s", val, unit); - if (ret < 0) - /* An error occurred creating the string, return */ - goto asprintf_failure; + if (virAsprintf(&poolInfoTexts[i].available, + "%.2lf %s", val, unit) < 0) + goto cleanup; } else { /* Capacity related information isn't available */ poolInfoTexts[i].capacity = vshStrdup(ctl, _("-")); @@ -1213,7 +1203,7 @@ cmdPoolList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) } /* Cleanup and return */ - functionReturn = true; + ret = true; goto cleanup; } @@ -1273,19 +1263,16 @@ cmdPoolList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) /* Create the output template. Each column is sized according to * the longest string. */ - ret = virAsprintf(&outputStr, - " %%-%lus %%-%lus %%-%lus %%-%lus %%%lus %%%lus %%%lus\n", - (unsigned long) nameStrLength, - (unsigned long) stateStrLength, - (unsigned long) autostartStrLength, - (unsigned long) persistStrLength, - (unsigned long) capStrLength, - (unsigned long) allocStrLength, - (unsigned long) availStrLength); - if (ret < 0) { - /* An error occurred creating the string, return */ - goto asprintf_failure; - } + if (virAsprintf(&outputStr, + " %%-%lus %%-%lus %%-%lus %%-%lus %%%lus %%%lus %%%lus\n", + (unsigned long) nameStrLength, + (unsigned long) stateStrLength, + (unsigned long) autostartStrLength, + (unsigned long) persistStrLength, + (unsigned long) capStrLength, + (unsigned long) allocStrLength, + (unsigned long) availStrLength) < 0) + goto cleanup; /* Display the header */ vshPrint(ctl, outputStr, _("Name"), _("State"), _("Autostart"), @@ -1310,21 +1297,7 @@ cmdPoolList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) } /* Cleanup and return */ - functionReturn = true; - goto cleanup; - -asprintf_failure: - /* Display an appropriate error message then cleanup and return */ - switch (errno) { - case ENOMEM: - /* Couldn't allocate memory */ - vshError(ctl, "%s", _("Out of memory")); - break; - default: - /* Some other error */ - vshError(ctl, _("virAsprintf failed (errno %d)"), errno); - } - functionReturn = false; + ret = true; cleanup: VIR_FREE(outputStr); @@ -1341,7 +1314,7 @@ cleanup: VIR_FREE(poolInfoTexts); vshStoragePoolListFree(list); - return functionReturn; + return ret; } /* diff --git a/tools/virsh-volume.c b/tools/virsh-volume.c index 839a17c..898b34b 100644 --- a/tools/virsh-volume.c +++ b/tools/virsh-volume.c @@ -1332,8 +1332,7 @@ cmdVolList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) double val; bool details = vshCommandOptBool(cmd, "details"); size_t i; - int ret; - bool functionReturn = false; + bool ret = false; int stringLength = 0; size_t allocStrLength = 0, capStrLength = 0; size_t nameStrLength = 0, pathStrLength = 0; @@ -1382,23 +1381,15 @@ cmdVolList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) volInfoTexts[i].type = vshStrdup(ctl, vshVolumeTypeToString(volumeInfo.type)); - /* Create the capacity output string */ val = vshPrettyCapacity(volumeInfo.capacity, &unit); - ret = virAsprintf(&volInfoTexts[i].capacity, - "%.2lf %s", val, unit); - if (ret < 0) { - /* An error occurred creating the string, return */ - goto asprintf_failure; - } - - /* Create the allocation output string */ + if (virAsprintf(&volInfoTexts[i].capacity, + "%.2lf %s", val, unit) < 0) + goto cleanup; + val = vshPrettyCapacity(volumeInfo.allocation, &unit); - ret = virAsprintf(&volInfoTexts[i].allocation, - "%.2lf %s", val, unit); - if (ret < 0) { - /* An error occurred creating the string, return */ - goto asprintf_failure; - } + if (virAsprintf(&volInfoTexts[i].allocation, + "%.2lf %s", val, unit) < 0) + goto cleanup; } /* Remember the largest length for each output string. @@ -1450,7 +1441,7 @@ cmdVolList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) } /* Cleanup and return */ - functionReturn = true; + ret = true; goto cleanup; } @@ -1493,18 +1484,14 @@ cmdVolList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) vshDebug(ctl, VSH_ERR_DEBUG, "Longest allocation string = %zu chars\n", allocStrLength); - /* Create the output template */ - ret = virAsprintf(&outputStr, - " %%-%lus %%-%lus %%-%lus %%%lus %%%lus\n", - (unsigned long) nameStrLength, - (unsigned long) pathStrLength, - (unsigned long) typeStrLength, - (unsigned long) capStrLength, - (unsigned long) allocStrLength); - if (ret < 0) { - /* An error occurred creating the string, return */ - goto asprintf_failure; - } + if (virAsprintf(&outputStr, + " %%-%lus %%-%lus %%-%lus %%%lus %%%lus\n", + (unsigned long) nameStrLength, + (unsigned long) pathStrLength, + (unsigned long) typeStrLength, + (unsigned long) capStrLength, + (unsigned long) allocStrLength) < 0) + goto cleanup; /* Display the header */ vshPrint(ctl, outputStr, _("Name"), _("Path"), _("Type"), @@ -1526,22 +1513,7 @@ cmdVolList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) } /* Cleanup and return */ - functionReturn = true; - goto cleanup; - -asprintf_failure: - - /* Display an appropriate error message then cleanup and return */ - switch (errno) { - case ENOMEM: - /* Couldn't allocate memory */ - vshError(ctl, "%s", _("Out of memory")); - break; - default: - /* Some other error */ - vshError(ctl, _("virAsprintf failed (errno %d)"), errno); - } - functionReturn = false; + ret = true; cleanup: @@ -1563,7 +1535,7 @@ cleanup: vshStorageVolListFree(list); /* Return the desired value */ - return functionReturn; + return ret; } /* -- 1.8.5.3
On 21.02.2014 23:59, Eric Blake wrote:
I noticed this while shortning switch statements via VIR_ENUM. Basically, the only ways virAsprintf can fail are if we pass a bogus format string (but we're not THAT bad) or if we run out of memory (but it already warns on our behalf in that case). Throw away the cruft that tries too hard to diagnose a printf failure.
* tools/virsh-volume.c (cmdVolList): Simplify. * tools/virsh-pool.c (cmdPoolList): Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com> --- tools/virsh-pool.c | 73 +++++++++++++++++----------------------------------- tools/virsh-volume.c | 66 ++++++++++++++--------------------------------- 2 files changed, 42 insertions(+), 97 deletions(-)
ACK Michal
On 02/24/2014 10:42 AM, Michal Privoznik wrote:
On 21.02.2014 23:59, Eric Blake wrote:
I noticed this while shortning switch statements via VIR_ENUM. Basically, the only ways virAsprintf can fail are if we pass a bogus format string (but we're not THAT bad) or if we run out of memory (but it already warns on our behalf in that case). Throw away the cruft that tries too hard to diagnose a printf failure.
* tools/virsh-volume.c (cmdVolList): Simplify. * tools/virsh-pool.c (cmdPoolList): Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com> --- tools/virsh-pool.c | 73 +++++++++++++++++----------------------------------- tools/virsh-volume.c | 66 ++++++++++++++--------------------------------- 2 files changed, 42 insertions(+), 97 deletions(-)
ACK
Thanks; I pushed the series. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (2)
-
Eric Blake -
Michal Privoznik