10:20 a.m.
The pool name has to be the same too to warrant rejecting a pool
definition as duplicate. This regression was introduced in commit
2184ade3a0546b915252cb3b6a5dc88e9a8d2ccf.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1236438
---
src/conf/storage_conf.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index 4bbed4f..4fa065d 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -2471,13 +2471,22 @@ virStoragePoolSourceFindDuplicate(virConnectPtr conn,
if (STREQ(pool->def->target.path, def->target.path))
matchpool = pool;
break;
- case VIR_STORAGE_POOL_NETFS:
+
case VIR_STORAGE_POOL_GLUSTER:
+ if (STREQ(pool->def->source.name, def->source.name) &&
+ STREQ(pool->def->source.dir, def->source.dir) &&
+ virStoragePoolSourceMatchSingleHost(&pool->def->source,
+ &def->source))
+ matchpool = pool;
+ break;
+
+ case VIR_STORAGE_POOL_NETFS:
if (STREQ(pool->def->source.dir, def->source.dir) &&
virStoragePoolSourceMatchSingleHost(&pool->def->source,
&def->source))
matchpool = pool;
break;
+
case VIR_STORAGE_POOL_SCSI:
if (pool->def->source.adapter.type ==
VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST &&
--
2.4.1
12:48 p.m.
On 06/30/2015 04:20 AM, Peter Krempa wrote:
The pool name has to be the same too to warrant rejecting a pool
definition as duplicate. This regression was introduced in commit
2184ade3a0546b915252cb3b6a5dc88e9a8d2ccf.
How did it check duplicates before that commit? The difference there
shows no checks being done on gluster at all... So semantically, not
sure it's a regression... That commit just added a check to ensure the
host names and 'dir' weren't the same... If anything it didn't go
"far
enough" to check the source.name.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1236438
---
src/conf/storage_conf.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
While rereading virStorageBackendGlusterOpen, I'm now wondering... Does
'dir' have to be provided? The virStorageBackendGlusterOpen indicates
that 'dir' could be NULL and would default to "/"
ACK (although perhaps the STREQ_NULLABLE may need to be used for dir)
John
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index 4bbed4f..4fa065d 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -2471,13 +2471,22 @@ virStoragePoolSourceFindDuplicate(virConnectPtr conn,
if (STREQ(pool->def->target.path, def->target.path))
matchpool = pool;
break;
- case VIR_STORAGE_POOL_NETFS:
+
case VIR_STORAGE_POOL_GLUSTER:
+ if (STREQ(pool->def->source.name, def->source.name) &&
+ STREQ(pool->def->source.dir, def->source.dir) &&
+ virStoragePoolSourceMatchSingleHost(&pool->def->source,
+ &def->source))
+ matchpool = pool;
+ break;
+
+ case VIR_STORAGE_POOL_NETFS:
if (STREQ(pool->def->source.dir, def->source.dir) &&
virStoragePoolSourceMatchSingleHost(&pool->def->source,
&def->source))
matchpool = pool;
break;
+
case VIR_STORAGE_POOL_SCSI:
if (pool->def->source.adapter.type ==
VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST &&
2:01 p.m.
On Tue, Jun 30, 2015 at 06:48:45 -0400, John Ferlan wrote:
On 06/30/2015 04:20 AM, Peter Krempa wrote:
> The pool name has to be the same too to warrant rejecting a pool
> definition as duplicate. This regression was introduced in commit
> 2184ade3a0546b915252cb3b6a5dc88e9a8d2ccf.
>
How did it check duplicates before that commit? The difference there
It was not checked in any way :), thus ...
shows no checks being done on gluster at all... So semantically, not
sure it's a regression... That commit just added a check to ensure the
host names and 'dir' weren't the same... If anything it didn't go
"far
enough" to check the source.name.
If you have a volume with different name on the same server and you want
to use the same path there then libvirt will forbid that and that would
not be forbidden before.
$ git desc 2184ade3a0546b915252cb3b6a5dc88e9a8d2ccf
v1.2.14-168-g2184ade
As it was released already it is a regression in legitimate behaviour.
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1236438
> ---
> src/conf/storage_conf.c | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
While rereading virStorageBackendGlusterOpen, I'm now wondering... Does
'dir' have to be provided? The virStorageBackendGlusterOpen indicates
that 'dir' could be NULL and would default to "/"
ACK (although perhaps the STREQ_NULLABLE may need to be used for dir)
You are right, "dir" actually at least according to the code in the
gluster backend can be NULL, so I'll add the _NULLABLE part.
John
Thanks.
Peter
3366
days inactive
3366
days old
2 comments
2 participants
participants (2)
-
John Ferlan -
Peter Krempa