[libvirt] [PATCH 0/3] test: Support NodeDeviceCreate/Destroy
The following small series provides the refactoring and implementation needed to support NodeDeviceCreate/Destroy in the test driver, particularly with respect to NPIV emulation. Cole Robinson (3): node device: Fix locking issue in virNodeDeviceDestroy node device: Break out get_wwns and get_parent_node helpers test: Support virNodeDeviceCreate and virNodeDeviceDestroy src/conf/node_device_conf.c | 89 +++++++++++++++++++++++ src/conf/node_device_conf.h | 11 +++ src/libvirt_private.syms | 2 + src/node_device/node_device_driver.c | 114 ++++-------------------------- src/test/test_driver.c | 129 +++++++++++++++++++++++++++++++++- 5 files changed, 244 insertions(+), 101 deletions(-)
Certain error paths won't unlock the node device object. Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/node_device/node_device_driver.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index 14b3098..21a4c8d 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -720,6 +720,8 @@ nodeDeviceDestroy(virNodeDevicePtr dev) } out: + if (obj) + virNodeDeviceObjUnlock(obj); VIR_FREE(parent_name); VIR_FREE(wwnn); VIR_FREE(wwpn); -- 1.6.0.6
2009/10/16 Cole Robinson <crobinso@redhat.com>:
Certain error paths won't unlock the node device object.
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/node_device/node_device_driver.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index 14b3098..21a4c8d 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -720,6 +720,8 @@ nodeDeviceDestroy(virNodeDevicePtr dev) }
out: + if (obj) + virNodeDeviceObjUnlock(obj); VIR_FREE(parent_name); VIR_FREE(wwnn); VIR_FREE(wwpn); -- 1.6.0.6
The only error path that won't unlock the node device object if the get_wwns() one. I would suggest to put the unlock call there: diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index 14b3098..43876c3 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -686,6 +686,7 @@ nodeDeviceDestroy(virNodeDevicePtr dev) } if (get_wwns(dev->conn, obj->def, &wwnn, &wwpn) == -1) { + virNodeDeviceObjUnlock(obj); goto out; } Matthias
Cole Robinson wrote:
Certain error paths won't unlock the node device object.
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/node_device/node_device_driver.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index 14b3098..21a4c8d 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -720,6 +720,8 @@ nodeDeviceDestroy(virNodeDevicePtr dev) }
out: + if (obj) + virNodeDeviceObjUnlock(obj); VIR_FREE(parent_name); VIR_FREE(wwnn); VIR_FREE(wwpn);
ACK to this patch. I think this way to do the unlock in the error case is correct. (I don't agree with Matthias' comment. As the code stands now, either way will work, but this way protects against problems in code that is added in the future.)
On 10/19/2009 05:28 PM, Dave Allan wrote:
Cole Robinson wrote:
Certain error paths won't unlock the node device object.
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/node_device/node_device_driver.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index 14b3098..21a4c8d 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -720,6 +720,8 @@ nodeDeviceDestroy(virNodeDevicePtr dev) }
out: + if (obj) + virNodeDeviceObjUnlock(obj); VIR_FREE(parent_name); VIR_FREE(wwnn); VIR_FREE(wwpn);
ACK to this patch. I think this way to do the unlock in the error case is correct. (I don't agree with Matthias' comment. As the code stands now, either way will work, but this way protects against problems in code that is added in the future.)
I agree, more future proof, and follows the same conventions of the other code in node_device_driver.c I've pushed this change now. Thanks, Cole
These will be used by the test driver, so move them to a shareable space. Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/conf/node_device_conf.c | 89 +++++++++++++++++++++++++++ src/conf/node_device_conf.h | 11 +++ src/libvirt_private.syms | 2 + src/node_device/node_device_driver.c | 112 ++++------------------------------ 4 files changed, 115 insertions(+), 99 deletions(-) diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index f09f814..77f7be3 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -1215,6 +1215,95 @@ virNodeDeviceDefParseFile(virConnectPtr conn, return virNodeDeviceDefParse(conn, NULL, filename, create); } +/* + * Return fc_host dev's WWNN and WWPN + */ +int +virNodeDeviceGetWWNs(virConnectPtr conn, + virNodeDeviceDefPtr def, + char **wwnn, + char **wwpn) +{ + virNodeDevCapsDefPtr cap = NULL; + int ret = 0; + + cap = def->caps; + while (cap != NULL) { + if (cap->type == VIR_NODE_DEV_CAP_SCSI_HOST && + cap->data.scsi_host.flags & VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST) { + *wwnn = strdup(cap->data.scsi_host.wwnn); + *wwpn = strdup(cap->data.scsi_host.wwpn); + break; + } + + cap = cap->next; + } + + if (cap == NULL) { + virNodeDeviceReportError(conn, VIR_ERR_NO_SUPPORT, + "%s", _("Device is not a fibre channel HBA")); + ret = -1; + } + + if (*wwnn == NULL || *wwpn == NULL) { + /* Free the other one, if allocated... */ + VIR_FREE(wwnn); + VIR_FREE(wwpn); + ret = -1; + virReportOOMError(conn); + } + + return ret; +} + +/* + * Return the NPIV dev's parent device name + */ +int +virNodeDeviceGetParentHost(virConnectPtr conn, + const virNodeDeviceObjListPtr devs, + const char *dev_name, + const char *parent_name, + int *parent_host) +{ + virNodeDeviceObjPtr parent = NULL; + virNodeDevCapsDefPtr cap = NULL; + int ret = 0; + + parent = virNodeDeviceFindByName(devs, parent_name); + if (parent == NULL) { + virNodeDeviceReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("Could not find parent HBA for '%s'"), + dev_name); + ret = -1; + goto out; + } + + cap = parent->def->caps; + while (cap != NULL) { + if (cap->type == VIR_NODE_DEV_CAP_SCSI_HOST && + (cap->data.scsi_host.flags & + VIR_NODE_DEV_CAP_FLAG_HBA_VPORT_OPS)) { + *parent_host = cap->data.scsi_host.host; + break; + } + + cap = cap->next; + } + + if (cap == NULL) { + virNodeDeviceReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("Parent HBA %s is not capable " + "of vport operations"), + parent->def->name); + ret = -1; + } + + virNodeDeviceObjUnlock(parent); + +out: + return ret; +} void virNodeDevCapsDefFree(virNodeDevCapsDefPtr caps) { diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h index 29a4d43..a7bb6c6 100644 --- a/src/conf/node_device_conf.h +++ b/src/conf/node_device_conf.h @@ -221,6 +221,17 @@ virNodeDeviceDefPtr virNodeDeviceDefParseNode(virConnectPtr conn, xmlNodePtr root, int create); +int virNodeDeviceGetWWNs(virConnectPtr conn, + virNodeDeviceDefPtr def, + char **wwnn, + char **wwpn); + +int virNodeDeviceGetParentHost(virConnectPtr conn, + const virNodeDeviceObjListPtr devs, + const char *dev_name, + const char *parent_name, + int *parent_host); + void virNodeDeviceDefFree(virNodeDeviceDefPtr def); void virNodeDeviceObjFree(virNodeDeviceObjPtr dev); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 37395ab..45d1069 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -318,6 +318,8 @@ virNodeDeviceDefParseString; virNodeDeviceObjLock; virNodeDeviceObjUnlock; virNodeDeviceAssignDef; +virNodeDeviceGetWWNs; +virNodeDeviceGetParentHost; # pci.h diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index 21a4c8d..f33ff48 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -459,92 +459,6 @@ cleanup: static int -get_wwns(virConnectPtr conn, - virNodeDeviceDefPtr def, - char **wwnn, - char **wwpn) -{ - virNodeDevCapsDefPtr cap = NULL; - int ret = 0; - - cap = def->caps; - while (cap != NULL) { - if (cap->type == VIR_NODE_DEV_CAP_SCSI_HOST && - cap->data.scsi_host.flags & VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST) { - *wwnn = strdup(cap->data.scsi_host.wwnn); - *wwpn = strdup(cap->data.scsi_host.wwpn); - break; - } - - cap = cap->next; - } - - if (cap == NULL) { - virNodeDeviceReportError(conn, VIR_ERR_NO_SUPPORT, - "%s", _("Device is not a fibre channel HBA")); - ret = -1; - } - - if (*wwnn == NULL || *wwpn == NULL) { - /* Free the other one, if allocated... */ - VIR_FREE(wwnn); - VIR_FREE(wwpn); - ret = -1; - virReportOOMError(conn); - } - - return ret; -} - - -static int -get_parent_host(virConnectPtr conn, - virDeviceMonitorStatePtr driver, - const char *dev_name, - const char *parent_name, - int *parent_host) -{ - virNodeDeviceObjPtr parent = NULL; - virNodeDevCapsDefPtr cap = NULL; - int ret = 0; - - parent = virNodeDeviceFindByName(&driver->devs, parent_name); - if (parent == NULL) { - virNodeDeviceReportError(conn, VIR_ERR_INTERNAL_ERROR, - _("Could not find parent HBA for '%s'"), - dev_name); - ret = -1; - goto out; - } - - cap = parent->def->caps; - while (cap != NULL) { - if (cap->type == VIR_NODE_DEV_CAP_SCSI_HOST && - (cap->data.scsi_host.flags & - VIR_NODE_DEV_CAP_FLAG_HBA_VPORT_OPS)) { - *parent_host = cap->data.scsi_host.host; - break; - } - - cap = cap->next; - } - - if (cap == NULL) { - virNodeDeviceReportError(conn, VIR_ERR_INTERNAL_ERROR, - _("Parent HBA %s is not capable " - "of vport operations"), - parent->def->name); - ret = -1; - } - - virNodeDeviceObjUnlock(parent); - -out: - return ret; -} - - -static int get_time(virConnectPtr conn, time_t *t) { int ret = 0; @@ -630,15 +544,15 @@ nodeDeviceCreateXML(virConnectPtr conn, goto cleanup; } - if (get_wwns(conn, def, &wwnn, &wwpn) == -1) { + if (virNodeDeviceGetWWNs(conn, def, &wwnn, &wwpn) == -1) { goto cleanup; } - if (get_parent_host(conn, - driver, - def->name, - def->parent, - &parent_host) == -1) { + if (virNodeDeviceGetParentHost(conn, + &driver->devs, + def->name, + def->parent, + &parent_host) == -1) { goto cleanup; } @@ -685,13 +599,13 @@ nodeDeviceDestroy(virNodeDevicePtr dev) goto out; } - if (get_wwns(dev->conn, obj->def, &wwnn, &wwpn) == -1) { + if (virNodeDeviceGetWWNs(dev->conn, obj->def, &wwnn, &wwpn) == -1) { goto out; } parent_name = strdup(obj->def->parent); - /* get_parent_host will cause the device object's lock to be + /* virNodeDeviceGetParentHost will cause the device object's lock to be * taken, so we have to dup the parent's name and drop the lock * before calling it. We don't need the reference to the object * any more once we have the parent's name. */ @@ -703,11 +617,11 @@ nodeDeviceDestroy(virNodeDevicePtr dev) goto out; } - if (get_parent_host(dev->conn, - driver, - dev->name, - parent_name, - &parent_host) == -1) { + if (virNodeDeviceGetParentHost(dev->conn, + &driver->devs, + dev->name, + parent_name, + &parent_host) == -1) { goto out; } -- 1.6.0.6
2009/10/16 Cole Robinson <crobinso@redhat.com>:
These will be used by the test driver, so move them to a shareable space.
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/conf/node_device_conf.c | 89 +++++++++++++++++++++++++++ src/conf/node_device_conf.h | 11 +++ src/libvirt_private.syms | 2 + src/node_device/node_device_driver.c | 112 ++++------------------------------ 4 files changed, 115 insertions(+), 99 deletions(-)
diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index f09f814..77f7be3 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -1215,6 +1215,95 @@ virNodeDeviceDefParseFile(virConnectPtr conn, return virNodeDeviceDefParse(conn, NULL, filename, create); }
+/* + * Return fc_host dev's WWNN and WWPN + */ +int +virNodeDeviceGetWWNs(virConnectPtr conn, + virNodeDeviceDefPtr def, + char **wwnn, + char **wwpn) +{ + virNodeDevCapsDefPtr cap = NULL; + int ret = 0; + + cap = def->caps; + while (cap != NULL) { + if (cap->type == VIR_NODE_DEV_CAP_SCSI_HOST && + cap->data.scsi_host.flags & VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST) { + *wwnn = strdup(cap->data.scsi_host.wwnn); + *wwpn = strdup(cap->data.scsi_host.wwpn); + break; + } + + cap = cap->next; + } + + if (cap == NULL) { + virNodeDeviceReportError(conn, VIR_ERR_NO_SUPPORT, + "%s", _("Device is not a fibre channel HBA")); + ret = -1; + } + + if (*wwnn == NULL || *wwpn == NULL) {
I know you have just moved this function to another file, but here seems to be a logical error that may result in a false-positive OOM error. If the while loop is left without finding the wanted device (cap == NULL), then *wwnn and *wwpn have not been set inside this function and point to their initial value (probably NULL). Because they have not been set inside this function (in the cap == NULL case), they should not be tested for NULL to detect an OOM condition. To fix this if (*wwnn == NULL || *wwpn == NULL) { could be changed to else if (*wwnn == NULL || *wwpn == NULL) { so the OOM check is only done if the wanted device has been found (cap != NULL).
+ /* Free the other one, if allocated... */ + VIR_FREE(wwnn); + VIR_FREE(wwpn); + ret = -1; + virReportOOMError(conn); + } + + return ret; +} +
The OOM detection should be fixed by an additional patch, so ACK to this one. Matthias
On 10/17/2009 08:58 AM, Matthias Bolte wrote:
diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index f09f814..77f7be3 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -1215,6 +1215,95 @@ virNodeDeviceDefParseFile(virConnectPtr conn, Â Â return virNodeDeviceDefParse(conn, NULL, filename, create); Â }
+/* + * Return fc_host dev's WWNN and WWPN + */ +int +virNodeDeviceGetWWNs(virConnectPtr conn, + Â Â Â Â Â Â Â Â Â Â virNodeDeviceDefPtr def, + Â Â Â Â Â Â Â Â Â Â char **wwnn, + Â Â Â Â Â Â Â Â Â Â char **wwpn) +{ + Â Â virNodeDevCapsDefPtr cap = NULL; + Â Â int ret = 0; + + Â Â cap = def->caps; + Â Â while (cap != NULL) { + Â Â Â Â if (cap->type == VIR_NODE_DEV_CAP_SCSI_HOST && + Â Â Â Â Â Â cap->data.scsi_host.flags & VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST) { + Â Â Â Â Â Â *wwnn = strdup(cap->data.scsi_host.wwnn); + Â Â Â Â Â Â *wwpn = strdup(cap->data.scsi_host.wwpn); + Â Â Â Â Â Â break; + Â Â Â Â } + + Â Â Â Â cap = cap->next; + Â Â } + + Â Â if (cap == NULL) { + Â Â Â Â virNodeDeviceReportError(conn, VIR_ERR_NO_SUPPORT, + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â "%s", _("Device is not a fibre channel HBA")); + Â Â Â Â ret = -1; + Â Â } + + Â Â if (*wwnn == NULL || *wwpn == NULL) {
I know you have just moved this function to another file, but here seems to be a logical error that may result in a false-positive OOM error.
If the while loop is left without finding the wanted device (cap == NULL), then *wwnn and *wwpn have not been set inside this function and point to their initial value (probably NULL). Because they have not been set inside this function (in the cap == NULL case), they should not be tested for NULL to detect an OOM condition.
To fix this
if (*wwnn == NULL || *wwpn == NULL) {
could be changed to
else if (*wwnn == NULL || *wwpn == NULL) {
so the OOM check is only done if the wanted device has been found (cap != NULL).
+ Â Â Â Â /* Free the other one, if allocated... */ + Â Â Â Â VIR_FREE(wwnn); + Â Â Â Â VIR_FREE(wwpn); + Â Â Â Â ret = -1; + Â Â Â Â virReportOOMError(conn); + Â Â } + + Â Â return ret; +} +
The OOM detection should be fixed by an additional patch, so ACK to this one.
Good catch. I agree a follow up patch is the way to go. Thanks, Cole
Cole Robinson wrote:
These will be used by the test driver, so move them to a shareable space.
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/conf/node_device_conf.c | 89 +++++++++++++++++++++++++++ src/conf/node_device_conf.h | 11 +++ src/libvirt_private.syms | 2 + src/node_device/node_device_driver.c | 112 ++++------------------------------ 4 files changed, 115 insertions(+), 99 deletions(-)
diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index f09f814..77f7be3 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -1215,6 +1215,95 @@ virNodeDeviceDefParseFile(virConnectPtr conn, return virNodeDeviceDefParse(conn, NULL, filename, create); }
+/* + * Return fc_host dev's WWNN and WWPN + */ +int +virNodeDeviceGetWWNs(virConnectPtr conn, + virNodeDeviceDefPtr def, + char **wwnn, + char **wwpn) +{ + virNodeDevCapsDefPtr cap = NULL; + int ret = 0; + + cap = def->caps; + while (cap != NULL) { + if (cap->type == VIR_NODE_DEV_CAP_SCSI_HOST && + cap->data.scsi_host.flags & VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST) { + *wwnn = strdup(cap->data.scsi_host.wwnn); + *wwpn = strdup(cap->data.scsi_host.wwpn); + break; + } + + cap = cap->next; + } + + if (cap == NULL) { + virNodeDeviceReportError(conn, VIR_ERR_NO_SUPPORT, + "%s", _("Device is not a fibre channel HBA")); + ret = -1; + } + + if (*wwnn == NULL || *wwpn == NULL) { + /* Free the other one, if allocated... */ + VIR_FREE(wwnn); + VIR_FREE(wwpn); + ret = -1; + virReportOOMError(conn); + } + + return ret; +} + +/* + * Return the NPIV dev's parent device name + */ +int +virNodeDeviceGetParentHost(virConnectPtr conn, + const virNodeDeviceObjListPtr devs, + const char *dev_name, + const char *parent_name, + int *parent_host) +{ + virNodeDeviceObjPtr parent = NULL; + virNodeDevCapsDefPtr cap = NULL; + int ret = 0; + + parent = virNodeDeviceFindByName(devs, parent_name); + if (parent == NULL) { + virNodeDeviceReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("Could not find parent HBA for '%s'"), + dev_name); + ret = -1; + goto out; + } + + cap = parent->def->caps; + while (cap != NULL) { + if (cap->type == VIR_NODE_DEV_CAP_SCSI_HOST && + (cap->data.scsi_host.flags & + VIR_NODE_DEV_CAP_FLAG_HBA_VPORT_OPS)) { + *parent_host = cap->data.scsi_host.host; + break; + } + + cap = cap->next; + } + + if (cap == NULL) { + virNodeDeviceReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("Parent HBA %s is not capable " + "of vport operations"), + parent->def->name); + ret = -1; + } + + virNodeDeviceObjUnlock(parent); + +out: + return ret; +}
void virNodeDevCapsDefFree(virNodeDevCapsDefPtr caps) { diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h index 29a4d43..a7bb6c6 100644 --- a/src/conf/node_device_conf.h +++ b/src/conf/node_device_conf.h @@ -221,6 +221,17 @@ virNodeDeviceDefPtr virNodeDeviceDefParseNode(virConnectPtr conn, xmlNodePtr root, int create);
+int virNodeDeviceGetWWNs(virConnectPtr conn, + virNodeDeviceDefPtr def, + char **wwnn, + char **wwpn); + +int virNodeDeviceGetParentHost(virConnectPtr conn, + const virNodeDeviceObjListPtr devs, + const char *dev_name, + const char *parent_name, + int *parent_host); + void virNodeDeviceDefFree(virNodeDeviceDefPtr def);
void virNodeDeviceObjFree(virNodeDeviceObjPtr dev); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 37395ab..45d1069 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -318,6 +318,8 @@ virNodeDeviceDefParseString; virNodeDeviceObjLock; virNodeDeviceObjUnlock; virNodeDeviceAssignDef; +virNodeDeviceGetWWNs; +virNodeDeviceGetParentHost;
# pci.h diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index 21a4c8d..f33ff48 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -459,92 +459,6 @@ cleanup:
static int -get_wwns(virConnectPtr conn, - virNodeDeviceDefPtr def, - char **wwnn, - char **wwpn) -{ - virNodeDevCapsDefPtr cap = NULL; - int ret = 0; - - cap = def->caps; - while (cap != NULL) { - if (cap->type == VIR_NODE_DEV_CAP_SCSI_HOST && - cap->data.scsi_host.flags & VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST) { - *wwnn = strdup(cap->data.scsi_host.wwnn); - *wwpn = strdup(cap->data.scsi_host.wwpn); - break; - } - - cap = cap->next; - } - - if (cap == NULL) { - virNodeDeviceReportError(conn, VIR_ERR_NO_SUPPORT, - "%s", _("Device is not a fibre channel HBA")); - ret = -1; - } - - if (*wwnn == NULL || *wwpn == NULL) { - /* Free the other one, if allocated... */ - VIR_FREE(wwnn); - VIR_FREE(wwpn); - ret = -1; - virReportOOMError(conn); - } - - return ret; -} - - -static int -get_parent_host(virConnectPtr conn, - virDeviceMonitorStatePtr driver, - const char *dev_name, - const char *parent_name, - int *parent_host) -{ - virNodeDeviceObjPtr parent = NULL; - virNodeDevCapsDefPtr cap = NULL; - int ret = 0; - - parent = virNodeDeviceFindByName(&driver->devs, parent_name); - if (parent == NULL) { - virNodeDeviceReportError(conn, VIR_ERR_INTERNAL_ERROR, - _("Could not find parent HBA for '%s'"), - dev_name); - ret = -1; - goto out; - } - - cap = parent->def->caps; - while (cap != NULL) { - if (cap->type == VIR_NODE_DEV_CAP_SCSI_HOST && - (cap->data.scsi_host.flags & - VIR_NODE_DEV_CAP_FLAG_HBA_VPORT_OPS)) { - *parent_host = cap->data.scsi_host.host; - break; - } - - cap = cap->next; - } - - if (cap == NULL) { - virNodeDeviceReportError(conn, VIR_ERR_INTERNAL_ERROR, - _("Parent HBA %s is not capable " - "of vport operations"), - parent->def->name); - ret = -1; - } - - virNodeDeviceObjUnlock(parent); - -out: - return ret; -} - - -static int get_time(virConnectPtr conn, time_t *t) { int ret = 0; @@ -630,15 +544,15 @@ nodeDeviceCreateXML(virConnectPtr conn, goto cleanup; }
- if (get_wwns(conn, def, &wwnn, &wwpn) == -1) { + if (virNodeDeviceGetWWNs(conn, def, &wwnn, &wwpn) == -1) { goto cleanup; }
- if (get_parent_host(conn, - driver, - def->name, - def->parent, - &parent_host) == -1) { + if (virNodeDeviceGetParentHost(conn, + &driver->devs, + def->name, + def->parent, + &parent_host) == -1) { goto cleanup; }
@@ -685,13 +599,13 @@ nodeDeviceDestroy(virNodeDevicePtr dev) goto out; }
- if (get_wwns(dev->conn, obj->def, &wwnn, &wwpn) == -1) { + if (virNodeDeviceGetWWNs(dev->conn, obj->def, &wwnn, &wwpn) == -1) { goto out; }
parent_name = strdup(obj->def->parent);
- /* get_parent_host will cause the device object's lock to be + /* virNodeDeviceGetParentHost will cause the device object's lock to be * taken, so we have to dup the parent's name and drop the lock * before calling it. We don't need the reference to the object * any more once we have the parent's name. */ @@ -703,11 +617,11 @@ nodeDeviceDestroy(virNodeDevicePtr dev) goto out; }
- if (get_parent_host(dev->conn, - driver, - dev->name, - parent_name, - &parent_host) == -1) { + if (virNodeDeviceGetParentHost(dev->conn, + &driver->devs, + dev->name, + parent_name, + &parent_host) == -1) { goto out; }
Ack Dave
On 10/20/2009 11:55 AM, Dave Allan wrote:
Cole Robinson wrote:
These will be used by the test driver, so move them to a shareable space.
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/conf/node_device_conf.c | 89 +++++++++++++++++++++++++++ src/conf/node_device_conf.h | 11 +++ src/libvirt_private.syms | 2 + src/node_device/node_device_driver.c | 112 ++++------------------------------ 4 files changed, 115 insertions(+), 99 deletions(-)
diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index f09f814..77f7be3 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -1215,6 +1215,95 @@ virNodeDeviceDefParseFile(virConnectPtr conn, return virNodeDeviceDefParse(conn, NULL, filename, create); }
+/* + * Return fc_host dev's WWNN and WWPN + */ +int +virNodeDeviceGetWWNs(virConnectPtr conn, + virNodeDeviceDefPtr def, + char **wwnn, + char **wwpn) +{ + virNodeDevCapsDefPtr cap = NULL; + int ret = 0; + + cap = def->caps; + while (cap != NULL) { + if (cap->type == VIR_NODE_DEV_CAP_SCSI_HOST && + cap->data.scsi_host.flags & VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST) { + *wwnn = strdup(cap->data.scsi_host.wwnn); + *wwpn = strdup(cap->data.scsi_host.wwpn); + break; + } + + cap = cap->next; + } + + if (cap == NULL) { + virNodeDeviceReportError(conn, VIR_ERR_NO_SUPPORT, + "%s", _("Device is not a fibre channel HBA")); + ret = -1; + } + + if (*wwnn == NULL || *wwpn == NULL) { + /* Free the other one, if allocated... */ + VIR_FREE(wwnn); + VIR_FREE(wwpn); + ret = -1; + virReportOOMError(conn); + } + + return ret; +} + +/* + * Return the NPIV dev's parent device name + */ +int +virNodeDeviceGetParentHost(virConnectPtr conn, + const virNodeDeviceObjListPtr devs, + const char *dev_name, + const char *parent_name, + int *parent_host) +{ + virNodeDeviceObjPtr parent = NULL; + virNodeDevCapsDefPtr cap = NULL; + int ret = 0; + + parent = virNodeDeviceFindByName(devs, parent_name); + if (parent == NULL) { + virNodeDeviceReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("Could not find parent HBA for '%s'"), + dev_name); + ret = -1; + goto out; + } + + cap = parent->def->caps; + while (cap != NULL) { + if (cap->type == VIR_NODE_DEV_CAP_SCSI_HOST && + (cap->data.scsi_host.flags & + VIR_NODE_DEV_CAP_FLAG_HBA_VPORT_OPS)) { + *parent_host = cap->data.scsi_host.host; + break; + } + + cap = cap->next; + } + + if (cap == NULL) { + virNodeDeviceReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("Parent HBA %s is not capable " + "of vport operations"), + parent->def->name); + ret = -1; + } + + virNodeDeviceObjUnlock(parent); + +out: + return ret; +}
void virNodeDevCapsDefFree(virNodeDevCapsDefPtr caps) { diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h index 29a4d43..a7bb6c6 100644 --- a/src/conf/node_device_conf.h +++ b/src/conf/node_device_conf.h @@ -221,6 +221,17 @@ virNodeDeviceDefPtr virNodeDeviceDefParseNode(virConnectPtr conn, xmlNodePtr root, int create);
+int virNodeDeviceGetWWNs(virConnectPtr conn, + virNodeDeviceDefPtr def, + char **wwnn, + char **wwpn); + +int virNodeDeviceGetParentHost(virConnectPtr conn, + const virNodeDeviceObjListPtr devs, + const char *dev_name, + const char *parent_name, + int *parent_host); + void virNodeDeviceDefFree(virNodeDeviceDefPtr def);
void virNodeDeviceObjFree(virNodeDeviceObjPtr dev); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 37395ab..45d1069 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -318,6 +318,8 @@ virNodeDeviceDefParseString; virNodeDeviceObjLock; virNodeDeviceObjUnlock; virNodeDeviceAssignDef; +virNodeDeviceGetWWNs; +virNodeDeviceGetParentHost;
# pci.h diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index 21a4c8d..f33ff48 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -459,92 +459,6 @@ cleanup:
static int -get_wwns(virConnectPtr conn, - virNodeDeviceDefPtr def, - char **wwnn, - char **wwpn) -{ - virNodeDevCapsDefPtr cap = NULL; - int ret = 0; - - cap = def->caps; - while (cap != NULL) { - if (cap->type == VIR_NODE_DEV_CAP_SCSI_HOST && - cap->data.scsi_host.flags & VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST) { - *wwnn = strdup(cap->data.scsi_host.wwnn); - *wwpn = strdup(cap->data.scsi_host.wwpn); - break; - } - - cap = cap->next; - } - - if (cap == NULL) { - virNodeDeviceReportError(conn, VIR_ERR_NO_SUPPORT, - "%s", _("Device is not a fibre channel HBA")); - ret = -1; - } - - if (*wwnn == NULL || *wwpn == NULL) { - /* Free the other one, if allocated... */ - VIR_FREE(wwnn); - VIR_FREE(wwpn); - ret = -1; - virReportOOMError(conn); - } - - return ret; -} - - -static int -get_parent_host(virConnectPtr conn, - virDeviceMonitorStatePtr driver, - const char *dev_name, - const char *parent_name, - int *parent_host) -{ - virNodeDeviceObjPtr parent = NULL; - virNodeDevCapsDefPtr cap = NULL; - int ret = 0; - - parent = virNodeDeviceFindByName(&driver->devs, parent_name); - if (parent == NULL) { - virNodeDeviceReportError(conn, VIR_ERR_INTERNAL_ERROR, - _("Could not find parent HBA for '%s'"), - dev_name); - ret = -1; - goto out; - } - - cap = parent->def->caps; - while (cap != NULL) { - if (cap->type == VIR_NODE_DEV_CAP_SCSI_HOST && - (cap->data.scsi_host.flags & - VIR_NODE_DEV_CAP_FLAG_HBA_VPORT_OPS)) { - *parent_host = cap->data.scsi_host.host; - break; - } - - cap = cap->next; - } - - if (cap == NULL) { - virNodeDeviceReportError(conn, VIR_ERR_INTERNAL_ERROR, - _("Parent HBA %s is not capable " - "of vport operations"), - parent->def->name); - ret = -1; - } - - virNodeDeviceObjUnlock(parent); - -out: - return ret; -} - - -static int get_time(virConnectPtr conn, time_t *t) { int ret = 0; @@ -630,15 +544,15 @@ nodeDeviceCreateXML(virConnectPtr conn, goto cleanup; }
- if (get_wwns(conn, def, &wwnn, &wwpn) == -1) { + if (virNodeDeviceGetWWNs(conn, def, &wwnn, &wwpn) == -1) { goto cleanup; }
- if (get_parent_host(conn, - driver, - def->name, - def->parent, - &parent_host) == -1) { + if (virNodeDeviceGetParentHost(conn, + &driver->devs, + def->name, + def->parent, + &parent_host) == -1) { goto cleanup; }
@@ -685,13 +599,13 @@ nodeDeviceDestroy(virNodeDevicePtr dev) goto out; }
- if (get_wwns(dev->conn, obj->def, &wwnn, &wwpn) == -1) { + if (virNodeDeviceGetWWNs(dev->conn, obj->def, &wwnn, &wwpn) == -1) { goto out; }
parent_name = strdup(obj->def->parent);
- /* get_parent_host will cause the device object's lock to be + /* virNodeDeviceGetParentHost will cause the device object's lock to be * taken, so we have to dup the parent's name and drop the lock * before calling it. We don't need the reference to the object * any more once we have the parent's name. */ @@ -703,11 +617,11 @@ nodeDeviceDestroy(virNodeDevicePtr dev) goto out; }
- if (get_parent_host(dev->conn, - driver, - dev->name, - parent_name, - &parent_host) == -1) { + if (virNodeDeviceGetParentHost(dev->conn, + &driver->devs, + dev->name, + parent_name, + &parent_host) == -1) { goto out; }
Ack
Dave
Thanks, I've pushed this now. - Cole
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/test/test_driver.c | 129 +++++++++++++++++++++++++++++++++++++++++++++++- 1 files changed, 127 insertions(+), 2 deletions(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 0541a73..888bc9c 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -4376,6 +4376,131 @@ cleanup: return ret; } +static virNodeDevicePtr +testNodeDeviceCreateXML(virConnectPtr conn, + const char *xmlDesc, + unsigned int flags ATTRIBUTE_UNUSED) +{ + testConnPtr driver = conn->privateData; + virNodeDeviceDefPtr def = NULL; + virNodeDeviceObjPtr obj = NULL; + char *wwnn = NULL, *wwpn = NULL; + int parent_host = -1; + virNodeDevicePtr dev = NULL; + virNodeDevCapsDefPtr caps; + + testDriverLock(driver); + + def = virNodeDeviceDefParseString(conn, xmlDesc, CREATE_DEVICE); + if (def == NULL) { + goto cleanup; + } + + /* We run these next two simply for validation */ + if (virNodeDeviceGetWWNs(conn, def, &wwnn, &wwpn) == -1) { + goto cleanup; + } + + if (virNodeDeviceGetParentHost(conn, + &driver->devs, + def->name, + def->parent, + &parent_host) == -1) { + goto cleanup; + } + + /* 'name' is supposed to be filled in by the node device backend, which + * we don't have. Use WWPN instead. */ + VIR_FREE(def->name); + if (!(def->name = strdup(wwpn))) { + virReportOOMError(dev->conn); + goto cleanup; + } + + /* Fill in a random 'host' value, since this would also come from + * the backend */ + caps = def->caps; + while (caps) { + if (caps->type != VIR_NODE_DEV_CAP_SCSI_HOST) + continue; + + caps->data.scsi_host.host = virRandom(1024); + caps = caps->next; + } + + + if (!(obj = virNodeDeviceAssignDef(conn, &driver->devs, def))) { + goto cleanup; + } + virNodeDeviceObjUnlock(obj); + + dev = virGetNodeDevice(conn, def->name); + def = NULL; +cleanup: + testDriverUnlock(driver); + if (def) + virNodeDeviceDefFree(def); + VIR_FREE(wwnn); + VIR_FREE(wwpn); + return dev; +} + +static int +testNodeDeviceDestroy(virNodeDevicePtr dev) +{ + int ret = 0; + testConnPtr driver = dev->conn->privateData; + virNodeDeviceObjPtr obj = NULL; + char *parent_name = NULL, *wwnn = NULL, *wwpn = NULL; + int parent_host = -1; + + testDriverLock(driver); + obj = virNodeDeviceFindByName(&driver->devs, dev->name); + testDriverUnlock(driver); + + if (!obj) { + virNodeDeviceReportError(dev->conn, VIR_ERR_NO_NODE_DEVICE, NULL); + goto out; + } + + if (virNodeDeviceGetWWNs(dev->conn, obj->def, &wwnn, &wwpn) == -1) { + goto out; + } + + parent_name = strdup(obj->def->parent); + if (parent_name == NULL) { + virReportOOMError(dev->conn); + goto out; + } + + /* virNodeDeviceGetParentHost will cause the device object's lock to be + * taken, so we have to dup the parent's name and drop the lock + * before calling it. We don't need the reference to the object + * any more once we have the parent's name. */ + virNodeDeviceObjUnlock(obj); + + /* We do this just for basic validation */ + if (virNodeDeviceGetParentHost(dev->conn, + &driver->devs, + dev->name, + parent_name, + &parent_host) == -1) { + obj = NULL; + goto out; + } + + virNodeDeviceObjLock(obj); + virNodeDeviceObjRemove(&driver->devs, obj); + +out: + if (obj) + virNodeDeviceObjUnlock(obj); + VIR_FREE(parent_name); + VIR_FREE(wwnn); + VIR_FREE(wwpn); + return ret; +} + /* Domain event implementations */ static int @@ -4650,8 +4775,8 @@ static virDeviceMonitor testDevMonitor = { .deviceGetParent = testNodeDeviceGetParent, .deviceNumOfCaps = testNodeDeviceNumOfCaps, .deviceListCaps = testNodeDeviceListCaps, - //.deviceCreateXML = nodeDeviceCreateXML; - //.deviceDestroy = nodeDeviceDestroy; + .deviceCreateXML = testNodeDeviceCreateXML, + .deviceDestroy = testNodeDeviceDestroy, }; static virSecretDriver testSecretDriver = { -- 1.6.0.6
2009/10/16 Cole Robinson <crobinso@redhat.com>:
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/test/test_driver.c | 129 +++++++++++++++++++++++++++++++++++++++++++++++- 1 files changed, 127 insertions(+), 2 deletions(-)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 0541a73..888bc9c 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -4376,6 +4376,131 @@ cleanup: return ret; }
+static virNodeDevicePtr +testNodeDeviceCreateXML(virConnectPtr conn, + const char *xmlDesc, + unsigned int flags ATTRIBUTE_UNUSED) +{ + testConnPtr driver = conn->privateData; + virNodeDeviceDefPtr def = NULL; + virNodeDeviceObjPtr obj = NULL; + char *wwnn = NULL, *wwpn = NULL; + int parent_host = -1; + virNodeDevicePtr dev = NULL; + virNodeDevCapsDefPtr caps; + + testDriverLock(driver); + + def = virNodeDeviceDefParseString(conn, xmlDesc, CREATE_DEVICE); + if (def == NULL) { + goto cleanup; + } + + /* We run these next two simply for validation */ + if (virNodeDeviceGetWWNs(conn, def, &wwnn, &wwpn) == -1) { + goto cleanup; + } + + if (virNodeDeviceGetParentHost(conn, + &driver->devs, + def->name, + def->parent, + &parent_host) == -1) { + goto cleanup; + } + + /* 'name' is supposed to be filled in by the node device backend, which + * we don't have. Use WWPN instead. */ + VIR_FREE(def->name); + if (!(def->name = strdup(wwpn))) { + virReportOOMError(dev->conn);
dev is NULL here, call virReportOOMError(conn) instead.
+ goto cleanup; + } + + /* Fill in a random 'host' value, since this would also come from + * the backend */ + caps = def->caps; + while (caps) { + if (caps->type != VIR_NODE_DEV_CAP_SCSI_HOST) + continue; + + caps->data.scsi_host.host = virRandom(1024); + caps = caps->next; + } + + + if (!(obj = virNodeDeviceAssignDef(conn, &driver->devs, def))) { + goto cleanup; + } + virNodeDeviceObjUnlock(obj); + + dev = virGetNodeDevice(conn, def->name); + def = NULL; +cleanup: + testDriverUnlock(driver); + if (def) + virNodeDeviceDefFree(def); + VIR_FREE(wwnn); + VIR_FREE(wwpn); + return dev; +} +
ACK Matthias
On 10/17/2009 09:10 AM, Matthias Bolte wrote:
2009/10/16 Cole Robinson <crobinso@redhat.com>:
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- Â src/test/test_driver.c | Â 129 +++++++++++++++++++++++++++++++++++++++++++++++- Â 1 files changed, 127 insertions(+), 2 deletions(-)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 0541a73..888bc9c 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -4376,6 +4376,131 @@ cleanup: Â Â return ret; Â }
+static virNodeDevicePtr +testNodeDeviceCreateXML(virConnectPtr conn, + Â Â Â Â Â Â Â Â Â Â Â Â const char *xmlDesc, + Â Â Â Â Â Â Â Â Â Â Â Â unsigned int flags ATTRIBUTE_UNUSED) +{ + Â Â testConnPtr driver = conn->privateData; + Â Â virNodeDeviceDefPtr def = NULL; + Â Â virNodeDeviceObjPtr obj = NULL; + Â Â char *wwnn = NULL, *wwpn = NULL; + Â Â int parent_host = -1; + Â Â virNodeDevicePtr dev = NULL; + Â Â virNodeDevCapsDefPtr caps; + + Â Â testDriverLock(driver); + + Â Â def = virNodeDeviceDefParseString(conn, xmlDesc, CREATE_DEVICE); + Â Â if (def == NULL) { + Â Â Â Â goto cleanup; + Â Â } + + Â Â /* We run these next two simply for validation */ + Â Â if (virNodeDeviceGetWWNs(conn, def, &wwnn, &wwpn) == -1) { + Â Â Â Â goto cleanup; + Â Â } + + Â Â if (virNodeDeviceGetParentHost(conn, + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â &driver->devs, + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â def->name, + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â def->parent, + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â &parent_host) == -1) { + Â Â Â Â goto cleanup; + Â Â } + + Â Â /* 'name' is supposed to be filled in by the node device backend, which + Â Â * we don't have. Use WWPN instead. */ + Â Â VIR_FREE(def->name); + Â Â if (!(def->name = strdup(wwpn))) { + Â Â Â Â virReportOOMError(dev->conn);
dev is NULL here, call virReportOOMError(conn) instead.
Whoops, yeah that's bad. Good catch! Thanks, Cole
+ Â Â Â Â goto cleanup; + Â Â } + + Â Â /* Fill in a random 'host' value, since this would also come from + Â Â * the backend */ + Â Â caps = def->caps; + Â Â while (caps) { + Â Â Â Â if (caps->type != VIR_NODE_DEV_CAP_SCSI_HOST) + Â Â Â Â Â Â continue; + + Â Â Â Â caps->data.scsi_host.host = virRandom(1024); + Â Â Â Â caps = caps->next; + Â Â } + + + Â Â if (!(obj = virNodeDeviceAssignDef(conn, &driver->devs, def))) { + Â Â Â Â goto cleanup; + Â Â } + Â Â virNodeDeviceObjUnlock(obj); + + Â Â dev = virGetNodeDevice(conn, def->name); + Â Â def = NULL; +cleanup: + Â Â testDriverUnlock(driver); + Â Â if (def) + Â Â Â Â virNodeDeviceDefFree(def); + Â Â VIR_FREE(wwnn); + Â Â VIR_FREE(wwpn); + Â Â return dev; +} +
ACK
Matthias
On 10/17/2009 09:10 AM, Matthias Bolte wrote:
2009/10/16 Cole Robinson <crobinso@redhat.com>:
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- Â src/test/test_driver.c | Â 129 +++++++++++++++++++++++++++++++++++++++++++++++- Â 1 files changed, 127 insertions(+), 2 deletions(-)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 0541a73..888bc9c 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -4376,6 +4376,131 @@ cleanup: Â Â return ret; Â }
+static virNodeDevicePtr +testNodeDeviceCreateXML(virConnectPtr conn, + Â Â Â Â Â Â Â Â Â Â Â Â const char *xmlDesc, + Â Â Â Â Â Â Â Â Â Â Â Â unsigned int flags ATTRIBUTE_UNUSED) +{ + Â Â testConnPtr driver = conn->privateData; + Â Â virNodeDeviceDefPtr def = NULL; + Â Â virNodeDeviceObjPtr obj = NULL; + Â Â char *wwnn = NULL, *wwpn = NULL; + Â Â int parent_host = -1; + Â Â virNodeDevicePtr dev = NULL; + Â Â virNodeDevCapsDefPtr caps; + + Â Â testDriverLock(driver); + + Â Â def = virNodeDeviceDefParseString(conn, xmlDesc, CREATE_DEVICE); + Â Â if (def == NULL) { + Â Â Â Â goto cleanup; + Â Â } + + Â Â /* We run these next two simply for validation */ + Â Â if (virNodeDeviceGetWWNs(conn, def, &wwnn, &wwpn) == -1) { + Â Â Â Â goto cleanup; + Â Â } + + Â Â if (virNodeDeviceGetParentHost(conn, + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â &driver->devs, + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â def->name, + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â def->parent, + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â &parent_host) == -1) { + Â Â Â Â goto cleanup; + Â Â } + + Â Â /* 'name' is supposed to be filled in by the node device backend, which + Â Â * we don't have. Use WWPN instead. */ + Â Â VIR_FREE(def->name); + Â Â if (!(def->name = strdup(wwpn))) { + Â Â Â Â virReportOOMError(dev->conn);
dev is NULL here, call virReportOOMError(conn) instead.
+ Â Â Â Â goto cleanup; + Â Â } + + Â Â /* Fill in a random 'host' value, since this would also come from + Â Â * the backend */ + Â Â caps = def->caps; + Â Â while (caps) { + Â Â Â Â if (caps->type != VIR_NODE_DEV_CAP_SCSI_HOST) + Â Â Â Â Â Â continue; + + Â Â Â Â caps->data.scsi_host.host = virRandom(1024); + Â Â Â Â caps = caps->next; + Â Â } + + + Â Â if (!(obj = virNodeDeviceAssignDef(conn, &driver->devs, def))) { + Â Â Â Â goto cleanup; + Â Â } + Â Â virNodeDeviceObjUnlock(obj); + + Â Â dev = virGetNodeDevice(conn, def->name); + Â Â def = NULL; +cleanup: + Â Â testDriverUnlock(driver); + Â Â if (def) + Â Â Â Â virNodeDeviceDefFree(def); + Â Â VIR_FREE(wwnn); + Â Â VIR_FREE(wwpn); + Â Â return dev; +} +
ACK
Matthias
I've pushed this with the fix you pointed out. Thanks, Cole
participants (3)
-
Cole Robinson -
Dave Allan -
Matthias Bolte