9:38 a.m.
*** BLURB HERE ***
Michal Prívozník (2):
virsh: Simplify vshTableRowAppend() calling in cmdList(), part one
virsh: Simplify vshTableRowAppend() calling in cmdList(), part two
tools/virsh-domain-monitor.c | 52 +++++++++++++++---------------------
1 file changed, 21 insertions(+), 31 deletions(-)
--
2.44.2
9:38 a.m.
New subject: [PATCH 1/2] virsh: Simplify vshTableRowAppend() calling in cmdList(), part one
All calls to vshTableRowAppend() inside of cmdList() share couple
of same arguments: domain ID, domain name and domain state. While
the first one is stored in a variable and then passed to all
vshTableRowAppend() calls, the others are passed as a function
call. Switch the latter to variables too.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
tools/virsh-domain-monitor.c | 26 ++++++++++++--------------
1 file changed, 12 insertions(+), 14 deletions(-)
diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
index 37184baa69..80b6857fad 100644
--- a/tools/virsh-domain-monitor.c
+++ b/tools/virsh-domain-monitor.c
@@ -1889,6 +1889,9 @@ cmdList(vshControl *ctl, const vshCmd *cmd)
ignore_value(virStrcpyStatic(id_buf, "-"));
if (optTable) {
+ const char *domName = virDomainGetName(dom);
+ const char *stateStr = NULL;
+
state = virshDomainState(ctl, dom, NULL);
/* Domain could've been removed in the meantime */
@@ -1896,8 +1899,11 @@ cmdList(vshControl *ctl, const vshCmd *cmd)
continue;
if (managed && state == VIR_DOMAIN_SHUTOFF &&
- virDomainHasManagedSaveImage(dom, 0) > 0)
- state = -2;
+ virDomainHasManagedSaveImage(dom, 0) > 0) {
+ stateStr = _("saved");
+ } else {
+ stateStr = virshDomainStateToString(state);
+ }
if (optTitle && !optUUID) {
g_autofree char *title = NULL;
@@ -1905,9 +1911,7 @@ cmdList(vshControl *ctl, const vshCmd *cmd)
if (!(title = virshGetDomainDescription(ctl, dom, true, 0)))
goto cleanup;
if (vshTableRowAppend(table, id_buf,
- virDomainGetName(dom),
- state == -2 ? _("saved")
- : virshDomainStateToString(state),
+ domName, stateStr,
title, NULL) < 0)
goto cleanup;
} else if (optUUID && !optTitle) {
@@ -1916,9 +1920,7 @@ cmdList(vshControl *ctl, const vshCmd *cmd)
goto cleanup;
}
if (vshTableRowAppend(table, id_buf,
- virDomainGetName(dom),
- state == -2 ? _("saved")
- : virshDomainStateToString(state),
+ domName, stateStr,
uuid, NULL) < 0)
goto cleanup;
} else if (optUUID && optTitle) {
@@ -1931,16 +1933,12 @@ cmdList(vshControl *ctl, const vshCmd *cmd)
goto cleanup;
}
if (vshTableRowAppend(table, id_buf,
- virDomainGetName(dom),
- state == -2 ? _("saved")
- : virshDomainStateToString(state),
+ domName, stateStr,
title, uuid, NULL) < 0)
goto cleanup;
} else {
if (vshTableRowAppend(table, id_buf,
- virDomainGetName(dom),
- state == -2 ? _("saved")
- : virshDomainStateToString(state),
+ domName, stateStr,
NULL) < 0)
goto cleanup;
}
--
2.44.2
10:27 a.m.
New subject: [PATCH 1/2] virsh: Simplify vshTableRowAppend() calling in
cmdList(), part one
On Mon, Aug 19, 2024 at 16:38:27 +0200, Michal Privoznik wrote:
All calls to vshTableRowAppend() inside of cmdList() share couple
of same arguments: domain ID, domain name and domain state. While
the first one is stored in a variable and then passed to all
vshTableRowAppend() calls, the others are passed as a function
call. Switch the latter to variables too.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
tools/virsh-domain-monitor.c | 26 ++++++++++++--------------
1 file changed, 12 insertions(+), 14 deletions(-)
Reviewed-by: Peter Krempa <pkrempa(a)redhat.com>
9:38 a.m.
New subject: [PATCH 2/2] virsh: Simplify vshTableRowAppend() calling in cmdList(), part two
Instead of having many if-else statements, each with its own
vshTableRowAppend() call, we can use a simple trick - have an
array of string pointers, set array members in the if bodies and
then call vshTableRowAppend() once.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
tools/virsh-domain-monitor.c | 34 +++++++++++++---------------------
1 file changed, 13 insertions(+), 21 deletions(-)
diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
index 80b6857fad..87efd86a69 100644
--- a/tools/virsh-domain-monitor.c
+++ b/tools/virsh-domain-monitor.c
@@ -1891,6 +1891,8 @@ cmdList(vshControl *ctl, const vshCmd *cmd)
if (optTable) {
const char *domName = virDomainGetName(dom);
const char *stateStr = NULL;
+ g_autofree char *title = NULL;
+ const char *arg[2] = {};
state = virshDomainState(ctl, dom, NULL);
@@ -1906,43 +1908,33 @@ cmdList(vshControl *ctl, const vshCmd *cmd)
}
if (optTitle && !optUUID) {
- g_autofree char *title = NULL;
-
if (!(title = virshGetDomainDescription(ctl, dom, true, 0)))
goto cleanup;
- if (vshTableRowAppend(table, id_buf,
- domName, stateStr,
- title, NULL) < 0)
- goto cleanup;
+
+ arg[0] = title;
} else if (optUUID && !optTitle) {
if (virDomainGetUUIDString(dom, uuid) < 0) {
vshError(ctl, "%s", _("Failed to get domain's
UUID"));
goto cleanup;
}
- if (vshTableRowAppend(table, id_buf,
- domName, stateStr,
- uuid, NULL) < 0)
- goto cleanup;
+
+ arg[0] = uuid;
} else if (optUUID && optTitle) {
- g_autofree char *title = NULL;
-
if (!(title = virshGetDomainDescription(ctl, dom, true, 0)))
goto cleanup;
if (virDomainGetUUIDString(dom, uuid) < 0) {
vshError(ctl, "%s", _("Failed to get domain's
UUID"));
goto cleanup;
}
- if (vshTableRowAppend(table, id_buf,
- domName, stateStr,
- title, uuid, NULL) < 0)
- goto cleanup;
- } else {
- if (vshTableRowAppend(table, id_buf,
- domName, stateStr,
- NULL) < 0)
- goto cleanup;
+
+ arg[0] = title;
+ arg[1] = uuid;
}
+ if (vshTableRowAppend(table, id_buf,
+ domName, stateStr,
+ arg[0], arg[1], NULL) < 0)
+ goto cleanup;
} else {
if (optUUID) {
if (virDomainGetUUIDString(dom, uuid) < 0) {
--
2.44.2
10:35 a.m.
New subject: [PATCH 2/2] virsh: Simplify vshTableRowAppend() calling in
cmdList(), part two
On Mon, Aug 19, 2024 at 16:38:28 +0200, Michal Privoznik wrote:
Instead of having many if-else statements, each with its own
vshTableRowAppend() call, we can use a simple trick - have an
array of string pointers, set array members in the if bodies and
then call vshTableRowAppend() once.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
tools/virsh-domain-monitor.c | 34 +++++++++++++---------------------
1 file changed, 13 insertions(+), 21 deletions(-)
[...]
+ if (vshTableRowAppend(table, id_buf,
+ domName, stateStr,
+ arg[0], arg[1], NULL) < 0)
+ goto cleanup;
While this works for this case when the arguments are at the end it will
not work in others.
When I've recently looked at that patch that motivated you to do this I
thought it would be cool if vshTable would have a visibility flag for
each column. That way you could always fill all of them and don't have
to deal with such scenarios.
} else {
if (optUUID) {
if (virDomainGetUUIDString(dom, uuid) < 0) {
anyways ...
Reviewed-by: Peter Krempa <pkrempa(a)redhat.com>
96
days inactive
96
days old
4 comments
2 participants
participants (2)
-
Michal Privoznik -
Peter Krempa