Open Stack

On 04/11/2014 07:54 AM, Deepak Shetty wrote:
> Hi,
>    I am using the nfs and glusterfs driver as reference here.
> 
> I see that load_shares_config is called everytime via
> _ensure_shares_mounted which I feel is incorrect mainly because
> ensure_shares_mounted loads the config file again w/o restarting the service
> 
> I think that the shares config file should only be loaded once (during
> service startup) as part of do_setup and never again.
> 

Wouldn't this change the functionality that this provides now, though?

Unless I'm missing something, since get_volume_stats calls
_ensure_shares_mounted(), this means you can add a new share to the
config file and have it become active in the driver.  (While I'm not
sure this was the original intent, it could be nice to have and should
at least be considered before ditching it.)

> If someone changes something in the conf file, one needs to restart service
> which calls do_setup again and the changes made in shares.conf is taken
> effect.
> 

I'm not sure this is correct given the above.

> In looking further.. the ensure_shares_mounted ends up calling
> remotefsclient.mount() which does _Nothing_ if the share is already
> mounted.. whcih is mostly the case. So even if someone changed something in
> the shares file (like added -o options) it won't take effect as the share
> is already mounted & service already running.
> 
> In fact today, if you restart the service, even then the changes in share
> won't take effect as the mount is not un-mounted, hence when the service is
> started next, the mount is existing and ensures_shares_mounted just returns
> w/o doing anything.
> 
> The only adv of calling load_shares_config in ensure_shares_mounted is if
> someone changed the shares server IP while the service is running ... it
> loads the new share usign the new server IP.. which again is wrong since
> ideally the person should restart service for any shares.conf changes to
> take effect.
> 

This won't work anyway because of how we track provider_location in the
database.  This particular case is planned to be addressed via this
blueprint with reworks configuration:

https://blueprints.launchpad.net/cinder/+spec/remotefs-share-cfg-improvements

> Hence i feel callign load_shares_config in ensure_shares_mounted is
> Incorrect and should be removed
> 
> Thoughts ?
> 
> thanx,
> deepak
>