Re: [libvirt] [PATCH] test: storage: Fill in default vol types for every pool
On 3/7/19 2:35 PM, Cole Robinson wrote:
Fill in a default volume type for every pool type, as reported
by the VolGetInfo API.
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
---
A more complete fix would take into account VolDef->type, so test
driver users could set this value via volume XML. That would require
adding a PostParse callback to the storage driver infrastructure.
This is still an improvement in the interim and would be part of
a complete fix anyways
src/test/test_driver.c | 27 ++++++++++++++++++++-------
1 file changed, 20 insertions(+), 7 deletions(-)
At first I wondered if this had to do with changes I made starting at
commit 4ad00278f going thru commit 538b83ae, but it doesn't seem so.
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index ce0df1f8e3..3f37b2ede7 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -5164,13 +5164,26 @@ testStorageVolDelete(virStorageVolPtr vol,
static int
testStorageVolumeTypeForPool(int pooltype)
{
- switch (pooltype) {
- case VIR_STORAGE_POOL_DIR:
- case VIR_STORAGE_POOL_FS:
- case VIR_STORAGE_POOL_NETFS:
- return VIR_STORAGE_VOL_FILE;
- default:
- return VIR_STORAGE_VOL_BLOCK;
+ switch ((virStoragePoolType) pooltype) {
+ case VIR_STORAGE_POOL_DIR:
+ case VIR_STORAGE_POOL_FS:
+ case VIR_STORAGE_POOL_NETFS:
+ case VIR_STORAGE_POOL_VSTORAGE:
+ return VIR_STORAGE_VOL_FILE;
+ case VIR_STORAGE_POOL_SHEEPDOG:
+ case VIR_STORAGE_POOL_ISCSI_DIRECT:
^^ This I would think should be BLOCK even though I see
virISCSIDirectRefreshVol uses NETWORK at volume creation.
Perhaps Michal can answer provide some insights.
+ case VIR_STORAGE_POOL_GLUSTER:
+ case VIR_STORAGE_POOL_RBD:
+ return VIR_STORAGE_VOL_NETWORK;
+ case VIR_STORAGE_POOL_LOGICAL:
+ case VIR_STORAGE_POOL_DISK:
+ case VIR_STORAGE_POOL_MPATH:
+ case VIR_STORAGE_POOL_ISCSI:
^^^ To a degree ISCSI could be NETWORK, but may also be BLOCK. Still the
default seems to be BLOCK (see below)
+ case VIR_STORAGE_POOL_SCSI:
+ case VIR_STORAGE_POOL_ZFS:
+ case VIR_STORAGE_POOL_LAST:
^^^ I agree w/ Andrea that this could an Enum error; however, it doesn't
seem the caller at this point can handle that.
+ default:
+ return VIR_STORAGE_VOL_BLOCK;
}
}
Using cscope I did an egrep search on "vol.*type.* = " as well as
"VIR_ALLOC.*vol" in order to help find places where a volume's type may
or should be set to see what kind of defaults are in use.
Beyond that consider that volume's in a pool volume list are recreated
at every refresh, so following the *Refresh logic to find types works
pretty well too.
For virStorageBackendRefreshLocal it's VIR_STORAGE_VOL_FILE
For virStorageBackendSCSINewLun it's VIR_STORAGE_VOL_BLOCK (used by scsi
and iscsi backends via calls to virStorageBackendSCSIFindLUs).
Perhaps a "more correct" answer comes from virStoragePoolTypeInfoLookup
or virStoragePoolOptionsForPoolType and adjustment of poolTypeInfo in
storage_conf.c?
John