Re: [libvirt] [PATCH 0/8] Add tests for Storage Pool startup command line generation

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