On 20/06/13 00:35, John Ferlan wrote:
On 06/07/2013 01:03 PM, Osier Yang wrote:This takes use of the two utils introduced in previous patches. Node device HAL backend represents PCI device like PCI_8086_2922, (I.E PCI_$vendor_$product), to get the PCI address, we have to traverse /sys/devices/ to find it out. And to get the current scsi host number assigned by the system for the scsi host device. We need to traverse /sys/bus/pci/devices/ with the found PCI address by getScsiHostParentAddress, and the specified unique_id. Later patch will allow to omit the "unique_id", by using the scsi host which has the smallest unique_id. --- src/storage/storage_backend_scsi.c | 81 +++++++++++++++++++++++++++++++++++++- 1 file changed, 79 insertions(+), 2 deletions(-) diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c index 13c498d..a77b9ae 100644 --- a/src/storage/storage_backend_scsi.c +++ b/src/storage/storage_backend_scsi.c @@ -591,6 +591,66 @@ out: return retval; }+/*+ * Node device udev backend and HAL backend represent PCI + * device in different style. Udev backend represents it like + * pci_0000_00_1f_2, and HAL backend represents it like + * pci_8086_2922. + * + * Covert the value of "parent" into PCI device address string + * (e.g. 0000:00:1f:2). Return the PCI address as string on + * success, or -1 on failure. + */ +static char * +getScsiHostParentAddress(const char *parent) +{ + char **tokens = NULL; + char *vendor = NULL; + char *product = NULL; + char *ret = NULL; + int len; + + if (!strchr(parent, '_')) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("'parent' of scsi_host adapter must " + "be consistent with name of node device")); + return NULL; + } + + if (!(tokens = virStringSplit(parent, "_", 0))) + return NULL;Given the following code, the call to Split could use 4 (or 5) for the max... Not that it matters.
hm, it should be 5.
+ + len = virStringListLength(tokens); + + switch (len) { + case 4: + if (!(ret = virStringJoin((const char **)(&tokens[1]), ":"))) + goto cleanup; + break; + + case 2: + vendor = tokens[1]; + product = tokens[2]; + if (!(ret = virFindPCIDeviceByVPD(NULL, vendor, product))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to find PCI device with vendor '%s' " + "product '%s'"), vendor, product); + goto cleanup; + } + + break; + default: + virReportError(VIR_ERR_XML_ERROR, "%s", + _("'parent' of scsi_host adapter must " + "be consistent with name of node device")); + goto cleanup; + } + +cleanup: + virStringFreeList(tokens); + return ret; +} + static int getHostNumber(const char *adapter_name, unsigned int *result) @@ -627,10 +687,27 @@ static char * getAdapterName(virStoragePoolSourceAdapter adapter) { char *name = NULL; + char *addr = NULL;if (adapter.type != VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST) {- if (VIR_STRDUP(name, adapter.data.scsi_host.name) < 0) - return NULL; + if (adapter.data.scsi_host.parent) { + if (!adapter.data.scsi_host.has_unique_id) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("'unique_id' is not specified for " + "scsi_host adapter"));so the optional unique_id isn't so optional after all for a couple of code paths. Issuing a message here could be confusing. Has this been tested within an environment where unique_id isn't specified?
It's made optional by later patches. Commit log clarifies it.
+ return NULL; + } else { + if (!(addr = getScsiHostParentAddress(adapter.data.scsi_host.parent))) + return NULL; + + name = virParseStableScsiHostAddress(NULL, addr, + adapter.data.scsi_host.unique_id);You need a VIR_FREE(addr); here John+ } + } else if (adapter.data.scsi_host.name) { + if (VIR_STRDUP(name, adapter.data.scsi_host.name) < 0) + return NULL; + } + return name; }
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list