<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Thu, Apr 17, 2014 at 10:00 PM, Deepak Shetty <span dir="ltr"><<a href="mailto:dpkshetty@gmail.com" target="_blank">dpkshetty@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote"><div class="">On Fri, Apr 11, 2014 at 8:25 PM, Eric Harney <span dir="ltr"><<a href="mailto:eharney@redhat.com" target="_blank">eharney@redhat.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div>On 04/11/2014 07:54 AM, Deepak Shetty wrote:<br>
> Hi,<br>
> I am using the nfs and glusterfs driver as reference here.<br>
><br>
> I see that load_shares_config is called everytime via<br>
> _ensure_shares_mounted which I feel is incorrect mainly because<br>
> ensure_shares_mounted loads the config file again w/o restarting the service<br>
><br>
> I think that the shares config file should only be loaded once (during<br>
> service startup) as part of do_setup and never again.<br>
><br>
<br>
</div>Wouldn't this change the functionality that this provides now, though?<br></blockquote><div><br></div></div><div>What functionality are you referring to.. ? didn't get you here<br> <br></div><div class=""><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
Unless I'm missing something, since get_volume_stats calls<br>
_ensure_shares_mounted(), this means you can add a new share to the<br>
config file and have it become active in the driver. (While I'm not<br>
sure this was the original intent, it could be nice to have and should<br>
at least be considered before ditching it.)<br></blockquote><div><br></div></div><div>That does sound like a good to have feature but it actually is a bug bcos for server IP changes, it is effected w/o restarting service, but if one adds -o options its not effected even if u restart service.. so i feel whats happening is un-intended and actually a bug!<br>
<br></div><div>Config should be loaded once and any changes to it should be effected post service restart<br></div></div></div></div></blockquote><div><br></div><div>forgot to add, that for the above to work consistently, we definitely need to have a framework / mechanism in cinder where the driver is provided a function/callback to gracefully cleanup its mounts (or any other thing) during service goign down. Today we don't have such a thing and hence drivers don't cleanup their mounts and hence when service starts up, it ensure_shares_mounted sees the mount being present and does nothing. this would work nice if drivers were given ability to clean up the mounts as part of service going down. <br>
<br></div><div>thanx,<br>deepak<br><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><br>
</div><div class=""><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div><br>
> If someone changes something in the conf file, one needs to restart service<br>
> which calls do_setup again and the changes made in shares.conf is taken<br>
> effect.<br>
><br>
<br>
</div>I'm not sure this is correct given the above.<br></blockquote><div><br></div></div><div>Pls see above.. it works in a incorrect way which is confusing to the admin/user<br> <br></div><div class=""><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div><br>
> In looking further.. the ensure_shares_mounted ends up calling<br>
> remotefsclient.mount() which does _Nothing_ if the share is already<br>
> mounted.. whcih is mostly the case. So even if someone changed something in<br>
> the shares file (like added -o options) it won't take effect as the share<br>
> is already mounted & service already running.<br>
><br>
> In fact today, if you restart the service, even then the changes in share<br>
> won't take effect as the mount is not un-mounted, hence when the service is<br>
> started next, the mount is existing and ensures_shares_mounted just returns<br>
> w/o doing anything.<br>
><br>
> The only adv of calling load_shares_config in ensure_shares_mounted is if<br>
> someone changed the shares server IP while the service is running ... it<br>
> loads the new share usign the new server IP.. which again is wrong since<br>
> ideally the person should restart service for any shares.conf changes to<br>
> take effect.<br>
><br>
<br>
</div>This won't work anyway because of how we track provider_location in the<br>
database. This particular case is planned to be addressed via this<br>
blueprint with reworks configuration:<br>
<br>
<a href="https://blueprints.launchpad.net/cinder/+spec/remotefs-share-cfg-improvements" target="_blank">https://blueprints.launchpad.net/cinder/+spec/remotefs-share-cfg-improvements</a><br></blockquote><div><br></div></div>
<div>
Agree, but until this is realized, we can fix the code/flow such that its sane.. in the sense, it works consistently for all cases.<br>today it doesn't.. for some change it works w/o service restart and for some it doesn't work even after service restart<br>
<br></div><div>thanx,<br>deepak<br></div></div><br></div></div>
</blockquote></div><br></div></div>