On Mon, May 04, 2009 at 04:58:21PM -0400, David Allan wrote:
@@ -258,6 +307,310 @@ cleanup: }
+static int +nodeDeviceVportCreateDelete(virConnectPtr conn, + const int parent_host, + const char *wwpn, + const char *wwnn, + int operation) +{ + int fd = -1; + int retval = 0; + char *operation_path; + const char *operation_file; + char *vport_name; + size_t towrite = 0; + unsigned int written = 0; + + switch (operation) { + case VPORT_CREATE: + operation_file = LINUX_SYSFS_VPORT_CREATE_POSTFIX; + break; + case VPORT_DELETE: + operation_file = LINUX_SYSFS_VPORT_DELETE_POSTFIX; + break; + default: + virNodeDeviceReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("Invalid vport operation (%d)"), operation); + retval = -1; + goto no_unwind; + break; + } + + if (virAsprintf(&operation_path, + "%shost%d%s", + LINUX_SYSFS_FC_HOST_PREFIX, + parent_host, + operation_file) < 0) { + + virReportOOMError(conn); + retval = -1; + goto no_unwind; + } + + VIR_DEBUG(_("Vport operation path is '%s'"), operation_path); + + fd = open(operation_path, O_WRONLY); + + if (fd < 0) { + virReportSystemError(conn, errno, + _("Could not open '%s' for vport operation"), + operation_path); + retval = -1; + goto free_path; + } + + if (virAsprintf(&vport_name, + "%s:%s", + wwpn, + wwnn) < 0) { + + virReportOOMError(conn); + retval = -1; + goto close_fd; + } + + towrite = strlen(vport_name); + written = safewrite(fd, vport_name, towrite); + if (written != towrite) { + virReportSystemError(conn, errno, + _("Write of '%s' to '%s' during " + "vport create/delete failed " + "(towrite: %lu written: %d)"), + vport_name, operation_path, + towrite, written); + retval = -1; + } + + VIR_FREE(vport_name); +close_fd: + close(fd); +free_path: + VIR_FREE(operation_path); +no_unwind: + VIR_DEBUG("%s", _("Vport operation complete")); + return retval; +}
I think it is a little overkill to have so many cleanup points. VIR_FREE is quite happy to get a NULL pointer, just being a no-op in this case. So if you make sure initialize the char * strins to NULL, you can just have cleanup: VIR_FREE(vport_name); if (fd != -1) close(fd); VIR_FREE(operation_path); VIR_DEBUG("%s", _("Vport operation complete"));
+ + +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 = cap->data.scsi_host.wwnn; + *wwpn = cap->data.scsi_host.wwnn; + break; + } + + cap = cap->next; + } + + if (cap == NULL) { + /* XXX This error code is wrong--it results in errors of the form: + "error: invalid node device pointer in Device foo is not a fibre channel HBA" + */ + virNodeDeviceReportError(conn, VIR_ERR_INVALID_NODE_DEVICE, + _("Device %s is not a fibre channel HBA"), + def->name); + ret = -1; + } + + return ret; +}
What we really want to indicate is that we only support device creation for SCSI vports. For everything else we need to return VIR_ERR_NO_SUPPORT with a message along the lines of 'This type of device cannot be created' The rest of the patch looks pretty reasonable to me. Only thing missing was an update to docs/schemas/nodedev.rng to list the new attributes that are allowed Regards, Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|