[pve-devel] [PATCH pve-storage 0/5] v5 fix regression and indentation

[Fabian Grünbichler]

On Fri, Jun 16, 2017 at 10:28:39PM +0200, Michael Rasmussen wrote:
> On Fri, 16 Jun 2017 14:21:34 +0200
> Fabian Grünbichler <f.gruenbichler at proxmox.com> wrote:
> 
> > 
> > one big point that I haven't really looked at is whether you need to
> > guard some operations with locks. especially the calls to iscsiadm are
> > triggered in a lot of code paths, and I don't know whether those are
> > problematic when executed concurrently or not. some of the higher level
> 
> I have not found any evidence that locks should be needed since, by
> nature, the scsi subsystem is thread safe. This is not a problem I have
> seen so far.

I was thinking more about logical races, along the lines of the
following:

- no existing session
- activate volume A
A- get_active_luns (note: this will already create an empty session)
A- freenas_get_lun_number
- concurrent activate volume B
B- get_active_luns (this will reuse session, but there are no active
LUNs at the moment)
A- get_sid
A- os_request to iscsiadm
A- sleep 1 (or udevadm ...)
B- get_sid
A- deactivate_luns (but with info from before B was activated)
B- os_request to iscisadm
B- sleep 1 (or udevadm ...)
B- deactivate_luns (but with info from before A was activated)

so now only B is activated, but no error was returned. or am I missing
something? (note that this was just the first that came to my mind, and
the whole code is very side-effecty, so I am pretty sure there are other
similar interactions)

> 
> > 
> > also haven't checked for how it would handle an already existing local
> > (open-)iscsi setup.. did you?
> > 
> My proof of concept (POC) was tested on an existing iscsi setup added
> libiscsi to the pool. There were no interfering with existing setup
> what so ever. The clever thing in my setup is that all interaction with
> the scsi subsystem is closely tied to a specific session which is
> invisible to other sessions. You can compared it to a database
> transaction.

sounds good (as you can probably tell, I am not an iSCSI expert
whatsoever ;)).