On 17.12.2015 17:55, John Ferlan wrote:
As a follow on to a recent series which created macros for various virsh-*.c command options, see:
http://www.redhat.com/archives/libvir-list/2015-December/msg00672.html
(although best to view by index since there are varying opinions on the matter)...
Modify virsh-domain.c and virsh-volume.c to "follow" the concept of macro-ifying some commonly used options. In order to choose, I used:
$ grep "{.name =" tools/virsh-*.c | \ grep -v "help" | grep -v "desc" | grep -v NULL | \ sort -i | uniq -c | sort -n
To generate a list by module. That generated a list where I randomly picked 9 as the nexus, leaving the following:
9 tools/virsh-domain.c: {.name = "persistent", 10 tools/virsh-snapshot.c: {.name = "domain", 12 tools/virsh-network.c: {.name = "network", 12 tools/virsh-pool.c: {.name = "pool", 13 tools/virsh-volume.c: {.name = "vol", 14 tools/virsh-domain.c: {.name = "file", 14 tools/virsh-domain-monitor.c: {.name = "domain", 14 tools/virsh-volume.c: {.name = "pool", 26 tools/virsh-domain.c: {.name = "current", 28 tools/virsh-domain.c: {.name = "config", 28 tools/virsh-domain.c: {.name = "live", 81 tools/virsh-domain.c: {.name = "domain",
Because virsh-domain-monitor.c, virsh-network.c, and virsh-snapshot.c only had one element with larger counts - I just left them as is and focused on the virsh-domain.c and virsh-volume.c.
The perhaps more controversial choice will be the "file" option in virsh-domain.c which had numerous different helpstr values. I chose to macro-ify them using the entire helpstr including the N_("xxx") rather than just "xxx" since the N_ is the I18N marker.
John Ferlan (8): virsh: Create macro for common "domain" option virsh: Create macro for common "persistent" option virsh: Create macro for common "config" option virsh: Create macro for common "live" option virsh: Create macro for common "current" option virsh: Create macro for common "file" option virsh: Create macros for common "pool" options virsh: Create macros for common "vol" options
tools/virsh-domain.c | 918 +++++++++++---------------------------------------- tools/virsh-volume.c | 155 +++------ 2 files changed, 247 insertions(+), 826 deletions(-)
Apart from Erik's suggestion, I wonder if we should put those definitions into a virsh-wide header so that we don't have to repeat them. For instance, plenty of commands in virsh-domain-monitor.c have --domain too. Other than that this looks very good. Michal