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

[Daniel P. Berrange]

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.

Regards,
Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|