[libvirt] [PATCH 1/1] Add trigger of host rescan

[Dave Allan]

Daniel Veillard wrote:
> On Fri, Apr 03, 2009 at 04:53:07PM -0400, David Allan wrote:
>> The scsi host code does not currently rescan for new LUs when refreshing
>> a pool.  This patch causes a scan for new LUs on all targets.  It does
>> not cause a LIP.
> 
>  LIP ?

Loop Initialization Primitive, required to discover new targets on fibre 
channel HBAs.  Causing one to be issued is disruptive to IO on the host, 
so this patch does not do that.  It does mean that there is no way 
through libvirt to discover new targets that have been zoned to the host 
subsequent to boot.

>>  static int
>> +virStorageBackendSCSIStartPool(virConnectPtr conn ATTRIBUTE_UNUSED,
>> +                               virStoragePoolObjPtr pool ATTRIBUTE_UNUSED)
>> +{
>> +    int retval = 0;
>> +
>> +    return retval;
>> +}
>> +
>> +
>> +static int
>> +virStorageBackendSCSIStopPool(virConnectPtr conn ATTRIBUTE_UNUSED,
>> +                              virStoragePoolObjPtr pool ATTRIBUTE_UNUSED)
>> +{
>> +    int retval = 0;
>> +
>> +    return retval;
>> +}
> 
>   Is that really better than suggesting the operation is not supported ?

These two functions are, of course, not required for this patch. 
They're there because I had started to implement NPIV support and then 
thought I should submit this patch before the NPIV patch.  I can take 
them out if you'd like and add them to the subsequent patch where they 
will have contents.

> [...]
>> +    if (virAsprintf(&path, "/sys/class/scsi_host/host%u/scan", host) < 0) {
>> +        virReportOOMError(conn);
>> +        retval = -1;
>> +        goto out;
>> +    }
>> +
>> +    VIR_DEBUG(_("Scan trigger path is '%s'"), path);
>> +
>> +    fd = open(path, O_WRONLY);
>> +
>> +    if (fd < 0) {
>> +        virReportSystemError(conn, errno,
>> +                             _("Could not open '%s' to trigger host scan"),
>> +                             path);
>> +        retval = -1;
>> +        goto cleanup;
>> +    }
>> +
>> +    if (write(fd,
>> +              LINUX_SYSFS_SCSI_HOST_SCAN_STRING,
>> +              sizeof(LINUX_SYSFS_SCSI_HOST_SCAN_STRING)) < 0) {
>> +
>> +        virReportSystemError(conn, errno,
>> +                             _("Write to '%s' to trigger host scan failed"),
>> +                             path);
>> +        retval = -1;
>> +        goto cleanup;
>> +    }
>> +
>> +    goto out;
> 
>   Seems to me that goto should be suppressed, it just generate a leak of path
> On the other hand fd is leaked for sure ... This really need some
> double-checking ;-)

Ugh, sorry about that.  Fixed, thanks.

>> +cleanup:
>> +    VIR_FREE(path);
>> +
>> +out:
>> +    VIR_DEBUG(_("Rescan of host %d complete"), host);
>> +    return retval;
>> +}
> 
>   Otherwise, sounds fine, as long as this doesn't generate a bus reset.

It doesn't generate a bus reset.  As I mentioned above, this code is not 
IO disruptive.  I haven't decided on what I think is the right way to do 
scans that are IO disruptive.

I've attached a patch with a fix for the leaks.  It also adds \n to the 
scan string.

Dave

-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: 0002-Fix-bugs-per-DV.patch
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20090406/1cdfd63d/attachment-0001.ksh>