On 12/03/2013 05:50 PM, Mark McLoughlin wrote:
> On Tue, 2013-12-03 at 16:23 -0600, Ben Nemec wrote:
>> On 2013-12-03 15:56, Sean Dague wrote:
>>> This cinder patch - https://review.openstack.org/#/c/48935/
>>>
>>> Is blocked on failing upgrade because the updated oslo lockutils won't
>>> function until there is a specific configuration variable added to the
>>> cinder.conf.
>>>
>>> That work around is proposed here -
>>> https://review.openstack.org/#/c/52070/3
>>>
>>> However I think this is exactly the kind of forward breaks that we want
>>> to prevent with grenade, as cinder failing to function after a rolling
>>> upgrade because a config item wasn't added is exactly the kind of pain
>>> we are trying to prevent happening to ops.
>>>
>>> So the question is, how is this done correctly so that a default can be
>>> set in the cinder code for this value, and it not require a config
>>> change to work?
> 
> You're absolutely correct, in principle - if the default value for
> lock_path worked for users before, we should absolutely continue to
> support it.
> 
>> I don't know that I have a good answer on how to handle this, but for 
>> context this change is the result of a nasty bug in lockutils that meant 
>> external locks were doing nothing if lock_path wasn't set.  Basically 
>> it's something we should never have allowed in the first place.
>>
>> As far as setting this in code, it's important that all of the processes 
>> for a service are using the same value to avoid the same bad situation 
>> we were in before.  For tests, we have a lockutils wrapper 
>> (https://github.com/openstack/oslo-incubator/blob/master/openstack/common/lockutils.py#L282)
>>  that sets an environment variable to address this, but that only works if 
>> all of the processes are going to be spawned from within the same wrapper, 
>> and I'm not sure how secure that is for production deployments since it puts 
>> all of the lock files in a temporary directory.
> 
> Right, I don't think the previous default really "worked" - if you used
> the default, then external locking was broken.
> 
> I suspect most distros do set a default - I see RDO has this in its
> default nova.conf:
> 
>   lock_path = /var/lib/nova/tmp
> 
> So, yes - this is all terrible.
> 
> IMHO, rather than raise an exception we should log a big fat warning
> about relying on the default and perhaps just treat the lock as an
> in-process lock in that case ... since that's essentially what it was
> before, right?

So a default of lock_path = /tmp will work (FHS says that path has to be
there), even if not optimal. Could we make it a default value like that
instead of the current default which is null (and hence the problem).

        -Sean

-- 
Sean Dague
http://dague.net

_______________________________________________
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Reply via email to