[libvirt] [PATCH 1/1] Add SCSI pool support.

[Dave Allan]

Daniel P. Berrange wrote:
> On Fri, Mar 27, 2009 at 11:16:42AM -0400, Dave Allan wrote:
>> Daniel P. Berrange wrote:
>>> The new generic 'NewLUN' method has lost this bit of retry / sleep logic.
>>> This unfortauntely causes us to randomly loose LUNs due to fact that 
>>> udev may not yet have created the /dev/ node or the stable path. 
>> [Forgive me if you've been through this before and I just don't know the 
>> history.]
>>
>> This is a race with no easy answer if udev settle does not provide the 
>> protection we need.  I removed the retries because I thought udevadm 
>> settle provided the correct protection against the race, so I thought 
>> the retries only added complexity.
>>
>> As your example demonstrates, though, that's not enough everywhere.  I 
>> can put the timeout and retry back to preserve the existing behavior, 
>> but that only makes it more likely that the path we want will win the 
>> race, it doesn't actually fix the problem.  The iSCSI login should not 
>> return until the devices have appeared in the kernel, which should 
>> populate the udev queue and cause udev settle to pause until the queue 
>> clears.  Is that assumption not valid everywhere?
>>
>> I think we need to understand why this is happening; if we have to pad 
>> the timing until the right guy wins the race, so be it, but I'd really 
>> like to understand the situation before we go there.  What OS is this 
>> on?  As you are, I am testing iSCSI with the IET.  I have 20 LUs, and I 
>> don't see the problem.  Does your system have udevadm settle?
> 
> Spot the obvious mistake...
> 
> 
>     virStorageBackendWaitForDevices(conn);
> 
>     if ((session = virStorageBackendISCSISession(conn, pool, 0)) == NULL)
>         goto cleanup;
>     if (virStorageBackendISCSIRescanLUNs(conn, pool, session) < 0)
>         goto cleanup;
>     if (virStorageBackendISCSIFindLUNs(conn, pool, session) < 0)
>         goto cleanup;
> 
> 
> Its calling udev settle before it has actually told the iSCSI client
> to rescan the target for new LUNs :-) Probably best to move the call
> to virStorageBackendWaitForDevices() into the method
> virStorageBackendISCSIFindLUNs() itself.

Hah...that would break things, wouldn't it?  I've moved the call.  Give 
the attached a go and see if it fixes it.

Dave

-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: 0001-Fixed-broken-placement-of-udevadm-noticed-by-DanPB.patch
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20090327/b8e0e3d4/attachment-0001.ksh>