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

[Dave Allan]

Daniel P. Berrange wrote:
> On Sat, Mar 28, 2009 at 10:40:48AM -0400, Dave Allan wrote:
>> Daniel P. Berrange wrote:
>>
>>> This unfortauntely does not work on RHEL5, because there are no targetX.X.X
>>> links here. Only the LUNs appear in this directory.
>>>
>>> The location which appears to be present on both old and new kernels is
>>> under:
>>>
>>>   /sys/class/scsi_host/host0/device/targetX.X.X
>>>
>>> This appears to be present for both SCSI and iSCSI hosts.
>> Unfortunately, that directory isn't present for FC hosts on F10.  :( 
>> I've put together a scan strategy that I think will for for both RHEL5 
>> and F10 for all transport types.  Let me know if it works for you.  The 
>> attached patch should be applied on top of the udev fix I sent 
>> yesterday.  This patch is another one that doesn't read well as a patch, 
>> but should be pretty straightforward once it's applied.
>>
>> I have not addressed the question of what device types to allow, because 
>> I want to think about that one a bit more.
>>
>> I also took out the unused struct you noticed.
>>
>> Thanks for all the feedback and testing.
> 
> This works now on RHEL-5, F9 and F10.
> 
> I still had to disable this code
> 
>     if (STREQLEN(devpath, vol->target.path, PATH_MAX)) {
>         VIR_DEBUG(_("No stable path found for '%s' in '%s'"),
>                   devpath, pool->def->target.path);
>         retval = -1;
>         goto free_vol;
>     }
> 
> because it breaks pools configured with a target dir of /dev/

That's a good point.  Let's allow people to use non-stable paths by 
specifying a target path of /dev.  That preserves the behavior I want, 
which is to prevent people from accidentally using non-stable paths when 
they asked for by-id, etc, but lets them use unstable paths if they want 
to.  That's a one-line change that I have implemented in the attached patch.

> And add device_type == 5 to allow CDROMs to work
> 
>     if (device_type != 0 &&
>         device_type != 5) {
>         retval = 0;
>         goto out;
>     }

The more I think about this question, the more I think we need to ensure 
that within a particular pool there is only one device type, also 
implemented in the attached patch.  The problem with mixing device types 
within the pool is that it forces the consumer application to do device 
type checking every time it uses a volume from the pool, because the 
different device types cannot be used identically in all cases.  I'm not 
entirely in agreement that we should allow device types other than disk 
in this API, but if we ensure that each pool contains only devices of a 
particular type, I don't see any harm in it.

The XML then looks like:

<pool type="scsi">
   <name>cdroms</name>
   <devicetype>cdrom</devicetype>
   <source>
     <adapter name="host1"/>
   </source>
   <target>
     <path>/dev/</path>
   </target>
</pool>

<snipped the rest of the previous patch>

Dave
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: 0001-Permit-pools-of-devices-other-than-disks.patch
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20090330/fdaa8f4e/attachment-0001.ksh>