7:38 p.m.
Lin Ma (12):
virsh: Add comp-methods completion to migrate command
virsh: Use VIR_ENUM_* for --target argument in cmdNodeSuspend
virsh: Add target completion to nodesuspend command
virsh: Use VIR_ENUM_* for --target argument in cmdDomPMSuspend
virsh: Add target completion to dompmsuspend command
virsh: Add format completion to blockcopy command
virsh-pool: Add virshPoolTypeCompleter in macro
VIRSH_COMMON_OPT_POOL_X_AS
virsh-pool: Remove static from virshStoragePoolList{Free,Collect}
virsh-volume: Introduce virshStorageVolKeyCompleter
virsh-volume: Add macros VIRSH_COMMON_OPT_VOL_*
virsh-volume: Apply virshStorageVolKeyCompleter to vol-{name,pool}
commands
virsh-volume: Apply virshStorageVolNameCompleter to vol-{key,path}
commands
tools/virsh-completer-domain.c | 37 +++++++++++++++++++++
tools/virsh-completer-domain.h | 11 +++++++
tools/virsh-completer-host.c | 20 ++++++++++++
tools/virsh-completer-host.h | 4 +++
tools/virsh-completer-volume.c | 53 ++++++++++++++++++++++++++++++
tools/virsh-completer-volume.h | 5 +++
tools/virsh-domain.c | 14 ++++----
tools/virsh-domain.h | 1 +
tools/virsh-host.c | 18 +++++-----
tools/virsh-host.h | 3 ++
tools/virsh-pool.c | 11 ++-----
tools/virsh-pool.h | 11 +++++++
tools/virsh-volume.c | 60 ++++++++++++++++------------------
13 files changed, 193 insertions(+), 55 deletions(-)
--
2.26.2
7:38 p.m.
New subject: [libvirt] [PATCH 01/12] virsh: Add comp-methods completion to migrate command
Signed-off-by: Lin Ma <lma(a)suse.com>
---
tools/virsh-completer-domain.c | 17 +++++++++++++++++
tools/virsh-completer-domain.h | 5 +++++
tools/virsh-domain.c | 1 +
3 files changed, 23 insertions(+)
diff --git a/tools/virsh-completer-domain.c b/tools/virsh-completer-domain.c
index 20d503ff09..256ac0b593 100644
--- a/tools/virsh-completer-domain.c
+++ b/tools/virsh-completer-domain.c
@@ -959,3 +959,20 @@ virshDomainCoreDumpFormatCompleter(vshControl *ctl G_GNUC_UNUSED,
return ret;
}
+
+
+char **
+virshDomainMigrateCompMethodsCompleter(vshControl *ctl,
+ const vshCmd *cmd,
+ unsigned int flags)
+{
+ const char *methods[] = {"xbzrle", "mt", NULL};
+ const char *method = NULL;
+
+ virCheckFlags(0, NULL);
+
+ if (vshCommandOptStringQuiet(ctl, cmd, "comp-methods", &method) <
0)
+ return NULL;
+
+ return virshCommaStringListComplete(method, methods);
+}
diff --git a/tools/virsh-completer-domain.h b/tools/virsh-completer-domain.h
index 4e16d84514..5d56090504 100644
--- a/tools/virsh-completer-domain.h
+++ b/tools/virsh-completer-domain.h
@@ -127,3 +127,8 @@ char **
virshDomainCoreDumpFormatCompleter(vshControl *ctl,
const vshCmd *cmd,
unsigned int flags);
+
+char **
+virshDomainMigrateCompMethodsCompleter(vshControl *ctl,
+ const vshCmd *cmd,
+ unsigned int flags);
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index ecad3a5e5d..0100652e76 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -10619,6 +10619,7 @@ static const vshCmdOptDef opts_migrate[] = {
},
{.name = "comp-methods",
.type = VSH_OT_STRING,
+ .completer = virshDomainMigrateCompMethodsCompleter,
.help = N_("comma separated list of compression methods to be used")
},
{.name = "comp-mt-level",
--
2.26.2
7:38 p.m.
New subject: [libvirt] [PATCH 02/12] virsh: Use VIR_ENUM_* for --target argument in cmdNodeSuspend
Signed-off-by: Lin Ma <lma(a)suse.com>
---
tools/virsh-host.c | 17 +++++++++--------
tools/virsh-host.h | 3 +++
2 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/tools/virsh-host.c b/tools/virsh-host.c
index 1eca0bc231..9d6d2b3645 100644
--- a/tools/virsh-host.c
+++ b/tools/virsh-host.c
@@ -950,6 +950,13 @@ cmdNodeMemStats(vshControl *ctl, const vshCmd *cmd)
/*
* "nodesuspend" command
*/
+
+VIR_ENUM_IMPL(virNodeSuspendTarget,
+ VIR_NODE_SUSPEND_TARGET_LAST,
+ "mem",
+ "disk",
+ "hybrid");
+
static const vshCmdInfo info_nodesuspend[] = {
{.name = "help",
.data = N_("suspend the host node for a given time duration")
@@ -980,7 +987,7 @@ static bool
cmdNodeSuspend(vshControl *ctl, const vshCmd *cmd)
{
const char *target = NULL;
- unsigned int suspendTarget;
+ int suspendTarget;
long long duration;
virshControl *priv = ctl->privData;
@@ -990,13 +997,7 @@ cmdNodeSuspend(vshControl *ctl, const vshCmd *cmd)
if (vshCommandOptLongLong(ctl, cmd, "duration", &duration) < 0)
return false;
- if (STREQ(target, "mem")) {
- suspendTarget = VIR_NODE_SUSPEND_TARGET_MEM;
- } else if (STREQ(target, "disk")) {
- suspendTarget = VIR_NODE_SUSPEND_TARGET_DISK;
- } else if (STREQ(target, "hybrid")) {
- suspendTarget = VIR_NODE_SUSPEND_TARGET_HYBRID;
- } else {
+ if ((suspendTarget = virNodeSuspendTargetTypeFromString(target)) < 0) {
vshError(ctl, "%s", _("Invalid target"));
return false;
}
diff --git a/tools/virsh-host.h b/tools/virsh-host.h
index 92328c7deb..840f0b4538 100644
--- a/tools/virsh-host.h
+++ b/tools/virsh-host.h
@@ -21,5 +21,8 @@
#pragma once
#include "vsh.h"
+#include "virenum.h"
extern const vshCmdDef hostAndHypervisorCmds[];
+
+VIR_ENUM_DECL(virNodeSuspendTarget);
--
2.26.2
7:38 p.m.
New subject: [libvirt] [PATCH 03/12] virsh: Add target completion to nodesuspend command
Signed-off-by: Lin Ma <lma(a)suse.com>
---
tools/virsh-completer-host.c | 20 ++++++++++++++++++++
tools/virsh-completer-host.h | 4 ++++
tools/virsh-host.c | 1 +
3 files changed, 25 insertions(+)
diff --git a/tools/virsh-completer-host.c b/tools/virsh-completer-host.c
index 213f029552..d17bc5f5ce 100644
--- a/tools/virsh-completer-host.c
+++ b/tools/virsh-completer-host.c
@@ -26,6 +26,7 @@
#include "virstring.h"
#include "virxml.h"
#include "virutil.h"
+#include "virsh-host.h"
static char *
virshPagesizeNodeToString(xmlNodePtr node)
@@ -167,3 +168,22 @@ virshNodeCpuCompleter(vshControl *ctl,
return g_steal_pointer(&tmp);
}
+
+
+char **
+virshNodeSuspendTargetCompleter(vshControl *ctl G_GNUC_UNUSED,
+ const vshCmd *cmd G_GNUC_UNUSED,
+ unsigned int flags)
+{
+ char **ret = NULL;
+ size_t i;
+
+ virCheckFlags(0, NULL);
+
+ ret = g_new0(char *, VIR_NODE_SUSPEND_TARGET_LAST + 1);
+
+ for (i = 0; i < VIR_NODE_SUSPEND_TARGET_LAST; i++)
+ ret[i] = g_strdup(virNodeSuspendTargetTypeToString(i));
+
+ return ret;
+}
diff --git a/tools/virsh-completer-host.h b/tools/virsh-completer-host.h
index a502216584..88106ec3db 100644
--- a/tools/virsh-completer-host.h
+++ b/tools/virsh-completer-host.h
@@ -33,3 +33,7 @@ char ** virshCellnoCompleter(vshControl *ctl,
char ** virshNodeCpuCompleter(vshControl *ctl,
const vshCmd *cmd,
unsigned int flags);
+
+char ** virshNodeSuspendTargetCompleter(vshControl *ctl,
+ const vshCmd *cmd,
+ unsigned int flags);
diff --git a/tools/virsh-host.c b/tools/virsh-host.c
index 9d6d2b3645..f42e995122 100644
--- a/tools/virsh-host.c
+++ b/tools/virsh-host.c
@@ -972,6 +972,7 @@ static const vshCmdOptDef opts_node_suspend[] = {
{.name = "target",
.type = VSH_OT_DATA,
.flags = VSH_OFLAG_REQ,
+ .completer = virshNodeSuspendTargetCompleter,
.help = N_("mem(Suspend-to-RAM), disk(Suspend-to-Disk), "
"hybrid(Hybrid-Suspend)")
},
--
2.26.2
7:38 p.m.
New subject: [libvirt] [PATCH 04/12] virsh: Use VIR_ENUM_* for --target argument in cmdDomPMSuspend
Signed-off-by: Lin Ma <lma(a)suse.com>
---
tools/virsh-domain.c | 10 ++--------
tools/virsh-domain.h | 1 +
2 files changed, 3 insertions(+), 8 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 0100652e76..3fb37ea5e6 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -3472,7 +3472,7 @@ cmdDomPMSuspend(vshControl *ctl, const vshCmd *cmd)
const char *name;
bool ret = false;
const char *target = NULL;
- unsigned int suspendTarget;
+ int suspendTarget;
unsigned long long duration = 0;
if (!(dom = virshCommandOptDomain(ctl, cmd, &name)))
@@ -3484,13 +3484,7 @@ cmdDomPMSuspend(vshControl *ctl, const vshCmd *cmd)
if (vshCommandOptStringReq(ctl, cmd, "target", &target) < 0)
goto cleanup;
- if (STREQ(target, "mem")) {
- suspendTarget = VIR_NODE_SUSPEND_TARGET_MEM;
- } else if (STREQ(target, "disk")) {
- suspendTarget = VIR_NODE_SUSPEND_TARGET_DISK;
- } else if (STREQ(target, "hybrid")) {
- suspendTarget = VIR_NODE_SUSPEND_TARGET_HYBRID;
- } else {
+ if ((suspendTarget = virNodeSuspendTargetTypeFromString(target)) < 0) {
vshError(ctl, "%s", _("Invalid target"));
goto cleanup;
}
diff --git a/tools/virsh-domain.h b/tools/virsh-domain.h
index 0c1cc7a630..b569dd8d91 100644
--- a/tools/virsh-domain.h
+++ b/tools/virsh-domain.h
@@ -44,3 +44,4 @@ VIR_ENUM_DECL(virDomainProcessSignal);
VIR_ENUM_DECL(virDomainLifecycle);
VIR_ENUM_DECL(virDomainLifecycleAction);
VIR_ENUM_DECL(virDomainCoreDumpFormat);
+VIR_ENUM_DECL(virNodeSuspendTarget);
--
2.26.2
3:37 a.m.
New subject: [libvirt] [PATCH 04/12] virsh: Use VIR_ENUM_* for --target argument in cmdDomPMSuspend
On 6/15/21 2:38 AM, Lin Ma wrote:
Signed-off-by: Lin Ma <lma(a)suse.com>
---
tools/virsh-domain.c | 10 ++--------
tools/virsh-domain.h | 1 +
2 files changed, 3 insertions(+), 8 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 0100652e76..3fb37ea5e6 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -3472,7 +3472,7 @@ cmdDomPMSuspend(vshControl *ctl, const vshCmd *cmd)
const char *name;
bool ret = false;
const char *target = NULL;
- unsigned int suspendTarget;
+ int suspendTarget;
unsigned long long duration = 0;
if (!(dom = virshCommandOptDomain(ctl, cmd, &name)))
@@ -3484,13 +3484,7 @@ cmdDomPMSuspend(vshControl *ctl, const vshCmd *cmd)
if (vshCommandOptStringReq(ctl, cmd, "target", &target) < 0)
goto cleanup;
- if (STREQ(target, "mem")) {
- suspendTarget = VIR_NODE_SUSPEND_TARGET_MEM;
- } else if (STREQ(target, "disk")) {
- suspendTarget = VIR_NODE_SUSPEND_TARGET_DISK;
- } else if (STREQ(target, "hybrid")) {
- suspendTarget = VIR_NODE_SUSPEND_TARGET_HYBRID;
- } else {
+ if ((suspendTarget = virNodeSuspendTargetTypeFromString(target)) < 0) {
vshError(ctl, "%s", _("Invalid target"));
goto cleanup;
}
diff --git a/tools/virsh-domain.h b/tools/virsh-domain.h
index 0c1cc7a630..b569dd8d91 100644
--- a/tools/virsh-domain.h
+++ b/tools/virsh-domain.h
@@ -44,3 +44,4 @@ VIR_ENUM_DECL(virDomainProcessSignal);
VIR_ENUM_DECL(virDomainLifecycle);
VIR_ENUM_DECL(virDomainLifecycleAction);
VIR_ENUM_DECL(virDomainCoreDumpFormat);
+VIR_ENUM_DECL(virNodeSuspendTarget);
This doesn't feel right. I understand why you added it - so that you can
use virNodeSuspendTargetTypeFromString() in the hunk above. However, the
enum is implemented and declared in virsh-host files. Thus, what this
patch should instead is include virsh-host.h from virsh-domain.c. IOW,
I'm squashing in the following:
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 3fb37ea5e6..de80b1c2d5 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -43,6 +43,7 @@
#include "virstring.h"
#include "virsh-console.h"
#include "virsh-domain-monitor.h"
+#include "virsh-host.h"
#include "virerror.h"
#include "virtime.h"
#include "virtypedparam.h"
diff --git a/tools/virsh-domain.h b/tools/virsh-domain.h
index b569dd8d91..0c1cc7a630 100644
--- a/tools/virsh-domain.h
+++ b/tools/virsh-domain.h
@@ -44,4 +44,3 @@ VIR_ENUM_DECL(virDomainProcessSignal);
VIR_ENUM_DECL(virDomainLifecycle);
VIR_ENUM_DECL(virDomainLifecycleAction);
VIR_ENUM_DECL(virDomainCoreDumpFormat);
-VIR_ENUM_DECL(virNodeSuspendTarget);
Michal
7:38 p.m.
New subject: [libvirt] [PATCH 05/12] virsh: Add target completion to dompmsuspend command
Signed-off-by: Lin Ma <lma(a)suse.com>
---
tools/virsh-domain.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 3fb37ea5e6..b3aa52b8a8 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -3453,6 +3453,7 @@ static const vshCmdOptDef opts_dom_pm_suspend[] = {
{.name = "target",
.type = VSH_OT_DATA,
.flags = VSH_OFLAG_REQ,
+ .completer = virshNodeSuspendTargetCompleter,
.help = N_("mem(Suspend-to-RAM), "
"disk(Suspend-to-Disk), "
"hybrid(Hybrid-Suspend)")
--
2.26.2
7:38 p.m.
New subject: [libvirt] [PATCH 06/12] virsh: Add format completion to blockcopy command
Signed-off-by: Lin Ma <lma(a)suse.com>
---
tools/virsh-completer-domain.c | 20 ++++++++++++++++++++
tools/virsh-completer-domain.h | 6 ++++++
tools/virsh-domain.c | 2 ++
3 files changed, 28 insertions(+)
diff --git a/tools/virsh-completer-domain.c b/tools/virsh-completer-domain.c
index 256ac0b593..14e4d95ec3 100644
--- a/tools/virsh-completer-domain.c
+++ b/tools/virsh-completer-domain.c
@@ -35,6 +35,7 @@
#include "virkeynametable_linux.h"
#include "virkeynametable_osx.h"
#include "virkeynametable_win32.h"
+#include "conf/storage_conf.h"
char **
virshDomainNameCompleter(vshControl *ctl,
@@ -976,3 +977,22 @@ virshDomainMigrateCompMethodsCompleter(vshControl *ctl,
return virshCommaStringListComplete(method, methods);
}
+
+
+char **
+virshDomainStorageFileFormatCompleter(vshControl *ctl G_GNUC_UNUSED,
+ const vshCmd *cmd G_GNUC_UNUSED,
+ unsigned int flags)
+{
+ char **ret = NULL;
+ size_t i;
+
+ virCheckFlags(0, NULL);
+
+ ret = g_new0(char *, VIR_STORAGE_FILE_LAST + 1);
+
+ for (i = 0; i < VIR_STORAGE_FILE_LAST; i++)
+ ret[i] = g_strdup(virStorageFileFormatTypeToString(i));
+
+ return ret;
+}
diff --git a/tools/virsh-completer-domain.h b/tools/virsh-completer-domain.h
index 5d56090504..45380906f9 100644
--- a/tools/virsh-completer-domain.h
+++ b/tools/virsh-completer-domain.h
@@ -132,3 +132,9 @@ char **
virshDomainMigrateCompMethodsCompleter(vshControl *ctl,
const vshCmd *cmd,
unsigned int flags);
+
+
+char **
+virshDomainStorageFileFormatCompleter(vshControl *ctl,
+ const vshCmd *cmd,
+ unsigned int flags);
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index b3aa52b8a8..183f53f913 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -2235,6 +2235,8 @@ static const vshCmdOptDef opts_blockcopy[] = {
},
{.name = "format",
.type = VSH_OT_STRING,
+ .flags = VSH_OFLAG_NONE,
+ .completer = virshDomainStorageFileFormatCompleter,
.help = N_("format of the destination file")
},
{.name = "granularity",
--
2.26.2
7:38 p.m.
New subject: [libvirt] [PATCH 07/12] virsh-pool: Add virshPoolTypeCompleter in macro VIRSH_COMMON_OPT_POOL_X_AS
Signed-off-by: Lin Ma <lma(a)suse.com>
---
tools/virsh-pool.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/tools/virsh-pool.c b/tools/virsh-pool.c
index e8d3c33506..34ed86152e 100644
--- a/tools/virsh-pool.c
+++ b/tools/virsh-pool.c
@@ -64,6 +64,7 @@
{.name = "type", \
.type = VSH_OT_DATA, \
.flags = VSH_OFLAG_REQ, \
+ .completer = virshPoolTypeCompleter, \
.help = N_("type of the pool") \
}, \
{.name = "print-xml", \
--
2.26.2
7:38 p.m.
New subject: [libvirt] [PATCH 08/12] virsh-pool: Remove static from virshStoragePoolList{Free, Collect}
The functions will be used by next patch.
Signed-off-by: Lin Ma <lma(a)suse.com>
---
tools/virsh-pool.c | 10 ++--------
tools/virsh-pool.h | 11 +++++++++++
2 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/tools/virsh-pool.c b/tools/virsh-pool.c
index 34ed86152e..18f3839a4c 100644
--- a/tools/virsh-pool.c
+++ b/tools/virsh-pool.c
@@ -835,13 +835,7 @@ virshStoragePoolSorter(const void *a, const void *b)
virStoragePoolGetName(*pb));
}
-struct virshStoragePoolList {
- virStoragePoolPtr *pools;
- size_t npools;
-};
-
-static void
-virshStoragePoolListFree(struct virshStoragePoolList *list)
+void virshStoragePoolListFree(struct virshStoragePoolList *list)
{
size_t i;
@@ -855,7 +849,7 @@ virshStoragePoolListFree(struct virshStoragePoolList *list)
g_free(list);
}
-static struct virshStoragePoolList *
+struct virshStoragePoolList *
virshStoragePoolListCollect(vshControl *ctl,
unsigned int flags)
{
diff --git a/tools/virsh-pool.h b/tools/virsh-pool.h
index 219f0eea42..d7bacd8731 100644
--- a/tools/virsh-pool.h
+++ b/tools/virsh-pool.h
@@ -40,3 +40,14 @@ typedef struct virshPoolEventCallback virshPoolEventCallback;
extern virshPoolEventCallback virshPoolEventCallbacks[];
extern const vshCmdDef storagePoolCmds[];
+
+struct virshStoragePoolList {
+ virStoragePoolPtr *pools;
+ size_t npools;
+};
+
+struct virshStoragePoolList *
+virshStoragePoolListCollect(vshControl *ctl,
+ unsigned int flags);
+
+void virshStoragePoolListFree(struct virshStoragePoolList *list);
--
2.26.2
7:38 p.m.
New subject: [libvirt] [PATCH 09/12] virsh-volume: Introduce virshStorageVolKeyCompleter
Signed-off-by: Lin Ma <lma(a)suse.com>
---
tools/virsh-completer-volume.c | 53 ++++++++++++++++++++++++++++++++++
tools/virsh-completer-volume.h | 5 ++++
2 files changed, 58 insertions(+)
diff --git a/tools/virsh-completer-volume.c b/tools/virsh-completer-volume.c
index 301ee982a5..14e9069c5a 100644
--- a/tools/virsh-completer-volume.c
+++ b/tools/virsh-completer-volume.c
@@ -69,3 +69,56 @@ virshStorageVolNameCompleter(vshControl *ctl,
g_free(vols);
return ret;
}
+
+char **
+virshStorageVolKeyCompleter(vshControl *ctl,
+ const vshCmd *cmd G_GNUC_UNUSED,
+ unsigned int flags)
+{
+ virshControl *priv = ctl->privData;
+ struct virshStoragePoolList *list = NULL;
+ virStorageVolPtr *vols = NULL;
+ int rc;
+ int nvols = 0;
+ size_t i = 0, j = 0, idx = 0;
+ char **ret = NULL;
+ g_auto(GStrv) tmp = NULL;
+
+ virCheckFlags(0, NULL);
+
+ flags = VIR_CONNECT_LIST_STORAGE_POOLS_ACTIVE;
+
+ if (!priv->conn || virConnectIsAlive(priv->conn) <= 0)
+ return NULL;
+
+ if (!(list = virshStoragePoolListCollect(ctl, flags)))
+ goto cleanup;
+
+ for (i = 0; i < list->npools; i++) {
+ if ((rc = virStoragePoolNumOfVolumes(list->pools[i])) < 0)
+ goto cleanup;
+ nvols += rc;
+ }
+
+ tmp = g_new0(char *, nvols + 1);
+
+ for (i = 0; i < list->npools; i++) {
+ if ((rc = virStoragePoolListAllVolumes(list->pools[i], &vols, 0)) < 0)
+ goto cleanup;
+ for (j = 0; j < rc; j++) {
+ const char *key = virStorageVolGetKey(vols[j]);
+ tmp[idx++] = g_strdup(key);
+ }
+ }
+
+ ret = g_steal_pointer(&tmp);
+
+ cleanup:
+ for (i = 0; i < list->npools; i++)
+ if ((rc = virStoragePoolListAllVolumes(list->pools[i], &vols, 0)) > 0)
+ for (j = 0; j < rc; j++)
+ virStorageVolFree(vols[j]);
+ g_free(vols);
+ virshStoragePoolListFree(list);
+ return ret;
+}
diff --git a/tools/virsh-completer-volume.h b/tools/virsh-completer-volume.h
index 6591e13fdf..b41d8f4f3e 100644
--- a/tools/virsh-completer-volume.h
+++ b/tools/virsh-completer-volume.h
@@ -26,3 +26,8 @@
char ** virshStorageVolNameCompleter(vshControl *ctl,
const vshCmd *cmd,
unsigned int flags);
+
+
+char ** virshStorageVolKeyCompleter(vshControl *ctl,
+ const vshCmd *cmd,
+ unsigned int flags);
--
2.26.2
3:36 a.m.
New subject: [libvirt] [PATCH 09/12] virsh-volume: Introduce virshStorageVolKeyCompleter
On 6/15/21 2:38 AM, Lin Ma wrote:
Signed-off-by: Lin Ma <lma(a)suse.com>
---
tools/virsh-completer-volume.c | 53 ++++++++++++++++++++++++++++++++++
tools/virsh-completer-volume.h | 5 ++++
2 files changed, 58 insertions(+)
diff --git a/tools/virsh-completer-volume.c b/tools/virsh-completer-volume.c
index 301ee982a5..14e9069c5a 100644
--- a/tools/virsh-completer-volume.c
+++ b/tools/virsh-completer-volume.c
@@ -69,3 +69,56 @@ virshStorageVolNameCompleter(vshControl *ctl,
g_free(vols);
return ret;
}
+
+char **
+virshStorageVolKeyCompleter(vshControl *ctl,
+ const vshCmd *cmd G_GNUC_UNUSED,
+ unsigned int flags)
+{
+ virshControl *priv = ctl->privData;
+ struct virshStoragePoolList *list = NULL;
+ virStorageVolPtr *vols = NULL;
+ int rc;
+ int nvols = 0;
+ size_t i = 0, j = 0, idx = 0;
+ char **ret = NULL;
+ g_auto(GStrv) tmp = NULL;
+
+ virCheckFlags(0, NULL);
+
+ flags = VIR_CONNECT_LIST_STORAGE_POOLS_ACTIVE;
I would much rather see this flag passed ...
+
+ if (!priv->conn || virConnectIsAlive(priv->conn) <= 0)
+ return NULL;
+
+ if (!(list = virshStoragePoolListCollect(ctl, flags)))
.. here directly, instead of overwriting @flags.
+ goto cleanup;
+
+ for (i = 0; i < list->npools; i++) {
+ if ((rc = virStoragePoolNumOfVolumes(list->pools[i])) < 0)
+ goto cleanup;
+ nvols += rc;
+ }
Alright, so here @nvols contains sum of all volumes from all active
pools. What if at this point a new volume is added into a pool (from a
different connection/outside of libvirt). What I'm getting at is [2].
+
+ tmp = g_new0(char *, nvols + 1);
+
+ for (i = 0; i < list->npools; i++) {
+ if ((rc = virStoragePoolListAllVolumes(list->pools[i], &vols, 0)) <
0)
+ goto cleanup;
+ for (j = 0; j < rc; j++) {
1: j < rc && idx < nvols
+ const char *key = virStorageVolGetKey(vols[j]);
+ tmp[idx++] = g_strdup(key);
2: this is potentially dangerous as idx may grow beyond nvols limit. I
think something like [1] will prevent that. But it won't prevent @vols
and individual virStorageVols from leaking.
+ }
+ }
+
+ ret = g_steal_pointer(&tmp);
+
+ cleanup:
+ for (i = 0; i < list->npools; i++)
+ if ((rc = virStoragePoolListAllVolumes(list->pools[i], &vols, 0)) >
0)
+ for (j = 0; j < rc; j++)
+ virStorageVolFree(vols[j]);
This does not free volumes really. This returns another allocated array
which is freed immediately. But the one allocated above is not freed. I
think this function should be reworked slightly, e.g. like this:
list = virshStoragePoolListCollect(ctl,
VIR_CONNECT_LIST_STORAGE_POOLS_ACTIVE);
for (i = 0; i < list->npools; i++) {
rc = virStoragePoolListAllVolumes(list->pools[i], vols, 0);
tmp = g_renew(char *, tmp, nvols + rc + 1);
/* memset new memory to 0 */
for (j = 0; j < rc; j++) {
tmp[nvols++] = volume key;
virStorageVolFree(vols[j]);
}
g_free(vols);
}
I've left out error checks, obviously. I'm stopping my review here as
the rest of patches rely on this one.
Michal
7:38 p.m.
New subject: [libvirt] [PATCH 10/12] virsh-volume: Add macros VIRSH_COMMON_OPT_VOL_*
Signed-off-by: Lin Ma <lma(a)suse.com>
---
tools/virsh-volume.c | 36 +++++++++++++++++++++++++-----------
1 file changed, 25 insertions(+), 11 deletions(-)
diff --git a/tools/virsh-volume.c b/tools/virsh-volume.c
index c7d5ab8c3b..41b366a833 100644
--- a/tools/virsh-volume.c
+++ b/tools/virsh-volume.c
@@ -54,14 +54,28 @@
.completer_flags = VIR_CONNECT_LIST_STORAGE_POOLS_ACTIVE, \
}
-#define VIRSH_COMMON_OPT_VOLUME_VOL \
+#define VIRSH_COMMON_OPT_VOL_NAME(_helpstr) \
{.name = "vol", \
.type = VSH_OT_DATA, \
.flags = VSH_OFLAG_REQ, \
- .help = N_("vol name, key or path"), \
+ .help = _helpstr, \
.completer = virshStorageVolNameCompleter, \
}
+#define VIRSH_COMMON_OPT_VOL_KEY(_helpstr) \
+ {.name = "vol", \
+ .type = VSH_OT_DATA, \
+ .flags = VSH_OFLAG_REQ, \
+ .help = _helpstr, \
+ .completer = virshStorageVolKeyCompleter, \
+ }
+
+#define VIRSH_COMMON_OPT_VOL_FULL \
+ VIRSH_COMMON_OPT_VOL_NAME(N_("vol name, key or path"))
+
+#define VIRSH_COMMON_OPT_VOL_BY_KEY \
+ VIRSH_COMMON_OPT_VOL_KEY(N_("volume key or path"))
+
virStorageVolPtr
virshCommandOptVolBy(vshControl *ctl, const vshCmd *cmd,
const char *optname,
@@ -440,7 +454,7 @@ static const vshCmdInfo info_vol_create_from[] = {
static const vshCmdOptDef opts_vol_create_from[] = {
VIRSH_COMMON_OPT_POOL_FULL,
VIRSH_COMMON_OPT_FILE(N_("file containing an XML vol description")),
- VIRSH_COMMON_OPT_VOLUME_VOL,
+ VIRSH_COMMON_OPT_VOL_FULL,
{.name = "inputpool",
.type = VSH_OT_STRING,
.help = N_("pool name or uuid of the input volume's pool")
@@ -550,7 +564,7 @@ static const vshCmdInfo info_vol_clone[] = {
};
static const vshCmdOptDef opts_vol_clone[] = {
- VIRSH_COMMON_OPT_VOLUME_VOL,
+ VIRSH_COMMON_OPT_VOL_FULL,
{.name = "newname",
.type = VSH_OT_DATA,
.flags = VSH_OFLAG_REQ,
@@ -646,7 +660,7 @@ static const vshCmdInfo info_vol_upload[] = {
};
static const vshCmdOptDef opts_vol_upload[] = {
- VIRSH_COMMON_OPT_VOLUME_VOL,
+ VIRSH_COMMON_OPT_VOL_FULL,
VIRSH_COMMON_OPT_FILE(N_("file")),
VIRSH_COMMON_OPT_POOL_OPTIONAL,
{.name = "offset",
@@ -768,7 +782,7 @@ static const vshCmdInfo info_vol_download[] = {
};
static const vshCmdOptDef opts_vol_download[] = {
- VIRSH_COMMON_OPT_VOLUME_VOL,
+ VIRSH_COMMON_OPT_VOL_FULL,
VIRSH_COMMON_OPT_FILE(N_("file")),
VIRSH_COMMON_OPT_POOL_OPTIONAL,
{.name = "offset",
@@ -889,7 +903,7 @@ static const vshCmdInfo info_vol_delete[] = {
};
static const vshCmdOptDef opts_vol_delete[] = {
- VIRSH_COMMON_OPT_VOLUME_VOL,
+ VIRSH_COMMON_OPT_VOL_FULL,
VIRSH_COMMON_OPT_POOL_OPTIONAL,
{.name = "delete-snapshots",
.type = VSH_OT_BOOL,
@@ -939,7 +953,7 @@ static const vshCmdInfo info_vol_wipe[] = {
};
static const vshCmdOptDef opts_vol_wipe[] = {
- VIRSH_COMMON_OPT_VOLUME_VOL,
+ VIRSH_COMMON_OPT_VOL_FULL,
VIRSH_COMMON_OPT_POOL_OPTIONAL,
{.name = "algorithm",
.type = VSH_OT_STRING,
@@ -1027,7 +1041,7 @@ static const vshCmdInfo info_vol_info[] = {
};
static const vshCmdOptDef opts_vol_info[] = {
- VIRSH_COMMON_OPT_VOLUME_VOL,
+ VIRSH_COMMON_OPT_VOL_FULL,
VIRSH_COMMON_OPT_POOL_OPTIONAL,
{.name = "bytes",
.type = VSH_OT_BOOL,
@@ -1117,7 +1131,7 @@ static const vshCmdInfo info_vol_resize[] = {
};
static const vshCmdOptDef opts_vol_resize[] = {
- VIRSH_COMMON_OPT_VOLUME_VOL,
+ VIRSH_COMMON_OPT_VOL_FULL,
{.name = "capacity",
.type = VSH_OT_DATA,
.flags = VSH_OFLAG_REQ,
@@ -1214,7 +1228,7 @@ static const vshCmdInfo info_vol_dumpxml[] = {
};
static const vshCmdOptDef opts_vol_dumpxml[] = {
- VIRSH_COMMON_OPT_VOLUME_VOL,
+ VIRSH_COMMON_OPT_VOL_FULL,
VIRSH_COMMON_OPT_POOL_OPTIONAL,
{.name = NULL}
};
--
2.26.2
7:38 p.m.
New subject: [libvirt] [PATCH 11/12] virsh-volume: Apply virshStorageVolKeyCompleter to vol-{name, pool} commands
Signed-off-by: Lin Ma <lma(a)suse.com>
---
tools/virsh-volume.c | 12 ++----------
1 file changed, 2 insertions(+), 10 deletions(-)
diff --git a/tools/virsh-volume.c b/tools/virsh-volume.c
index 41b366a833..daa205ea67 100644
--- a/tools/virsh-volume.c
+++ b/tools/virsh-volume.c
@@ -1552,11 +1552,7 @@ static const vshCmdInfo info_vol_name[] = {
};
static const vshCmdOptDef opts_vol_name[] = {
- {.name = "vol",
- .type = VSH_OT_DATA,
- .flags = VSH_OFLAG_REQ,
- .help = N_("volume key or path")
- },
+ VIRSH_COMMON_OPT_VOL_BY_KEY,
{.name = NULL}
};
@@ -1588,11 +1584,7 @@ static const vshCmdInfo info_vol_pool[] = {
};
static const vshCmdOptDef opts_vol_pool[] = {
- {.name = "vol",
- .type = VSH_OT_DATA,
- .flags = VSH_OFLAG_REQ,
- .help = N_("volume key or path")
- },
+ VIRSH_COMMON_OPT_VOL_BY_KEY,
{.name = "uuid",
.type = VSH_OT_BOOL,
.help = N_("return the pool uuid rather than pool name")
--
2.26.2
7:38 p.m.
New subject: [libvirt] [PATCH 12/12] virsh-volume: Apply virshStorageVolNameCompleter to vol-{key, path} commands
Signed-off-by: Lin Ma <lma(a)suse.com>
---
tools/virsh-volume.c | 12 ++----------
1 file changed, 2 insertions(+), 10 deletions(-)
diff --git a/tools/virsh-volume.c b/tools/virsh-volume.c
index daa205ea67..1da9b7217f 100644
--- a/tools/virsh-volume.c
+++ b/tools/virsh-volume.c
@@ -1643,11 +1643,7 @@ static const vshCmdInfo info_vol_key[] = {
};
static const vshCmdOptDef opts_vol_key[] = {
- {.name = "vol",
- .type = VSH_OT_DATA,
- .flags = VSH_OFLAG_REQ,
- .help = N_("volume name or path")
- },
+ VIRSH_COMMON_OPT_VOL_NAME(N_("volume name or path")),
VIRSH_COMMON_OPT_POOL_OPTIONAL,
{.name = NULL}
};
@@ -1679,11 +1675,7 @@ static const vshCmdInfo info_vol_path[] = {
};
static const vshCmdOptDef opts_vol_path[] = {
- {.name = "vol",
- .type = VSH_OT_DATA,
- .flags = VSH_OFLAG_REQ,
- .help = N_("volume name or key")
- },
+ VIRSH_COMMON_OPT_VOL_NAME(N_("volume name or key")),
VIRSH_COMMON_OPT_POOL_OPTIONAL,
{.name = NULL}
};
--
2.26.2
3:35 a.m.
On 6/15/21 2:38 AM, Lin Ma wrote:
Lin Ma (12):
virsh: Add comp-methods completion to migrate command
virsh: Use VIR_ENUM_* for --target argument in cmdNodeSuspend
virsh: Add target completion to nodesuspend command
virsh: Use VIR_ENUM_* for --target argument in cmdDomPMSuspend
virsh: Add target completion to dompmsuspend command
virsh: Add format completion to blockcopy command
virsh-pool: Add virshPoolTypeCompleter in macro
VIRSH_COMMON_OPT_POOL_X_AS
virsh-pool: Remove static from virshStoragePoolList{Free,Collect}
virsh-volume: Introduce virshStorageVolKeyCompleter
virsh-volume: Add macros VIRSH_COMMON_OPT_VOL_*
virsh-volume: Apply virshStorageVolKeyCompleter to vol-{name,pool}
commands
virsh-volume: Apply virshStorageVolNameCompleter to vol-{key,path}
commands
tools/virsh-completer-domain.c | 37 +++++++++++++++++++++
tools/virsh-completer-domain.h | 11 +++++++
tools/virsh-completer-host.c | 20 ++++++++++++
tools/virsh-completer-host.h | 4 +++
tools/virsh-completer-volume.c | 53 ++++++++++++++++++++++++++++++
tools/virsh-completer-volume.h | 5 +++
tools/virsh-domain.c | 14 ++++----
tools/virsh-domain.h | 1 +
tools/virsh-host.c | 18 +++++-----
tools/virsh-host.h | 3 ++
tools/virsh-pool.c | 11 ++-----
tools/virsh-pool.h | 11 +++++++
tools/virsh-volume.c | 60 ++++++++++++++++------------------
13 files changed, 193 insertions(+), 55 deletions(-)
ACK to 01-08. However, I'm pushing 01-07 only because 08 refers to 09
which needs to be reworked.
Michal
1257
days inactive
1257
days old
15 comments
3 participants
participants (3)
-
Lin Ma -
Michal Privoznik -
Michal Prívozník