[libvirt] [PATCH 0/2] glusterfs related fixes
Hi, I noticed that "virsh pool-list --type gluster" is not giving me an empty list as expected, but rather the list of active pools. This is happening because virConnectListAllStoragePools() is returning VIR_ERROR_INVALID_ARG because VIR_CONNECT_LIST_STORAGE_POOLS_GLUSTER is unrecognized. vshStoragePoolListCollect then goes to retry with only VIR_CONNECT_LIST_STORAGE_POOLS_ACTIVE | VIR_CONNECT_LIST_STORAGE_POOLS_INACTIVE in the flags, which is questionable, but explains the result I got. Adding VIR_CONNECT_LIST_STORAGE_POOLS_GLUSTER to VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_POOL_TYPE is enough to fix that. I've played a bit with adding some compile time checks to avoid this in the future, but while they work, they are a bit ugly-looking, and they would require some changes in docs/apibuild.py to teach it to parse 1 << N, or to make it ignore values in #ifdef VIR_ENUM_SENTINELS, so before spending too much time on this, I'd like to know if these checks would be a welcome/useful addition. The checks are: commit 2aee2a1827497ebde19738c88d2ababfe2dbb13d Author: Christophe Fergeau <cfergeau@redhat.com> Date: Thu Feb 6 17:39:43 2014 +0100 add pool related compile-time checks diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index b3ce000..9d79ae6 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -3104,6 +3104,10 @@ typedef enum { VIR_CONNECT_LIST_STORAGE_POOLS_RBD = 1 << 14, VIR_CONNECT_LIST_STORAGE_POOLS_SHEEPDOG = 1 << 15, VIR_CONNECT_LIST_STORAGE_POOLS_GLUSTER = 1 << 16, + +#ifdef VIR_ENUM_SENTINELS + VIR_CONNECT_LIST_STORAGE_POOLS_LAST +#endif } virConnectListAllStoragePoolsFlags; int virConnectListAllStoragePools(virConnectPtr conn, diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index eaa9325..c8d4df4 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -36,6 +36,7 @@ #include "virerror.h" #include "datatypes.h" #include "storage_conf.h" +#include "verify.h" #include "virstoragefile.h" #include "virxml.h" @@ -51,6 +52,16 @@ #define DEFAULT_POOL_PERM_MODE 0755 #define DEFAULT_VOL_PERM_MODE 0600 +/* Check if VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_POOL_TYPE seems to + * contain all virConnectListAllStoragePoolsFlags elements + */ +verify(VIR_CONNECT_LIST_STORAGE_POOLS_LAST < VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_POOL_TYPE); + +/* Check if virConnectListAllStorageFlags has one flag per known storage + * pool type */ +verify(1 << (5+VIR_STORAGE_POOL_LAST) == VIR_CONNECT_LIST_STORAGE_POOLS_LAST - 1); + + VIR_ENUM_IMPL(virStorageVol, VIR_STORAGE_VOL_LAST, "file", "block", "dir", "network", "netdir") @@ -281,6 +292,8 @@ static virStoragePoolTypeInfo poolTypeInfo[] = { } }; +/* Check if poolTypeInfo has one entry per known storage pool type */ +verify(ARRAY_CARDINALITY(poolTypeInfo) == VIR_STORAGE_POOL_LAST); static virStoragePoolTypeInfoPtr virStoragePoolTypeInfoLookup(int type)
If it's not present in this list, we won't be able to get only glusterfs pools when using virConnectListAllStoragePools. --- src/conf/storage_conf.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h index 62ac749..cada861 100644 --- a/src/conf/storage_conf.h +++ b/src/conf/storage_conf.h @@ -575,7 +575,8 @@ VIR_ENUM_DECL(virStoragePartedFsType) VIR_CONNECT_LIST_STORAGE_POOLS_SCSI | \ VIR_CONNECT_LIST_STORAGE_POOLS_MPATH | \ VIR_CONNECT_LIST_STORAGE_POOLS_RBD | \ - VIR_CONNECT_LIST_STORAGE_POOLS_SHEEPDOG) + VIR_CONNECT_LIST_STORAGE_POOLS_SHEEPDOG | \ + VIR_CONNECT_LIST_STORAGE_POOLS_GLUSTER) # define VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_ALL \ (VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_ACTIVE | \ -- 1.8.5.3
Sorry, screwed up sending the patches, will send a v2, ignore this thread. Christophe On Thu, Feb 06, 2014 at 07:03:25PM +0100, Christophe Fergeau wrote:
Hi,
I noticed that "virsh pool-list --type gluster" is not giving me an empty list as expected, but rather the list of active pools. This is happening because virConnectListAllStoragePools() is returning VIR_ERROR_INVALID_ARG because VIR_CONNECT_LIST_STORAGE_POOLS_GLUSTER is unrecognized.
vshStoragePoolListCollect then goes to retry with only VIR_CONNECT_LIST_STORAGE_POOLS_ACTIVE | VIR_CONNECT_LIST_STORAGE_POOLS_INACTIVE in the flags, which is questionable, but explains the result I got.
Adding VIR_CONNECT_LIST_STORAGE_POOLS_GLUSTER to VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_POOL_TYPE is enough to fix that.
I've played a bit with adding some compile time checks to avoid this in the future, but while they work, they are a bit ugly-looking, and they would require some changes in docs/apibuild.py to teach it to parse 1 << N, or to make it ignore values in #ifdef VIR_ENUM_SENTINELS, so before spending too much time on this, I'd like to know if these checks would be a welcome/useful addition.
The checks are:
commit 2aee2a1827497ebde19738c88d2ababfe2dbb13d Author: Christophe Fergeau <cfergeau@redhat.com> Date: Thu Feb 6 17:39:43 2014 +0100
add pool related compile-time checks
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index b3ce000..9d79ae6 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -3104,6 +3104,10 @@ typedef enum { VIR_CONNECT_LIST_STORAGE_POOLS_RBD = 1 << 14, VIR_CONNECT_LIST_STORAGE_POOLS_SHEEPDOG = 1 << 15, VIR_CONNECT_LIST_STORAGE_POOLS_GLUSTER = 1 << 16, + +#ifdef VIR_ENUM_SENTINELS + VIR_CONNECT_LIST_STORAGE_POOLS_LAST +#endif } virConnectListAllStoragePoolsFlags;
int virConnectListAllStoragePools(virConnectPtr conn, diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index eaa9325..c8d4df4 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -36,6 +36,7 @@ #include "virerror.h" #include "datatypes.h" #include "storage_conf.h" +#include "verify.h" #include "virstoragefile.h"
#include "virxml.h" @@ -51,6 +52,16 @@ #define DEFAULT_POOL_PERM_MODE 0755 #define DEFAULT_VOL_PERM_MODE 0600
+/* Check if VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_POOL_TYPE seems to + * contain all virConnectListAllStoragePoolsFlags elements + */ +verify(VIR_CONNECT_LIST_STORAGE_POOLS_LAST < VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_POOL_TYPE); + +/* Check if virConnectListAllStorageFlags has one flag per known storage + * pool type */ +verify(1 << (5+VIR_STORAGE_POOL_LAST) == VIR_CONNECT_LIST_STORAGE_POOLS_LAST - 1); + + VIR_ENUM_IMPL(virStorageVol, VIR_STORAGE_VOL_LAST, "file", "block", "dir", "network", "netdir") @@ -281,6 +292,8 @@ static virStoragePoolTypeInfo poolTypeInfo[] = { } };
+/* Check if poolTypeInfo has one entry per known storage pool type */ +verify(ARRAY_CARDINALITY(poolTypeInfo) == VIR_STORAGE_POOL_LAST);
static virStoragePoolTypeInfoPtr virStoragePoolTypeInfoLookup(int type)
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
participants (1)
-
Christophe Fergeau