[openstack-dev] [cinder] should we use fsync when writing iscsi config file?

Chris Friesen chris.friesen at windriver.com
Thu Sep 24 16:54:58 UTC 2015


On 09/22/2015 06:19 PM, John Griffith wrote:
> On Tue, Sep 22, 2015 at 6:17 PM, John Griffith <john.griffith8 at gmail.com

>     ​That target file pretty much "is" the persistence record, the db entry is
>     the iqn and provider info only.  I think that adding the fdatasync isn't a
>     bad idea at all.  At the very least it doesn't hurt.  Power losses on attach
>     I would expect to be problematic regardless.
>
> ​Let me clarify the statement above, if you loose power to the node in the
> middle of an attach process and the file wasn't written properly you're most
> likely 'stuck' and will have to detach (which deletes the file) or it will be in
> an error state and rebuild the file when you try the attach again anyway IIRC,
> it's been a while since we've mucked with that code (thank goodness)!!

I took another look at the code and realized that the file *should* get rebuilt 
on restart after a power outage--if the file already exists it will print a 
warning message in the logs but it should still overwrite the contents of the 
file with the desired contents.  However, that didn't happen in my case.

That made me confused about how I ever ended up with an empty persistence file. 
  I went back to my logs and found this:

File "./usr/lib64/python2.7/site-packages/cinder/volume/manager.py", line 334, 
in init_host
File "/usr/lib64/python2.7/site-packages/osprofiler/profiler.py", line 105, in 
wrapper
File "./usr/lib64/python2.7/site-packages/cinder/volume/drivers/lvm.py", line 
603, in ensure_export
File "./usr/lib64/python2.7/site-packages/cinder/volume/targets/iscsi.py", line 
296, in ensure_export
File "./usr/lib64/python2.7/site-packages/cinder/volume/targets/tgt.py", line 
185, in create_iscsi_target
TypeError: not enough arguments for format string


So it seems like we might have a bug in the handling of an empty file.


In kilo/stable, line 185 code looks like this:

	chap_str = 'incominguser %s %s' % chap_auth

This means that "chap_auth" must not be None coming into this function.  In 
volume.targets.iscsi.ISCSITarget.ensure_export() we call

	chap_auth = self._get_target_chap_auth(context, iscsi_name)

which I think would return None given the empty file.

I tried to reproduce with devstack running stable/kilo by booting from volume 
and then removing the persistance file and restarting cinder-volume.  It 
recreated the persistance file, but the "incominguser" line was missing 
completely from the regenerated file.

I think I need to try to reproduce this in our load with some extra debugging, 
see if I can figure out exactly what's going on.
	

Chris



More information about the OpenStack-dev mailing list