10:47 a.m.
Similar to qemuxml2argv and storagevolxml2argv, add storagepoolxml2argvtest
in order to check the command line creation for the pool 'Start' commands.
Only applicable for pool types with a "@startPool" function - that is disk,
fs, iscsi, logical, scsi, and vstorage and further restricted if the pool
doesn't use virCommandPtr type processing.
This initial series only addresses fs and logical so that it's not "too
many patches to review".
The iscsi and vstorage pools could have their own tests, with the iscsi
ones being more challenging to write.
The disk pool does not have the normal command line processing to start
something - rather the command line processing is used to validate that
the pool about to be started is of a valid type based on the label seen
at startup compared to the pool XML. This processing is not easily mocked.
The scsi (for NPIV) doesn't use the virCommandPtr style interfaces, but at
least that processing is tested in other ways using testCreateVport from
test_driver.c.
John Ferlan (8):
storage: Extract out mount command creation for FS Backend
storage: Move FS backend mount creation command helper
storage: Move virStorageBackendFileSystemGetPoolSource
tests: Introduce tests for storage pool xml to argv checks
tests: Add storagepool xml test for netfs-auto
storage: Rework virStorageBackendFileSystemMountCmd
logical: Fix @on argument type
storage: Add tests for logical backend startup
src/storage/storage_backend_fs.c | 77 +-------
src/storage/storage_backend_logical.c | 12 +-
src/storage/storage_util.c | 122 ++++++++++++
src/storage/storage_util.h | 11 ++
tests/Makefile.am | 12 ++
tests/storagepoolxml2argvdata/pool-fs.argv | 1 +
.../pool-logical-create.argv | 1 +
.../pool-logical-noname.argv | 1 +
.../pool-logical-nopath.argv | 1 +
.../storagepoolxml2argvdata/pool-logical.argv | 1 +
.../pool-netfs-auto.argv | 1 +
.../pool-netfs-cifs.argv | 1 +
.../pool-netfs-gluster.argv | 1 +
tests/storagepoolxml2argvdata/pool-netfs.argv | 1 +
tests/storagepoolxml2argvtest.c | 175 ++++++++++++++++++
.../storagepoolxml2xmlin/pool-netfs-auto.xml | 19 ++
.../storagepoolxml2xmlout/pool-netfs-auto.xml | 20 ++
tests/storagepoolxml2xmltest.c | 1 +
18 files changed, 375 insertions(+), 83 deletions(-)
create mode 100644 tests/storagepoolxml2argvdata/pool-fs.argv
create mode 100644 tests/storagepoolxml2argvdata/pool-logical-create.argv
create mode 100644 tests/storagepoolxml2argvdata/pool-logical-noname.argv
create mode 100644 tests/storagepoolxml2argvdata/pool-logical-nopath.argv
create mode 100644 tests/storagepoolxml2argvdata/pool-logical.argv
create mode 100644 tests/storagepoolxml2argvdata/pool-netfs-auto.argv
create mode 100644 tests/storagepoolxml2argvdata/pool-netfs-cifs.argv
create mode 100644 tests/storagepoolxml2argvdata/pool-netfs-gluster.argv
create mode 100644 tests/storagepoolxml2argvdata/pool-netfs.argv
create mode 100644 tests/storagepoolxml2argvtest.c
create mode 100644 tests/storagepoolxml2xmlin/pool-netfs-auto.xml
create mode 100644 tests/storagepoolxml2xmlout/pool-netfs-auto.xml
--
2.17.2
10:47 a.m.
New subject: [libvirt] [PATCH 1/8] storage: Extract out mount command creation for FS Backend
Extract out the code that is used to create the MOUNT command
for starting the pool. We can use this for Storage Pool XML
to Argv testing to ensure code changes don't alter how a
storage pool is started.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/storage/storage_backend_fs.c | 70 +++++++++++++++++++-------------
1 file changed, 41 insertions(+), 29 deletions(-)
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index 0837443391..4bf411b330 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -330,19 +330,11 @@ virStorageBackendFileSystemIsMounted(virStoragePoolObjPtr pool)
return ret;
}
-/**
- * @pool storage pool to mount
- *
- * Ensure that a FS storage pool is mounted on its target location.
- * If already mounted, this is a no-op
- *
- * Returns 0 if successfully mounted, -1 on error
- */
-static int
-virStorageBackendFileSystemMount(virStoragePoolObjPtr pool)
+
+static virCommandPtr
+virStorageBackendFileSystemMountCmd(virStoragePoolDefPtr def,
+ const char *src)
{
- virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool);
- char *src = NULL;
/* 'mount -t auto' doesn't seem to auto determine nfs (or cifs),
* while plain 'mount' does. We have to craft separate argvs to
* accommodate this */
@@ -353,23 +345,6 @@ virStorageBackendFileSystemMount(virStoragePoolObjPtr pool)
bool cifsfs = (def->type == VIR_STORAGE_POOL_NETFS &&
def->source.format == VIR_STORAGE_POOL_NETFS_CIFS);
virCommandPtr cmd = NULL;
- int ret = -1;
- int rc;
-
- if (virStorageBackendFileSystemIsValid(pool) < 0)
- return -1;
-
- if ((rc = virStorageBackendFileSystemIsMounted(pool)) < 0)
- return -1;
-
- /* Short-circuit if already mounted */
- if (rc == 1) {
- VIR_INFO("Target '%s' is already mounted",
def->target.path);
- return 0;
- }
-
- if (!(src = virStorageBackendFileSystemGetPoolSource(pool)))
- return -1;
if (netauto)
cmd = virCommandNewArgList(MOUNT,
@@ -404,6 +379,43 @@ virStorageBackendFileSystemMount(virStoragePoolObjPtr pool)
def->target.path,
NULL);
+ return cmd;
+}
+
+
+/**
+ * @pool storage pool to mount
+ *
+ * Ensure that a FS storage pool is mounted on its target location.
+ * If already mounted, this is a no-op
+ *
+ * Returns 0 if successfully mounted, -1 on error
+ */
+static int
+virStorageBackendFileSystemMount(virStoragePoolObjPtr pool)
+{
+ virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool);
+ char *src = NULL;
+ virCommandPtr cmd = NULL;
+ int ret = -1;
+ int rc;
+
+ if (virStorageBackendFileSystemIsValid(pool) < 0)
+ return -1;
+
+ if ((rc = virStorageBackendFileSystemIsMounted(pool)) < 0)
+ return -1;
+
+ /* Short-circuit if already mounted */
+ if (rc == 1) {
+ VIR_INFO("Target '%s' is already mounted",
def->target.path);
+ return 0;
+ }
+
+ if (!(src = virStorageBackendFileSystemGetPoolSource(pool)))
+ return -1;
+
+ cmd = virStorageBackendFileSystemMountCmd(def, src);
if (virCommandRun(cmd, NULL) < 0)
goto cleanup;
--
2.17.2
10:47 a.m.
New subject: [libvirt] [PATCH 2/8] storage: Move FS backend mount creation command helper
Move virStorageBackendFileSystemMountCmd to storage_util so that
it can be used by the test harness.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/storage/storage_backend_fs.c | 52 --------------------------------
src/storage/storage_util.c | 52 ++++++++++++++++++++++++++++++++
src/storage/storage_util.h | 4 +++
3 files changed, 56 insertions(+), 52 deletions(-)
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index 4bf411b330..b341ba84fa 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -331,58 +331,6 @@ virStorageBackendFileSystemIsMounted(virStoragePoolObjPtr pool)
}
-static virCommandPtr
-virStorageBackendFileSystemMountCmd(virStoragePoolDefPtr def,
- const char *src)
-{
- /* 'mount -t auto' doesn't seem to auto determine nfs (or cifs),
- * while plain 'mount' does. We have to craft separate argvs to
- * accommodate this */
- bool netauto = (def->type == VIR_STORAGE_POOL_NETFS &&
- def->source.format == VIR_STORAGE_POOL_NETFS_AUTO);
- bool glusterfs = (def->type == VIR_STORAGE_POOL_NETFS &&
- def->source.format == VIR_STORAGE_POOL_NETFS_GLUSTERFS);
- bool cifsfs = (def->type == VIR_STORAGE_POOL_NETFS &&
- def->source.format == VIR_STORAGE_POOL_NETFS_CIFS);
- virCommandPtr cmd = NULL;
-
- if (netauto)
- cmd = virCommandNewArgList(MOUNT,
- src,
- def->target.path,
- NULL);
- else if (glusterfs)
- cmd = virCommandNewArgList(MOUNT,
- "-t",
-
virStoragePoolFormatFileSystemNetTypeToString(def->source.format),
- src,
- "-o",
- "direct-io-mode=1",
- def->target.path,
- NULL);
- else if (cifsfs)
- cmd = virCommandNewArgList(MOUNT,
- "-t",
-
virStoragePoolFormatFileSystemNetTypeToString(def->source.format),
- src,
- def->target.path,
- "-o",
- "guest",
- NULL);
- else
- cmd = virCommandNewArgList(MOUNT,
- "-t",
- (def->type == VIR_STORAGE_POOL_FS ?
-
virStoragePoolFormatFileSystemTypeToString(def->source.format) :
-
virStoragePoolFormatFileSystemNetTypeToString(def->source.format)),
- src,
- def->target.path,
- NULL);
-
- return cmd;
-}
-
-
/**
* @pool storage pool to mount
*
diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index 318a556656..180d7b1fa3 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -4226,3 +4226,55 @@ virStorageBackendZeroPartitionTable(const char *path,
return storageBackendVolWipeLocalFile(path, VIR_STORAGE_VOL_WIPE_ALG_ZERO,
size, true);
}
+
+
+virCommandPtr
+virStorageBackendFileSystemMountCmd(virStoragePoolDefPtr def,
+ const char *src)
+{
+ /* 'mount -t auto' doesn't seem to auto determine nfs (or cifs),
+ * while plain 'mount' does. We have to craft separate argvs to
+ * accommodate this */
+ bool netauto = (def->type == VIR_STORAGE_POOL_NETFS &&
+ def->source.format == VIR_STORAGE_POOL_NETFS_AUTO);
+ bool glusterfs = (def->type == VIR_STORAGE_POOL_NETFS &&
+ def->source.format == VIR_STORAGE_POOL_NETFS_GLUSTERFS);
+ bool cifsfs = (def->type == VIR_STORAGE_POOL_NETFS &&
+ def->source.format == VIR_STORAGE_POOL_NETFS_CIFS);
+ virCommandPtr cmd = NULL;
+
+ if (netauto)
+ cmd = virCommandNewArgList(MOUNT,
+ src,
+ def->target.path,
+ NULL);
+ else if (glusterfs)
+ cmd = virCommandNewArgList(MOUNT,
+ "-t",
+
virStoragePoolFormatFileSystemNetTypeToString(def->source.format),
+ src,
+ "-o",
+ "direct-io-mode=1",
+ def->target.path,
+ NULL);
+ else if (cifsfs)
+ cmd = virCommandNewArgList(MOUNT,
+ "-t",
+
virStoragePoolFormatFileSystemNetTypeToString(def->source.format),
+ src,
+ def->target.path,
+ "-o",
+ "guest",
+ NULL);
+ else
+ cmd = virCommandNewArgList(MOUNT,
+ "-t",
+ (def->type == VIR_STORAGE_POOL_FS ?
+
virStoragePoolFormatFileSystemTypeToString(def->source.format) :
+
virStoragePoolFormatFileSystemNetTypeToString(def->source.format)),
+ src,
+ def->target.path,
+ NULL);
+
+ return cmd;
+}
diff --git a/src/storage/storage_util.h b/src/storage/storage_util.h
index 58b991c772..5b0baf56c4 100644
--- a/src/storage/storage_util.h
+++ b/src/storage/storage_util.h
@@ -177,4 +177,8 @@ int
virStorageBackendZeroPartitionTable(const char *path,
unsigned long long size);
+virCommandPtr
+virStorageBackendFileSystemMountCmd(virStoragePoolDefPtr def,
+ const char *src);
+
#endif /* __VIR_STORAGE_UTIL_H__ */
--
2.17.2
10:47 a.m.
New subject: [libvirt] [PATCH 3/8] storage: Move virStorageBackendFileSystemGetPoolSource
Move into storage_util for reuse by test harness
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/storage/storage_backend_fs.c | 33 --------------------------------
src/storage/storage_util.c | 33 ++++++++++++++++++++++++++++++++
src/storage/storage_util.h | 3 +++
3 files changed, 36 insertions(+), 33 deletions(-)
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index b341ba84fa..c5e75627b5 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -245,39 +245,6 @@ virStorageBackendFileSystemIsValid(virStoragePoolObjPtr pool)
}
-/**
- * virStorageBackendFileSystemGetPoolSource
- * @pool: storage pool object pointer
- *
- * Allocate/return a string representing the FS storage pool source.
- * It is up to the caller to VIR_FREE the allocated string
- */
-static char *
-virStorageBackendFileSystemGetPoolSource(virStoragePoolObjPtr pool)
-{
- virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool);
- char *src = NULL;
-
- if (def->type == VIR_STORAGE_POOL_NETFS) {
- if (def->source.format == VIR_STORAGE_POOL_NETFS_CIFS) {
- if (virAsprintf(&src, "//%s/%s",
- def->source.hosts[0].name,
- def->source.dir) < 0)
- return NULL;
- } else {
- if (virAsprintf(&src, "%s:%s",
- def->source.hosts[0].name,
- def->source.dir) < 0)
- return NULL;
- }
- } else {
- if (VIR_STRDUP(src, def->source.devices[0].path) < 0)
- return NULL;
- }
- return src;
-}
-
-
/**
* @pool storage pool to check for status
*
diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index 180d7b1fa3..c9f6096687 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -4228,6 +4228,39 @@ virStorageBackendZeroPartitionTable(const char *path,
}
+/**
+ * virStorageBackendFileSystemGetPoolSource
+ * @pool: storage pool object pointer
+ *
+ * Allocate/return a string representing the FS storage pool source.
+ * It is up to the caller to VIR_FREE the allocated string
+ */
+char *
+virStorageBackendFileSystemGetPoolSource(virStoragePoolObjPtr pool)
+{
+ virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool);
+ char *src = NULL;
+
+ if (def->type == VIR_STORAGE_POOL_NETFS) {
+ if (def->source.format == VIR_STORAGE_POOL_NETFS_CIFS) {
+ if (virAsprintf(&src, "//%s/%s",
+ def->source.hosts[0].name,
+ def->source.dir) < 0)
+ return NULL;
+ } else {
+ if (virAsprintf(&src, "%s:%s",
+ def->source.hosts[0].name,
+ def->source.dir) < 0)
+ return NULL;
+ }
+ } else {
+ if (VIR_STRDUP(src, def->source.devices[0].path) < 0)
+ return NULL;
+ }
+ return src;
+}
+
+
virCommandPtr
virStorageBackendFileSystemMountCmd(virStoragePoolDefPtr def,
const char *src)
diff --git a/src/storage/storage_util.h b/src/storage/storage_util.h
index 5b0baf56c4..28b3e0b9c9 100644
--- a/src/storage/storage_util.h
+++ b/src/storage/storage_util.h
@@ -177,6 +177,9 @@ int
virStorageBackendZeroPartitionTable(const char *path,
unsigned long long size);
+char *
+virStorageBackendFileSystemGetPoolSource(virStoragePoolObjPtr pool);
+
virCommandPtr
virStorageBackendFileSystemMountCmd(virStoragePoolDefPtr def,
const char *src);
--
2.17.2
10:47 a.m.
New subject: [libvirt] [PATCH 4/8] tests: Introduce tests for storage pool xml to argv checks
Similar to qemuxml2argv and storagevolxml2argv, let's create some
tests to ensure that the XML generates a consistent command line.
Using the same list of pools as storagepoolxml2xmltest, start with
the file system tests (fs, netfs, netfs-cifs, netfs-gluster).
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
tests/Makefile.am | 12 ++
tests/storagepoolxml2argvdata/pool-fs.argv | 1 +
.../pool-netfs-cifs.argv | 1 +
.../pool-netfs-gluster.argv | 1 +
tests/storagepoolxml2argvdata/pool-netfs.argv | 1 +
tests/storagepoolxml2argvtest.c | 171 ++++++++++++++++++
6 files changed, 187 insertions(+)
create mode 100644 tests/storagepoolxml2argvdata/pool-fs.argv
create mode 100644 tests/storagepoolxml2argvdata/pool-netfs-cifs.argv
create mode 100644 tests/storagepoolxml2argvdata/pool-netfs-gluster.argv
create mode 100644 tests/storagepoolxml2argvdata/pool-netfs.argv
create mode 100644 tests/storagepoolxml2argvtest.c
diff --git a/tests/Makefile.am b/tests/Makefile.am
index d7ec7e3a6f..bec0930c11 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -140,6 +140,7 @@ EXTRA_DIST = \
storagepoolschemadata \
storagepoolxml2xmlin \
storagepoolxml2xmlout \
+ storagepoolxml2argvdata \
storagevolschemadata \
storagevolxml2argvdata \
storagevolxml2xmlin \
@@ -363,6 +364,7 @@ endif WITH_NWFILTER
if WITH_STORAGE
test_programs += storagevolxml2argvtest
+test_programs += storagepoolxml2argvtest
test_programs += virstorageutiltest
endif WITH_STORAGE
@@ -901,6 +903,16 @@ storagevolxml2argvtest_LDADD = \
../src/libvirt_util.la \
$(LDADDS)
+storagepoolxml2argvtest_SOURCES = \
+ storagepoolxml2argvtest.c \
+ testutils.c testutils.h
+storagepoolxml2argvtest_LDADD = \
+ $(LIBXML_LIBS) \
+ ../src/libvirt_driver_storage_impl.la \
+ ../src/libvirt_conf.la \
+ ../src/libvirt_util.la \
+ $(LDADDS)
+
else ! WITH_STORAGE
EXTRA_DIST += storagevolxml2argvtest.c
EXTRA_DIST += virstorageutiltest.c
diff --git a/tests/storagepoolxml2argvdata/pool-fs.argv
b/tests/storagepoolxml2argvdata/pool-fs.argv
new file mode 100644
index 0000000000..4a94148114
--- /dev/null
+++ b/tests/storagepoolxml2argvdata/pool-fs.argv
@@ -0,0 +1 @@
+/usr/bin/mount -t ext3 /dev/sda6 /mnt
diff --git a/tests/storagepoolxml2argvdata/pool-netfs-cifs.argv
b/tests/storagepoolxml2argvdata/pool-netfs-cifs.argv
new file mode 100644
index 0000000000..7a06f86f66
--- /dev/null
+++ b/tests/storagepoolxml2argvdata/pool-netfs-cifs.argv
@@ -0,0 +1 @@
+/usr/bin/mount -t cifs //example.com/samba_share /mnt/cifs -o guest
diff --git a/tests/storagepoolxml2argvdata/pool-netfs-gluster.argv
b/tests/storagepoolxml2argvdata/pool-netfs-gluster.argv
new file mode 100644
index 0000000000..90e94f6f13
--- /dev/null
+++ b/tests/storagepoolxml2argvdata/pool-netfs-gluster.argv
@@ -0,0 +1 @@
+/usr/bin/mount -t glusterfs example.com:/volume -o direct-io-mode=1 /mnt/gluster
diff --git a/tests/storagepoolxml2argvdata/pool-netfs.argv
b/tests/storagepoolxml2argvdata/pool-netfs.argv
new file mode 100644
index 0000000000..f890a03f7e
--- /dev/null
+++ b/tests/storagepoolxml2argvdata/pool-netfs.argv
@@ -0,0 +1 @@
+/usr/bin/mount -t nfs localhost:/var/lib/libvirt/images /mnt
diff --git a/tests/storagepoolxml2argvtest.c b/tests/storagepoolxml2argvtest.c
new file mode 100644
index 0000000000..74f8561379
--- /dev/null
+++ b/tests/storagepoolxml2argvtest.c
@@ -0,0 +1,171 @@
+#include <config.h>
+
+#include "internal.h"
+#include "testutils.h"
+#include "datatypes.h"
+#include "storage/storage_util.h"
+#include "testutilsqemu.h"
+#include "virstring.h"
+
+#define VIR_FROM_THIS VIR_FROM_NONE
+
+
+static int
+testCompareXMLToArgvFiles(bool shouldFail,
+ const char *poolxml,
+ const char *cmdline)
+{
+ VIR_AUTOFREE(char *) actualCmdline = NULL;
+ VIR_AUTOFREE(char *) src = NULL;
+ int ret = -1;
+ virCommandPtr cmd = NULL;
+ virStoragePoolDefPtr def = NULL;
+ virStoragePoolObjPtr pool = NULL;
+
+ if (!(def = virStoragePoolDefParseFile(poolxml)))
+ goto cleanup;
+
+ switch ((virStoragePoolType)def->type) {
+ case VIR_STORAGE_POOL_FS:
+ case VIR_STORAGE_POOL_NETFS:
+ if (!(pool = virStoragePoolObjNew())) {
+ VIR_TEST_DEBUG("pool type %d alloc pool obj fails\n",
def->type);
+ virStoragePoolDefFree(def);
+ goto cleanup;
+ }
+ virStoragePoolObjSetDef(pool, def);
+
+ if (!(src = virStorageBackendFileSystemGetPoolSource(pool))) {
+ VIR_TEST_DEBUG("pool type %d has no pool source\n", def->type);
+ goto cleanup;
+ }
+
+ cmd = virStorageBackendFileSystemMountCmd(def, src);
+ break;
+
+ case VIR_STORAGE_POOL_DIR:
+ case VIR_STORAGE_POOL_LOGICAL:
+ case VIR_STORAGE_POOL_DISK:
+ case VIR_STORAGE_POOL_ISCSI:
+ case VIR_STORAGE_POOL_ISCSI_DIRECT:
+ case VIR_STORAGE_POOL_SCSI:
+ case VIR_STORAGE_POOL_MPATH:
+ case VIR_STORAGE_POOL_RBD:
+ case VIR_STORAGE_POOL_SHEEPDOG:
+ case VIR_STORAGE_POOL_GLUSTER:
+ case VIR_STORAGE_POOL_ZFS:
+ case VIR_STORAGE_POOL_VSTORAGE:
+ case VIR_STORAGE_POOL_LAST:
+ default:
+ VIR_TEST_DEBUG("pool type %d has no xml2argv test\n", def->type);
+ goto cleanup;
+ };
+
+ if (!(actualCmdline = virCommandToString(cmd))) {
+ VIR_TEST_DEBUG("pool type %d failed to get commandline\n",
def->type);
+ goto cleanup;
+ }
+
+ if (virTestCompareToFile(actualCmdline, cmdline) < 0)
+ goto cleanup;
+
+ ret = 0;
+
+ cleanup:
+ virCommandFree(cmd);
+ VIR_FREE(actualCmdline);
+ virStoragePoolObjEndAPI(&pool);
+ if (shouldFail) {
+ virResetLastError();
+ ret = 0;
+ }
+ return ret;
+}
+
+struct testInfo {
+ bool shouldFail;
+ const char *pool;
+};
+
+static int
+testCompareXMLToArgvHelper(const void *data)
+{
+ int result = -1;
+ const struct testInfo *info = data;
+ char *poolxml = NULL;
+ char *cmdline = NULL;
+
+ if (virAsprintf(&poolxml, "%s/storagepoolxml2xmlin/%s.xml",
+ abs_srcdir, info->pool) < 0)
+ goto cleanup;
+
+ if (virAsprintf(&cmdline, "%s/storagepoolxml2argvdata/%s.argv",
+ abs_srcdir, info->pool) < 0 && !info->shouldFail)
+ goto cleanup;
+
+ result = testCompareXMLToArgvFiles(info->shouldFail, poolxml, cmdline);
+
+ cleanup:
+ VIR_FREE(poolxml);
+ VIR_FREE(cmdline);
+
+ return result;
+}
+
+
+static int
+mymain(void)
+{
+ int ret = 0;
+
+#define DO_TEST_FULL(shouldFail, pool) \
+ do { \
+ struct testInfo info = { shouldFail, pool }; \
+ if (virTestRun("Storage Pool XML-2-argv " pool, \
+ testCompareXMLToArgvHelper, &info) < 0) \
+ ret = -1; \
+ } \
+ while (0);
+
+#define DO_TEST(pool, ...) \
+ DO_TEST_FULL(false, pool)
+
+#define DO_TEST_FAIL(pool, ...) \
+ DO_TEST_FULL(true, pool)
+
+ DO_TEST_FAIL("pool-dir");
+ DO_TEST_FAIL("pool-dir-naming");
+ DO_TEST("pool-fs");
+ DO_TEST_FAIL("pool-logical");
+ DO_TEST_FAIL("pool-logical-nopath");
+ DO_TEST_FAIL("pool-logical-create");
+ DO_TEST_FAIL("pool-logical-noname");
+ DO_TEST_FAIL("pool-disk");
+ DO_TEST_FAIL("pool-disk-device-nopartsep");
+ DO_TEST_FAIL("pool-iscsi");
+ DO_TEST_FAIL("pool-iscsi-auth");
+ DO_TEST("pool-netfs");
+ DO_TEST("pool-netfs-gluster");
+ DO_TEST("pool-netfs-cifs");
+ DO_TEST_FAIL("pool-scsi");
+ DO_TEST_FAIL("pool-scsi-type-scsi-host");
+ DO_TEST_FAIL("pool-scsi-type-fc-host");
+ DO_TEST_FAIL("pool-scsi-type-fc-host-managed");
+ DO_TEST_FAIL("pool-mpath");
+ DO_TEST_FAIL("pool-iscsi-multiiqn");
+ DO_TEST_FAIL("pool-iscsi-vendor-product");
+ DO_TEST_FAIL("pool-sheepdog");
+ DO_TEST_FAIL("pool-gluster");
+ DO_TEST_FAIL("pool-gluster-sub");
+ DO_TEST_FAIL("pool-scsi-type-scsi-host-stable");
+ DO_TEST_FAIL("pool-zfs");
+ DO_TEST_FAIL("pool-zfs-sourcedev");
+ DO_TEST_FAIL("pool-rbd");
+ DO_TEST_FAIL("pool-vstorage");
+ DO_TEST_FAIL("pool-iscsi-direct-auth");
+ DO_TEST_FAIL("pool-iscsi-direct");
+
+ return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
+}
+
+VIR_TEST_MAIN(mymain)
--
2.17.2
8:04 a.m.
New subject: [libvirt] [PATCH 4/8] tests: Introduce tests for storage pool xml to argv checks
On 12/4/18 5:47 PM, John Ferlan wrote:
Similar to qemuxml2argv and storagevolxml2argv, let's create
some
tests to ensure that the XML generates a consistent command line.
Using the same list of pools as storagepoolxml2xmltest, start with
the file system tests (fs, netfs, netfs-cifs, netfs-gluster).
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
tests/Makefile.am | 12 ++
tests/storagepoolxml2argvdata/pool-fs.argv | 1 +
.../pool-netfs-cifs.argv | 1 +
.../pool-netfs-gluster.argv | 1 +
tests/storagepoolxml2argvdata/pool-netfs.argv | 1 +
tests/storagepoolxml2argvtest.c | 171 ++++++++++++++++++
6 files changed, 187 insertions(+)
create mode 100644 tests/storagepoolxml2argvdata/pool-fs.argv
create mode 100644 tests/storagepoolxml2argvdata/pool-netfs-cifs.argv
create mode 100644 tests/storagepoolxml2argvdata/pool-netfs-gluster.argv
create mode 100644 tests/storagepoolxml2argvdata/pool-netfs.argv
create mode 100644 tests/storagepoolxml2argvtest.c
diff --git a/tests/Makefile.am b/tests/Makefile.am
index d7ec7e3a6f..bec0930c11 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -140,6 +140,7 @@ EXTRA_DIST = \
storagepoolschemadata \
storagepoolxml2xmlin \
storagepoolxml2xmlout \
+ storagepoolxml2argvdata \
storagevolschemadata \
storagevolxml2argvdata \
storagevolxml2xmlin \
@@ -363,6 +364,7 @@ endif WITH_NWFILTER
if WITH_STORAGE
test_programs += storagevolxml2argvtest
+test_programs += storagepoolxml2argvtest
test_programs += virstorageutiltest
endif WITH_STORAGE
@@ -901,6 +903,16 @@ storagevolxml2argvtest_LDADD = \
../src/libvirt_util.la \
$(LDADDS)
+storagepoolxml2argvtest_SOURCES = \
+ storagepoolxml2argvtest.c \
+ testutils.c testutils.h
+storagepoolxml2argvtest_LDADD = \
+ $(LIBXML_LIBS) \
+ ../src/libvirt_driver_storage_impl.la \
+ ../src/libvirt_conf.la \
+ ../src/libvirt_util.la \
+ $(LDADDS)
+
else ! WITH_STORAGE
EXTRA_DIST += storagevolxml2argvtest.c
EXTRA_DIST += virstorageutiltest.c
diff --git a/tests/storagepoolxml2argvdata/pool-fs.argv
b/tests/storagepoolxml2argvdata/pool-fs.argv
new file mode 100644
index 0000000000..4a94148114
--- /dev/null
+++ b/tests/storagepoolxml2argvdata/pool-fs.argv
@@ -0,0 +1 @@
+/usr/bin/mount -t ext3 /dev/sda6 /mnt
Problem is, on my system (Gentoo), it is /usr/mount so this is failing
for me.
I wonder if we should have a placeholder here, say "MOUNT" and then
replace it with actual location of 'mount' (taken from config.h) at runtime.
Michal
3:33 p.m.
New subject: [libvirt] [PATCH 4/8] tests: Introduce tests for storage pool xml to argv checks
On 12/12/18 9:04 AM, Michal Privoznik wrote:
On 12/4/18 5:47 PM, John Ferlan wrote:
> Similar to qemuxml2argv and storagevolxml2argv, let's create some
> tests to ensure that the XML generates a consistent command line.
>
> Using the same list of pools as storagepoolxml2xmltest, start with
> the file system tests (fs, netfs, netfs-cifs, netfs-gluster).
>
> Signed-off-by: John Ferlan <jferlan(a)redhat.com>
> ---
> tests/Makefile.am | 12 ++
> tests/storagepoolxml2argvdata/pool-fs.argv | 1 +
> .../pool-netfs-cifs.argv | 1 +
> .../pool-netfs-gluster.argv | 1 +
> tests/storagepoolxml2argvdata/pool-netfs.argv | 1 +
> tests/storagepoolxml2argvtest.c | 171 ++++++++++++++++++
> 6 files changed, 187 insertions(+)
> create mode 100644 tests/storagepoolxml2argvdata/pool-fs.argv
> create mode 100644 tests/storagepoolxml2argvdata/pool-netfs-cifs.argv
> create mode 100644 tests/storagepoolxml2argvdata/pool-netfs-gluster.argv
> create mode 100644 tests/storagepoolxml2argvdata/pool-netfs.argv
> create mode 100644 tests/storagepoolxml2argvtest.c
>
> diff --git a/tests/Makefile.am b/tests/Makefile.am
> index d7ec7e3a6f..bec0930c11 100644
> --- a/tests/Makefile.am
> +++ b/tests/Makefile.am
> @@ -140,6 +140,7 @@ EXTRA_DIST = \
> storagepoolschemadata \
> storagepoolxml2xmlin \
> storagepoolxml2xmlout \
> + storagepoolxml2argvdata \
> storagevolschemadata \
> storagevolxml2argvdata \
> storagevolxml2xmlin \
> @@ -363,6 +364,7 @@ endif WITH_NWFILTER
>
> if WITH_STORAGE
> test_programs += storagevolxml2argvtest
> +test_programs += storagepoolxml2argvtest
> test_programs += virstorageutiltest
> endif WITH_STORAGE
>
> @@ -901,6 +903,16 @@ storagevolxml2argvtest_LDADD = \
> ../src/libvirt_util.la \
> $(LDADDS)
>
> +storagepoolxml2argvtest_SOURCES = \
> + storagepoolxml2argvtest.c \
> + testutils.c testutils.h
> +storagepoolxml2argvtest_LDADD = \
> + $(LIBXML_LIBS) \
> + ../src/libvirt_driver_storage_impl.la \
> + ../src/libvirt_conf.la \
> + ../src/libvirt_util.la \
> + $(LDADDS)
> +
> else ! WITH_STORAGE
> EXTRA_DIST += storagevolxml2argvtest.c
> EXTRA_DIST += virstorageutiltest.c
> diff --git a/tests/storagepoolxml2argvdata/pool-fs.argv
b/tests/storagepoolxml2argvdata/pool-fs.argv
> new file mode 100644
> index 0000000000..4a94148114
> --- /dev/null
> +++ b/tests/storagepoolxml2argvdata/pool-fs.argv
> @@ -0,0 +1 @@
> +/usr/bin/mount -t ext3 /dev/sda6 /mnt
Problem is, on my system (Gentoo), it is /usr/mount so this is failing
for me.
I wonder if we should have a placeholder here, say "MOUNT" and then
replace it with actual location of 'mount' (taken from config.h) at runtime.
Michal
If I add a call to virTestClearCommandPath(actualCmdline); just prior to
the if (virTestCompareToFile(actualCmdline, cmdline) < 0) *and* I modify
each of the .argv files to remove the leading path, then things work
just fine.
Would you like to see or is the explanation good enough? IDC either way.
The .argv files would be affected/changed here, patch5 (netfs-auto), and
patch8 (logical).
Tks -
John
3:08 a.m.
New subject: [libvirt] [PATCH 4/8] tests: Introduce tests for storage pool xml to argv checks
On 12/12/18 10:33 PM, John Ferlan wrote:
On 12/12/18 9:04 AM, Michal Privoznik wrote:
> On 12/4/18 5:47 PM, John Ferlan wrote:
>> Similar to qemuxml2argv and storagevolxml2argv, let's create some
>> tests to ensure that the XML generates a consistent command line.
>>
>> Using the same list of pools as storagepoolxml2xmltest, start with
>> the file system tests (fs, netfs, netfs-cifs, netfs-gluster).
>>
>> Signed-off-by: John Ferlan <jferlan(a)redhat.com>
>> ---
>> tests/Makefile.am | 12 ++
>> tests/storagepoolxml2argvdata/pool-fs.argv | 1 +
>> .../pool-netfs-cifs.argv | 1 +
>> .../pool-netfs-gluster.argv | 1 +
>> tests/storagepoolxml2argvdata/pool-netfs.argv | 1 +
>> tests/storagepoolxml2argvtest.c | 171 ++++++++++++++++++
>> 6 files changed, 187 insertions(+)
>> create mode 100644 tests/storagepoolxml2argvdata/pool-fs.argv
>> create mode 100644 tests/storagepoolxml2argvdata/pool-netfs-cifs.argv
>> create mode 100644 tests/storagepoolxml2argvdata/pool-netfs-gluster.argv
>> create mode 100644 tests/storagepoolxml2argvdata/pool-netfs.argv
>> create mode 100644 tests/storagepoolxml2argvtest.c
>>
>> diff --git a/tests/Makefile.am b/tests/Makefile.am
>> index d7ec7e3a6f..bec0930c11 100644
>> --- a/tests/Makefile.am
>> +++ b/tests/Makefile.am
>> @@ -140,6 +140,7 @@ EXTRA_DIST = \
>> storagepoolschemadata \
>> storagepoolxml2xmlin \
>> storagepoolxml2xmlout \
>> + storagepoolxml2argvdata \
>> storagevolschemadata \
>> storagevolxml2argvdata \
>> storagevolxml2xmlin \
>> @@ -363,6 +364,7 @@ endif WITH_NWFILTER
>>
>> if WITH_STORAGE
>> test_programs += storagevolxml2argvtest
>> +test_programs += storagepoolxml2argvtest
>> test_programs += virstorageutiltest
>> endif WITH_STORAGE
>>
>> @@ -901,6 +903,16 @@ storagevolxml2argvtest_LDADD = \
>> ../src/libvirt_util.la \
>> $(LDADDS)
>>
>> +storagepoolxml2argvtest_SOURCES = \
>> + storagepoolxml2argvtest.c \
>> + testutils.c testutils.h
>> +storagepoolxml2argvtest_LDADD = \
>> + $(LIBXML_LIBS) \
>> + ../src/libvirt_driver_storage_impl.la \
>> + ../src/libvirt_conf.la \
>> + ../src/libvirt_util.la \
>> + $(LDADDS)
>> +
>> else ! WITH_STORAGE
>> EXTRA_DIST += storagevolxml2argvtest.c
>> EXTRA_DIST += virstorageutiltest.c
>> diff --git a/tests/storagepoolxml2argvdata/pool-fs.argv
b/tests/storagepoolxml2argvdata/pool-fs.argv
>> new file mode 100644
>> index 0000000000..4a94148114
>> --- /dev/null
>> +++ b/tests/storagepoolxml2argvdata/pool-fs.argv
>> @@ -0,0 +1 @@
>> +/usr/bin/mount -t ext3 /dev/sda6 /mnt
>
> Problem is, on my system (Gentoo), it is /usr/mount so this is failing
> for me.
>
> I wonder if we should have a placeholder here, say "MOUNT" and then
> replace it with actual location of 'mount' (taken from config.h) at runtime.
>
> Michal
>
If I add a call to virTestClearCommandPath(actualCmdline); just prior to
the if (virTestCompareToFile(actualCmdline, cmdline) < 0) *and* I modify
each of the .argv files to remove the leading path, then things work
just fine.
Ah, had no idea we have such nifty function.
Would you like to see or is the explanation good enough? IDC either way.
I believe you can place the function call at the right place and
regenerate the test outputs.
ACK series then.
Michal
10:47 a.m.
New subject: [libvirt] [PATCH 5/8] tests: Add storagepool xml test for netfs-auto
Cover the case where @netauto would be used to create the command
line in virStorageBackendFileSystemMountCmd. Essentially when the
pool type is "netfs", but the "source.format" is empty, create the
command line properly.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
.../pool-netfs-auto.argv | 1 +
tests/storagepoolxml2argvtest.c | 1 +
.../storagepoolxml2xmlin/pool-netfs-auto.xml | 19 ++++++++++++++++++
.../storagepoolxml2xmlout/pool-netfs-auto.xml | 20 +++++++++++++++++++
tests/storagepoolxml2xmltest.c | 1 +
5 files changed, 42 insertions(+)
create mode 100644 tests/storagepoolxml2argvdata/pool-netfs-auto.argv
create mode 100644 tests/storagepoolxml2xmlin/pool-netfs-auto.xml
create mode 100644 tests/storagepoolxml2xmlout/pool-netfs-auto.xml
diff --git a/tests/storagepoolxml2argvdata/pool-netfs-auto.argv
b/tests/storagepoolxml2argvdata/pool-netfs-auto.argv
new file mode 100644
index 0000000000..217fcc786b
--- /dev/null
+++ b/tests/storagepoolxml2argvdata/pool-netfs-auto.argv
@@ -0,0 +1 @@
+/usr/bin/mount localhost:/var/lib/libvirt/images /mnt
diff --git a/tests/storagepoolxml2argvtest.c b/tests/storagepoolxml2argvtest.c
index 74f8561379..42044e3518 100644
--- a/tests/storagepoolxml2argvtest.c
+++ b/tests/storagepoolxml2argvtest.c
@@ -145,6 +145,7 @@ mymain(void)
DO_TEST_FAIL("pool-iscsi");
DO_TEST_FAIL("pool-iscsi-auth");
DO_TEST("pool-netfs");
+ DO_TEST("pool-netfs-auto");
DO_TEST("pool-netfs-gluster");
DO_TEST("pool-netfs-cifs");
DO_TEST_FAIL("pool-scsi");
diff --git a/tests/storagepoolxml2xmlin/pool-netfs-auto.xml
b/tests/storagepoolxml2xmlin/pool-netfs-auto.xml
new file mode 100644
index 0000000000..d7f7ce8168
--- /dev/null
+++ b/tests/storagepoolxml2xmlin/pool-netfs-auto.xml
@@ -0,0 +1,19 @@
+<pool type='netfs'>
+ <name>nfsimages</name>
+ <uuid>7641d5a8-af11-f730-a34e-0a7dfcede71f</uuid>
+ <capacity>0</capacity>
+ <allocation>0</allocation>
+ <available>0</available>
+ <source>
+ <host name='localhost'/>
+ <dir path='/var/lib/libvirt/images'/>
+ </source>
+ <target>
+ <path>/mnt</path>
+ <permissions>
+ <mode>0700</mode>
+ <owner>0</owner>
+ <group>0</group>
+ </permissions>
+ </target>
+</pool>
diff --git a/tests/storagepoolxml2xmlout/pool-netfs-auto.xml
b/tests/storagepoolxml2xmlout/pool-netfs-auto.xml
new file mode 100644
index 0000000000..a180ca521c
--- /dev/null
+++ b/tests/storagepoolxml2xmlout/pool-netfs-auto.xml
@@ -0,0 +1,20 @@
+<pool type='netfs'>
+ <name>nfsimages</name>
+ <uuid>7641d5a8-af11-f730-a34e-0a7dfcede71f</uuid>
+ <capacity unit='bytes'>0</capacity>
+ <allocation unit='bytes'>0</allocation>
+ <available unit='bytes'>0</available>
+ <source>
+ <host name='localhost'/>
+ <dir path='/var/lib/libvirt/images'/>
+ <format type='auto'/>
+ </source>
+ <target>
+ <path>/mnt</path>
+ <permissions>
+ <mode>0700</mode>
+ <owner>0</owner>
+ <group>0</group>
+ </permissions>
+ </target>
+</pool>
diff --git a/tests/storagepoolxml2xmltest.c b/tests/storagepoolxml2xmltest.c
index 8230dc8ddc..707d09f5c2 100644
--- a/tests/storagepoolxml2xmltest.c
+++ b/tests/storagepoolxml2xmltest.c
@@ -82,6 +82,7 @@ mymain(void)
DO_TEST("pool-iscsi");
DO_TEST("pool-iscsi-auth");
DO_TEST("pool-netfs");
+ DO_TEST("pool-netfs-auto");
DO_TEST("pool-netfs-gluster");
DO_TEST("pool-netfs-cifs");
DO_TEST("pool-scsi");
--
2.17.2
10:47 a.m.
New subject: [libvirt] [PATCH 6/8] storage: Rework virStorageBackendFileSystemMountCmd
Let's create helpers for each style of command line created. This
primarily is easier on the eyes rather than the large multi line
if-then-else-else clause used, but may also be useful if in the
future any particular pool needs to add to the command line based
on pool xml format.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/storage/storage_util.c | 84 +++++++++++++++++++++++++-------------
1 file changed, 55 insertions(+), 29 deletions(-)
diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index c9f6096687..789f270f2a 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -4261,6 +4261,56 @@ virStorageBackendFileSystemGetPoolSource(virStoragePoolObjPtr
pool)
}
+static void
+virStorageBackendFileSystemMountNFSArgs(virCommandPtr cmd,
+ const char *src,
+ virStoragePoolDefPtr def)
+{
+ virCommandAddArgList(cmd, src, def->target.path, NULL);
+}
+
+
+static void
+virStorageBackendFileSystemMountGlusterArgs(virCommandPtr cmd,
+ const char *src,
+ virStoragePoolDefPtr def)
+{
+ const char *fmt;
+
+ fmt = virStoragePoolFormatFileSystemNetTypeToString(def->source.format);
+ virCommandAddArgList(cmd, "-t", fmt, src, "-o",
"direct-io-mode=1",
+ def->target.path, NULL);
+}
+
+
+static void
+virStorageBackendFileSystemMountCIFSArgs(virCommandPtr cmd,
+ const char *src,
+ virStoragePoolDefPtr def)
+{
+ const char *fmt;
+
+ fmt = virStoragePoolFormatFileSystemNetTypeToString(def->source.format);
+ virCommandAddArgList(cmd, "-t", fmt, src, def->target.path,
+ "-o", "guest", NULL);
+}
+
+
+static void
+virStorageBackendFileSystemMountDefaultArgs(virCommandPtr cmd,
+ const char *src,
+ virStoragePoolDefPtr def)
+{
+ const char *fmt;
+
+ if (def->type == VIR_STORAGE_POOL_FS)
+ fmt = virStoragePoolFormatFileSystemTypeToString(def->source.format);
+ else
+ fmt = virStoragePoolFormatFileSystemNetTypeToString(def->source.format);
+ virCommandAddArgList(cmd, "-t", fmt, src, def->target.path, NULL);
+}
+
+
virCommandPtr
virStorageBackendFileSystemMountCmd(virStoragePoolDefPtr def,
const char *src)
@@ -4276,38 +4326,14 @@ virStorageBackendFileSystemMountCmd(virStoragePoolDefPtr def,
def->source.format == VIR_STORAGE_POOL_NETFS_CIFS);
virCommandPtr cmd = NULL;
+ cmd = virCommandNew(MOUNT);
if (netauto)
- cmd = virCommandNewArgList(MOUNT,
- src,
- def->target.path,
- NULL);
+ virStorageBackendFileSystemMountNFSArgs(cmd, src, def);
else if (glusterfs)
- cmd = virCommandNewArgList(MOUNT,
- "-t",
-
virStoragePoolFormatFileSystemNetTypeToString(def->source.format),
- src,
- "-o",
- "direct-io-mode=1",
- def->target.path,
- NULL);
+ virStorageBackendFileSystemMountGlusterArgs(cmd, src, def);
else if (cifsfs)
- cmd = virCommandNewArgList(MOUNT,
- "-t",
-
virStoragePoolFormatFileSystemNetTypeToString(def->source.format),
- src,
- def->target.path,
- "-o",
- "guest",
- NULL);
+ virStorageBackendFileSystemMountCIFSArgs(cmd, src, def);
else
- cmd = virCommandNewArgList(MOUNT,
- "-t",
- (def->type == VIR_STORAGE_POOL_FS ?
-
virStoragePoolFormatFileSystemTypeToString(def->source.format) :
-
virStoragePoolFormatFileSystemNetTypeToString(def->source.format)),
- src,
- def->target.path,
- NULL);
-
+ virStorageBackendFileSystemMountDefaultArgs(cmd, src, def);
return cmd;
}
--
2.17.2
10:47 a.m.
New subject: [libvirt] [PATCH 7/8] logical: Fix @on argument type
It's only pass as 0 or 1 and used as a bool, let's just use a bool
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/storage/storage_backend_logical.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/storage/storage_backend_logical.c
b/src/storage/storage_backend_logical.c
index cad88756cb..44cff61af7 100644
--- a/src/storage/storage_backend_logical.c
+++ b/src/storage/storage_backend_logical.c
@@ -48,7 +48,7 @@ VIR_LOG_INIT("storage.storage_backend_logical");
static int
virStorageBackendLogicalSetActive(virStoragePoolObjPtr pool,
- int on)
+ bool on)
{
int ret;
virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool);
@@ -731,7 +731,7 @@ virStorageBackendLogicalStartPool(virStoragePoolObjPtr pool)
* that the pool's source devices match the pvs output.
*/
if (!virStorageBackendLogicalMatchPoolSource(pool) ||
- virStorageBackendLogicalSetActive(pool, 1) < 0)
+ virStorageBackendLogicalSetActive(pool, true) < 0)
return -1;
return 0;
@@ -858,7 +858,7 @@ virStorageBackendLogicalRefreshPool(virStoragePoolObjPtr pool)
static int
virStorageBackendLogicalStopPool(virStoragePoolObjPtr pool)
{
- if (virStorageBackendLogicalSetActive(pool, 0) < 0)
+ if (virStorageBackendLogicalSetActive(pool, false) < 0)
return -1;
return 0;
--
2.17.2
10:47 a.m.
New subject: [libvirt] [PATCH 8/8] storage: Add tests for logical backend startup
Add the logical storage pool startup validation (xml2argv) tests.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/storage/storage_backend_logical.c | 6 +-----
src/storage/storage_util.c | 11 +++++++++++
src/storage/storage_util.h | 4 ++++
.../pool-logical-create.argv | 1 +
.../pool-logical-noname.argv | 1 +
.../pool-logical-nopath.argv | 1 +
tests/storagepoolxml2argvdata/pool-logical.argv | 1 +
tests/storagepoolxml2argvtest.c | 13 ++++++++-----
8 files changed, 28 insertions(+), 10 deletions(-)
create mode 100644 tests/storagepoolxml2argvdata/pool-logical-create.argv
create mode 100644 tests/storagepoolxml2argvdata/pool-logical-noname.argv
create mode 100644 tests/storagepoolxml2argvdata/pool-logical-nopath.argv
create mode 100644 tests/storagepoolxml2argvdata/pool-logical.argv
diff --git a/src/storage/storage_backend_logical.c
b/src/storage/storage_backend_logical.c
index 44cff61af7..12fff651e8 100644
--- a/src/storage/storage_backend_logical.c
+++ b/src/storage/storage_backend_logical.c
@@ -52,11 +52,7 @@ virStorageBackendLogicalSetActive(virStoragePoolObjPtr pool,
{
int ret;
virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool);
- virCommandPtr cmd =
- virCommandNewArgList(VGCHANGE,
- on ? "-aly" : "-aln",
- def->source.name,
- NULL);
+ virCommandPtr cmd = virStorageBackendLogicalChangeCmd(def, on);
ret = virCommandRun(cmd, NULL);
virCommandFree(cmd);
diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index 789f270f2a..01f3c93008 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -4337,3 +4337,14 @@ virStorageBackendFileSystemMountCmd(virStoragePoolDefPtr def,
virStorageBackendFileSystemMountDefaultArgs(cmd, src, def);
return cmd;
}
+
+
+virCommandPtr
+virStorageBackendLogicalChangeCmd(virStoragePoolDefPtr def,
+ bool on)
+{
+ return virCommandNewArgList(VGCHANGE,
+ on ? "-aly" : "-aln",
+ def->source.name,
+ NULL);
+}
diff --git a/src/storage/storage_util.h b/src/storage/storage_util.h
index 28b3e0b9c9..a2ef2ac07d 100644
--- a/src/storage/storage_util.h
+++ b/src/storage/storage_util.h
@@ -184,4 +184,8 @@ virCommandPtr
virStorageBackendFileSystemMountCmd(virStoragePoolDefPtr def,
const char *src);
+virCommandPtr
+virStorageBackendLogicalChangeCmd(virStoragePoolDefPtr def,
+ bool on);
+
#endif /* __VIR_STORAGE_UTIL_H__ */
diff --git a/tests/storagepoolxml2argvdata/pool-logical-create.argv
b/tests/storagepoolxml2argvdata/pool-logical-create.argv
new file mode 100644
index 0000000000..203da86e48
--- /dev/null
+++ b/tests/storagepoolxml2argvdata/pool-logical-create.argv
@@ -0,0 +1 @@
+/usr/sbin/vgchange -aly HostVG
diff --git a/tests/storagepoolxml2argvdata/pool-logical-noname.argv
b/tests/storagepoolxml2argvdata/pool-logical-noname.argv
new file mode 100644
index 0000000000..0813467120
--- /dev/null
+++ b/tests/storagepoolxml2argvdata/pool-logical-noname.argv
@@ -0,0 +1 @@
+/usr/sbin/vgchange -aly zily
diff --git a/tests/storagepoolxml2argvdata/pool-logical-nopath.argv
b/tests/storagepoolxml2argvdata/pool-logical-nopath.argv
new file mode 100644
index 0000000000..203da86e48
--- /dev/null
+++ b/tests/storagepoolxml2argvdata/pool-logical-nopath.argv
@@ -0,0 +1 @@
+/usr/sbin/vgchange -aly HostVG
diff --git a/tests/storagepoolxml2argvdata/pool-logical.argv
b/tests/storagepoolxml2argvdata/pool-logical.argv
new file mode 100644
index 0000000000..203da86e48
--- /dev/null
+++ b/tests/storagepoolxml2argvdata/pool-logical.argv
@@ -0,0 +1 @@
+/usr/sbin/vgchange -aly HostVG
diff --git a/tests/storagepoolxml2argvtest.c b/tests/storagepoolxml2argvtest.c
index 42044e3518..26b12c8fc3 100644
--- a/tests/storagepoolxml2argvtest.c
+++ b/tests/storagepoolxml2argvtest.c
@@ -43,8 +43,11 @@ testCompareXMLToArgvFiles(bool shouldFail,
cmd = virStorageBackendFileSystemMountCmd(def, src);
break;
- case VIR_STORAGE_POOL_DIR:
case VIR_STORAGE_POOL_LOGICAL:
+ cmd = virStorageBackendLogicalChangeCmd(def, true);
+ break;
+
+ case VIR_STORAGE_POOL_DIR:
case VIR_STORAGE_POOL_DISK:
case VIR_STORAGE_POOL_ISCSI:
case VIR_STORAGE_POOL_ISCSI_DIRECT:
@@ -136,10 +139,10 @@ mymain(void)
DO_TEST_FAIL("pool-dir");
DO_TEST_FAIL("pool-dir-naming");
DO_TEST("pool-fs");
- DO_TEST_FAIL("pool-logical");
- DO_TEST_FAIL("pool-logical-nopath");
- DO_TEST_FAIL("pool-logical-create");
- DO_TEST_FAIL("pool-logical-noname");
+ DO_TEST("pool-logical");
+ DO_TEST("pool-logical-nopath");
+ DO_TEST("pool-logical-create");
+ DO_TEST("pool-logical-noname");
DO_TEST_FAIL("pool-disk");
DO_TEST_FAIL("pool-disk-device-nopartsep");
DO_TEST_FAIL("pool-iscsi");
--
2.17.2
8:04 a.m.
New subject: [libvirt] [PATCH 8/8] storage: Add tests for logical backend startup
On 12/4/18 5:47 PM, John Ferlan wrote:
Add the logical storage pool startup validation (xml2argv) tests.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/storage/storage_backend_logical.c | 6 +-----
src/storage/storage_util.c | 11 +++++++++++
src/storage/storage_util.h | 4 ++++
.../pool-logical-create.argv | 1 +
.../pool-logical-noname.argv | 1 +
.../pool-logical-nopath.argv | 1 +
tests/storagepoolxml2argvdata/pool-logical.argv | 1 +
tests/storagepoolxml2argvtest.c | 13 ++++++++-----
8 files changed, 28 insertions(+), 10 deletions(-)
create mode 100644 tests/storagepoolxml2argvdata/pool-logical-create.argv
create mode 100644 tests/storagepoolxml2argvdata/pool-logical-noname.argv
create mode 100644 tests/storagepoolxml2argvdata/pool-logical-nopath.argv
create mode 100644 tests/storagepoolxml2argvdata/pool-logical.argv
diff --git a/src/storage/storage_backend_logical.c
b/src/storage/storage_backend_logical.c
index 44cff61af7..12fff651e8 100644
--- a/src/storage/storage_backend_logical.c
+++ b/src/storage/storage_backend_logical.c
@@ -52,11 +52,7 @@ virStorageBackendLogicalSetActive(virStoragePoolObjPtr pool,
{
int ret;
virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool);
- virCommandPtr cmd =
- virCommandNewArgList(VGCHANGE,
- on ? "-aly" : "-aln",
- def->source.name,
- NULL);
+ virCommandPtr cmd = virStorageBackendLogicalChangeCmd(def, on);
ret = virCommandRun(cmd, NULL);
virCommandFree(cmd);
diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index 789f270f2a..01f3c93008 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -4337,3 +4337,14 @@ virStorageBackendFileSystemMountCmd(virStoragePoolDefPtr def,
virStorageBackendFileSystemMountDefaultArgs(cmd, src, def);
return cmd;
}
+
+
+virCommandPtr
+virStorageBackendLogicalChangeCmd(virStoragePoolDefPtr def,
+ bool on)
+{
+ return virCommandNewArgList(VGCHANGE,
+ on ? "-aly" : "-aln",
+ def->source.name,
+ NULL);
+}
diff --git a/src/storage/storage_util.h b/src/storage/storage_util.h
index 28b3e0b9c9..a2ef2ac07d 100644
--- a/src/storage/storage_util.h
+++ b/src/storage/storage_util.h
@@ -184,4 +184,8 @@ virCommandPtr
virStorageBackendFileSystemMountCmd(virStoragePoolDefPtr def,
const char *src);
+virCommandPtr
+virStorageBackendLogicalChangeCmd(virStoragePoolDefPtr def,
+ bool on);
+
#endif /* __VIR_STORAGE_UTIL_H__ */
diff --git a/tests/storagepoolxml2argvdata/pool-logical-create.argv
b/tests/storagepoolxml2argvdata/pool-logical-create.argv
new file mode 100644
index 0000000000..203da86e48
--- /dev/null
+++ b/tests/storagepoolxml2argvdata/pool-logical-create.argv
@@ -0,0 +1 @@
+/usr/sbin/vgchange -aly HostVG
The same point here. On my system it is /sbin/vgchange.
Michal
4:29 p.m.
ping?
Thanks,
John
On 12/4/18 11:47 AM, John Ferlan wrote:
Similar to qemuxml2argv and storagevolxml2argv, add
storagepoolxml2argvtest
in order to check the command line creation for the pool 'Start' commands.
Only applicable for pool types with a "@startPool" function - that is disk,
fs, iscsi, logical, scsi, and vstorage and further restricted if the pool
doesn't use virCommandPtr type processing.
This initial series only addresses fs and logical so that it's not "too
many patches to review".
The iscsi and vstorage pools could have their own tests, with the iscsi
ones being more challenging to write.
The disk pool does not have the normal command line processing to start
something - rather the command line processing is used to validate that
the pool about to be started is of a valid type based on the label seen
at startup compared to the pool XML. This processing is not easily mocked.
The scsi (for NPIV) doesn't use the virCommandPtr style interfaces, but at
least that processing is tested in other ways using testCreateVport from
test_driver.c.
John Ferlan (8):
storage: Extract out mount command creation for FS Backend
storage: Move FS backend mount creation command helper
storage: Move virStorageBackendFileSystemGetPoolSource
tests: Introduce tests for storage pool xml to argv checks
tests: Add storagepool xml test for netfs-auto
storage: Rework virStorageBackendFileSystemMountCmd
logical: Fix @on argument type
storage: Add tests for logical backend startup
src/storage/storage_backend_fs.c | 77 +-------
src/storage/storage_backend_logical.c | 12 +-
src/storage/storage_util.c | 122 ++++++++++++
src/storage/storage_util.h | 11 ++
tests/Makefile.am | 12 ++
tests/storagepoolxml2argvdata/pool-fs.argv | 1 +
.../pool-logical-create.argv | 1 +
.../pool-logical-noname.argv | 1 +
.../pool-logical-nopath.argv | 1 +
.../storagepoolxml2argvdata/pool-logical.argv | 1 +
.../pool-netfs-auto.argv | 1 +
.../pool-netfs-cifs.argv | 1 +
.../pool-netfs-gluster.argv | 1 +
tests/storagepoolxml2argvdata/pool-netfs.argv | 1 +
tests/storagepoolxml2argvtest.c | 175 ++++++++++++++++++
.../storagepoolxml2xmlin/pool-netfs-auto.xml | 19 ++
.../storagepoolxml2xmlout/pool-netfs-auto.xml | 20 ++
tests/storagepoolxml2xmltest.c | 1 +
18 files changed, 375 insertions(+), 83 deletions(-)
create mode 100644 tests/storagepoolxml2argvdata/pool-fs.argv
create mode 100644 tests/storagepoolxml2argvdata/pool-logical-create.argv
create mode 100644 tests/storagepoolxml2argvdata/pool-logical-noname.argv
create mode 100644 tests/storagepoolxml2argvdata/pool-logical-nopath.argv
create mode 100644 tests/storagepoolxml2argvdata/pool-logical.argv
create mode 100644 tests/storagepoolxml2argvdata/pool-netfs-auto.argv
create mode 100644 tests/storagepoolxml2argvdata/pool-netfs-cifs.argv
create mode 100644 tests/storagepoolxml2argvdata/pool-netfs-gluster.argv
create mode 100644 tests/storagepoolxml2argvdata/pool-netfs.argv
create mode 100644 tests/storagepoolxml2argvtest.c
create mode 100644 tests/storagepoolxml2xmlin/pool-netfs-auto.xml
create mode 100644 tests/storagepoolxml2xmlout/pool-netfs-auto.xml
8:04 a.m.
On 12/4/18 5:47 PM, John Ferlan wrote:
Similar to qemuxml2argv and storagevolxml2argv, add
storagepoolxml2argvtest
in order to check the command line creation for the pool 'Start' commands.
Only applicable for pool types with a "@startPool" function - that is disk,
fs, iscsi, logical, scsi, and vstorage and further restricted if the pool
doesn't use virCommandPtr type processing.
This initial series only addresses fs and logical so that it's not "too
many patches to review".
The iscsi and vstorage pools could have their own tests, with the iscsi
ones being more challenging to write.
The disk pool does not have the normal command line processing to start
something - rather the command line processing is used to validate that
the pool about to be started is of a valid type based on the label seen
at startup compared to the pool XML. This processing is not easily mocked.
The scsi (for NPIV) doesn't use the virCommandPtr style interfaces, but at
least that processing is tested in other ways using testCreateVport from
test_driver.c.
John Ferlan (8):
storage: Extract out mount command creation for FS Backend
storage: Move FS backend mount creation command helper
storage: Move virStorageBackendFileSystemGetPoolSource
tests: Introduce tests for storage pool xml to argv checks
tests: Add storagepool xml test for netfs-auto
storage: Rework virStorageBackendFileSystemMountCmd
logical: Fix @on argument type
storage: Add tests for logical backend startup
src/storage/storage_backend_fs.c | 77 +-------
src/storage/storage_backend_logical.c | 12 +-
src/storage/storage_util.c | 122 ++++++++++++
src/storage/storage_util.h | 11 ++
tests/Makefile.am | 12 ++
tests/storagepoolxml2argvdata/pool-fs.argv | 1 +
.../pool-logical-create.argv | 1 +
.../pool-logical-noname.argv | 1 +
.../pool-logical-nopath.argv | 1 +
.../storagepoolxml2argvdata/pool-logical.argv | 1 +
.../pool-netfs-auto.argv | 1 +
.../pool-netfs-cifs.argv | 1 +
.../pool-netfs-gluster.argv | 1 +
tests/storagepoolxml2argvdata/pool-netfs.argv | 1 +
tests/storagepoolxml2argvtest.c | 175 ++++++++++++++++++
.../storagepoolxml2xmlin/pool-netfs-auto.xml | 19 ++
.../storagepoolxml2xmlout/pool-netfs-auto.xml | 20 ++
tests/storagepoolxml2xmltest.c | 1 +
18 files changed, 375 insertions(+), 83 deletions(-)
create mode 100644 tests/storagepoolxml2argvdata/pool-fs.argv
create mode 100644 tests/storagepoolxml2argvdata/pool-logical-create.argv
create mode 100644 tests/storagepoolxml2argvdata/pool-logical-noname.argv
create mode 100644 tests/storagepoolxml2argvdata/pool-logical-nopath.argv
create mode 100644 tests/storagepoolxml2argvdata/pool-logical.argv
create mode 100644 tests/storagepoolxml2argvdata/pool-netfs-auto.argv
create mode 100644 tests/storagepoolxml2argvdata/pool-netfs-cifs.argv
create mode 100644 tests/storagepoolxml2argvdata/pool-netfs-gluster.argv
create mode 100644 tests/storagepoolxml2argvdata/pool-netfs.argv
create mode 100644 tests/storagepoolxml2argvtest.c
create mode 100644 tests/storagepoolxml2xmlin/pool-netfs-auto.xml
create mode 100644 tests/storagepoolxml2xmlout/pool-netfs-auto.xml
The patches look good from logical POV. However, there is an issue with
mount/vgchange location on different systems.
Michal
8:42 a.m.
On 12/12/18 9:04 AM, Michal Privoznik wrote:
On 12/4/18 5:47 PM, John Ferlan wrote:
> Similar to qemuxml2argv and storagevolxml2argv, add storagepoolxml2argvtest
> in order to check the command line creation for the pool 'Start' commands.
>
> Only applicable for pool types with a "@startPool" function - that is
disk,
> fs, iscsi, logical, scsi, and vstorage and further restricted if the pool
> doesn't use virCommandPtr type processing.
>
> This initial series only addresses fs and logical so that it's not "too
> many patches to review".
>
> The iscsi and vstorage pools could have their own tests, with the iscsi
> ones being more challenging to write.
>
> The disk pool does not have the normal command line processing to start
> something - rather the command line processing is used to validate that
> the pool about to be started is of a valid type based on the label seen
> at startup compared to the pool XML. This processing is not easily mocked.
>
> The scsi (for NPIV) doesn't use the virCommandPtr style interfaces, but at
> least that processing is tested in other ways using testCreateVport from
> test_driver.c.
>
> John Ferlan (8):
> storage: Extract out mount command creation for FS Backend
> storage: Move FS backend mount creation command helper
> storage: Move virStorageBackendFileSystemGetPoolSource
> tests: Introduce tests for storage pool xml to argv checks
> tests: Add storagepool xml test for netfs-auto
> storage: Rework virStorageBackendFileSystemMountCmd
> logical: Fix @on argument type
> storage: Add tests for logical backend startup
>
> src/storage/storage_backend_fs.c | 77 +-------
> src/storage/storage_backend_logical.c | 12 +-
> src/storage/storage_util.c | 122 ++++++++++++
> src/storage/storage_util.h | 11 ++
> tests/Makefile.am | 12 ++
> tests/storagepoolxml2argvdata/pool-fs.argv | 1 +
> .../pool-logical-create.argv | 1 +
> .../pool-logical-noname.argv | 1 +
> .../pool-logical-nopath.argv | 1 +
> .../storagepoolxml2argvdata/pool-logical.argv | 1 +
> .../pool-netfs-auto.argv |
> .../pool-netfs-cifs.argv | 1 +
> .../pool-netfs-gluster.argv | 1 +
> tests/storagepoolxml2argvdata/pool-netfs.argv | 1 +
> tests/storagepoolxml2argvtest.c | 175 ++++++++++++++++++
> .../storagepoolxml2xmlin/pool-netfs-auto.xml | 19 ++
> .../storagepoolxml2xmlout/pool-netfs-auto.xml | 20 ++
> tests/storagepoolxml2xmltest.c | 1 +
> 18 files changed, 375 insertions(+), 83 deletions(-)
> create mode 100644 tests/storagepoolxml2argvdata/pool-fs.argv
> create mode 100644 tests/storagepoolxml2argvdata/pool-logical-create.argv
> create mode 100644 tests/storagepoolxml2argvdata/pool-logical-noname.argv
> create mode 100644 tests/storagepoolxml2argvdata/pool-logical-nopath.argv
> create mode 100644 tests/storagepoolxml2argvdata/pool-logical.argv
> create mode 100644 tests/storagepoolxml2argvdata/pool-netfs-auto.argv
> create mode 100644 tests/storagepoolxml2argvdata/pool-netfs-cifs.argv
> create mode 100644 tests/storagepoolxml2argvdata/pool-netfs-gluster.argv
> create mode 100644 tests/storagepoolxml2argvdata/pool-netfs.argv
> create mode 100644 tests/storagepoolxml2argvtest.c
> create mode 100644 tests/storagepoolxml2xmlin/pool-netfs-auto.xml
> create mode 100644 tests/storagepoolxml2xmlout/pool-netfs-auto.xml
>
The patches look good from logical POV. However, there is an issue with
mount/vgchange location on different systems.
Michal
Hmm... good to know... Maybe I can strip the path to mount/vgchange from
the output... That shouldn't be hard to do (hah!).
I'll see if I can come up with something...
Tks -
John
2171
days inactive
2180
days old
15 comments
2 participants
participants (2)
-
John Ferlan -
Michal Privoznik