Re: [libvirt] [PATCH 3/3] gluster: Fix key attribute for gluster volumes

2014-02-25 Thread Peter Krempa
On 02/24/14 21:59, Eric Blake wrote:
 On 02/24/2014 08:21 AM, Peter Krempa wrote:
 According to our documentation the key value has the following
 meaning: Providing an identifier for the volume which is globally
 unique. This cannot be set when creating a volume: it is always
 generated. The currently used keys for gluster volumes consist of the
 gluster volume name and file path. This can't be considered unique as a
 different storage server can serve a volume with the same name.

 Use the full URI as the key for the volume to avoid ambiguity problems.
 
 The full URI cannot be considered unique, either, as both
 gluster://hosta/volume/file and gluster://hostb/volume/file may resolve
 to the same file.  I think we are better off documenting that a key is

This depends on what we consider as unique in this context:

The problem you are describing here is that two different keys may map
to a single volume. The issue I'm trying to solve is that one key may
map to two distinct volumes.

As a first step we should thus clarify which way the key should be unique.

If we want to make sure the mapping is always just 1:1, we might want to
choose the gluster volume UUID as the first part of the key instead of
the name.

 unique for some pools, but best effort for others, and not change what
 we have already been outputting.  But if we DO keep this patch, you also
 need to change the documentation that gives examples of gluster keys.
 

Peter




signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 3/3] gluster: Fix key attribute for gluster volumes

2014-02-25 Thread Daniel P. Berrange
On Mon, Feb 24, 2014 at 01:59:30PM -0700, Eric Blake wrote:
 On 02/24/2014 08:21 AM, Peter Krempa wrote:
  According to our documentation the key value has the following
  meaning: Providing an identifier for the volume which is globally
  unique. This cannot be set when creating a volume: it is always
  generated. The currently used keys for gluster volumes consist of the
  gluster volume name and file path. This can't be considered unique as a
  different storage server can serve a volume with the same name.
  
  Use the full URI as the key for the volume to avoid ambiguity problems.
 
 The full URI cannot be considered unique, either, as both
 gluster://hosta/volume/file and gluster://hostb/volume/file may resolve
 to the same file.

You're talking about a different type of uniqueness here. The important
aspect of key uniqueness is that a single 'key' must be unambigous about
what volume it is associated with. ie a single 'key' must not be capable
of refering to 2 completely different volumes. Your example is about
whether each volume has precisely 1 unique key. In the context of the
storage pools I believe only the former point is important.

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH 3/3] gluster: Fix key attribute for gluster volumes

2014-02-25 Thread Eric Blake
On 02/25/2014 03:36 AM, Peter Krempa wrote:

 The problem you are describing here is that two different keys may map
 to a single volume. The issue I'm trying to solve is that one key may
 map to two distinct volumes.
 
 As a first step we should thus clarify which way the key should be unique.
 
 If we want to make sure the mapping is always just 1:1, we might want to
 choose the gluster volume UUID as the first part of the key instead of
 the name.

That actually sounds like a reasonable idea - if we are trying to ensure
that a key lookup will find exactly one volume across multiple pools,
then encoding the pool UUID into the volume key name seems reasonable.

 
 unique for some pools, but best effort for others, and not change what
 we have already been outputting.  But if we DO keep this patch, you also
 need to change the documentation that gives examples of gluster keys.

This is still true - we'd need to document the choice, as well as
mention that early implementations of gluster pools were buggy with
regards to key generation.  As it would be a bug fix, I think we can
still try to get this done in time for 1.2.2.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 3/3] gluster: Fix key attribute for gluster volumes

2014-02-24 Thread Peter Krempa
According to our documentation the key value has the following
meaning: Providing an identifier for the volume which is globally
unique. This cannot be set when creating a volume: it is always
generated. The currently used keys for gluster volumes consist of the
gluster volume name and file path. This can't be considered unique as a
different storage server can serve a volume with the same name.

Use the full URI as the key for the volume to avoid ambiguity problems.
---
 src/storage/storage_backend_gluster.c | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/src/storage/storage_backend_gluster.c 
b/src/storage/storage_backend_gluster.c
index 202a441..bb13463 100644
--- a/src/storage/storage_backend_gluster.c
+++ b/src/storage/storage_backend_gluster.c
@@ -185,6 +185,7 @@ 
virStorageBackendGlusterSetMetadata(virStorageBackendGlusterStatePtr state,
 const char *name)
 {
 int ret = -1;
+char *path = NULL;
 char *tmp;

 VIR_FREE(vol-key);
@@ -199,12 +200,12 @@ 
virStorageBackendGlusterSetMetadata(virStorageBackendGlusterStatePtr state,
 goto cleanup;
 }

-if (virAsprintf(vol-key, %s%s%s, state-volname, state-dir,
+if (virAsprintf(path, %s%s%s, state-volname, state-dir,
 vol-name)  0)
 goto cleanup;

 tmp = state-uri-path;
-if (virAsprintf(state-uri-path, /%s, vol-key)  0) {
+if (virAsprintf(state-uri-path, /%s, path)  0) {
 state-uri-path = tmp;
 goto cleanup;
 }
@@ -216,9 +217,14 @@ 
virStorageBackendGlusterSetMetadata(virStorageBackendGlusterStatePtr state,
 VIR_FREE(state-uri-path);
 state-uri-path = tmp;

+/* the path is unique enough to serve as a volume key */
+if (VIR_STRDUP(vol-key, vol-target.path)  0)
+goto cleanup;
+
 ret = 0;

 cleanup:
+VIR_FREE(path);
 return ret;
 }

-- 
1.8.5.5

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH 3/3] gluster: Fix key attribute for gluster volumes

2014-02-24 Thread Eric Blake
On 02/24/2014 08:21 AM, Peter Krempa wrote:
 According to our documentation the key value has the following
 meaning: Providing an identifier for the volume which is globally
 unique. This cannot be set when creating a volume: it is always
 generated. The currently used keys for gluster volumes consist of the
 gluster volume name and file path. This can't be considered unique as a
 different storage server can serve a volume with the same name.
 
 Use the full URI as the key for the volume to avoid ambiguity problems.

The full URI cannot be considered unique, either, as both
gluster://hosta/volume/file and gluster://hostb/volume/file may resolve
to the same file.  I think we are better off documenting that a key is
unique for some pools, but best effort for others, and not change what
we have already been outputting.  But if we DO keep this patch, you also
need to change the documentation that gives examples of gluster keys.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list