[Gluster-devel] trusted.glusterfs.location

[Anand Avati]

> Hmm, is it really an arbitrary subvolume, i.e. not the one that Gluster
> actually uses as the primary copy? A comma separated and ordered list would
> be perfect for Disco.

You should really assume that it is arbitrary. It may so happen that
you are getting the value from the first subvolume (for now) but with
proper load balancing it would be actually arbitrary. You really need
that explicit handling of "user.glusterfs.location" key in replicate
to branch out the getxattr calls and merge the values with comma
separation in the callback and return it upwards.

>>>        if (loc->inode && S_ISREG (loc->inode->st_mode) && name &&
>>> -           (strcmp (name, "trusted.glusterfs.location") == 0)) {
>>> -                ret = dict_set_static_ptr (dict,
>>> -                                           "trusted.glusterfs.location",
>>> -                                           priv->hostname);
>>> +           (strcmp (name, "user.glusterfs.location") == 0)) {
>>> +                ret = dict_set_str (dict,
>>> +                              "user.glusterfs.location",
>>> +                                priv->hostname);
>>
>> dict_set_static_str is the right function to use here. For your case
>> use dict_set_static_bin() with len as strlen(priv->hostname).
>
> I'm not sure what you mean by dict_set_static_str? At least dict.c in 2.0.6
> doesn't have such a function.
>
> I tried to understand how different dict_set_* functions work, and looking
> at dict_set_str, it seems to do just
>
>        data_t *data = get_new_data ();
>        data->len = strlen (value) + 1;
>
>        data->data = value;
>        data->is_static = 1;
>
> whereas dict_set_static_bin does
>
>        data_t *data = get_new_data ();
>
>        data->is_static = 1;
>        data->len = len;
>        data->data = value;
>
> To me using dict_set_static_bin(dict, "...", priv->hostname,
> strlen(priv->hostname) + 1) seem exactly same as dict_set_str(). I wonder
> what you meant by "Using dict_set_str in this case is dangerous and will
> result in double freeing of the priv->hostname pointer".

It was my bad for confusing dict_set_*_str with dict_set_*_ptr. What
you actually need in posix_getxattr for setting the location is
dict_set_static_bin(), which is similar to dict_set_str, except you
get to set the length (where you can skip the trailing nul count).

Avati