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 :|