[libvirt] [PATCH v2 0/3] Resolve libvirtd crash matching scsi_host
https://bugzilla.redhat.com/show_bug.cgi?id=1146837 Rewrite the v1 patch to take a different method to attack the problem. Expose the getHostNumber from scsi check/startup/refresh in the form of virGetSCSIHostNumber. Additionally, expose part of the getAdapterName code in the form of virGetSCSIHostNameByParentaddr. Then use the two functions to determine whether the incoming def matches any of the defined pool defs. v1 is here: http://www.redhat.com/archives/libvir-list/2014-September/msg01746.html John Ferlan (3): virutil: Introduce virGetSCSIHostNumber virutil: Introduce virGetSCSIHostNameByParentaddr storage_conf: Resolve libvirtd crash matching scsi_host src/conf/storage_conf.c | 69 ++++++++++++++---------- src/libvirt_private.syms | 2 + src/storage/storage_backend_scsi.c | 59 ++++----------------- src/util/virutil.c | 106 +++++++++++++++++++++++++++++++++++++ src/util/virutil.h | 10 ++++ 5 files changed, 170 insertions(+), 76 deletions(-) -- 1.9.3
Create/use virGetSCSIHostNumber to replace the static getHostNumber Removed the "if (result &&" since result is now required to be non NULL on input. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/libvirt_private.syms | 1 + src/storage/storage_backend_scsi.c | 40 +++------------------------ src/util/virutil.c | 56 ++++++++++++++++++++++++++++++++++++++ src/util/virutil.h | 4 +++ 4 files changed, 65 insertions(+), 36 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index d6265ac..189e07c 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2151,6 +2151,7 @@ virGetGroupList; virGetGroupName; virGetHostname; virGetListenFDs; +virGetSCSIHostNumber; virGetSelfLastChanged; virGetUnprivSGIOSysfsPath; virGetUserCacheDirectory; diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c index 16ed328..d9f3ff2 100644 --- a/src/storage/storage_backend_scsi.c +++ b/src/storage/storage_backend_scsi.c @@ -511,38 +511,6 @@ virStorageBackendSCSITriggerRescan(uint32_t host) return retval; } -static int -getHostNumber(const char *adapter_name, - unsigned int *result) -{ - char *host = (char *)adapter_name; - - /* Specifying adapter like 'host5' is still supported for - * back-compat reason. - */ - if (STRPREFIX(host, "scsi_host")) { - host += strlen("scsi_host"); - } else if (STRPREFIX(host, "fc_host")) { - host += strlen("fc_host"); - } else if (STRPREFIX(host, "host")) { - host += strlen("host"); - } else { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Invalid adapter name '%s' for SCSI pool"), - adapter_name); - return -1; - } - - if (result && virStrToLong_ui(host, NULL, 10, result) == -1) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Invalid adapter name '%s' for SCSI pool"), - adapter_name); - return -1; - } - - return 0; -} - static char * getAdapterName(virStoragePoolSourceAdapter adapter) { @@ -610,7 +578,7 @@ createVport(virStoragePoolSourceAdapter adapter) return -1; } - if (getHostNumber(adapter.data.fchost.parent, &parent_host) < 0) + if (virGetSCSIHostNumber(adapter.data.fchost.parent, &parent_host) < 0) return -1; if (virManageVport(parent_host, adapter.data.fchost.wwpn, @@ -643,7 +611,7 @@ deleteVport(virStoragePoolSourceAdapter adapter) adapter.data.fchost.wwpn))) return -1; - if (getHostNumber(adapter.data.fchost.parent, &parent_host) < 0) + if (virGetSCSIHostNumber(adapter.data.fchost.parent, &parent_host) < 0) goto cleanup; if (virManageVport(parent_host, adapter.data.fchost.wwpn, @@ -683,7 +651,7 @@ virStorageBackendSCSICheckPool(virConnectPtr conn ATTRIBUTE_UNUSED, } } - if (getHostNumber(name, &host) < 0) + if (virGetSCSIHostNumber(name, &host) < 0) goto cleanup; if (virAsprintf(&path, "%s/host%d", @@ -712,7 +680,7 @@ virStorageBackendSCSIRefreshPool(virConnectPtr conn ATTRIBUTE_UNUSED, if (!(name = getAdapterName(pool->def->source.adapter))) return -1; - if (getHostNumber(name, &host) < 0) + if (virGetSCSIHostNumber(name, &host) < 0) goto out; VIR_DEBUG("Scanning host%u", host); diff --git a/src/util/virutil.c b/src/util/virutil.c index 5197969..1c23d7c 100644 --- a/src/util/virutil.c +++ b/src/util/virutil.c @@ -1838,6 +1838,54 @@ virFindSCSIHostByPCI(const char *sysfs_prefix, return ret; } +/* virGetSCSIHostNumber: + * @adapter_name: Name of the host adapter + * @result: Return the entry value as unsigned int + * + * Convert the various forms of scsi_host names into the numeric + * host# value that can be used in order to scan sysfs looking for + * the specific host. + * + * Names can be either "scsi_host#" or just "host#", where + * "host#" is the back-compat format, but both equate to + * the same source adapter. First check if both pool and def + * are using same format (easier) - if so, then compare + * + * Returns 0 on success, and @result has the host number. + * Otherwise returns -1. + */ +int +virGetSCSIHostNumber(const char *adapter_name, + unsigned int *result) +{ + char *host = (char *)adapter_name; + + /* Specifying adapter like 'host5' is still supported for + * back-compat reason. + */ + if (STRPREFIX(host, "scsi_host")) { + host += strlen("scsi_host"); + } else if (STRPREFIX(host, "fc_host")) { + host += strlen("fc_host"); + } else if (STRPREFIX(host, "host")) { + host += strlen("host"); + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Invalid adapter name '%s' for SCSI pool"), + adapter_name); + return -1; + } + + if (virStrToLong_ui(host, NULL, 10, result) == -1) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Invalid adapter name '%s' for SCSI pool"), + adapter_name); + return -1; + } + + return 0; +} + /* virReadFCHost: * @sysfs_prefix: "fc_host" sysfs path, defaults to SYSFS_FC_HOST_PATH * @host: Host number, E.g. 5 of "fc_host/host5" @@ -2209,6 +2257,14 @@ virFindSCSIHostByPCI(const char *sysfs_prefix ATTRIBUTE_UNUSED, } int +virGetSCSIHostNumber(const char *adapter_name ATTRIBUTE_UNUSED, + unsigned int *result ATTRIBUTE_UNUSED) +{ + virReportSystemError(ENOSYS, "%s", _("Not supported on this platform")); + return NULL; +} + +int virReadFCHost(const char *sysfs_prefix ATTRIBUTE_UNUSED, int host ATTRIBUTE_UNUSED, const char *entry ATTRIBUTE_UNUSED, diff --git a/src/util/virutil.h b/src/util/virutil.h index 54f1148..442c998 100644 --- a/src/util/virutil.h +++ b/src/util/virutil.h @@ -173,6 +173,10 @@ char * virFindSCSIHostByPCI(const char *sysfs_prefix, const char *parentaddr, unsigned int unique_id); +int +virGetSCSIHostNumber(const char *adapter_name, + unsigned int *result) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); int virReadFCHost(const char *sysfs_prefix, int host, const char *entry, -- 1.9.3
On 06.10.2014 23:49, John Ferlan wrote:
Create/use virGetSCSIHostNumber to replace the static getHostNumber
Removed the "if (result &&" since result is now required to be non NULL on input.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/libvirt_private.syms | 1 + src/storage/storage_backend_scsi.c | 40 +++------------------------ src/util/virutil.c | 56 ++++++++++++++++++++++++++++++++++++++ src/util/virutil.h | 4 +++ 4 files changed, 65 insertions(+), 36 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index d6265ac..189e07c 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2151,6 +2151,7 @@ virGetGroupList; virGetGroupName; virGetHostname; virGetListenFDs; +virGetSCSIHostNumber; virGetSelfLastChanged; virGetUnprivSGIOSysfsPath; virGetUserCacheDirectory; diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c index 16ed328..d9f3ff2 100644 --- a/src/storage/storage_backend_scsi.c +++ b/src/storage/storage_backend_scsi.c @@ -511,38 +511,6 @@ virStorageBackendSCSITriggerRescan(uint32_t host) return retval; }
-static int -getHostNumber(const char *adapter_name, - unsigned int *result) -{ - char *host = (char *)adapter_name; - - /* Specifying adapter like 'host5' is still supported for - * back-compat reason. - */ - if (STRPREFIX(host, "scsi_host")) { - host += strlen("scsi_host"); - } else if (STRPREFIX(host, "fc_host")) { - host += strlen("fc_host"); - } else if (STRPREFIX(host, "host")) { - host += strlen("host"); - } else { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Invalid adapter name '%s' for SCSI pool"), - adapter_name); - return -1; - } - - if (result && virStrToLong_ui(host, NULL, 10, result) == -1) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Invalid adapter name '%s' for SCSI pool"), - adapter_name); - return -1; - } - - return 0; -} - static char * getAdapterName(virStoragePoolSourceAdapter adapter) { @@ -610,7 +578,7 @@ createVport(virStoragePoolSourceAdapter adapter) return -1; }
- if (getHostNumber(adapter.data.fchost.parent, &parent_host) < 0) + if (virGetSCSIHostNumber(adapter.data.fchost.parent, &parent_host) < 0) return -1;
if (virManageVport(parent_host, adapter.data.fchost.wwpn, @@ -643,7 +611,7 @@ deleteVport(virStoragePoolSourceAdapter adapter) adapter.data.fchost.wwpn))) return -1;
- if (getHostNumber(adapter.data.fchost.parent, &parent_host) < 0) + if (virGetSCSIHostNumber(adapter.data.fchost.parent, &parent_host) < 0) goto cleanup;
if (virManageVport(parent_host, adapter.data.fchost.wwpn, @@ -683,7 +651,7 @@ virStorageBackendSCSICheckPool(virConnectPtr conn ATTRIBUTE_UNUSED, } }
- if (getHostNumber(name, &host) < 0) + if (virGetSCSIHostNumber(name, &host) < 0) goto cleanup;
if (virAsprintf(&path, "%s/host%d", @@ -712,7 +680,7 @@ virStorageBackendSCSIRefreshPool(virConnectPtr conn ATTRIBUTE_UNUSED, if (!(name = getAdapterName(pool->def->source.adapter))) return -1;
- if (getHostNumber(name, &host) < 0) + if (virGetSCSIHostNumber(name, &host) < 0) goto out;
VIR_DEBUG("Scanning host%u", host); diff --git a/src/util/virutil.c b/src/util/virutil.c index 5197969..1c23d7c 100644 --- a/src/util/virutil.c +++ b/src/util/virutil.c @@ -1838,6 +1838,54 @@ virFindSCSIHostByPCI(const char *sysfs_prefix, return ret; }
+/* virGetSCSIHostNumber: + * @adapter_name: Name of the host adapter + * @result: Return the entry value as unsigned int + * + * Convert the various forms of scsi_host names into the numeric + * host# value that can be used in order to scan sysfs looking for + * the specific host. + * + * Names can be either "scsi_host#" or just "host#", where + * "host#" is the back-compat format, but both equate to + * the same source adapter. First check if both pool and def + * are using same format (easier) - if so, then compare + * + * Returns 0 on success, and @result has the host number. + * Otherwise returns -1. + */ +int +virGetSCSIHostNumber(const char *adapter_name, + unsigned int *result) +{ + char *host = (char *)adapter_name;
I think this isn't needed. I mean, not only typecast, but the @host variable itself.
+ + /* Specifying adapter like 'host5' is still supported for + * back-compat reason. + */ + if (STRPREFIX(host, "scsi_host")) { + host += strlen("scsi_host"); + } else if (STRPREFIX(host, "fc_host")) { + host += strlen("fc_host"); + } else if (STRPREFIX(host, "host")) { + host += strlen("host"); + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Invalid adapter name '%s' for SCSI pool"), + adapter_name); + return -1; + } + + if (virStrToLong_ui(host, NULL, 10, result) == -1) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Invalid adapter name '%s' for SCSI pool"), + adapter_name); + return -1; + } + + return 0; +} + /* virReadFCHost: * @sysfs_prefix: "fc_host" sysfs path, defaults to SYSFS_FC_HOST_PATH * @host: Host number, E.g. 5 of "fc_host/host5" @@ -2209,6 +2257,14 @@ virFindSCSIHostByPCI(const char *sysfs_prefix ATTRIBUTE_UNUSED, }
int +virGetSCSIHostNumber(const char *adapter_name ATTRIBUTE_UNUSED, + unsigned int *result ATTRIBUTE_UNUSED) +{ + virReportSystemError(ENOSYS, "%s", _("Not supported on this platform")); + return NULL; +} + +int virReadFCHost(const char *sysfs_prefix ATTRIBUTE_UNUSED, int host ATTRIBUTE_UNUSED, const char *entry ATTRIBUTE_UNUSED, diff --git a/src/util/virutil.h b/src/util/virutil.h index 54f1148..442c998 100644 --- a/src/util/virutil.h +++ b/src/util/virutil.h @@ -173,6 +173,10 @@ char * virFindSCSIHostByPCI(const char *sysfs_prefix, const char *parentaddr, unsigned int unique_id); +int +virGetSCSIHostNumber(const char *adapter_name, + unsigned int *result) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); int virReadFCHost(const char *sysfs_prefix, int host, const char *entry,
Michal
On 10/28/2014 07:36 PM, Michal Privoznik wrote:
On 06.10.2014 23:49, John Ferlan wrote:
Create/use virGetSCSIHostNumber to replace the static getHostNumber
Removed the "if (result &&" since result is now required to be non NULL on input.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/libvirt_private.syms | 1 + src/storage/storage_backend_scsi.c | 40 +++------------------------ src/util/virutil.c | 56 ++++++++++++++++++++++++++++++++++++++ src/util/virutil.h | 4 +++ 4 files changed, 65 insertions(+), 36 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index d6265ac..189e07c 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2151,6 +2151,7 @@ virGetGroupList; virGetGroupName; virGetHostname; virGetListenFDs; +virGetSCSIHostNumber; virGetSelfLastChanged; virGetUnprivSGIOSysfsPath; virGetUserCacheDirectory; diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c index 16ed328..d9f3ff2 100644 --- a/src/storage/storage_backend_scsi.c +++ b/src/storage/storage_backend_scsi.c @@ -511,38 +511,6 @@ virStorageBackendSCSITriggerRescan(uint32_t host) return retval; }
-static int -getHostNumber(const char *adapter_name, - unsigned int *result) -{ - char *host = (char *)adapter_name; - - /* Specifying adapter like 'host5' is still supported for - * back-compat reason. - */ - if (STRPREFIX(host, "scsi_host")) { - host += strlen("scsi_host"); - } else if (STRPREFIX(host, "fc_host")) { - host += strlen("fc_host"); - } else if (STRPREFIX(host, "host")) { - host += strlen("host"); - } else { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Invalid adapter name '%s' for SCSI pool"), - adapter_name); - return -1; - } - - if (result && virStrToLong_ui(host, NULL, 10, result) == -1) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Invalid adapter name '%s' for SCSI pool"), - adapter_name); - return -1; - } - - return 0; -} - static char * getAdapterName(virStoragePoolSourceAdapter adapter) { @@ -610,7 +578,7 @@ createVport(virStoragePoolSourceAdapter adapter) return -1; }
- if (getHostNumber(adapter.data.fchost.parent, &parent_host) < 0) + if (virGetSCSIHostNumber(adapter.data.fchost.parent, &parent_host) < 0) return -1;
if (virManageVport(parent_host, adapter.data.fchost.wwpn, @@ -643,7 +611,7 @@ deleteVport(virStoragePoolSourceAdapter adapter) adapter.data.fchost.wwpn))) return -1;
- if (getHostNumber(adapter.data.fchost.parent, &parent_host) < 0) + if (virGetSCSIHostNumber(adapter.data.fchost.parent, &parent_host) < 0) goto cleanup;
if (virManageVport(parent_host, adapter.data.fchost.wwpn, @@ -683,7 +651,7 @@ virStorageBackendSCSICheckPool(virConnectPtr conn ATTRIBUTE_UNUSED, } }
- if (getHostNumber(name, &host) < 0) + if (virGetSCSIHostNumber(name, &host) < 0) goto cleanup;
if (virAsprintf(&path, "%s/host%d", @@ -712,7 +680,7 @@ virStorageBackendSCSIRefreshPool(virConnectPtr conn ATTRIBUTE_UNUSED, if (!(name = getAdapterName(pool->def->source.adapter))) return -1;
- if (getHostNumber(name, &host) < 0) + if (virGetSCSIHostNumber(name, &host) < 0) goto out;
VIR_DEBUG("Scanning host%u", host); diff --git a/src/util/virutil.c b/src/util/virutil.c index 5197969..1c23d7c 100644 --- a/src/util/virutil.c +++ b/src/util/virutil.c @@ -1838,6 +1838,54 @@ virFindSCSIHostByPCI(const char *sysfs_prefix, return ret; }
+/* virGetSCSIHostNumber: + * @adapter_name: Name of the host adapter + * @result: Return the entry value as unsigned int + * + * Convert the various forms of scsi_host names into the numeric + * host# value that can be used in order to scan sysfs looking for + * the specific host. + * + * Names can be either "scsi_host#" or just "host#", where + * "host#" is the back-compat format, but both equate to + * the same source adapter. First check if both pool and def + * are using same format (easier) - if so, then compare + * + * Returns 0 on success, and @result has the host number. + * Otherwise returns -1. + */ +int +virGetSCSIHostNumber(const char *adapter_name, + unsigned int *result) +{ + char *host = (char *)adapter_name;
I think this isn't needed. I mean, not only typecast, but the @host variable itself.
You'll see it's just a cut-n-paste/copy of the former getHostNumber function above. I see your point though - it's not a 'const char const *', so adjusting the adapter_name pointer is perfectly reasonable and what I went with. John
+ + /* Specifying adapter like 'host5' is still supported for + * back-compat reason. + */ + if (STRPREFIX(host, "scsi_host")) { + host += strlen("scsi_host"); + } else if (STRPREFIX(host, "fc_host")) { + host += strlen("fc_host"); + } else if (STRPREFIX(host, "host")) { + host += strlen("host"); + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Invalid adapter name '%s' for SCSI pool"), + adapter_name); + return -1; + } + + if (virStrToLong_ui(host, NULL, 10, result) == -1) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Invalid adapter name '%s' for SCSI pool"), + adapter_name); + return -1; + } + + return 0; +} + /* virReadFCHost: * @sysfs_prefix: "fc_host" sysfs path, defaults to SYSFS_FC_HOST_PATH * @host: Host number, E.g. 5 of "fc_host/host5" @@ -2209,6 +2257,14 @@ virFindSCSIHostByPCI(const char *sysfs_prefix ATTRIBUTE_UNUSED, }
int +virGetSCSIHostNumber(const char *adapter_name ATTRIBUTE_UNUSED, + unsigned int *result ATTRIBUTE_UNUSED) +{ + virReportSystemError(ENOSYS, "%s", _("Not supported on this platform")); + return NULL; +} + +int virReadFCHost(const char *sysfs_prefix ATTRIBUTE_UNUSED, int host ATTRIBUTE_UNUSED, const char *entry ATTRIBUTE_UNUSED, diff --git a/src/util/virutil.h b/src/util/virutil.h index 54f1148..442c998 100644 --- a/src/util/virutil.h +++ b/src/util/virutil.h @@ -173,6 +173,10 @@ char * virFindSCSIHostByPCI(const char *sysfs_prefix, const char *parentaddr, unsigned int unique_id); +int +virGetSCSIHostNumber(const char *adapter_name, + unsigned int *result) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); int virReadFCHost(const char *sysfs_prefix, int host, const char *entry,
Michal
Create the function from the code in getAdapterName() in order to return the "host#" name for the provided parentaddr values. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/libvirt_private.syms | 1 + src/storage/storage_backend_scsi.c | 19 +++++---------- src/util/virutil.c | 50 ++++++++++++++++++++++++++++++++++++++ src/util/virutil.h | 6 +++++ 4 files changed, 63 insertions(+), 13 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 189e07c..b29f77d 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2151,6 +2151,7 @@ virGetGroupList; virGetGroupName; virGetHostname; virGetListenFDs; +virGetSCSIHostNameByParentaddr; virGetSCSIHostNumber; virGetSelfLastChanged; virGetUnprivSGIOSysfsPath; diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c index d9f3ff2..02160bc 100644 --- a/src/storage/storage_backend_scsi.c +++ b/src/storage/storage_backend_scsi.c @@ -519,22 +519,15 @@ getAdapterName(virStoragePoolSourceAdapter adapter) if (adapter.type == VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_SCSI_HOST) { if (adapter.data.scsi_host.has_parent) { + virDevicePCIAddress addr = adapter.data.scsi_host.parentaddr; unsigned int unique_id = adapter.data.scsi_host.unique_id; - if (virAsprintf(&parentaddr, "%04x:%02x:%02x.%01x", - adapter.data.scsi_host.parentaddr.domain, - adapter.data.scsi_host.parentaddr.bus, - adapter.data.scsi_host.parentaddr.slot, - adapter.data.scsi_host.parentaddr.function) < 0) + if (!(name = virGetSCSIHostNameByParentaddr(addr.domain, + addr.bus, + addr.slot, + addr.function, + unique_id))) goto cleanup; - if (!(name = virFindSCSIHostByPCI(NULL, parentaddr, - unique_id))) { - virReportError(VIR_ERR_XML_ERROR, - _("Failed to find scsi_host using PCI '%s' " - "and unique_id='%u'"), - parentaddr, unique_id); - goto cleanup; - } } else { ignore_value(VIR_STRDUP(name, adapter.data.scsi_host.name)); } diff --git a/src/util/virutil.c b/src/util/virutil.c index 1c23d7c..eddfb08 100644 --- a/src/util/virutil.c +++ b/src/util/virutil.c @@ -1886,6 +1886,45 @@ virGetSCSIHostNumber(const char *adapter_name, return 0; } +/* virGetSCSIHostNameByParentaddr: + * @domain: The domain from the scsi_host parentaddr + * @bus: The bus from the scsi_host parentaddr + * @slot: The slot from the scsi_host parentaddr + * @function: The function from the scsi_host parentaddr + * @unique_id: The unique id value for parentaddr + * + * Generate a parentaddr and find the scsi_host host# for + * the provided parentaddr PCI address fields. + * + * Returns the "host#" string which must be free'd by + * the caller or NULL on error + */ +char * +virGetSCSIHostNameByParentaddr(unsigned int domain, + unsigned int bus, + unsigned int slot, + unsigned int function, + unsigned int unique_id) +{ + char *name = NULL; + char *parentaddr = NULL; + + if (virAsprintf(&parentaddr, "%04x:%02x:%02x.%01x", + domain, bus, slot, function) < 0) + goto cleanup; + if (!(name = virFindSCSIHostByPCI(NULL, parentaddr, unique_id))) { + virReportError(VIR_ERR_XML_ERROR, + _("Failed to find scsi_host using PCI '%s' " + "and unique_id='%u'"), + parentaddr, unique_id); + goto cleanup; + } + + cleanup: + VIR_FREE(parentaddr); + return name; +} + /* virReadFCHost: * @sysfs_prefix: "fc_host" sysfs path, defaults to SYSFS_FC_HOST_PATH * @host: Host number, E.g. 5 of "fc_host/host5" @@ -2264,6 +2303,17 @@ virGetSCSIHostNumber(const char *adapter_name ATTRIBUTE_UNUSED, return NULL; } +char * +virGetSCSIHostNameByParentaddr(unsigned int domain ATTRIBUTE_UNUSED, + unsigned int bus ATTRIBUTE_UNUSED, + unsigned int slot ATTRIBUTE_UNUSED, + unsigned int function ATTRIBUTE_UNUSED, + unsigned int unique_id ATTRIBUTE_UNUSED) +{ + virReportSystemError(ENOSYS, "%s", _("Not supported on this platform")); + return NULL; +} + int virReadFCHost(const char *sysfs_prefix ATTRIBUTE_UNUSED, int host ATTRIBUTE_UNUSED, diff --git a/src/util/virutil.h b/src/util/virutil.h index 442c998..f31bf88 100644 --- a/src/util/virutil.h +++ b/src/util/virutil.h @@ -177,6 +177,12 @@ int virGetSCSIHostNumber(const char *adapter_name, unsigned int *result) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); +char * +virGetSCSIHostNameByParentaddr(unsigned int domain, + unsigned int bus, + unsigned int slot, + unsigned int function, + unsigned int unique_id); int virReadFCHost(const char *sysfs_prefix, int host, const char *entry, -- 1.9.3
https://bugzilla.redhat.com/show_bug.cgi?id=1146837 Resolve a crash in libvirtd resulting from commit id 'a4bd62ad' (1.0.6) which added parentaddr and unique_id to allow unique identification of a scsi_host, but assumed that all the pool entries and the incoming definition would be similarly defined. If the existing pool uses the 'name' attribute and an incoming pool is using the parentaddr/unique_id, then the code will attempt to compare the existing name string against the incoming name string which doesn't exist (is NULL) and results in a core (STREQ). Conversely, if the existing pool used the parentaddr/unique_id and the to be defined pool used the name, then the comparison would be against the parentaddr, but since the incoming pool doesn't have one - that would leave the comparison against a parentaddr of all 0's and a unique_id of 0, which will always comparison to fail. This means someone could define the same source adapter for two pools In order to resolve this, adjust the code to get the 'host#' to be used by the storage scsi backend in order to check/start the pool and make sure the incoming definition doesn't match any of the existing pool defs. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/conf/storage_conf.c | 69 ++++++++++++++++++++++++++++++------------------- 1 file changed, 42 insertions(+), 27 deletions(-) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 36696a4..19c452b 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -2062,26 +2062,37 @@ virStoragePoolObjIsDuplicate(virStoragePoolObjListPtr pools, return ret; } -static bool -matchSCSIAdapterParent(virStoragePoolObjPtr pool, - virStoragePoolDefPtr def) +static int +getSCSIHostNumber(virStoragePoolSourceAdapter adapter, + unsigned int *hostnum) { - virDevicePCIAddressPtr pooladdr = - &pool->def->source.adapter.data.scsi_host.parentaddr; - virDevicePCIAddressPtr defaddr = - &def->source.adapter.data.scsi_host.parentaddr; - int pool_unique_id = - pool->def->source.adapter.data.scsi_host.unique_id; - int def_unique_id = - def->source.adapter.data.scsi_host.unique_id; - if (pooladdr->domain == defaddr->domain && - pooladdr->bus == defaddr->bus && - pooladdr->slot == defaddr->slot && - pooladdr->function == defaddr->function && - pool_unique_id == def_unique_id) { - return true; - } - return false; + int ret = -1; + unsigned int num; + char *name = NULL; + + if (adapter.data.scsi_host.has_parent) { + virDevicePCIAddress addr = adapter.data.scsi_host.parentaddr; + unsigned int unique_id = adapter.data.scsi_host.unique_id; + + if (!(name = virGetSCSIHostNameByParentaddr(addr.domain, + addr.bus, + addr.slot, + addr.function, + unique_id))) + goto cleanup; + if (virGetSCSIHostNumber(name, &num) < 0) + goto cleanup; + } else { + if (virGetSCSIHostNumber(adapter.data.scsi_host.name, &num) < 0) + goto cleanup; + } + + *hostnum = num; + ret = 0; + + cleanup: + VIR_FREE(name); + return ret; } int @@ -2130,14 +2141,14 @@ virStoragePoolSourceFindDuplicate(virStoragePoolObjListPtr pools, VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_SCSI_HOST && def->source.adapter.type == VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_SCSI_HOST) { - if (pool->def->source.adapter.data.scsi_host.name) { - if (STREQ(pool->def->source.adapter.data.scsi_host.name, - def->source.adapter.data.scsi_host.name)) - matchpool = pool; - } else { - if (matchSCSIAdapterParent(pool, def)) - matchpool = pool; - } + unsigned int pool_hostnum, def_hostnum; + + if (getSCSIHostNumber(pool->def->source.adapter, + &pool_hostnum) < 0 || + getSCSIHostNumber(def->source.adapter, &def_hostnum) < 0) + goto error; + if (pool_hostnum == def_hostnum) + matchpool = pool; } break; case VIR_STORAGE_POOL_ISCSI: @@ -2177,6 +2188,10 @@ virStoragePoolSourceFindDuplicate(virStoragePoolObjListPtr pools, ret = -1; } return ret; + + error: + virStoragePoolObjUnlock(pool); + return -1; } void -- 1.9.3
On 10/06/2014 11:49 PM, John Ferlan wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1146837
Resolve a crash in libvirtd resulting from commit id 'a4bd62ad' (1.0.6) which added parentaddr and unique_id to allow unique identification of a scsi_host, but assumed that all the pool entries and the incoming definition would be similarly defined. If the existing pool uses the 'name' attribute and an incoming pool is using the parentaddr/unique_id, then the code will attempt to compare the existing name string against the incoming name string which doesn't exist (is NULL) and results in a core (STREQ).
Conversely, if the existing pool used the parentaddr/unique_id and the to be defined pool used the name, then the comparison would be against the parentaddr, but since the incoming pool doesn't have one - that would leave the comparison against a parentaddr of all 0's and a unique_id of 0, which will always comparison to fail. This means someone could define the same source adapter for two pools
In order to resolve this, adjust the code to get the 'host#' to be used by the storage scsi backend in order to check/start the pool and make sure the incoming definition doesn't match any of the existing pool defs.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/conf/storage_conf.c | 69 ++++++++++++++++++++++++++++++------------------- 1 file changed, 42 insertions(+), 27 deletions(-)
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 36696a4..19c452b 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -2062,26 +2062,37 @@ virStoragePoolObjIsDuplicate(virStoragePoolObjListPtr pools, return ret; }
-static bool -matchSCSIAdapterParent(virStoragePoolObjPtr pool, - virStoragePoolDefPtr def) +static int +getSCSIHostNumber(virStoragePoolSourceAdapter adapter, + unsigned int *hostnum) { - virDevicePCIAddressPtr pooladdr = - &pool->def->source.adapter.data.scsi_host.parentaddr; - virDevicePCIAddressPtr defaddr = - &def->source.adapter.data.scsi_host.parentaddr; - int pool_unique_id = - pool->def->source.adapter.data.scsi_host.unique_id; - int def_unique_id = - def->source.adapter.data.scsi_host.unique_id; - if (pooladdr->domain == defaddr->domain && - pooladdr->bus == defaddr->bus && - pooladdr->slot == defaddr->slot && - pooladdr->function == defaddr->function && - pool_unique_id == def_unique_id) { - return true; - } - return false; + int ret = -1; + unsigned int num; + char *name = NULL; + + if (adapter.data.scsi_host.has_parent) { + virDevicePCIAddress addr = adapter.data.scsi_host.parentaddr; + unsigned int unique_id = adapter.data.scsi_host.unique_id; + + if (!(name = virGetSCSIHostNameByParentaddr(addr.domain, + addr.bus, + addr.slot, + addr.function, + unique_id))) + goto cleanup;
This still has the same problem v1 does: https://www.redhat.com/archives/libvir-list/2014-October/msg00117.html A valid pool definition for an existing device can be refused because another definition, the one we already accepted, is invalid. I think this is strange behavior and I would rather allow duplicit pools if the user went through the trouble of bypassing our checks. Jan
On 10/29/2014 07:31 AM, Ján Tomko wrote:
On 10/06/2014 11:49 PM, John Ferlan wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1146837
Resolve a crash in libvirtd resulting from commit id 'a4bd62ad' (1.0.6) which added parentaddr and unique_id to allow unique identification of a scsi_host, but assumed that all the pool entries and the incoming definition would be similarly defined. If the existing pool uses the 'name' attribute and an incoming pool is using the parentaddr/unique_id, then the code will attempt to compare the existing name string against the incoming name string which doesn't exist (is NULL) and results in a core (STREQ).
Conversely, if the existing pool used the parentaddr/unique_id and the to be defined pool used the name, then the comparison would be against the parentaddr, but since the incoming pool doesn't have one - that would leave the comparison against a parentaddr of all 0's and a unique_id of 0, which will always comparison to fail. This means someone could define the same source adapter for two pools
In order to resolve this, adjust the code to get the 'host#' to be used by the storage scsi backend in order to check/start the pool and make sure the incoming definition doesn't match any of the existing pool defs.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/conf/storage_conf.c | 69 ++++++++++++++++++++++++++++++------------------- 1 file changed, 42 insertions(+), 27 deletions(-)
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 36696a4..19c452b 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -2062,26 +2062,37 @@ virStoragePoolObjIsDuplicate(virStoragePoolObjListPtr pools, return ret; }
-static bool -matchSCSIAdapterParent(virStoragePoolObjPtr pool, - virStoragePoolDefPtr def) +static int +getSCSIHostNumber(virStoragePoolSourceAdapter adapter, + unsigned int *hostnum) { - virDevicePCIAddressPtr pooladdr = - &pool->def->source.adapter.data.scsi_host.parentaddr; - virDevicePCIAddressPtr defaddr = - &def->source.adapter.data.scsi_host.parentaddr; - int pool_unique_id = - pool->def->source.adapter.data.scsi_host.unique_id; - int def_unique_id = - def->source.adapter.data.scsi_host.unique_id; - if (pooladdr->domain == defaddr->domain && - pooladdr->bus == defaddr->bus && - pooladdr->slot == defaddr->slot && - pooladdr->function == defaddr->function && - pool_unique_id == def_unique_id) { - return true; - } - return false; + int ret = -1; + unsigned int num; + char *name = NULL; + + if (adapter.data.scsi_host.has_parent) { + virDevicePCIAddress addr = adapter.data.scsi_host.parentaddr; + unsigned int unique_id = adapter.data.scsi_host.unique_id; + + if (!(name = virGetSCSIHostNameByParentaddr(addr.domain, + addr.bus, + addr.slot, + addr.function, + unique_id))) + goto cleanup;
This still has the same problem v1 does: https://www.redhat.com/archives/libvir-list/2014-October/msg00117.html
A valid pool definition for an existing device can be refused because another definition, the one we already accepted, is invalid. I think this is strange behavior and I would rather allow duplicit pools if the user went through the trouble of bypassing our checks.
I think I'm missing your point. The finding of duplicate sources for storage pools and disallowing them to be defined (or created) has been around since commit id '5a1f27287". I'm not sure what you meant by your first sentence - perhaps an example would help me especially in the accepted, but invalid condition. John
On 10/29/2014 01:49 PM, John Ferlan wrote:
On 10/29/2014 07:31 AM, Ján Tomko wrote:
On 10/06/2014 11:49 PM, John Ferlan wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1146837
Resolve a crash in libvirtd resulting from commit id 'a4bd62ad' (1.0.6) which added parentaddr and unique_id to allow unique identification of a scsi_host, but assumed that all the pool entries and the incoming definition would be similarly defined. If the existing pool uses the 'name' attribute and an incoming pool is using the parentaddr/unique_id, then the code will attempt to compare the existing name string against the incoming name string which doesn't exist (is NULL) and results in a core (STREQ).
Conversely, if the existing pool used the parentaddr/unique_id and the to be defined pool used the name, then the comparison would be against the parentaddr, but since the incoming pool doesn't have one - that would leave the comparison against a parentaddr of all 0's and a unique_id of 0, which will always comparison to fail. This means someone could define the same source adapter for two pools
In order to resolve this, adjust the code to get the 'host#' to be used by the storage scsi backend in order to check/start the pool and make sure the incoming definition doesn't match any of the existing pool defs.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/conf/storage_conf.c | 69 ++++++++++++++++++++++++++++++------------------- 1 file changed, 42 insertions(+), 27 deletions(-)
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 36696a4..19c452b 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -2062,26 +2062,37 @@ virStoragePoolObjIsDuplicate(virStoragePoolObjListPtr pools, return ret; }
-static bool -matchSCSIAdapterParent(virStoragePoolObjPtr pool, - virStoragePoolDefPtr def) +static int +getSCSIHostNumber(virStoragePoolSourceAdapter adapter, + unsigned int *hostnum) { - virDevicePCIAddressPtr pooladdr = - &pool->def->source.adapter.data.scsi_host.parentaddr; - virDevicePCIAddressPtr defaddr = - &def->source.adapter.data.scsi_host.parentaddr; - int pool_unique_id = - pool->def->source.adapter.data.scsi_host.unique_id; - int def_unique_id = - def->source.adapter.data.scsi_host.unique_id; - if (pooladdr->domain == defaddr->domain && - pooladdr->bus == defaddr->bus && - pooladdr->slot == defaddr->slot && - pooladdr->function == defaddr->function && - pool_unique_id == def_unique_id) { - return true; - } - return false; + int ret = -1; + unsigned int num; + char *name = NULL; + + if (adapter.data.scsi_host.has_parent) { + virDevicePCIAddress addr = adapter.data.scsi_host.parentaddr; + unsigned int unique_id = adapter.data.scsi_host.unique_id; + + if (!(name = virGetSCSIHostNameByParentaddr(addr.domain, + addr.bus, + addr.slot, + addr.function, + unique_id))) + goto cleanup;
This still has the same problem v1 does: https://www.redhat.com/archives/libvir-list/2014-October/msg00117.html
A valid pool definition for an existing device can be refused because another definition, the one we already accepted, is invalid. I think this is strange behavior and I would rather allow duplicit pools if the user went through the trouble of bypassing our checks.
I think I'm missing your point. The finding of duplicate sources for storage pools and disallowing them to be defined (or created) has been around since commit id '5a1f27287".
I'm not sure what you meant by your first sentence - perhaps an example would help me especially in the accepted, but invalid condition.
Perhaps 'invalid' wasn't the best choice - the definition itself is valid, but it doesn't refer to an existing device so the pool cannot be started up. It's possible to define a pool using parentaddr (even on a host with no SCSI, as the address is only used on pool startup, not definition): <adapter type='scsi_host'> <parentaddr unique_id='1'> <address domain='0x0000' bus='0x00' slot='0x1f' function='0x0'/> </parentaddr> </adapter> After that, defining a pool with scsi_host source fails. Both via name: <adapter type='scsi_host' name='scsi_host13'/> and via parenaddr: <adapter type='scsi_host'> <parentaddr unique_id='1'> <address domain='0x0000' bus='0x00' slot='0x10' function='0x0'/> </parentaddr> </adapter> because of the first, already accepted definition: error: Failed to define pool from scsi2.xml error: XML error: Failed to find scsi_host using PCI '0000:00:1f.0' and unique_id='1' Jan
On 10/29/2014 09:33 AM, Ján Tomko wrote:
On 10/29/2014 01:49 PM, John Ferlan wrote:
<...snip...>
I think I'm missing your point. The finding of duplicate sources for storage pools and disallowing them to be defined (or created) has been around since commit id '5a1f27287".
I'm not sure what you meant by your first sentence - perhaps an example would help me especially in the accepted, but invalid condition.
Perhaps 'invalid' wasn't the best choice - the definition itself is valid, but it doesn't refer to an existing device so the pool cannot be started up.
It's possible to define a pool using parentaddr (even on a host with no SCSI, as the address is only used on pool startup, not definition): <adapter type='scsi_host'> <parentaddr unique_id='1'> <address domain='0x0000' bus='0x00' slot='0x1f' function='0x0'/> </parentaddr> </adapter>
After that, defining a pool with scsi_host source fails. Both via name: <adapter type='scsi_host' name='scsi_host13'/> and via parenaddr: <adapter type='scsi_host'> <parentaddr unique_id='1'> <address domain='0x0000' bus='0x00' slot='0x10' function='0x0'/> </parentaddr> </adapter> because of the first, already accepted definition: error: Failed to define pool from scsi2.xml error: XML error: Failed to find scsi_host using PCI '0000:00:1f.0' and unique_id='1'
OK - so the issue you are bringing up resolves around an "incorrect" definition of a pool that will fail on startup and why would restrict someone from creating a second, but perhaps correct definition rather than perhaps fixing their first definition. As an interesting corollary how is this any different than the following example using a DIR pool with a bad target : # cat x1.xml <pool type='dir'> <name>x1</name> <source> </source> <target> <path>/avr/lib/libvirt/images</path> <permissions> <mode>0755</mode> <owner>-1</owner> <group>-1</group> </permissions> </target> </pool> # cat x2.xml <pool type='dir'> <name>x2</name> <source> </source> <target> <path>/var/lib/libvirt/images</path> <permissions> <mode>0755</mode> <owner>-1</owner> <group>-1</group> </permissions> </target> </pool> # virsh pool-define x1.xml Pool x1 defined from x1.xml # virsh pool-define x2.xml error: Failed to define pool from x2.xml error: operation failed: Storage source conflict with pool: 'x1' # virsh pool-start x1 error: Failed to start pool x1 error: cannot open path '/avr/lib/libvirt/images': No such file or directory While I see your point, I guess I would expect someone to edit their current incorrect definition rather than trying to create a new one and use that instead. I think this code follows the spirit of what other code does and enforces only 1 target per pool at definition time regardless of whether that target is valid.That's a different problem which is always a bit controversial - allowing bogus definitions to exist. John
On 10/29/2014 03:56 PM, John Ferlan wrote:
On 10/29/2014 09:33 AM, Ján Tomko wrote:
On 10/29/2014 01:49 PM, John Ferlan wrote:
<...snip...>
I think I'm missing your point. The finding of duplicate sources for storage pools and disallowing them to be defined (or created) has been around since commit id '5a1f27287".
I'm not sure what you meant by your first sentence - perhaps an example would help me especially in the accepted, but invalid condition.
Perhaps 'invalid' wasn't the best choice - the definition itself is valid, but it doesn't refer to an existing device so the pool cannot be started up.
It's possible to define a pool using parentaddr (even on a host with no SCSI, as the address is only used on pool startup, not definition): <adapter type='scsi_host'> <parentaddr unique_id='1'> <address domain='0x0000' bus='0x00' slot='0x1f' function='0x0'/> </parentaddr> </adapter>
After that, defining a pool with scsi_host source fails. Both via name: <adapter type='scsi_host' name='scsi_host13'/> and via parenaddr: <adapter type='scsi_host'> <parentaddr unique_id='1'> <address domain='0x0000' bus='0x00' slot='0x10' function='0x0'/> </parentaddr> </adapter> because of the first, already accepted definition: error: Failed to define pool from scsi2.xml error: XML error: Failed to find scsi_host using PCI '0000:00:1f.0' and unique_id='1'
OK - so the issue you are bringing up resolves around an "incorrect" definition of a pool that will fail on startup and why would restrict someone from creating a second, but perhaps correct definition rather than perhaps fixing their first definition.
As an interesting corollary how is this any different than the following example using a DIR pool with a bad target :
# cat x1.xml <pool type='dir'> <name>x1</name> <source> </source> <target> <path>/avr/lib/libvirt/images</path> <permissions> <mode>0755</mode> <owner>-1</owner> <group>-1</group> </permissions> </target> </pool>
# cat x2.xml <pool type='dir'> <name>x2</name> <source> </source> <target> <path>/var/lib/libvirt/images</path> <permissions> <mode>0755</mode> <owner>-1</owner> <group>-1</group> </permissions> </target> </pool>
# virsh pool-define x1.xml Pool x1 defined from x1.xml
# virsh pool-define x2.xml error: Failed to define pool from x2.xml error: operation failed: Storage source conflict with pool: 'x1'
This is not what happens with current libvirt - the pool is defined successfully. But if we did this for a directory pool it would be equally strange - the paths are obviously different so there is no conflict.
# virsh pool-start x1 error: Failed to start pool x1 error: cannot open path '/avr/lib/libvirt/images': No such file or directory
While I see your point, I guess I would expect someone to edit their current incorrect definition rather than trying to create a new one and use that instead.
Reporting errors related to the old pool when trying to define a new one seems confusing to me.
I think this code follows the spirit of what other code does and enforces only 1 target per pool at definition time regardless of whether that target is valid.That's a different problem which is always a bit controversial - allowing bogus definitions to exist.
It's not foolproof - you can mount -o bind the directory somewhere else and define the pool successfully. Regarding bogus definitions and newly introduced checks, I think it's nice to leave them defined and only report errors on startup (that way you can still use management tools to fix/delete them after libvirt upgrade). And a bogus pool definition shouldn't block the sane pool definitions, which is what happens here. Jan
ping Tks, John On 10/06/2014 05:49 PM, John Ferlan wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1146837
Rewrite the v1 patch to take a different method to attack the problem. Expose the getHostNumber from scsi check/startup/refresh in the form of virGetSCSIHostNumber. Additionally, expose part of the getAdapterName code in the form of virGetSCSIHostNameByParentaddr. Then use the two functions to determine whether the incoming def matches any of the defined pool defs.
v1 is here:
http://www.redhat.com/archives/libvir-list/2014-September/msg01746.html
John Ferlan (3): virutil: Introduce virGetSCSIHostNumber virutil: Introduce virGetSCSIHostNameByParentaddr storage_conf: Resolve libvirtd crash matching scsi_host
src/conf/storage_conf.c | 69 ++++++++++++++---------- src/libvirt_private.syms | 2 + src/storage/storage_backend_scsi.c | 59 ++++----------------- src/util/virutil.c | 106 +++++++++++++++++++++++++++++++++++++ src/util/virutil.h | 10 ++++ 5 files changed, 170 insertions(+), 76 deletions(-)
Ping again - Tks John On 10/06/2014 05:49 PM, John Ferlan wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1146837
Rewrite the v1 patch to take a different method to attack the problem. Expose the getHostNumber from scsi check/startup/refresh in the form of virGetSCSIHostNumber. Additionally, expose part of the getAdapterName code in the form of virGetSCSIHostNameByParentaddr. Then use the two functions to determine whether the incoming def matches any of the defined pool defs.
v1 is here:
http://www.redhat.com/archives/libvir-list/2014-September/msg01746.html
John Ferlan (3): virutil: Introduce virGetSCSIHostNumber virutil: Introduce virGetSCSIHostNameByParentaddr storage_conf: Resolve libvirtd crash matching scsi_host
src/conf/storage_conf.c | 69 ++++++++++++++---------- src/libvirt_private.syms | 2 + src/storage/storage_backend_scsi.c | 59 ++++----------------- src/util/virutil.c | 106 +++++++++++++++++++++++++++++++++++++ src/util/virutil.h | 10 ++++ 5 files changed, 170 insertions(+), 76 deletions(-)
On 06.10.2014 23:49, John Ferlan wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1146837
Rewrite the v1 patch to take a different method to attack the problem. Expose the getHostNumber from scsi check/startup/refresh in the form of virGetSCSIHostNumber. Additionally, expose part of the getAdapterName code in the form of virGetSCSIHostNameByParentaddr. Then use the two functions to determine whether the incoming def matches any of the defined pool defs.
v1 is here:
http://www.redhat.com/archives/libvir-list/2014-September/msg01746.html
John Ferlan (3): virutil: Introduce virGetSCSIHostNumber virutil: Introduce virGetSCSIHostNameByParentaddr storage_conf: Resolve libvirtd crash matching scsi_host
src/conf/storage_conf.c | 69 ++++++++++++++---------- src/libvirt_private.syms | 2 + src/storage/storage_backend_scsi.c | 59 ++++----------------- src/util/virutil.c | 106 +++++++++++++++++++++++++++++++++++++ src/util/virutil.h | 10 ++++ 5 files changed, 170 insertions(+), 76 deletions(-)
ACK, but see my comment to 1/3. Michal
participants (3)
-
John Ferlan -
Ján Tomko -
Michal Privoznik