[libvirt] [PATCH 0/4 v3] New API virNodeDeviceLookupSCSIHostByWWN

For easy reviewing, this is splitted from [1]. v2 - v3: * Validate the specified wwnn,wwpn pair before applying it to the new API in virsh-nodedev.c v1 - v2: * Per Daniel's suggestion, change the API name from virNodeDeviceLookupByWWN into virNodeDeviceLookupSCSIHostByWWN. Osier Yang (4): Introduce API virNodeDeviceLookupSCSIHostByWWN remote: Wire up the remote protocol nodedev: Implement virNodeDeviceLookupSCSIHostByWWN virsh: Use virNodeDeviceLookupSCSIHostByWWN include/libvirt/libvirt.h.in | 5 ++ src/driver.h | 6 ++ src/libvirt.c | 46 ++++++++++++++++ src/libvirt_public.syms | 1 + src/node_device/node_device_driver.c | 13 +++-- src/node_device/node_device_driver.h | 4 ++ src/node_device/node_device_hal.c | 1 + src/node_device/node_device_udev.c | 1 + src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 13 ++++- src/remote_protocol-structs | 9 +++ src/rpc/gendispatch.pl | 5 ++- tools/virsh-nodedev.c | 97 ++++++++++++++++++++++++++-------- tools/virsh.pod | 15 +++-- 14 files changed, 181 insertions(+), 36 deletions(-) [1] https://www.redhat.com/archives/libvir-list/2013-January/msg00328.html Regards, Osier

Since the name (like scsi_host10) is not stable for vHBA, (it can be changed either after recreating or system rebooting), current API virNodeDeviceLookupByName is not nice to use for management app in this case. (E.g. one wants to destroy the vHBA whose name has been changed after system rebooting, he has to find out current name first). Later patches will support the persistent vHBA via storage pool, with which one can identify the vHBA stably by the wwnn && wwpn pair. So this new API comes. --- include/libvirt/libvirt.h.in | 5 ++++ src/driver.h | 6 +++++ src/libvirt.c | 46 ++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 1 + 4 files changed, 58 insertions(+), 0 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index c1233f6..563ca27 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -3156,6 +3156,11 @@ int virConnectListAllNodeDevices (virConnectPtr conn, virNodeDevicePtr virNodeDeviceLookupByName (virConnectPtr conn, const char *name); +virNodeDevicePtr virNodeDeviceLookupSCSIHostByWWN (virConnectPtr conn, + const char *wwnn, + const char *wwpn, + unsigned int flags); + const char * virNodeDeviceGetName (virNodeDevicePtr dev); const char * virNodeDeviceGetParent (virNodeDevicePtr dev); diff --git a/src/driver.h b/src/driver.h index 01c95cf..bdac101 100644 --- a/src/driver.h +++ b/src/driver.h @@ -1547,6 +1547,11 @@ typedef int (*virDevMonListAllNodeDevices)(virConnectPtr conn, typedef virNodeDevicePtr (*virDevMonDeviceLookupByName)(virConnectPtr conn, const char *name); +typedef virNodeDevicePtr (*virDevMonDeviceLookupSCSIHostByWWN)(virConnectPtr conn, + const char *wwnn, + const char *wwpn, + unsigned int flags); + typedef char * (*virDevMonDeviceGetXMLDesc)(virNodeDevicePtr dev, unsigned int flags); @@ -1578,6 +1583,7 @@ struct _virDeviceMonitor { virDevMonListDevices listDevices; virDevMonListAllNodeDevices listAllNodeDevices; virDevMonDeviceLookupByName deviceLookupByName; + virDevMonDeviceLookupSCSIHostByWWN deviceLookupSCSIHostByWWN; virDevMonDeviceGetXMLDesc deviceGetXMLDesc; virDevMonDeviceGetParent deviceGetParent; virDevMonDeviceNumOfCaps deviceNumOfCaps; diff --git a/src/libvirt.c b/src/libvirt.c index a783fa6..0a2ade7 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -14334,6 +14334,52 @@ error: return NULL; } +/** + * virNodeDeviceLookupSCSIHostByWWN: + * @conn: pointer to the hypervisor connection + * @wwnn: WWNN of the SCSI Host. + * @wwpn: WWPN of the SCSI Host. + * @flags: extra flags; not used yet, so callers should always pass 0 + * + * Lookup SCSI Host which is capable with 'fc_host' by its WWNN and WWPN. + * + * Returns a virNodeDevicePtr if found, NULL otherwise. + */ +virNodeDevicePtr +virNodeDeviceLookupSCSIHostByWWN(virConnectPtr conn, + const char *wwnn, + const char *wwpn, + unsigned int flags) +{ + VIR_DEBUG("conn=%p, wwnn=%p, wwpn=%p, flags=%x", conn, wwnn, wwpn, flags); + + virResetLastError(); + + if (!VIR_IS_CONNECT(conn)) { + virLibConnError(VIR_ERR_INVALID_CONN, __FUNCTION__); + virDispatchError(NULL); + return NULL; + } + + virCheckNonNullArgGoto(wwnn, error); + virCheckNonNullArgGoto(wwpn, error); + + if (conn->deviceMonitor && + conn->deviceMonitor->deviceLookupSCSIHostByWWN) { + virNodeDevicePtr ret; + ret = conn->deviceMonitor->deviceLookupSCSIHostByWWN(conn, wwnn, + wwpn, flags); + if (!ret) + goto error; + return ret; + } + + virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: + virDispatchError(conn); + return NULL; +} /** * virNodeDeviceGetXMLDesc: diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 2107519..abd5d4a 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -583,6 +583,7 @@ LIBVIRT_1.0.1 { LIBVIRT_1.0.2 { global: virDomainOpenChannel; + virNodeDeviceLookupSCSIHostByWWN; } LIBVIRT_1.0.1; # .... define new API here using predicted next version number .... -- 1.7.7.6

Like virNodeDeviceCreateXML, virNodeDeviceLookupSCSIHostByWWN has to be treated specially when generating the RPC codes. Also new rules are added in fixup_name to keep the name SCSIHostByWWN. --- src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 13 ++++++++++++- src/remote_protocol-structs | 9 +++++++++ src/rpc/gendispatch.pl | 5 ++++- 4 files changed, 26 insertions(+), 2 deletions(-) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 862e474..ae86ba1 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -6290,6 +6290,7 @@ static virDeviceMonitor dev_monitor = { .listDevices = remoteNodeListDevices, /* 0.5.0 */ .listAllNodeDevices = remoteConnectListAllNodeDevices, /* 0.10.2 */ .deviceLookupByName = remoteNodeDeviceLookupByName, /* 0.5.0 */ + .deviceLookupSCSIHostByWWN = remoteNodeDeviceLookupSCSIHostByWWN, /* 1.0.2 */ .deviceGetXMLDesc = remoteNodeDeviceGetXMLDesc, /* 0.5.0 */ .deviceGetParent = remoteNodeDeviceGetParent, /* 0.5.0 */ .deviceNumOfCaps = remoteNodeDeviceNumOfCaps, /* 0.5.0 */ diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 9035776..5ace31f 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -1852,6 +1852,16 @@ struct remote_node_device_lookup_by_name_ret { remote_nonnull_node_device dev; }; +struct remote_node_device_lookup_scsi_host_by_wwn_args { + remote_nonnull_string wwnn; + remote_nonnull_string wwpn; + unsigned int flags; +}; + +struct remote_node_device_lookup_scsi_host_by_wwn_ret { + remote_nonnull_node_device dev; +}; + struct remote_node_device_get_xml_desc_args { remote_nonnull_string name; unsigned int flags; @@ -3049,7 +3059,8 @@ enum remote_procedure { REMOTE_PROC_NODE_GET_CPU_MAP = 293, /* skipgen skipgen */ REMOTE_PROC_DOMAIN_FSTRIM = 294, /* autogen autogen */ REMOTE_PROC_DOMAIN_SEND_PROCESS_SIGNAL = 295, /* autogen autogen */ - REMOTE_PROC_DOMAIN_OPEN_CHANNEL = 296 /* autogen autogen | readstream@2 */ + REMOTE_PROC_DOMAIN_OPEN_CHANNEL = 296, /* autogen autogen | readstream@2 */ + REMOTE_PROC_NODE_DEVICE_LOOKUP_SCSI_HOST_BY_WWN = 297 /* autogen autogen priority:high */ /* * Notice how the entries are grouped in sets of 10 ? diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index 91414d4..b8ca88b 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -1400,6 +1400,14 @@ struct remote_node_device_lookup_by_name_args { struct remote_node_device_lookup_by_name_ret { remote_nonnull_node_device dev; }; +struct remote_node_device_lookup_scsi_host_by_wwn_args { + remote_nonull_string wwnn; + remote_nonull_string wwpn; + unsigned int flags; +}; +struct remote_node_device_lookup_scsi_host_by_wwn_ret { + remote_nonnull_node_device dev; +}; struct remote_node_device_get_xml_desc_args { remote_nonnull_string name; u_int flags; @@ -2453,4 +2461,5 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_FSTRIM = 294, REMOTE_PROC_DOMAIN_SEND_PROCESS_SIGNAL = 295, REMOTE_PROC_DOMAIN_OPEN_CHANNEL = 296, + REMOTE_PROC_NODE_DEVICE_LOOKUP_SCSI_HOST_BY_WWN = 297, }; diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl index 899f4bc..c29133a 100755 --- a/src/rpc/gendispatch.pl +++ b/src/rpc/gendispatch.pl @@ -45,6 +45,8 @@ sub fixup_name { $name =~ s/Nmi$/NMI/; $name =~ s/Pm/PM/; $name =~ s/Fstrim$/FSTrim/; + $name =~ s/Scsi/SCSI/; + $name =~ s/Wwn$/WWN/; return $name; } @@ -402,7 +404,8 @@ elsif ($opt_b) { # node device is special, as it's identified by name if ($argtype =~ m/^remote_node_device_/ and !($argtype =~ m/^remote_node_device_lookup_by_name_/) and - !($argtype =~ m/^remote_node_device_create_xml_/)) { + !($argtype =~ m/^remote_node_device_create_xml_/) and + !($argtype =~ m/^remote_node_device_lookup_scsi_host_by_wwn_/)) { $has_node_device = 1; push(@vars_list, "virNodeDevicePtr dev = NULL"); push(@getters_list, -- 1.7.7.6

This just simply changes nodeDeviceLookupByWWN to be not static, and its name into nodeDeviceLookupSCSIHostByWWN. And use that for udev and HAL backends. --- src/node_device/node_device_driver.c | 13 ++++++++----- src/node_device/node_device_driver.h | 4 ++++ src/node_device/node_device_hal.c | 1 + src/node_device/node_device_udev.c | 1 + 4 files changed, 14 insertions(+), 5 deletions(-) diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index 522af99..73b824f 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -224,10 +224,11 @@ cleanup: } -static virNodeDevicePtr -nodeDeviceLookupByWWN(virConnectPtr conn, - const char *wwnn, - const char *wwpn) +virNodeDevicePtr +nodeDeviceLookupSCSIHostByWWN(virConnectPtr conn, + const char *wwnn, + const char *wwpn, + unsigned int flags) { unsigned int i; virDeviceMonitorStatePtr driver = conn->devMonPrivateData; @@ -236,6 +237,8 @@ nodeDeviceLookupByWWN(virConnectPtr conn, virNodeDeviceObjPtr obj = NULL; virNodeDevicePtr dev = NULL; + virCheckFlags(0, NULL); + nodeDeviceLock(driver); for (i = 0; i < devs->count; i++) { @@ -546,7 +549,7 @@ find_new_device(virConnectPtr conn, const char *wwnn, const char *wwpn) virFileWaitForDevices(); - dev = nodeDeviceLookupByWWN(conn, wwnn, wwpn); + dev = nodeDeviceLookupSCSIHostByWWN(conn, wwnn, wwpn, 0); if (dev != NULL) { break; diff --git a/src/node_device/node_device_driver.h b/src/node_device/node_device_driver.h index 4cec07c..718e444 100644 --- a/src/node_device/node_device_driver.h +++ b/src/node_device/node_device_driver.h @@ -77,6 +77,10 @@ int nodeListAllNodeDevices(virConnectPtr conn, virNodeDevicePtr **devices, unsigned int flags); virNodeDevicePtr nodeDeviceLookupByName(virConnectPtr conn, const char *name); +virNodeDevicePtr nodeDeviceLookupSCSIHostByWWN(virConnectPtr conn, + const char *wwnn, + const char *wwpn, + unsigned int flags); char *nodeDeviceGetXMLDesc(virNodeDevicePtr dev, unsigned int flags); char *nodeDeviceGetParent(virNodeDevicePtr dev); int nodeDeviceNumOfCaps(virNodeDevicePtr dev); diff --git a/src/node_device/node_device_hal.c b/src/node_device/node_device_hal.c index 610df8d..20714d3 100644 --- a/src/node_device/node_device_hal.c +++ b/src/node_device/node_device_hal.c @@ -765,6 +765,7 @@ static virDeviceMonitor halDeviceMonitor = { .listDevices = nodeListDevices, /* 0.5.0 */ .listAllNodeDevices = nodeListAllNodeDevices, /* 0.10.2 */ .deviceLookupByName = nodeDeviceLookupByName, /* 0.5.0 */ + .deviceLookupSCSIHostByWWN = nodeDeviceLookupSCSIHostByWWN, /* 1.0.2 */ .deviceGetXMLDesc = nodeDeviceGetXMLDesc, /* 0.5.0 */ .deviceGetParent = nodeDeviceGetParent, /* 0.5.0 */ .deviceNumOfCaps = nodeDeviceNumOfCaps, /* 0.5.0 */ diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index a9b30b2..1be8526 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -1754,6 +1754,7 @@ static virDeviceMonitor udevDeviceMonitor = { .listDevices = nodeListDevices, /* 0.7.3 */ .listAllNodeDevices = nodeListAllNodeDevices, /* 0.10.2 */ .deviceLookupByName = nodeDeviceLookupByName, /* 0.7.3 */ + .deviceLookupSCSIHostByWWN = nodeDeviceLookupSCSIHostByWWN, /* 1.0.2 */ .deviceGetXMLDesc = nodeDeviceGetXMLDesc, /* 0.7.3 */ .deviceGetParent = nodeDeviceGetParent, /* 0.7.3 */ .deviceNumOfCaps = nodeDeviceNumOfCaps, /* 0.7.3 */ -- 1.7.7.6

Only nodedev-destroy and nodedev-dumpxml can benifit from the new API, other commands like nodedev-detach only works for PCI devices, WWN makes no sense for them. --- tools/virsh-nodedev.c | 97 +++++++++++++++++++++++++++++++++++++------------ tools/virsh.pod | 15 +++++--- 2 files changed, 83 insertions(+), 29 deletions(-) diff --git a/tools/virsh-nodedev.c b/tools/virsh-nodedev.c index f9517cc..ddbf7ed 100644 --- a/tools/virsh-nodedev.c +++ b/tools/virsh-nodedev.c @@ -97,8 +97,9 @@ static const vshCmdInfo info_node_device_destroy[] = { }; static const vshCmdOptDef opts_node_device_destroy[] = { - {"name", VSH_OT_DATA, VSH_OFLAG_REQ, - N_("name of the device to be destroyed")}, + {"name", VSH_OT_ALIAS, 0, "device"}, + {"device", VSH_OT_DATA, VSH_OFLAG_REQ, + N_("device name or wwn pair in 'wwnn,wwpn' format")}, {NULL, 0, 0, NULL} }; @@ -106,21 +107,47 @@ static bool cmdNodeDeviceDestroy(vshControl *ctl, const vshCmd *cmd) { virNodeDevicePtr dev = NULL; - bool ret = true; - const char *name = NULL; + bool ret = false; + const char *device_value = NULL; + char **arr = NULL; + int narr; - if (vshCommandOptString(cmd, "name", &name) <= 0) + if (vshCommandOptString(cmd, "device", &device_value) <= 0) return false; - dev = virNodeDeviceLookupByName(ctl->conn, name); + if (strchr(device_value, ',')) { + narr = vshStringToArray(device_value, &arr); + if (narr != 2) { + vshError(ctl, _("Malformed device value '%s'"), device_value); + goto cleanup; + } + + if (!virValidateWWN(arr[0]) || !virValidateWWN(arr[1])) + goto cleanup; + + dev = virNodeDeviceLookupSCSIHostByWWN(ctl->conn, arr[0], arr[1], 0); + } else { + dev = virNodeDeviceLookupByName(ctl->conn, device_value); + } + + if (!dev) { + vshError(ctl, "%s '%s'", _("Could not find matching device"), device_value); + goto cleanup; + } if (virNodeDeviceDestroy(dev) == 0) { - vshPrint(ctl, _("Destroyed node device '%s'\n"), name); + vshPrint(ctl, _("Destroyed node device '%s'\n"), device_value); } else { - vshError(ctl, _("Failed to destroy node device '%s'"), name); - ret = false; + vshError(ctl, _("Failed to destroy node device '%s'"), device_value); + goto cleanup; } + ret = true; +cleanup: + if (arr) { + VIR_FREE(*arr); + VIR_FREE(arr); + } virNodeDeviceFree(dev); return ret; } @@ -459,34 +486,58 @@ static const vshCmdInfo info_node_device_dumpxml[] = { static const vshCmdOptDef opts_node_device_dumpxml[] = { - {"device", VSH_OT_DATA, VSH_OFLAG_REQ, N_("device key")}, + {"device", VSH_OT_DATA, VSH_OFLAG_REQ, + N_("device name or wwn pair in 'wwnn,wwpn' format")}, {NULL, 0, 0, NULL} }; static bool cmdNodeDeviceDumpXML(vshControl *ctl, const vshCmd *cmd) { - const char *name = NULL; - virNodeDevicePtr device; - char *xml; - - if (vshCommandOptString(cmd, "device", &name) <= 0) - return false; - if (!(device = virNodeDeviceLookupByName(ctl->conn, name))) { - vshError(ctl, "%s '%s'", _("Could not find matching device"), name); + virNodeDevicePtr device = NULL; + char *xml = NULL; + const char *device_value = NULL; + char **arr = NULL; + int narr; + bool ret = false; + + if (vshCommandOptString(cmd, "device", &device_value) <= 0) return false; + + if (strchr(device_value, ',')) { + narr = vshStringToArray(device_value, &arr); + if (narr != 2) { + vshError(ctl, _("Malformed device value '%s'"), device_value); + goto cleanup; + } + + if (!virValidateWWN(arr[0]) || !virValidateWWN(arr[1])) + goto cleanup; + + device = virNodeDeviceLookupSCSIHostByWWN(ctl->conn, arr[0], arr[1], 0); + } else { + device = virNodeDeviceLookupByName(ctl->conn, device_value); } - xml = virNodeDeviceGetXMLDesc(device, 0); - if (!xml) { - virNodeDeviceFree(device); - return false; + if (!device) { + vshError(ctl, "%s '%s'", _("Could not find matching device"), device_value); + goto cleanup; } + if (!(xml = virNodeDeviceGetXMLDesc(device, 0))) + goto cleanup; + vshPrint(ctl, "%s\n", xml); + ret = true; + +cleanup: + if (arr) { + VIR_FREE(*arr); + VIR_FREE(arr); + } VIR_FREE(xml); virNodeDeviceFree(device); - return true; + return ret; } /* diff --git a/tools/virsh.pod b/tools/virsh.pod index 3687a4d..a2da9ef 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1950,11 +1950,12 @@ host nodes are available for use, but this allows registration of host hardware that libvirt did not automatically detect. I<file> contains xml for a top-level <device> description of a node device. -=item B<nodedev-destroy> I<nodedev> +=item B<nodedev-destroy> I<device> -Destroy (stop) a device on the host. Note that this makes libvirt -quit managing a host device, and may even make that device unusable -by the rest of the physical host until a reboot. +Destroy (stop) a device on the host. I<device> can be either device +name or wwn pair in "wwnn,wwpn" format (only works for HBA). Note +that this makes libvirt quit managing a host device, and may even make +that device unusable by the rest of the physical host until a reboot. =item B<nodedev-detach> I<nodedev> @@ -1964,12 +1965,14 @@ B<nodedev-reattach>, and is done automatically for managed devices. For compatibility purposes, this command can also be spelled B<nodedev-dettach>. -=item B<nodedev-dumpxml> I<nodedev> +=item B<nodedev-dumpxml> I<device> Dump a <device> XML representation for the given node device, including such information as the device name, which bus owns the device, the vendor and product id, and any capabilities of the device usable by -libvirt (such as whether device reset is supported). +libvirt (such as whether device reset is supported). I<device> can +be either device name or wwn pair in "wwnn,wwpn" format (only works +for HBA). =item B<nodedev-list> I<cap> I<--tree> -- 1.7.7.6

On 2013年01月14日 11:09, Osier Yang wrote:
For easy reviewing, this is splitted from [1].
Ping, anyone can review this?
v2 - v3: * Validate the specified wwnn,wwpn pair before applying it to the new API in virsh-nodedev.c
v1 - v2: * Per Daniel's suggestion, change the API name from virNodeDeviceLookupByWWN into virNodeDeviceLookupSCSIHostByWWN.
Osier Yang (4): Introduce API virNodeDeviceLookupSCSIHostByWWN remote: Wire up the remote protocol nodedev: Implement virNodeDeviceLookupSCSIHostByWWN virsh: Use virNodeDeviceLookupSCSIHostByWWN
include/libvirt/libvirt.h.in | 5 ++ src/driver.h | 6 ++ src/libvirt.c | 46 ++++++++++++++++ src/libvirt_public.syms | 1 + src/node_device/node_device_driver.c | 13 +++-- src/node_device/node_device_driver.h | 4 ++ src/node_device/node_device_hal.c | 1 + src/node_device/node_device_udev.c | 1 + src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 13 ++++- src/remote_protocol-structs | 9 +++ src/rpc/gendispatch.pl | 5 ++- tools/virsh-nodedev.c | 97 ++++++++++++++++++++++++++-------- tools/virsh.pod | 15 +++-- 14 files changed, 181 insertions(+), 36 deletions(-)
[1] https://www.redhat.com/archives/libvir-list/2013-January/msg00328.html
Regards, Osier
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
participants (1)
-
Osier Yang