[libvirt] [PATCH 0/9 v2] Miscs of nodedev

This is the second part of [1]. Some of them are preparations for persistent vHBA support in storage pool, some are fixes, some are improvements. v1 - v2: * One more patch: 9/9 Osier Yang (9): nodedev: Remove the unused enum nodedev: Introduce two new flags for listAll API util: Add one helper virReadFCHost to read the value of fc_host entry nodedev: Use access instead of stat nodedev: Refactor the helpers nodedev: Dump max vports and vports in use for HBA's XML nodedev: Fix the improper logic when enumerating SRIOV VF nodedev: Abstract nodeDeviceVportCreateDelete as util function cleanup libvirt_private.syms docs/formatnode.html.in | 10 +- docs/schemas/nodedev.rng | 6 + include/libvirt/libvirt.h.in | 20 +- src/conf/node_device_conf.c | 41 +- src/conf/node_device_conf.h | 16 +- src/libvirt.c | 2 + src/libvirt_private.syms | 1160 +++++++++++++++-------------- src/node_device/node_device_driver.c | 110 +--- src/node_device/node_device_driver.h | 19 +- src/node_device/node_device_hal.c | 15 +- src/node_device/node_device_linux_sysfs.c | 245 ++---- src/node_device/node_device_udev.c | 14 +- src/util/virpci.c | 36 +- src/util/virutil.c | 224 ++++++ src/util/virutil.h | 19 + tools/virsh-nodedev.c | 6 + tools/virsh.pod | 7 +- 17 files changed, 1034 insertions(+), 916 deletions(-) Regards, Osier

Guess it was created for the fc_host and vports_ops capabilities purpose, but there is enum virNodeDevScsiHostCapFlags for them, and enum virNodeDevHBACapType is unused, and actually both VIR_ENUM_DECL and VIR_ENUM_IMPL use the wrong enum name "virNodeDevHBACap". --- src/conf/node_device_conf.c | 5 ----- src/conf/node_device_conf.h | 8 -------- 2 files changed, 0 insertions(+), 13 deletions(-) diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index 53b6af2..48e8190 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -56,11 +56,6 @@ VIR_ENUM_IMPL(virNodeDevNetCap, VIR_NODE_DEV_CAP_NET_LAST, "80203", "80211") -VIR_ENUM_IMPL(virNodeDevHBACap, VIR_NODE_DEV_CAP_HBA_LAST, - "fc_host", - "vport_ops") - - static int virNodeDevCapsDefParseString(const char *xpath, xmlXPathContextPtr ctxt, diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h index 12c36d8..36bf5ac 100644 --- a/src/conf/node_device_conf.h +++ b/src/conf/node_device_conf.h @@ -55,16 +55,8 @@ enum virNodeDevNetCapType { VIR_NODE_DEV_CAP_NET_LAST }; -enum virNodeDevHBACapType { - /* Keep in sync with VIR_ENUM_IMPL in node_device_conf.c */ - VIR_NODE_DEV_CAP_HBA_FC_HOST, /* fibre channel HBA */ - VIR_NODE_DEV_CAP_HBA_VPORT_OPS, /* capable of vport operations */ - VIR_NODE_DEV_CAP_HBA_LAST -}; - VIR_ENUM_DECL(virNodeDevCap) VIR_ENUM_DECL(virNodeDevNetCap) -VIR_ENUM_DECL(virNodeDevHBACap) enum virNodeDevStorageCapFlags { VIR_NODE_DEV_CAP_STORAGE_REMOVABLE = (1 << 0), -- 1.7.7.6

On 14.01.2013 15:34, Osier Yang wrote:
Guess it was created for the fc_host and vports_ops capabilities purpose, but there is enum virNodeDevScsiHostCapFlags for them, and enum virNodeDevHBACapType is unused, and actually both VIR_ENUM_DECL and VIR_ENUM_IMPL use the wrong enum name "virNodeDevHBACap". --- src/conf/node_device_conf.c | 5 ----- src/conf/node_device_conf.h | 8 -------- 2 files changed, 0 insertions(+), 13 deletions(-)
ACK Michal

VIR_CONNECT_LIST_NODE_DEVICES_CAP_FC_HOST to filter the FC HBA, and VIR_CONNECT_LIST_NODE_DEVICES_CAP_VPORTS to filter the FC HBA which supports vport. --- include/libvirt/libvirt.h.in | 20 +++++++++++--------- src/conf/node_device_conf.c | 29 ++++++++++++++++++++++++++--- src/conf/node_device_conf.h | 6 +++++- src/libvirt.c | 2 ++ tools/virsh-nodedev.c | 6 ++++++ tools/virsh.pod | 7 ++++--- 6 files changed, 54 insertions(+), 16 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 563ca27..6c5dfe6 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -3138,15 +3138,17 @@ int virNodeListDevices (virConnectPtr conn, * type. */ typedef enum { - VIR_CONNECT_LIST_NODE_DEVICES_CAP_SYSTEM = 1 << 0, /* System capability */ - VIR_CONNECT_LIST_NODE_DEVICES_CAP_PCI_DEV = 1 << 1, /* PCI device */ - VIR_CONNECT_LIST_NODE_DEVICES_CAP_USB_DEV = 1 << 2, /* USB device */ - VIR_CONNECT_LIST_NODE_DEVICES_CAP_USB_INTERFACE = 1 << 3, /* USB interface */ - VIR_CONNECT_LIST_NODE_DEVICES_CAP_NET = 1 << 4, /* Network device */ - VIR_CONNECT_LIST_NODE_DEVICES_CAP_SCSI_HOST = 1 << 5, /* SCSI Host Bus Adapter */ - VIR_CONNECT_LIST_NODE_DEVICES_CAP_SCSI_TARGET = 1 << 6, /* SCSI Target */ - VIR_CONNECT_LIST_NODE_DEVICES_CAP_SCSI = 1 << 7, /* SCSI device */ - VIR_CONNECT_LIST_NODE_DEVICES_CAP_STORAGE = 1 << 8, /* Storage device */ + VIR_CONNECT_LIST_NODE_DEVICES_CAP_SYSTEM = 1 << 0, /* System capability */ + VIR_CONNECT_LIST_NODE_DEVICES_CAP_PCI_DEV = 1 << 1, /* PCI device */ + VIR_CONNECT_LIST_NODE_DEVICES_CAP_USB_DEV = 1 << 2, /* USB device */ + VIR_CONNECT_LIST_NODE_DEVICES_CAP_USB_INTERFACE = 1 << 3, /* USB interface */ + VIR_CONNECT_LIST_NODE_DEVICES_CAP_NET = 1 << 4, /* Network device */ + VIR_CONNECT_LIST_NODE_DEVICES_CAP_SCSI_HOST = 1 << 5, /* SCSI Host Bus Adapter */ + VIR_CONNECT_LIST_NODE_DEVICES_CAP_SCSI_TARGET = 1 << 6, /* SCSI Target */ + VIR_CONNECT_LIST_NODE_DEVICES_CAP_SCSI = 1 << 7, /* SCSI device */ + VIR_CONNECT_LIST_NODE_DEVICES_CAP_STORAGE = 1 << 8, /* Storage device */ + VIR_CONNECT_LIST_NODE_DEVICES_CAP_FC_HOST = 1 << 9, /* FC Host Bus Adapter */ + VIR_CONNECT_LIST_NODE_DEVICES_CAP_VPORTS = 1 << 10, /* Capable of vport */ } virConnectListAllNodeDeviceFlags; int virConnectListAllNodeDevices (virConnectPtr conn, diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index 48e8190..819e6af 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -50,7 +50,9 @@ VIR_ENUM_IMPL(virNodeDevCap, VIR_NODE_DEV_CAP_LAST, "scsi_host", "scsi_target", "scsi", - "storage") + "storage", + "fc_host", + "vports") VIR_ENUM_IMPL(virNodeDevNetCap, VIR_NODE_DEV_CAP_NET_LAST, "80203", @@ -467,8 +469,10 @@ char *virNodeDeviceDefFormat(const virNodeDeviceDefPtr def) virBufferAddLit(&buf, " <capability type='hotpluggable' />\n"); break; + case VIR_NODE_DEV_CAP_FC_HOST: + case VIR_NODE_DEV_CAP_VPORTS: case VIR_NODE_DEV_CAP_LAST: - /* ignore special LAST value */ + default: break; } @@ -1409,7 +1413,10 @@ void virNodeDevCapsDefFree(virNodeDevCapsDefPtr caps) VIR_FREE(data->storage.serial); VIR_FREE(data->storage.media_label); break; + case VIR_NODE_DEV_CAP_FC_HOST: + case VIR_NODE_DEV_CAP_VPORTS: case VIR_NODE_DEV_CAP_LAST: + default: /* This case is here to shutup the compiler */ break; } @@ -1437,6 +1444,18 @@ virNodeDeviceCapMatch(virNodeDeviceObjPtr devobj, for (cap = devobj->def->caps; cap; cap = cap->next) { if (type == cap->type) return true; + + if (cap->type == VIR_NODE_DEV_CAP_SCSI_HOST) { + if (type == VIR_CONNECT_LIST_NODE_DEVICES_CAP_FC_HOST && + (cap->data.scsi_host.flags & + VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST)) + return true; + + if (type == VIR_CONNECT_LIST_NODE_DEVICES_CAP_VPORTS && + (cap->data.scsi_host.flags & + VIR_NODE_DEV_CAP_FLAG_HBA_VPORT_OPS)) + return true; + } } return false; @@ -1466,7 +1485,11 @@ virNodeDeviceMatch(virNodeDeviceObjPtr devobj, (MATCH(VIR_CONNECT_LIST_NODE_DEVICES_CAP_SCSI) && virNodeDeviceCapMatch(devobj, VIR_NODE_DEV_CAP_SCSI)) || (MATCH(VIR_CONNECT_LIST_NODE_DEVICES_CAP_STORAGE) && - virNodeDeviceCapMatch(devobj, VIR_NODE_DEV_CAP_STORAGE)))) + virNodeDeviceCapMatch(devobj, VIR_NODE_DEV_CAP_STORAGE)) || + (MATCH(VIR_CONNECT_LIST_NODE_DEVICES_CAP_FC_HOST) && + virNodeDeviceCapMatch(devobj, VIR_NODE_DEV_CAP_FC_HOST)) || + (MATCH(VIR_CONNECT_LIST_NODE_DEVICES_CAP_VPORTS) && + virNodeDeviceCapMatch(devobj, VIR_NODE_DEV_CAP_VPORTS)))) return false; } diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h index 36bf5ac..145d699 100644 --- a/src/conf/node_device_conf.h +++ b/src/conf/node_device_conf.h @@ -45,6 +45,8 @@ enum virNodeDevCapType { VIR_NODE_DEV_CAP_SCSI_TARGET, /* SCSI Target */ VIR_NODE_DEV_CAP_SCSI, /* SCSI device */ VIR_NODE_DEV_CAP_STORAGE, /* Storage device */ + VIR_NODE_DEV_CAP_FC_HOST, /* FC Host Bus Adapter */ + VIR_NODE_DEV_CAP_VPORTS, /* HBA which is capable of vports */ VIR_NODE_DEV_CAP_LAST }; @@ -262,7 +264,9 @@ void virNodeDeviceObjUnlock(virNodeDeviceObjPtr obj); VIR_CONNECT_LIST_NODE_DEVICES_CAP_SCSI_HOST | \ VIR_CONNECT_LIST_NODE_DEVICES_CAP_SCSI_TARGET | \ VIR_CONNECT_LIST_NODE_DEVICES_CAP_SCSI | \ - VIR_CONNECT_LIST_NODE_DEVICES_CAP_STORAGE) + VIR_CONNECT_LIST_NODE_DEVICES_CAP_STORAGE | \ + VIR_CONNECT_LIST_NODE_DEVICES_CAP_FC_HOST | \ + VIR_CONNECT_LIST_NODE_DEVICES_CAP_VPORTS) int virNodeDeviceList(virConnectPtr conn, virNodeDeviceObjList devobjs, diff --git a/src/libvirt.c b/src/libvirt.c index 0a2ade7..f0c452e 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -14202,6 +14202,8 @@ error: * VIR_CONNECT_LIST_NODE_DEVICES_CAP_SCSI_TARGET * VIR_CONNECT_LIST_NODE_DEVICES_CAP_SCSI * VIR_CONNECT_LIST_NODE_DEVICES_CAP_STORAGE + * VIR_CONNECT_LIST_NODE_DEVICES_CAP_FC_HOST + * VIR_CONNECT_LIST_NODE_DEVICES_CAP_VPORTS * * Returns the number of node devices found or -1 and sets @devices to NULL in * case of error. On success, the array stored into @devices is guaranteed to diff --git a/tools/virsh-nodedev.c b/tools/virsh-nodedev.c index ddbf7ed..ef4a1a4 100644 --- a/tools/virsh-nodedev.c +++ b/tools/virsh-nodedev.c @@ -420,6 +420,12 @@ cmdNodeListDevices(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) case VIR_NODE_DEV_CAP_STORAGE: flags |= VIR_CONNECT_LIST_NODE_DEVICES_CAP_STORAGE; break; + case VIR_NODE_DEV_CAP_FC_HOST: + flags |= VIR_CONNECT_LIST_NODE_DEVICES_CAP_FC_HOST; + break; + case VIR_NODE_DEV_CAP_VPORTS: + flags |= VIR_CONNECT_LIST_NODE_DEVICES_CAP_VPORTS; + break; default: break; } diff --git a/tools/virsh.pod b/tools/virsh.pod index a2da9ef..368618c 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1980,9 +1980,10 @@ List all of the devices available on the node that are known by libvirt. I<cap> is used to filter the list by capability types, the types must be separated by comma, e.g. --cap pci,scsi, valid capability types include 'system', 'pci', 'usb_device', 'usb', 'net', 'scsi_host', 'scsi_target', -'scsi', 'storage'. If I<--tree> is used, the output is formatted in a tree -representing parents of each node. I<cap> and I<--tree> are mutually -exclusive. +'scsi', 'storage', 'fc_host', 'vports'. If I<--tree> is used, the output +is formatted in a tree representing parents of each node. I<cap> and +I<--tree> are mutually exclusive. + =item B<nodedev-reattach> I<nodedev> Declare that I<nodedev> is no longer in use by any guests, and that -- 1.7.7.6

On 14.01.2013 15:34, Osier Yang wrote:
VIR_CONNECT_LIST_NODE_DEVICES_CAP_FC_HOST to filter the FC HBA, and VIR_CONNECT_LIST_NODE_DEVICES_CAP_VPORTS to filter the FC HBA which supports vport. --- include/libvirt/libvirt.h.in | 20 +++++++++++--------- src/conf/node_device_conf.c | 29 ++++++++++++++++++++++++++--- src/conf/node_device_conf.h | 6 +++++- src/libvirt.c | 2 ++ tools/virsh-nodedev.c | 6 ++++++ tools/virsh.pod | 7 ++++--- 6 files changed, 54 insertions(+), 16 deletions(-)
ACK Michal

"open_wwn_file" in node_device_linux_sysfs.c is redundant, on one hand it duplicates work of virFileReadAll, on the other hand, it's waste to use a function for it, as there is no other users of it. So I don't see why the file opening work cannot be done in "read_wwn_linux". "read_wwn_linux" can be abstracted as an util function. As what all it does is to read the sysfs entry. So this patch removes "open_wwn_file", and abstract "read_wwn_linux" as an util function "virReadFCHost" (a more general name, because after changes, it can read each of the fc_host entry now). * src/util/virutil.h: (Declare virReadFCHost) * src/util/virutil.c: (Implement virReadFCHost) * src/node_device/node_device_linux_sysfs.c: (Remove open_wwn_file, and read_wwn_linux) src/node_device/node_device_driver.h: (Remove the declaration of read_wwn_linux, and the related macros) src/libvirt_private.syms: (Export virReadFCHost) --- src/libvirt_private.syms | 1 + src/node_device/node_device_driver.h | 4 - src/node_device/node_device_linux_sysfs.c | 92 ++++------------------------- src/util/virutil.c | 67 +++++++++++++++++++++ src/util/virutil.h | 5 ++ 5 files changed, 85 insertions(+), 84 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 7be58ee..9e84b40 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1299,6 +1299,7 @@ virIsDevMapperDevice; virParseNumber; virParseVersionString; virPipeReadUntilEOF; +virReadFCHost; virScaleInteger; virSetBlocking; virSetCloseExec; diff --git a/src/node_device/node_device_driver.h b/src/node_device/node_device_driver.h index 718e444..a153b18 100644 --- a/src/node_device/node_device_driver.h +++ b/src/node_device/node_device_driver.h @@ -59,14 +59,10 @@ int check_fc_host_linux(union _virNodeDevCapData *d); # define check_vport_capable(d) check_vport_capable_linux(d) int check_vport_capable_linux(union _virNodeDevCapData *d); -# define read_wwn(host, file, wwn) read_wwn_linux(host, file, wwn) -int read_wwn_linux(int host, const char *file, char **wwn); - # else /* __linux__ */ # define check_fc_host(d) (-1) # define check_vport_capable(d) (-1) -# define read_wwn(host, file, wwn) # endif /* __linux__ */ diff --git a/src/node_device/node_device_linux_sysfs.c b/src/node_device/node_device_linux_sysfs.c index 9c305d3..742a0bc 100644 --- a/src/node_device/node_device_linux_sysfs.c +++ b/src/node_device/node_device_linux_sysfs.c @@ -37,77 +37,6 @@ #ifdef __linux__ -static int open_wwn_file(const char *prefix, - int host, - const char *file, - int *fd) -{ - int retval = 0; - char *wwn_path = NULL; - - if (virAsprintf(&wwn_path, "%s/host%d/%s", prefix, host, file) < 0) { - virReportOOMError(); - retval = -1; - goto out; - } - - /* fd will be closed by caller */ - if ((*fd = open(wwn_path, O_RDONLY)) != -1) { - VIR_DEBUG("Opened WWN path '%s' for reading", - wwn_path); - } else { - VIR_ERROR(_("Failed to open WWN path '%s' for reading"), - wwn_path); - } - -out: - VIR_FREE(wwn_path); - return retval; -} - - -int read_wwn_linux(int host, const char *file, char **wwn) -{ - char *p = NULL; - int fd = -1, retval = 0; - char buf[65] = ""; - - if (open_wwn_file(LINUX_SYSFS_FC_HOST_PREFIX, host, file, &fd) < 0) { - goto out; - } - - if (saferead(fd, buf, sizeof(buf) - 1) < 0) { - retval = -1; - VIR_DEBUG("Failed to read WWN for host%d '%s'", - host, file); - goto out; - } - - p = strstr(buf, "0x"); - if (p != NULL) { - p += strlen("0x"); - } else { - p = buf; - } - - *wwn = strndup(p, sizeof(buf)); - if (*wwn == NULL) { - virReportOOMError(); - retval = -1; - goto out; - } - - p = strchr(*wwn, '\n'); - if (p != NULL) { - *p = '\0'; - } - -out: - VIR_FORCE_CLOSE(fd); - return retval; -} - - int check_fc_host_linux(union _virNodeDevCapData *d) { char *sysfs_path = NULL; @@ -131,26 +60,29 @@ int check_fc_host_linux(union _virNodeDevCapData *d) d->scsi_host.flags |= VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST; - if (read_wwn(d->scsi_host.host, - "port_name", - &d->scsi_host.wwpn) == -1) { + if (virReadFCHost(NULL, + d->scsi_host.host, + "port_name", + &d->scsi_host.wwpn) == -1) { VIR_ERROR(_("Failed to read WWPN for host%d"), d->scsi_host.host); retval = -1; goto out; } - if (read_wwn(d->scsi_host.host, - "node_name", - &d->scsi_host.wwnn) == -1) { + if (virReadFCHost(NULL, + d->scsi_host.host, + "node_name", + &d->scsi_host.wwnn) == -1) { VIR_ERROR(_("Failed to read WWNN for host%d"), d->scsi_host.host); retval = -1; } - if (read_wwn(d->scsi_host.host, - "fabric_name", - &d->scsi_host.fabric_wwn) == -1) { + if (virReadFCHost(NULL, + d->scsi_host.host, + "fabric_name", + &d->scsi_host.fabric_wwn) == -1) { VIR_ERROR(_("Failed to read fabric WWN for host%d"), d->scsi_host.host); retval = -1; diff --git a/src/util/virutil.c b/src/util/virutil.c index 7c54bea..9442da3 100644 --- a/src/util/virutil.c +++ b/src/util/virutil.c @@ -3260,3 +3260,70 @@ cleanup: VIR_FREE(buf); return ret; } + +#ifdef __linux__ +# define SYSFS_FC_HOST_PATH "/sys/class/fc_host/" + +/* virReadFCHost: + * @sysfs_prefix: "fc_host" sysfs path, defaults to SYSFS_FC_HOST_PATH + * @host: Host number, E.g. 5 of "fc_host/host5" + * @entry: Name of the sysfs entry to read + * @result: Return the entry value as string + * + * Read the value of sysfs "fc_host" entry. + * + * Returns 0 on success, and @result is filled with the entry value. + * as string, Otherwise returns -1. Caller must free @result after + * use. + */ +int +virReadFCHost(const char *sysfs_prefix, + int host, + const char *entry, + char **result) +{ + char *sysfs_path = NULL; + char *p = NULL; + int ret = -1; + char *buf = NULL; + + if (virAsprintf(&sysfs_path, "%s/host%d/%s", + sysfs_prefix ? sysfs_prefix : SYSFS_FC_HOST_PATH, + host, entry) < 0) { + virReportOOMError(); + goto cleanup; + } + + if (virFileReadAll(sysfs_path, 1024, &buf) < 0) + goto cleanup; + + if ((p = strchr(buf, '\n'))) + *p = '\0'; + + if ((p = strstr(buf, "0x"))) + p += strlen("0x"); + else + p = buf; + + if (!(*result = strndup(p, sizeof(buf)))) { + virReportOOMError(); + goto cleanup; + } + + ret = 0; +cleanup: + VIR_FREE(sysfs_path); + VIR_FREE(buf); + return ret; +} +#else +int +virReadFCHost(const char *sysfs_prefix ATTRIBUTE_UNUSED, + int host ATTRIBUTE_UNUSED, + const char *entry ATTRIBUTE_UNUSED, + char **result ATTRIBUTE_UNUSED) +{ + virReportSystemError(ENOSYS, "%s", _("Not supported on this platform")); + return -1; +} +#endif /* __linux__ */ diff --git a/src/util/virutil.h b/src/util/virutil.h index 5a08c81..373c48c 100644 --- a/src/util/virutil.h +++ b/src/util/virutil.h @@ -293,5 +293,10 @@ int virGetDeviceUnprivSGIO(const char *path, int *unpriv_sgio); char * virGetUnprivSGIOSysfsPath(const char *path, const char *sysfs_dir); +int virReadFCHost(const char *sysfs_prefix, + int host, + const char *entry, + char **result) + ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4); #endif /* __VIR_UTIL_H__ */ -- 1.7.7.6

On 14.01.2013 15:34, Osier Yang wrote:
"open_wwn_file" in node_device_linux_sysfs.c is redundant, on one hand it duplicates work of virFileReadAll, on the other hand, it's waste to use a function for it, as there is no other users of it. So I don't see why the file opening work cannot be done in "read_wwn_linux".
"read_wwn_linux" can be abstracted as an util function. As what all it does is to read the sysfs entry.
So this patch removes "open_wwn_file", and abstract "read_wwn_linux" as an util function "virReadFCHost" (a more general name, because after changes, it can read each of the fc_host entry now).
* src/util/virutil.h: (Declare virReadFCHost) * src/util/virutil.c: (Implement virReadFCHost) * src/node_device/node_device_linux_sysfs.c: (Remove open_wwn_file, and read_wwn_linux) src/node_device/node_device_driver.h: (Remove the declaration of read_wwn_linux, and the related macros) src/libvirt_private.syms: (Export virReadFCHost) --- src/libvirt_private.syms | 1 + src/node_device/node_device_driver.h | 4 - src/node_device/node_device_linux_sysfs.c | 92 ++++------------------------- src/util/virutil.c | 67 +++++++++++++++++++++ src/util/virutil.h | 5 ++ 5 files changed, 85 insertions(+), 84 deletions(-)
ACK Michal

The use of 'stat' in nodeDeviceVportCreateDelete is only to check if the file exists or not, it's a bit overkill, and safe to replace with the wrapper of access(2) (virFileExists). --- src/node_device/node_device_driver.c | 6 ++---- 1 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index 73b824f..51e8115 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -28,7 +28,6 @@ #include <errno.h> #include <fcntl.h> #include <time.h> -#include <sys/stat.h> #include "virerror.h" #include "datatypes.h" @@ -422,7 +421,6 @@ nodeDeviceVportCreateDelete(const int parent_host, int retval = 0; char *operation_path = NULL, *vport_name = NULL; const char *operation_file = NULL; - struct stat st; switch (operation) { case VPORT_CREATE: @@ -450,7 +448,7 @@ nodeDeviceVportCreateDelete(const int parent_host, goto cleanup; } - if (stat(operation_path, &st) != 0) { + if (!virFileExists(operation_path)) { VIR_FREE(operation_path); if (virAsprintf(&operation_path, "%shost%d%s", @@ -462,7 +460,7 @@ nodeDeviceVportCreateDelete(const int parent_host, goto cleanup; } - if (stat(operation_path, &st) != 0) { + if (!virFileExists(operation_path)) { VIR_ERROR(_("No vport operation path found for host%d"), parent_host); retval = -1; -- 1.7.7.6

On 14.01.2013 15:34, Osier Yang wrote:
The use of 'stat' in nodeDeviceVportCreateDelete is only to check if the file exists or not, it's a bit overkill, and safe to replace with the wrapper of access(2) (virFileExists). --- src/node_device/node_device_driver.c | 6 ++---- 1 files changed, 2 insertions(+), 4 deletions(-)
ACK Michal

This adds two util functions (virIsCapableFCHost and virIsCapableVport), and rename helper check_fc_host_linux as detect_scsi_host_caps, check_capable_vport_linux is removed, as it's abstracted to the util function virIsCapableVport. detect_scsi_host_caps nows detect both the fc_host and vport_ops capabilities. "stat(2)" is replaced with "access(2)" for saving. * src/util/virutil.h: - Declare virIsCapableFCHost and virIsCapableVport * src/util/virutil.c: - Implement virIsCapableFCHost and virIsCapableVport * src/node_device/node_device_linux_sysfs.c: - Remove check_capable_vport_linux - Rename check_fc_host_linux as detect_scsi_host_caps, and refactor it a bit to detect both fc_host and vport_os capabilities * src/node_device/node_device_driver.h: - Change/remove the related declarations * src/node_device/node_device_udev.c: (Use detect_scsi_host_caps) * src/node_device/node_device_hal.c: (Likewise) * src/node_device/node_device_driver.c (Likewise) --- src/libvirt_private.syms | 2 + src/node_device/node_device_driver.c | 7 +- src/node_device/node_device_driver.h | 10 +-- src/node_device/node_device_hal.c | 4 +- src/node_device/node_device_linux_sysfs.c | 135 ++++++++--------------------- src/node_device/node_device_udev.c | 3 +- src/util/virutil.c | 73 ++++++++++++++++ src/util/virutil.h | 3 + 8 files changed, 123 insertions(+), 114 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 9e84b40..2945917 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1295,6 +1295,8 @@ virGetUserName; virGetUserRuntimeDirectory; virHexToBin; virIndexToDiskName; +virIsCapableFCHost; +virIsCapableVport; virIsDevMapperDevice; virParseNumber; virParseVersionString; diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index 51e8115..050ea62 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -48,7 +48,7 @@ static int update_caps(virNodeDeviceObjPtr dev) while (cap) { /* The only caps that currently need updating are FC related. */ if (cap->type == VIR_NODE_DEV_CAP_SCSI_HOST) { - check_fc_host(&dev->def->caps->data); + detect_scsi_host_caps(&dev->def->caps->data); } cap = cap->next; } @@ -241,18 +241,15 @@ nodeDeviceLookupSCSIHostByWWN(virConnectPtr conn, nodeDeviceLock(driver); for (i = 0; i < devs->count; i++) { - obj = devs->objs[i]; virNodeDeviceObjLock(obj); cap = obj->def->caps; while (cap) { - if (cap->type == VIR_NODE_DEV_CAP_SCSI_HOST) { - check_fc_host(&cap->data); + detect_scsi_host_caps(&cap->data); if (cap->data.scsi_host.flags & VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST) { - if (STREQ(cap->data.scsi_host.wwnn, wwnn) && STREQ(cap->data.scsi_host.wwpn, wwpn)) { dev = virGetNodeDevice(conn, obj->def->name); diff --git a/src/node_device/node_device_driver.h b/src/node_device/node_device_driver.h index a153b18..7775a59 100644 --- a/src/node_device/node_device_driver.h +++ b/src/node_device/node_device_driver.h @@ -53,16 +53,12 @@ int nodedevRegister(void); # ifdef __linux__ -# define check_fc_host(d) check_fc_host_linux(d) -int check_fc_host_linux(union _virNodeDevCapData *d); - -# define check_vport_capable(d) check_vport_capable_linux(d) -int check_vport_capable_linux(union _virNodeDevCapData *d); +# define detect_scsi_host_caps(d) detect_scsi_host_caps_linux(d) +int detect_scsi_host_caps_linux(union _virNodeDevCapData *d); # else /* __linux__ */ -# define check_fc_host(d) (-1) -# define check_vport_capable(d) (-1) +# define detect_scsi_host_caps(d) (-1) # endif /* __linux__ */ diff --git a/src/node_device/node_device_hal.c b/src/node_device/node_device_hal.c index 20714d3..1afa21c 100644 --- a/src/node_device/node_device_hal.c +++ b/src/node_device/node_device_hal.c @@ -234,14 +234,12 @@ static int gather_scsi_host_cap(LibHalContext *ctx, const char *udi, (void)get_int_prop(ctx, udi, "scsi_host.host", (int *)&d->scsi_host.host); - retval = check_fc_host(d); + retval = detect_scsi_host_caps(d); if (retval == -1) { goto out; } - retval = check_vport_capable(d); - out: return retval; } diff --git a/src/node_device/node_device_linux_sysfs.c b/src/node_device/node_device_linux_sysfs.c index 742a0bc..054afea 100644 --- a/src/node_device/node_device_linux_sysfs.c +++ b/src/node_device/node_device_linux_sysfs.c @@ -1,8 +1,8 @@ /* - * node_device_hal_linuc.c: Linux specific code to gather device data + * node_device_linux_sysfs.c: Linux specific code to gather device data * not available through HAL. * - * Copyright (C) 2009-2011 Red Hat, Inc. + * Copyright (C) 2009-2013 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -37,112 +37,53 @@ #ifdef __linux__ -int check_fc_host_linux(union _virNodeDevCapData *d) +int +detect_scsi_host_caps_linux(union _virNodeDevCapData *d) { - char *sysfs_path = NULL; - int retval = 0; - struct stat st; + int ret = -1; VIR_DEBUG("Checking if host%d is an FC HBA", d->scsi_host.host); - if (virAsprintf(&sysfs_path, "%shost%d", - LINUX_SYSFS_FC_HOST_PREFIX, - d->scsi_host.host) < 0) { - virReportOOMError(); - retval = -1; - goto out; + if (virIsCapableFCHost(NULL, d->scsi_host.host)) { + d->scsi_host.flags |= VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST; + + if (virReadFCHost(NULL, + d->scsi_host.host, + "port_name", + &d->scsi_host.wwpn) == -1) { + VIR_ERROR(_("Failed to read WWPN for host%d"), d->scsi_host.host); + goto cleanup; + } + + if (virReadFCHost(NULL, + d->scsi_host.host, + "node_name", + &d->scsi_host.wwnn) == -1) { + VIR_ERROR(_("Failed to read WWNN for host%d"), d->scsi_host.host); + goto cleanup; + } + + if (virReadFCHost(NULL, + d->scsi_host.host, + "fabric_name", + &d->scsi_host.fabric_wwn) == -1) { + VIR_ERROR(_("Failed to read fabric WWN for host%d"), + d->scsi_host.host); + goto cleanup; + } } - if (stat(sysfs_path, &st) != 0) { - /* Not an FC HBA; not an error, either. */ - goto out; - } - - d->scsi_host.flags |= VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST; - - if (virReadFCHost(NULL, - d->scsi_host.host, - "port_name", - &d->scsi_host.wwpn) == -1) { - VIR_ERROR(_("Failed to read WWPN for host%d"), - d->scsi_host.host); - retval = -1; - goto out; - } - - if (virReadFCHost(NULL, - d->scsi_host.host, - "node_name", - &d->scsi_host.wwnn) == -1) { - VIR_ERROR(_("Failed to read WWNN for host%d"), - d->scsi_host.host); - retval = -1; - } - - if (virReadFCHost(NULL, - d->scsi_host.host, - "fabric_name", - &d->scsi_host.fabric_wwn) == -1) { - VIR_ERROR(_("Failed to read fabric WWN for host%d"), - d->scsi_host.host); - retval = -1; - goto out; - } + if (virIsCapableVport(NULL, d->scsi_host.host) == 0) + d->scsi_host.flags |= VIR_NODE_DEV_CAP_FLAG_HBA_VPORT_OPS; -out: - if (retval == -1) { + ret = 0; +cleanup: + if (ret == -1) { VIR_FREE(d->scsi_host.wwnn); VIR_FREE(d->scsi_host.wwpn); VIR_FREE(d->scsi_host.fabric_wwn); } - VIR_FREE(sysfs_path); - return retval; -} - - -int check_vport_capable_linux(union _virNodeDevCapData *d) -{ - char *sysfs_path = NULL; - struct stat st; - int retval = 0; - - if (virAsprintf(&sysfs_path, - "%shost%d%s", - LINUX_SYSFS_FC_HOST_PREFIX, - d->scsi_host.host, - LINUX_SYSFS_VPORT_CREATE_POSTFIX) < 0) { - virReportOOMError(); - retval = -1; - goto out; - } - - if (stat(sysfs_path, &st) == 0) { - d->scsi_host.flags |= VIR_NODE_DEV_CAP_FLAG_HBA_VPORT_OPS; - goto out; - } - - VIR_FREE(sysfs_path); - if (virAsprintf(&sysfs_path, - "%shost%d%s", - LINUX_SYSFS_SCSI_HOST_PREFIX, - d->scsi_host.host, - LINUX_SYSFS_VPORT_CREATE_POSTFIX) < 0) { - virReportOOMError(); - retval = -1; - goto out; - } - - if (stat(sysfs_path, &st) == 0) { - d->scsi_host.flags |= VIR_NODE_DEV_CAP_FLAG_HBA_VPORT_OPS; - } else { - /* Not a vport capable HBA; not an error, either. */ - VIR_DEBUG("No vport operation path found for host%d", - d->scsi_host.host); - } - -out: - VIR_FREE(sysfs_path); - return retval; + return ret; } #endif /* __linux__ */ diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index 1be8526..87f1000 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -663,8 +663,7 @@ static int udevProcessSCSIHost(struct udev_device *device ATTRIBUTE_UNUSED, goto out; } - check_fc_host(&def->caps->data); - check_vport_capable(&def->caps->data); + detect_scsi_host_caps(&def->caps->data); if (udevGenerateDeviceName(device, def, NULL) != 0) { goto out; diff --git a/src/util/virutil.c b/src/util/virutil.c index 9442da3..8b74afb 100644 --- a/src/util/virutil.c +++ b/src/util/virutil.c @@ -3263,6 +3263,7 @@ cleanup: #ifdef __linux__ # define SYSFS_FC_HOST_PATH "/sys/class/fc_host/" +# define SYSFS_SCSI_HOST_PATH "/sys/class/scsi_host/" /* virReadFCHost: * @sysfs_prefix: "fc_host" sysfs path, defaults to SYSFS_FC_HOST_PATH @@ -3316,6 +3317,63 @@ cleanup: VIR_FREE(buf); return ret; } + +int +virIsCapableFCHost(const char *sysfs_prefix, + int host) +{ + char *sysfs_path = NULL; + int ret = -1; + + if (virAsprintf(&sysfs_path, "%shost%d", + sysfs_prefix ? sysfs_prefix : SYSFS_FC_HOST_PATH, + host) < 0) { + virReportOOMError(); + return -1; + } + + if (access(sysfs_path, F_OK) == 0) + ret = 0; + + VIR_FREE(sysfs_path); + return ret; +} + +int +virIsCapableVport(const char *sysfs_prefix, + int host) +{ + char *scsi_host_path = NULL; + char *fc_host_path = NULL; + int ret = -1; + + if (virAsprintf(&fc_host_path, + "%shost%d%s", + sysfs_prefix ? sysfs_prefix : SYSFS_FC_HOST_PATH, + host, + "vport_create") < 0) { + virReportOOMError(); + return -1; + } + + if (virAsprintf(&scsi_host_path, + "%shost%d%s", + sysfs_prefix ? sysfs_prefix : SYSFS_SCSI_HOST_PATH, + host, + "vport_create") < 0) { + virReportOOMError(); + goto cleanup; + } + + if ((access(fc_host_path, F_OK) == 0) || + (access(scsi_host_path, F_OK) == 0)) + ret = 0; + +cleanup: + VIR_FREE(fc_host_path); + VIR_FREE(scsi_host_path); + return ret; +} #else int virReadFCHost(const char *sysfs_prefix ATTRIBUTE_UNUSED, @@ -3326,4 +3384,19 @@ virReadFCHost(const char *sysfs_prefix ATTRIBUTE_UNUSED, virReportSystemError(ENOSYS, "%s", _("Not supported on this platform")); return -1; } + +int +virIsCapableFCHost(int host ATTRIBUTE_UNUSED) +{ + virReportSystemError(ENOSYS, "%s", _("Not supported on this platform")); + return -1; +} + +int +virIsCapbleVport(const char *sysfs_prefix ATTRIBUTE_UNUSED, + int host ATTRIBUTE_UNUSED) +{ + virReportSystemError(ENOSYS, "%s", _("Not supported on this platform")); + return -1; +} #endif /* __linux__ */ diff --git a/src/util/virutil.h b/src/util/virutil.h index 373c48c..d87aa92 100644 --- a/src/util/virutil.h +++ b/src/util/virutil.h @@ -299,4 +299,7 @@ int virReadFCHost(const char *sysfs_prefix, char **result) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4); +int virIsCapableFCHost(const char *sysfs_prefix, int host); +int virIsCapableVport(const char *sysfs_prefix, int host); + #endif /* __VIR_UTIL_H__ */ -- 1.7.7.6

On 14.01.2013 15:34, Osier Yang wrote:
This adds two util functions (virIsCapableFCHost and virIsCapableVport), and rename helper check_fc_host_linux as detect_scsi_host_caps, check_capable_vport_linux is removed, as it's abstracted to the util function virIsCapableVport. detect_scsi_host_caps nows detect both the fc_host and vport_ops capabilities. "stat(2)" is replaced with "access(2)" for saving.
* src/util/virutil.h: - Declare virIsCapableFCHost and virIsCapableVport * src/util/virutil.c: - Implement virIsCapableFCHost and virIsCapableVport * src/node_device/node_device_linux_sysfs.c: - Remove check_capable_vport_linux - Rename check_fc_host_linux as detect_scsi_host_caps, and refactor it a bit to detect both fc_host and vport_os capabilities * src/node_device/node_device_driver.h: - Change/remove the related declarations * src/node_device/node_device_udev.c: (Use detect_scsi_host_caps) * src/node_device/node_device_hal.c: (Likewise) * src/node_device/node_device_driver.c (Likewise) --- src/libvirt_private.syms | 2 + src/node_device/node_device_driver.c | 7 +- src/node_device/node_device_driver.h | 10 +-- src/node_device/node_device_hal.c | 4 +- src/node_device/node_device_linux_sysfs.c | 135 ++++++++--------------------- src/node_device/node_device_udev.c | 3 +- src/util/virutil.c | 73 ++++++++++++++++ src/util/virutil.h | 3 + 8 files changed, 123 insertions(+), 114 deletions(-)
diff --git a/src/node_device/node_device_linux_sysfs.c b/src/node_device/node_device_linux_sysfs.c index 742a0bc..054afea 100644 --- a/src/node_device/node_device_linux_sysfs.c +++ b/src/node_device/node_device_linux_sysfs.c @@ -1,8 +1,8 @@ /* - * node_device_hal_linuc.c: Linux specific code to gather device data + * node_device_linux_sysfs.c: Linux specific code to gather device data * not available through HAL. * - * Copyright (C) 2009-2011 Red Hat, Inc. + * Copyright (C) 2009-2013 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -37,112 +37,53 @@
#ifdef __linux__
-int check_fc_host_linux(union _virNodeDevCapData *d) +int +detect_scsi_host_caps_linux(union _virNodeDevCapData *d) { - char *sysfs_path = NULL; - int retval = 0; - struct stat st; + int ret = -1;
VIR_DEBUG("Checking if host%d is an FC HBA", d->scsi_host.host);
- if (virAsprintf(&sysfs_path, "%shost%d", - LINUX_SYSFS_FC_HOST_PREFIX, - d->scsi_host.host) < 0) { - virReportOOMError(); - retval = -1; - goto out; + if (virIsCapableFCHost(NULL, d->scsi_host.host)) { + d->scsi_host.flags |= VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST; + + if (virReadFCHost(NULL, + d->scsi_host.host, + "port_name", + &d->scsi_host.wwpn) == -1) { + VIR_ERROR(_("Failed to read WWPN for host%d"), d->scsi_host.host); + goto cleanup; + } + + if (virReadFCHost(NULL, + d->scsi_host.host, + "node_name", + &d->scsi_host.wwnn) == -1) { + VIR_ERROR(_("Failed to read WWNN for host%d"), d->scsi_host.host); + goto cleanup; + } + + if (virReadFCHost(NULL, + d->scsi_host.host, + "fabric_name", + &d->scsi_host.fabric_wwn) == -1) { + VIR_ERROR(_("Failed to read fabric WWN for host%d"), + d->scsi_host.host); + goto cleanup; + } }
- if (stat(sysfs_path, &st) != 0) { - /* Not an FC HBA; not an error, either. */ - goto out; - } - - d->scsi_host.flags |= VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST; - - if (virReadFCHost(NULL, - d->scsi_host.host, - "port_name", - &d->scsi_host.wwpn) == -1) { - VIR_ERROR(_("Failed to read WWPN for host%d"), - d->scsi_host.host); - retval = -1; - goto out; - } - - if (virReadFCHost(NULL, - d->scsi_host.host, - "node_name", - &d->scsi_host.wwnn) == -1) { - VIR_ERROR(_("Failed to read WWNN for host%d"), - d->scsi_host.host); - retval = -1; - } - - if (virReadFCHost(NULL, - d->scsi_host.host, - "fabric_name", - &d->scsi_host.fabric_wwn) == -1) { - VIR_ERROR(_("Failed to read fabric WWN for host%d"), - d->scsi_host.host); - retval = -1; - goto out; - } + if (virIsCapableVport(NULL, d->scsi_host.host) == 0) + d->scsi_host.flags |= VIR_NODE_DEV_CAP_FLAG_HBA_VPORT_OPS;
-out: - if (retval == -1) { + ret = 0; +cleanup: + if (ret == -1) {
I'd write this as 'if (ret < 0) {' so we don't have to change this line if we ever invent a new error code for this function. Moreover, it's common practice over libvirt code. ACK if you fix this. Michal

On 2013年01月15日 03:42, Michal Privoznik wrote:
On 14.01.2013 15:34, Osier Yang wrote:
This adds two util functions (virIsCapableFCHost and virIsCapableVport), and rename helper check_fc_host_linux as detect_scsi_host_caps, check_capable_vport_linux is removed, as it's abstracted to the util function virIsCapableVport. detect_scsi_host_caps nows detect both the fc_host and vport_ops capabilities. "stat(2)" is replaced with "access(2)" for saving.
* src/util/virutil.h: - Declare virIsCapableFCHost and virIsCapableVport * src/util/virutil.c: - Implement virIsCapableFCHost and virIsCapableVport * src/node_device/node_device_linux_sysfs.c: - Remove check_capable_vport_linux - Rename check_fc_host_linux as detect_scsi_host_caps, and refactor it a bit to detect both fc_host and vport_os capabilities * src/node_device/node_device_driver.h: - Change/remove the related declarations * src/node_device/node_device_udev.c: (Use detect_scsi_host_caps) * src/node_device/node_device_hal.c: (Likewise) * src/node_device/node_device_driver.c (Likewise) --- src/libvirt_private.syms | 2 + src/node_device/node_device_driver.c | 7 +- src/node_device/node_device_driver.h | 10 +-- src/node_device/node_device_hal.c | 4 +- src/node_device/node_device_linux_sysfs.c | 135 ++++++++--------------------- src/node_device/node_device_udev.c | 3 +- src/util/virutil.c | 73 ++++++++++++++++ src/util/virutil.h | 3 + 8 files changed, 123 insertions(+), 114 deletions(-)
diff --git a/src/node_device/node_device_linux_sysfs.c b/src/node_device/node_device_linux_sysfs.c index 742a0bc..054afea 100644 --- a/src/node_device/node_device_linux_sysfs.c +++ b/src/node_device/node_device_linux_sysfs.c @@ -1,8 +1,8 @@ /* - * node_device_hal_linuc.c: Linux specific code to gather device data + * node_device_linux_sysfs.c: Linux specific code to gather device data * not available through HAL. * - * Copyright (C) 2009-2011 Red Hat, Inc. + * Copyright (C) 2009-2013 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -37,112 +37,53 @@
#ifdef __linux__
-int check_fc_host_linux(union _virNodeDevCapData *d) +int +detect_scsi_host_caps_linux(union _virNodeDevCapData *d) { - char *sysfs_path = NULL; - int retval = 0; - struct stat st; + int ret = -1;
VIR_DEBUG("Checking if host%d is an FC HBA", d->scsi_host.host);
- if (virAsprintf(&sysfs_path, "%shost%d", - LINUX_SYSFS_FC_HOST_PREFIX, - d->scsi_host.host)< 0) { - virReportOOMError(); - retval = -1; - goto out; + if (virIsCapableFCHost(NULL, d->scsi_host.host)) { + d->scsi_host.flags |= VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST; + + if (virReadFCHost(NULL, + d->scsi_host.host, + "port_name", +&d->scsi_host.wwpn) == -1) { + VIR_ERROR(_("Failed to read WWPN for host%d"), d->scsi_host.host); + goto cleanup; + } + + if (virReadFCHost(NULL, + d->scsi_host.host, + "node_name", +&d->scsi_host.wwnn) == -1) { + VIR_ERROR(_("Failed to read WWNN for host%d"), d->scsi_host.host); + goto cleanup; + } + + if (virReadFCHost(NULL, + d->scsi_host.host, + "fabric_name", +&d->scsi_host.fabric_wwn) == -1) { + VIR_ERROR(_("Failed to read fabric WWN for host%d"), + d->scsi_host.host); + goto cleanup; + } }
- if (stat(sysfs_path,&st) != 0) { - /* Not an FC HBA; not an error, either. */ - goto out; - } - - d->scsi_host.flags |= VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST; - - if (virReadFCHost(NULL, - d->scsi_host.host, - "port_name", -&d->scsi_host.wwpn) == -1) { - VIR_ERROR(_("Failed to read WWPN for host%d"), - d->scsi_host.host); - retval = -1; - goto out; - } - - if (virReadFCHost(NULL, - d->scsi_host.host, - "node_name", -&d->scsi_host.wwnn) == -1) { - VIR_ERROR(_("Failed to read WWNN for host%d"), - d->scsi_host.host); - retval = -1; - } - - if (virReadFCHost(NULL, - d->scsi_host.host, - "fabric_name", -&d->scsi_host.fabric_wwn) == -1) { - VIR_ERROR(_("Failed to read fabric WWN for host%d"), - d->scsi_host.host); - retval = -1; - goto out; - } + if (virIsCapableVport(NULL, d->scsi_host.host) == 0) + d->scsi_host.flags |= VIR_NODE_DEV_CAP_FLAG_HBA_VPORT_OPS;
-out: - if (retval == -1) { + ret = 0; +cleanup: + if (ret == -1) {
I'd write this as 'if (ret< 0) {' so we don't have to change this line if we ever invent a new error code for this function. Moreover, it's common practice over libvirt code.
Okay, it was inherited from the old codes, I will change it when pushing. Thanks.
ACK if you fix this.
Michal

This enrichs HBA's xml by dumping the number of max vports and vports in use. Format is like: <capability type='vport_ops'> <max_vports>164</max_vports> <vports>5</vports> </capability> * docs/formatnode.html.in: (Document the new XML) * docs/schemas/nodedev.rng: (Add the schema) * src/conf/node_device_conf.h: (New member for data.scsi_host) * src/node_device/node_device_linux_sysfs.c: (Collect the value of max_vports and vports) --- docs/formatnode.html.in | 10 ++++-- docs/schemas/nodedev.rng | 6 +++ src/conf/node_device_conf.c | 7 +++- src/conf/node_device_conf.h | 2 + src/node_device/node_device_linux_sysfs.c | 48 ++++++++++++++++++++++++++-- 5 files changed, 65 insertions(+), 8 deletions(-) diff --git a/docs/formatnode.html.in b/docs/formatnode.html.in index fcaaaaf..5712bcf 100644 --- a/docs/formatnode.html.in +++ b/docs/formatnode.html.in @@ -136,9 +136,13 @@ <dd>The SCSI host number.</dd> <dt><code>capability</code></dt> <dd>Current capabilities include "vports_ops" (indicates - vport operations are supported) and "fc_host", the later - implies following sub-elements: <code>wwnn</code>, - <code>wwpn</code>, <code>fabric_wwn</code>. + vport operations are supported) and "fc_host". "vport_ops" + could contain two optional sub-elements: <code>vports</code>, + and <code>max_vports</code>. <code>vports</code> shows the + number of vport in use. <code>max_vports</code> shows the + maximum vports the HBA supports. "fc_host" implies following + sub-elements: <code>wwnn</code>, <code>wwpn</code>, and + <code>fabric_wwn</code>. </dd> </dl> </dd> diff --git a/docs/schemas/nodedev.rng b/docs/schemas/nodedev.rng index 7c85815..b94cce6 100644 --- a/docs/schemas/nodedev.rng +++ b/docs/schemas/nodedev.rng @@ -267,6 +267,12 @@ <attribute name='type'> <value>vports_ops</value> </attribute> + <element name='max_vports'> + <ref name='unsignedInt'/> + </element> + <element name='vports'> + <ref name='unsignedInt'/> + </element> </define> <define name='capscsihost'> diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index 819e6af..5962d58 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -392,7 +392,12 @@ char *virNodeDeviceDefFormat(const virNodeDeviceDefPtr def) virBufferAddLit(&buf, " </capability>\n"); } if (data->scsi_host.flags & VIR_NODE_DEV_CAP_FLAG_HBA_VPORT_OPS) { - virBufferAddLit(&buf, " <capability type='vport_ops' />\n"); + virBufferAddLit(&buf, " <capability type='vport_ops'>\n"); + virBufferAsprintf(&buf, " <max_vports>%d</max_vports>\n", + data->scsi_host.max_vports); + virBufferAsprintf(&buf, " <vports>%d</vports>\n", + data->scsi_host.vports); + virBufferAddLit(&buf, " </capability>\n"); } break; diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h index 145d699..4e584a3 100644 --- a/src/conf/node_device_conf.h +++ b/src/conf/node_device_conf.h @@ -137,6 +137,8 @@ struct _virNodeDevCapsDef { char *wwpn; char *fabric_wwn; unsigned int flags; + int max_vports; + int vports; } scsi_host; struct { char *name; diff --git a/src/node_device/node_device_linux_sysfs.c b/src/node_device/node_device_linux_sysfs.c index 054afea..b498726 100644 --- a/src/node_device/node_device_linux_sysfs.c +++ b/src/node_device/node_device_linux_sysfs.c @@ -40,6 +40,8 @@ int detect_scsi_host_caps_linux(union _virNodeDevCapData *d) { + char *max_vports = NULL; + char *vports = NULL; int ret = -1; VIR_DEBUG("Checking if host%d is an FC HBA", d->scsi_host.host); @@ -50,7 +52,7 @@ detect_scsi_host_caps_linux(union _virNodeDevCapData *d) if (virReadFCHost(NULL, d->scsi_host.host, "port_name", - &d->scsi_host.wwpn) == -1) { + &d->scsi_host.wwpn) < 0) { VIR_ERROR(_("Failed to read WWPN for host%d"), d->scsi_host.host); goto cleanup; } @@ -58,7 +60,7 @@ detect_scsi_host_caps_linux(union _virNodeDevCapData *d) if (virReadFCHost(NULL, d->scsi_host.host, "node_name", - &d->scsi_host.wwnn) == -1) { + &d->scsi_host.wwnn) < 0) { VIR_ERROR(_("Failed to read WWNN for host%d"), d->scsi_host.host); goto cleanup; } @@ -66,23 +68,61 @@ detect_scsi_host_caps_linux(union _virNodeDevCapData *d) if (virReadFCHost(NULL, d->scsi_host.host, "fabric_name", - &d->scsi_host.fabric_wwn) == -1) { + &d->scsi_host.fabric_wwn) < 0) { VIR_ERROR(_("Failed to read fabric WWN for host%d"), d->scsi_host.host); goto cleanup; } } - if (virIsCapableVport(NULL, d->scsi_host.host) == 0) + if (virIsCapableVport(NULL, d->scsi_host.host) == 0) { d->scsi_host.flags |= VIR_NODE_DEV_CAP_FLAG_HBA_VPORT_OPS; + if (virReadFCHost(NULL, + d->scsi_host.max_vports, + "max_npiv_vports", + &max_vports) < 0) { + VIR_ERROR(_("Failed to read max_npiv_vports for host%d"), + d->scsi_host.host); + goto cleanup; + } + + if (virReadFCHost(NULL, + d->scsi_host.max_vports, + "npiv_vports_inuse", + &vports) < 0) { + VIR_ERROR(_("Failed to read npiv_vports_inuse for host%d"), + d->scsi_host.host); + goto cleanup; + } + + if (virStrToLong_i(max_vports, NULL, 10, + &d->scsi_host.max_vports) < 0) { + VIR_ERROR(_("Failed to parse value of max_npiv_vports '%s'"), + max_vports); + goto cleanup; + } + + if (virStrToLong_i(vports, NULL, 10, + &d->scsi_host.vports) < 0) { + VIR_ERROR(_("Failed to parse value of npiv_vports_inuse '%s'"), + vports); + goto cleanup; + } + } + ret = 0; cleanup: if (ret == -1) { + /* Clear the flags in case of producing confusing XML output */ + d->scsi_host.flags = 0; + VIR_FREE(d->scsi_host.wwnn); VIR_FREE(d->scsi_host.wwpn); VIR_FREE(d->scsi_host.fabric_wwn); } + VIR_FREE(max_vports); + VIR_FREE(vports); return ret; } -- 1.7.7.6

On 14.01.2013 15:34, Osier Yang wrote:
This enrichs HBA's xml by dumping the number of max vports and vports in use. Format is like:
<capability type='vport_ops'> <max_vports>164</max_vports> <vports>5</vports> </capability>
* docs/formatnode.html.in: (Document the new XML) * docs/schemas/nodedev.rng: (Add the schema) * src/conf/node_device_conf.h: (New member for data.scsi_host) * src/node_device/node_device_linux_sysfs.c: (Collect the value of max_vports and vports) --- docs/formatnode.html.in | 10 ++++-- docs/schemas/nodedev.rng | 6 +++ src/conf/node_device_conf.c | 7 +++- src/conf/node_device_conf.h | 2 + src/node_device/node_device_linux_sysfs.c | 48 ++++++++++++++++++++++++++-- 5 files changed, 65 insertions(+), 8 deletions(-)
diff --git a/src/node_device/node_device_linux_sysfs.c b/src/node_device/node_device_linux_sysfs.c index 054afea..b498726 100644 --- a/src/node_device/node_device_linux_sysfs.c +++ b/src/node_device/node_device_linux_sysfs.c @@ -40,6 +40,8 @@ int detect_scsi_host_caps_linux(union _virNodeDevCapData *d) { + char *max_vports = NULL; + char *vports = NULL; int ret = -1;
VIR_DEBUG("Checking if host%d is an FC HBA", d->scsi_host.host); @@ -50,7 +52,7 @@ detect_scsi_host_caps_linux(union _virNodeDevCapData *d) if (virReadFCHost(NULL, d->scsi_host.host, "port_name", - &d->scsi_host.wwpn) == -1) { + &d->scsi_host.wwpn) < 0) { VIR_ERROR(_("Failed to read WWPN for host%d"), d->scsi_host.host); goto cleanup; } @@ -58,7 +60,7 @@ detect_scsi_host_caps_linux(union _virNodeDevCapData *d) if (virReadFCHost(NULL, d->scsi_host.host, "node_name", - &d->scsi_host.wwnn) == -1) { + &d->scsi_host.wwnn) < 0) { VIR_ERROR(_("Failed to read WWNN for host%d"), d->scsi_host.host); goto cleanup; } @@ -66,23 +68,61 @@ detect_scsi_host_caps_linux(union _virNodeDevCapData *d) if (virReadFCHost(NULL, d->scsi_host.host, "fabric_name", - &d->scsi_host.fabric_wwn) == -1) { + &d->scsi_host.fabric_wwn) < 0) { VIR_ERROR(_("Failed to read fabric WWN for host%d"), d->scsi_host.host); goto cleanup; } }
- if (virIsCapableVport(NULL, d->scsi_host.host) == 0) + if (virIsCapableVport(NULL, d->scsi_host.host) == 0) { d->scsi_host.flags |= VIR_NODE_DEV_CAP_FLAG_HBA_VPORT_OPS;
+ if (virReadFCHost(NULL, + d->scsi_host.max_vports, + "max_npiv_vports", + &max_vports) < 0) { + VIR_ERROR(_("Failed to read max_npiv_vports for host%d"), + d->scsi_host.host); + goto cleanup; + } + + if (virReadFCHost(NULL, + d->scsi_host.max_vports, + "npiv_vports_inuse", + &vports) < 0) { + VIR_ERROR(_("Failed to read npiv_vports_inuse for host%d"), + d->scsi_host.host); + goto cleanup; + } + + if (virStrToLong_i(max_vports, NULL, 10, + &d->scsi_host.max_vports) < 0) { + VIR_ERROR(_("Failed to parse value of max_npiv_vports '%s'"), + max_vports); + goto cleanup; + } + + if (virStrToLong_i(vports, NULL, 10, + &d->scsi_host.vports) < 0) { + VIR_ERROR(_("Failed to parse value of npiv_vports_inuse '%s'"), + vports); + goto cleanup; + } + } + ret = 0; cleanup: if (ret == -1) {
And again. I'd rewrite this condition as well. But you don't have to.
+ /* Clear the flags in case of producing confusing XML output */ + d->scsi_host.flags = 0;
I don't think so. Shouldn't we be just clearing those two flags and don't touch the others?
+ VIR_FREE(d->scsi_host.wwnn); VIR_FREE(d->scsi_host.wwpn); VIR_FREE(d->scsi_host.fabric_wwn); } + VIR_FREE(max_vports); + VIR_FREE(vports); return ret; }
Michal

On 2013年01月15日 03:42, Michal Privoznik wrote:
On 14.01.2013 15:34, Osier Yang wrote:
This enrichs HBA's xml by dumping the number of max vports and vports in use. Format is like:
<capability type='vport_ops'> <max_vports>164</max_vports> <vports>5</vports> </capability>
* docs/formatnode.html.in: (Document the new XML) * docs/schemas/nodedev.rng: (Add the schema) * src/conf/node_device_conf.h: (New member for data.scsi_host) * src/node_device/node_device_linux_sysfs.c: (Collect the value of max_vports and vports) --- docs/formatnode.html.in | 10 ++++-- docs/schemas/nodedev.rng | 6 +++ src/conf/node_device_conf.c | 7 +++- src/conf/node_device_conf.h | 2 + src/node_device/node_device_linux_sysfs.c | 48 ++++++++++++++++++++++++++-- 5 files changed, 65 insertions(+), 8 deletions(-)
diff --git a/src/node_device/node_device_linux_sysfs.c b/src/node_device/node_device_linux_sysfs.c index 054afea..b498726 100644 --- a/src/node_device/node_device_linux_sysfs.c +++ b/src/node_device/node_device_linux_sysfs.c @@ -40,6 +40,8 @@ int detect_scsi_host_caps_linux(union _virNodeDevCapData *d) { + char *max_vports = NULL; + char *vports = NULL; int ret = -1;
VIR_DEBUG("Checking if host%d is an FC HBA", d->scsi_host.host); @@ -50,7 +52,7 @@ detect_scsi_host_caps_linux(union _virNodeDevCapData *d) if (virReadFCHost(NULL, d->scsi_host.host, "port_name", -&d->scsi_host.wwpn) == -1) { +&d->scsi_host.wwpn)< 0) { VIR_ERROR(_("Failed to read WWPN for host%d"), d->scsi_host.host); goto cleanup; } @@ -58,7 +60,7 @@ detect_scsi_host_caps_linux(union _virNodeDevCapData *d) if (virReadFCHost(NULL, d->scsi_host.host, "node_name", -&d->scsi_host.wwnn) == -1) { +&d->scsi_host.wwnn)< 0) { VIR_ERROR(_("Failed to read WWNN for host%d"), d->scsi_host.host); goto cleanup; } @@ -66,23 +68,61 @@ detect_scsi_host_caps_linux(union _virNodeDevCapData *d) if (virReadFCHost(NULL, d->scsi_host.host, "fabric_name", -&d->scsi_host.fabric_wwn) == -1) { +&d->scsi_host.fabric_wwn)< 0) { VIR_ERROR(_("Failed to read fabric WWN for host%d"), d->scsi_host.host); goto cleanup; } }
- if (virIsCapableVport(NULL, d->scsi_host.host) == 0) + if (virIsCapableVport(NULL, d->scsi_host.host) == 0) { d->scsi_host.flags |= VIR_NODE_DEV_CAP_FLAG_HBA_VPORT_OPS;
+ if (virReadFCHost(NULL, + d->scsi_host.max_vports, + "max_npiv_vports", +&max_vports)< 0) { + VIR_ERROR(_("Failed to read max_npiv_vports for host%d"), + d->scsi_host.host); + goto cleanup; + } + + if (virReadFCHost(NULL, + d->scsi_host.max_vports, + "npiv_vports_inuse", +&vports)< 0) { + VIR_ERROR(_("Failed to read npiv_vports_inuse for host%d"), + d->scsi_host.host); + goto cleanup; + } + + if (virStrToLong_i(max_vports, NULL, 10, +&d->scsi_host.max_vports)< 0) { + VIR_ERROR(_("Failed to parse value of max_npiv_vports '%s'"), + max_vports); + goto cleanup; + } + + if (virStrToLong_i(vports, NULL, 10, +&d->scsi_host.vports)< 0) { + VIR_ERROR(_("Failed to parse value of npiv_vports_inuse '%s'"), + vports); + goto cleanup; + } + } + ret = 0; cleanup: if (ret == -1) {
And again. I'd rewrite this condition as well. But you don't have to.
+ /* Clear the flags in case of producing confusing XML output */ + d->scsi_host.flags = 0;
I don't think so. Shouldn't we be just clearing those two flags and don't touch the others?
Agreed. Though there are only the two flags. I will post a v2.
+ VIR_FREE(d->scsi_host.wwnn); VIR_FREE(d->scsi_host.wwpn); VIR_FREE(d->scsi_host.fabric_wwn); } + VIR_FREE(max_vports); + VIR_FREE(vports); return ret; }
Michal

This enrichs HBA's xml by dumping the number of max vports and vports in use. Format is like: <capability type='vport_ops'> <max_vports>164</max_vports> <vports>5</vports> </capability> * docs/formatnode.html.in: (Document the new XML) * docs/schemas/nodedev.rng: (Add the schema) * src/conf/node_device_conf.h: (New member for data.scsi_host) * src/node_device/node_device_linux_sysfs.c: (Collect the value of max_vports and vports) --- docs/formatnode.html.in | 10 ++++-- docs/schemas/nodedev.rng | 6 +++ src/conf/node_device_conf.c | 7 +++- src/conf/node_device_conf.h | 2 + src/node_device/node_device_linux_sysfs.c | 49 ++++++++++++++++++++++++++-- 5 files changed, 66 insertions(+), 8 deletions(-) diff --git a/docs/formatnode.html.in b/docs/formatnode.html.in index fcaaaaf..5712bcf 100644 --- a/docs/formatnode.html.in +++ b/docs/formatnode.html.in @@ -136,9 +136,13 @@ <dd>The SCSI host number.</dd> <dt><code>capability</code></dt> <dd>Current capabilities include "vports_ops" (indicates - vport operations are supported) and "fc_host", the later - implies following sub-elements: <code>wwnn</code>, - <code>wwpn</code>, <code>fabric_wwn</code>. + vport operations are supported) and "fc_host". "vport_ops" + could contain two optional sub-elements: <code>vports</code>, + and <code>max_vports</code>. <code>vports</code> shows the + number of vport in use. <code>max_vports</code> shows the + maximum vports the HBA supports. "fc_host" implies following + sub-elements: <code>wwnn</code>, <code>wwpn</code>, and + <code>fabric_wwn</code>. </dd> </dl> </dd> diff --git a/docs/schemas/nodedev.rng b/docs/schemas/nodedev.rng index 7c85815..b94cce6 100644 --- a/docs/schemas/nodedev.rng +++ b/docs/schemas/nodedev.rng @@ -267,6 +267,12 @@ <attribute name='type'> <value>vports_ops</value> </attribute> + <element name='max_vports'> + <ref name='unsignedInt'/> + </element> + <element name='vports'> + <ref name='unsignedInt'/> + </element> </define> <define name='capscsihost'> diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index 819e6af..5962d58 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -392,7 +392,12 @@ char *virNodeDeviceDefFormat(const virNodeDeviceDefPtr def) virBufferAddLit(&buf, " </capability>\n"); } if (data->scsi_host.flags & VIR_NODE_DEV_CAP_FLAG_HBA_VPORT_OPS) { - virBufferAddLit(&buf, " <capability type='vport_ops' />\n"); + virBufferAddLit(&buf, " <capability type='vport_ops'>\n"); + virBufferAsprintf(&buf, " <max_vports>%d</max_vports>\n", + data->scsi_host.max_vports); + virBufferAsprintf(&buf, " <vports>%d</vports>\n", + data->scsi_host.vports); + virBufferAddLit(&buf, " </capability>\n"); } break; diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h index 145d699..4e584a3 100644 --- a/src/conf/node_device_conf.h +++ b/src/conf/node_device_conf.h @@ -137,6 +137,8 @@ struct _virNodeDevCapsDef { char *wwpn; char *fabric_wwn; unsigned int flags; + int max_vports; + int vports; } scsi_host; struct { char *name; diff --git a/src/node_device/node_device_linux_sysfs.c b/src/node_device/node_device_linux_sysfs.c index 85bbab6..a1c3637 100644 --- a/src/node_device/node_device_linux_sysfs.c +++ b/src/node_device/node_device_linux_sysfs.c @@ -40,6 +40,8 @@ int detect_scsi_host_caps_linux(union _virNodeDevCapData *d) { + char *max_vports = NULL; + char *vports = NULL; int ret = -1; VIR_DEBUG("Checking if host%d is an FC HBA", d->scsi_host.host); @@ -50,7 +52,7 @@ detect_scsi_host_caps_linux(union _virNodeDevCapData *d) if (virReadFCHost(NULL, d->scsi_host.host, "port_name", - &d->scsi_host.wwpn) == -1) { + &d->scsi_host.wwpn) < 0) { VIR_ERROR(_("Failed to read WWPN for host%d"), d->scsi_host.host); goto cleanup; } @@ -58,7 +60,7 @@ detect_scsi_host_caps_linux(union _virNodeDevCapData *d) if (virReadFCHost(NULL, d->scsi_host.host, "node_name", - &d->scsi_host.wwnn) == -1) { + &d->scsi_host.wwnn) < 0) { VIR_ERROR(_("Failed to read WWNN for host%d"), d->scsi_host.host); goto cleanup; } @@ -66,23 +68,62 @@ detect_scsi_host_caps_linux(union _virNodeDevCapData *d) if (virReadFCHost(NULL, d->scsi_host.host, "fabric_name", - &d->scsi_host.fabric_wwn) == -1) { + &d->scsi_host.fabric_wwn) < 0) { VIR_ERROR(_("Failed to read fabric WWN for host%d"), d->scsi_host.host); goto cleanup; } } - if (virIsCapableVport(NULL, d->scsi_host.host) == 0) + if (virIsCapableVport(NULL, d->scsi_host.host) == 0) { d->scsi_host.flags |= VIR_NODE_DEV_CAP_FLAG_HBA_VPORT_OPS; + if (virReadFCHost(NULL, + d->scsi_host.max_vports, + "max_npiv_vports", + &max_vports) < 0) { + VIR_ERROR(_("Failed to read max_npiv_vports for host%d"), + d->scsi_host.host); + goto cleanup; + } + + if (virReadFCHost(NULL, + d->scsi_host.max_vports, + "npiv_vports_inuse", + &vports) < 0) { + VIR_ERROR(_("Failed to read npiv_vports_inuse for host%d"), + d->scsi_host.host); + goto cleanup; + } + + if (virStrToLong_i(max_vports, NULL, 10, + &d->scsi_host.max_vports) < 0) { + VIR_ERROR(_("Failed to parse value of max_npiv_vports '%s'"), + max_vports); + goto cleanup; + } + + if (virStrToLong_i(vports, NULL, 10, + &d->scsi_host.vports) < 0) { + VIR_ERROR(_("Failed to parse value of npiv_vports_inuse '%s'"), + vports); + goto cleanup; + } + } + ret = 0; cleanup: if (ret < 0) { + /* Clear the two flags in case of producing confusing XML output */ + d->scsi_host.flags &= ~(VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST | + VIR_NODE_DEV_CAP_FLAG_HBA_VPORT_OPS); + VIR_FREE(d->scsi_host.wwnn); VIR_FREE(d->scsi_host.wwpn); VIR_FREE(d->scsi_host.fabric_wwn); } + VIR_FREE(max_vports); + VIR_FREE(vports); return ret; } -- 1.7.7.6

On 15.01.2013 10:38, Osier Yang wrote:
This enrichs HBA's xml by dumping the number of max vports and vports in use. Format is like:
<capability type='vport_ops'> <max_vports>164</max_vports> <vports>5</vports> </capability>
* docs/formatnode.html.in: (Document the new XML) * docs/schemas/nodedev.rng: (Add the schema) * src/conf/node_device_conf.h: (New member for data.scsi_host) * src/node_device/node_device_linux_sysfs.c: (Collect the value of max_vports and vports) --- docs/formatnode.html.in | 10 ++++-- docs/schemas/nodedev.rng | 6 +++ src/conf/node_device_conf.c | 7 +++- src/conf/node_device_conf.h | 2 + src/node_device/node_device_linux_sysfs.c | 49 ++++++++++++++++++++++++++-- 5 files changed, 66 insertions(+), 8 deletions(-)
diff --git a/docs/formatnode.html.in b/docs/formatnode.html.in index fcaaaaf..5712bcf 100644 --- a/docs/formatnode.html.in +++ b/docs/formatnode.html.in @@ -136,9 +136,13 @@ <dd>The SCSI host number.</dd> <dt><code>capability</code></dt> <dd>Current capabilities include "vports_ops" (indicates - vport operations are supported) and "fc_host", the later - implies following sub-elements: <code>wwnn</code>, - <code>wwpn</code>, <code>fabric_wwn</code>. + vport operations are supported) and "fc_host". "vport_ops" + could contain two optional sub-elements: <code>vports</code>, + and <code>max_vports</code>. <code>vports</code> shows the + number of vport in use. <code>max_vports</code> shows the + maximum vports the HBA supports. "fc_host" implies following + sub-elements: <code>wwnn</code>, <code>wwpn</code>, and + <code>fabric_wwn</code>. </dd> </dl> </dd> diff --git a/docs/schemas/nodedev.rng b/docs/schemas/nodedev.rng index 7c85815..b94cce6 100644 --- a/docs/schemas/nodedev.rng +++ b/docs/schemas/nodedev.rng @@ -267,6 +267,12 @@ <attribute name='type'> <value>vports_ops</value> </attribute> + <element name='max_vports'> + <ref name='unsignedInt'/> + </element> + <element name='vports'> + <ref name='unsignedInt'/> + </element> </define>
<define name='capscsihost'> diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index 819e6af..5962d58 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -392,7 +392,12 @@ char *virNodeDeviceDefFormat(const virNodeDeviceDefPtr def) virBufferAddLit(&buf, " </capability>\n"); } if (data->scsi_host.flags & VIR_NODE_DEV_CAP_FLAG_HBA_VPORT_OPS) { - virBufferAddLit(&buf, " <capability type='vport_ops' />\n"); + virBufferAddLit(&buf, " <capability type='vport_ops'>\n"); + virBufferAsprintf(&buf, " <max_vports>%d</max_vports>\n", + data->scsi_host.max_vports); + virBufferAsprintf(&buf, " <vports>%d</vports>\n", + data->scsi_host.vports); + virBufferAddLit(&buf, " </capability>\n"); }
break; diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h index 145d699..4e584a3 100644 --- a/src/conf/node_device_conf.h +++ b/src/conf/node_device_conf.h @@ -137,6 +137,8 @@ struct _virNodeDevCapsDef { char *wwpn; char *fabric_wwn; unsigned int flags; + int max_vports; + int vports; } scsi_host; struct { char *name; diff --git a/src/node_device/node_device_linux_sysfs.c b/src/node_device/node_device_linux_sysfs.c index 85bbab6..a1c3637 100644 --- a/src/node_device/node_device_linux_sysfs.c +++ b/src/node_device/node_device_linux_sysfs.c @@ -40,6 +40,8 @@ int detect_scsi_host_caps_linux(union _virNodeDevCapData *d) { + char *max_vports = NULL; + char *vports = NULL; int ret = -1;
VIR_DEBUG("Checking if host%d is an FC HBA", d->scsi_host.host); @@ -50,7 +52,7 @@ detect_scsi_host_caps_linux(union _virNodeDevCapData *d) if (virReadFCHost(NULL, d->scsi_host.host, "port_name", - &d->scsi_host.wwpn) == -1) { + &d->scsi_host.wwpn) < 0) { VIR_ERROR(_("Failed to read WWPN for host%d"), d->scsi_host.host); goto cleanup; } @@ -58,7 +60,7 @@ detect_scsi_host_caps_linux(union _virNodeDevCapData *d) if (virReadFCHost(NULL, d->scsi_host.host, "node_name", - &d->scsi_host.wwnn) == -1) { + &d->scsi_host.wwnn) < 0) { VIR_ERROR(_("Failed to read WWNN for host%d"), d->scsi_host.host); goto cleanup; } @@ -66,23 +68,62 @@ detect_scsi_host_caps_linux(union _virNodeDevCapData *d) if (virReadFCHost(NULL, d->scsi_host.host, "fabric_name", - &d->scsi_host.fabric_wwn) == -1) { + &d->scsi_host.fabric_wwn) < 0) { VIR_ERROR(_("Failed to read fabric WWN for host%d"), d->scsi_host.host); goto cleanup; } }
- if (virIsCapableVport(NULL, d->scsi_host.host) == 0) + if (virIsCapableVport(NULL, d->scsi_host.host) == 0) { d->scsi_host.flags |= VIR_NODE_DEV_CAP_FLAG_HBA_VPORT_OPS;
+ if (virReadFCHost(NULL, + d->scsi_host.max_vports, + "max_npiv_vports", + &max_vports) < 0) { + VIR_ERROR(_("Failed to read max_npiv_vports for host%d"), + d->scsi_host.host); + goto cleanup; + } + + if (virReadFCHost(NULL, + d->scsi_host.max_vports, + "npiv_vports_inuse", + &vports) < 0) { + VIR_ERROR(_("Failed to read npiv_vports_inuse for host%d"), + d->scsi_host.host); + goto cleanup; + } + + if (virStrToLong_i(max_vports, NULL, 10, + &d->scsi_host.max_vports) < 0) { + VIR_ERROR(_("Failed to parse value of max_npiv_vports '%s'"), + max_vports); + goto cleanup; + } + + if (virStrToLong_i(vports, NULL, 10, + &d->scsi_host.vports) < 0) { + VIR_ERROR(_("Failed to parse value of npiv_vports_inuse '%s'"), + vports); + goto cleanup; + } + } + ret = 0; cleanup: if (ret < 0) { + /* Clear the two flags in case of producing confusing XML output */ + d->scsi_host.flags &= ~(VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST | + VIR_NODE_DEV_CAP_FLAG_HBA_VPORT_OPS); +
Yeah, this is what I thought. ACK Michal
VIR_FREE(d->scsi_host.wwnn); VIR_FREE(d->scsi_host.wwpn); VIR_FREE(d->scsi_host.fabric_wwn); } + VIR_FREE(max_vports); + VIR_FREE(vports); return ret; }

pciGetVirtualFunctions returns 0 even if there is no "virtfn" entry under the device sysfs path. And pciGetVirtualFunctions returns -1 when it fails to get the PCI config space of one VF, however, with keeping the the VFs already detected. That's why udevProcessPCI and gather_pci_cap use logic like: if (!pciGetVirtualFunctions(syspath, &data->pci_dev.virtual_functions, &data->pci_dev.num_virtual_functions) || data->pci_dev.num_virtual_functions > 0) data->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION; to tag the PCI device with "virtual_function" cap. However, this results in a VF will aslo get "virtual_function" cap. This patch fixes it by: * Ignoring the VF which has failure of getting PCI config space (given that the successfully detected VFs are kept , it makes sense to not give up on the failure of one VF too) with a warning, so pciGetVirtualFunctions will not return -1 except out of memory. * Free the allocated *virtual_functions when out of memory And thus the logic can be changed to: /* Out of memory */ int rc = pciGetVirtualFunctions(syspath, &data->pci_dev.virtual_functions, &data->pci_dev.num_virtual_functions); if (ret < 0 ) goto out; else if (!ret && (data->pci_dev.num_virtual_functions > 0)) data->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION; --- src/node_device/node_device_hal.c | 11 ++++++++--- src/node_device/node_device_udev.c | 11 ++++++++--- src/util/virpci.c | 36 +++++++++++++++++++++++------------- 3 files changed, 39 insertions(+), 19 deletions(-) diff --git a/src/node_device/node_device_hal.c b/src/node_device/node_device_hal.c index 1afa21c..d0c419d 100644 --- a/src/node_device/node_device_hal.c +++ b/src/node_device/node_device_hal.c @@ -151,10 +151,15 @@ static int gather_pci_cap(LibHalContext *ctx, const char *udi, if (!pciGetPhysicalFunction(sysfs_path, &d->pci_dev.physical_function)) d->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_PHYSICAL_FUNCTION; - if (!pciGetVirtualFunctions(sysfs_path, &d->pci_dev.virtual_functions, - &d->pci_dev.num_virtual_functions) || - d->pci_dev.num_virtual_functions > 0) + int ret = pciGetVirtualFunctions(sysfs_path, + &d->pci_dev.virtual_functions, + &d->pci_dev.num_virtual_functions); + if (ret < 0) { + VIR_FREE(sysfs_path); + return -1; + } else if (!ret && (d->pci_dev.num_virtual_functions > 0)) { d->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION; + } VIR_FREE(sysfs_path); } diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index 87f1000..47ac4f4 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -416,6 +416,7 @@ static int udevProcessPCI(struct udev_device *device, union _virNodeDevCapData *data = &def->caps->data; int ret = -1; char *p; + int rc; syspath = udev_device_get_syspath(device); @@ -484,9 +485,13 @@ static int udevProcessPCI(struct udev_device *device, if (!pciGetPhysicalFunction(syspath, &data->pci_dev.physical_function)) data->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_PHYSICAL_FUNCTION; - if (!pciGetVirtualFunctions(syspath, &data->pci_dev.virtual_functions, - &data->pci_dev.num_virtual_functions) || - data->pci_dev.num_virtual_functions > 0) + rc = pciGetVirtualFunctions(syspath, + &data->pci_dev.virtual_functions, + &data->pci_dev.num_virtual_functions); + /* Out of memory */ + if (rc < 0) + goto out; + else if (!rc && (data->pci_dev.num_virtual_functions > 0)) data->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION; ret = 0; diff --git a/src/util/virpci.c b/src/util/virpci.c index 0fb9923..9f4f3c7 100644 --- a/src/util/virpci.c +++ b/src/util/virpci.c @@ -1968,6 +1968,7 @@ pciGetVirtualFunctions(const char *sysfs_path, unsigned int *num_virtual_functions) { int ret = -1; + int i; DIR *dir = NULL; struct dirent *entry = NULL; char *device_link = NULL; @@ -1989,6 +1990,7 @@ pciGetVirtualFunctions(const char *sysfs_path, *num_virtual_functions = 0; while ((entry = readdir(dir))) { if (STRPREFIX(entry->d_name, "virtfn")) { + struct pci_config_address *config_addr = NULL; if (virBuildPath(&device_link, sysfs_path, entry->d_name) == -1) { virReportOOMError(); @@ -1997,24 +1999,23 @@ pciGetVirtualFunctions(const char *sysfs_path, VIR_DEBUG("Number of virtual functions: %d", *num_virtual_functions); - if (VIR_REALLOC_N(*virtual_functions, - (*num_virtual_functions) + 1) != 0) { - virReportOOMError(); - VIR_FREE(device_link); - goto out; - } if (pciGetPciConfigAddressFromSysfsDeviceLink(device_link, - &((*virtual_functions)[*num_virtual_functions])) != + &config_addr) != SRIOV_FOUND) { - /* We should not get back SRIOV_NOT_FOUND in this - * case, so if we do, it's an error. */ - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Failed to get SR IOV function from device " - "link '%s'"), device_link); + VIR_WARN("Failed to get SRIOV function from device " + "link '%s'", device_link); VIR_FREE(device_link); - goto out; + continue; } else { + if (VIR_ALLOC_N(*virtual_functions, + *num_virtual_functions + 1) < 0) { + virReportOOMError(); + VIR_FREE(config_addr); + goto out; + } + + (*virtual_functions)[*num_virtual_functions] = config_addr; (*num_virtual_functions)++; } VIR_FREE(device_link); @@ -2022,8 +2023,17 @@ pciGetVirtualFunctions(const char *sysfs_path, } ret = 0; + goto cleanup; out: + if (*virtual_functions) { + for (i = 0; i < *num_virtual_functions; i++) + VIR_FREE((*virtual_functions)[i]); + VIR_FREE(*virtual_functions); + } + +cleanup: + VIR_FREE(device_link); if (dir) closedir(dir); -- 1.7.7.6

On 14.01.2013 15:34, Osier Yang wrote:
pciGetVirtualFunctions returns 0 even if there is no "virtfn" entry under the device sysfs path.
And pciGetVirtualFunctions returns -1 when it fails to get the PCI config space of one VF, however, with keeping the the VFs already detected.
That's why udevProcessPCI and gather_pci_cap use logic like:
if (!pciGetVirtualFunctions(syspath, &data->pci_dev.virtual_functions, &data->pci_dev.num_virtual_functions) || data->pci_dev.num_virtual_functions > 0) data->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION;
to tag the PCI device with "virtual_function" cap.
However, this results in a VF will aslo get "virtual_function" cap.
This patch fixes it by: * Ignoring the VF which has failure of getting PCI config space (given that the successfully detected VFs are kept , it makes sense to not give up on the failure of one VF too) with a warning, so pciGetVirtualFunctions will not return -1 except out of memory.
* Free the allocated *virtual_functions when out of memory
And thus the logic can be changed to:
/* Out of memory */ int rc = pciGetVirtualFunctions(syspath, &data->pci_dev.virtual_functions, &data->pci_dev.num_virtual_functions);
s/int rc/int ret/
if (ret < 0 ) goto out; else if (!ret && (data->pci_dev.num_virtual_functions > 0)) data->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION;
Hm.. what about the second condition being: else if (data->pci_dev.num_virtual_functions > 0) data->pci_dev.flags |= ...; My rationale is - the first 'if' catches all the errors, so we don't have to check 'ret' again for success. We already know it succeeded because we are taking the 'else' branch. Having said that, what about going one step further? int ret = pciGetVirtualFunctions(...); if (ret < 0) goto error; if (data->pci_dev.num_virtual_functions) data->pci_dev.flags |= ...; That is - we can leave the 'else' out since we are doing 'goto'. Likewise, '> 0' can be left out because pciGetVirtualFunctions() sets nonzero value there.
--- src/node_device/node_device_hal.c | 11 ++++++++--- src/node_device/node_device_udev.c | 11 ++++++++--- src/util/virpci.c | 36 +++++++++++++++++++++++------------- 3 files changed, 39 insertions(+), 19 deletions(-)
diff --git a/src/node_device/node_device_hal.c b/src/node_device/node_device_hal.c index 1afa21c..d0c419d 100644 --- a/src/node_device/node_device_hal.c +++ b/src/node_device/node_device_hal.c @@ -151,10 +151,15 @@ static int gather_pci_cap(LibHalContext *ctx, const char *udi, if (!pciGetPhysicalFunction(sysfs_path, &d->pci_dev.physical_function)) d->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_PHYSICAL_FUNCTION;
- if (!pciGetVirtualFunctions(sysfs_path, &d->pci_dev.virtual_functions, - &d->pci_dev.num_virtual_functions) || - d->pci_dev.num_virtual_functions > 0) + int ret = pciGetVirtualFunctions(sysfs_path, + &d->pci_dev.virtual_functions, + &d->pci_dev.num_virtual_functions); + if (ret < 0) { + VIR_FREE(sysfs_path); + return -1; + } else if (!ret && (d->pci_dev.num_virtual_functions > 0)) { d->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION; + }
VIR_FREE(sysfs_path); } diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index 87f1000..47ac4f4 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -416,6 +416,7 @@ static int udevProcessPCI(struct udev_device *device, union _virNodeDevCapData *data = &def->caps->data; int ret = -1; char *p; + int rc;
syspath = udev_device_get_syspath(device);
@@ -484,9 +485,13 @@ static int udevProcessPCI(struct udev_device *device, if (!pciGetPhysicalFunction(syspath, &data->pci_dev.physical_function)) data->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_PHYSICAL_FUNCTION;
- if (!pciGetVirtualFunctions(syspath, &data->pci_dev.virtual_functions, - &data->pci_dev.num_virtual_functions) || - data->pci_dev.num_virtual_functions > 0) + rc = pciGetVirtualFunctions(syspath, + &data->pci_dev.virtual_functions, + &data->pci_dev.num_virtual_functions); + /* Out of memory */ + if (rc < 0) + goto out; + else if (!rc && (data->pci_dev.num_virtual_functions > 0)) data->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION;
ret = 0; diff --git a/src/util/virpci.c b/src/util/virpci.c index 0fb9923..9f4f3c7 100644 --- a/src/util/virpci.c +++ b/src/util/virpci.c @@ -1968,6 +1968,7 @@ pciGetVirtualFunctions(const char *sysfs_path, unsigned int *num_virtual_functions) { int ret = -1; + int i; DIR *dir = NULL; struct dirent *entry = NULL; char *device_link = NULL; @@ -1989,6 +1990,7 @@ pciGetVirtualFunctions(const char *sysfs_path, *num_virtual_functions = 0; while ((entry = readdir(dir))) { if (STRPREFIX(entry->d_name, "virtfn")) { + struct pci_config_address *config_addr = NULL;
if (virBuildPath(&device_link, sysfs_path, entry->d_name) == -1) { virReportOOMError(); @@ -1997,24 +1999,23 @@ pciGetVirtualFunctions(const char *sysfs_path,
VIR_DEBUG("Number of virtual functions: %d", *num_virtual_functions); - if (VIR_REALLOC_N(*virtual_functions, - (*num_virtual_functions) + 1) != 0) { - virReportOOMError(); - VIR_FREE(device_link); - goto out; - }
if (pciGetPciConfigAddressFromSysfsDeviceLink(device_link, - &((*virtual_functions)[*num_virtual_functions])) != + &config_addr) != SRIOV_FOUND) { - /* We should not get back SRIOV_NOT_FOUND in this - * case, so if we do, it's an error. */ - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Failed to get SR IOV function from device " - "link '%s'"), device_link); + VIR_WARN("Failed to get SRIOV function from device " + "link '%s'", device_link); VIR_FREE(device_link); - goto out; + continue;
This 'continue' causes immediate jump to the beginning of the next iteration, so the 'else' is a bit overkill. But I can live with that.
} else { + if (VIR_ALLOC_N(*virtual_functions, + *num_virtual_functions + 1) < 0) { + virReportOOMError(); + VIR_FREE(config_addr); + goto out; + } + + (*virtual_functions)[*num_virtual_functions] = config_addr; (*num_virtual_functions)++; } VIR_FREE(device_link); @@ -2022,8 +2023,17 @@ pciGetVirtualFunctions(const char *sysfs_path, }
ret = 0; + goto cleanup;
out: + if (*virtual_functions) { + for (i = 0; i < *num_virtual_functions; i++) + VIR_FREE((*virtual_functions)[i]); + VIR_FREE(*virtual_functions); + } + +cleanup: + VIR_FREE(device_link); if (dir) closedir(dir);
Re: these 'out' and 'cleanup' labels. I think the preferred way is: ...(some code)... ret = 0; cleanup: VIR_FREE(some_var); return ret; error: VIR_FREE(other_var); <either return val < 0 or goto cleanup> } So if you can spare us the 'out' label which is there a while, that'd be great. Michal

On 2013年01月15日 03:42, Michal Privoznik wrote:
On 14.01.2013 15:34, Osier Yang wrote:
pciGetVirtualFunctions returns 0 even if there is no "virtfn" entry under the device sysfs path.
And pciGetVirtualFunctions returns -1 when it fails to get the PCI config space of one VF, however, with keeping the the VFs already detected.
That's why udevProcessPCI and gather_pci_cap use logic like:
if (!pciGetVirtualFunctions(syspath, &data->pci_dev.virtual_functions, &data->pci_dev.num_virtual_functions) || data->pci_dev.num_virtual_functions> 0) data->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION;
to tag the PCI device with "virtual_function" cap.
However, this results in a VF will aslo get "virtual_function" cap.
This patch fixes it by: * Ignoring the VF which has failure of getting PCI config space (given that the successfully detected VFs are kept , it makes sense to not give up on the failure of one VF too) with a warning, so pciGetVirtualFunctions will not return -1 except out of memory.
* Free the allocated *virtual_functions when out of memory
And thus the logic can be changed to:
/* Out of memory */ int rc = pciGetVirtualFunctions(syspath, &data->pci_dev.virtual_functions, &data->pci_dev.num_virtual_functions);
s/int rc/int ret/
if (ret< 0 ) goto out; else if (!ret&& (data->pci_dev.num_virtual_functions> 0)) data->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION;
Hm.. what about the second condition being:
else if (data->pci_dev.num_virtual_functions> 0) data->pci_dev.flags |= ...;
My rationale is - the first 'if' catches all the errors, so we don't have to check 'ret' again for success. We already know it succeeded because we are taking the 'else' branch. Having said that, what about going one step further?
int ret = pciGetVirtualFunctions(...); if (ret< 0) goto error; if (data->pci_dev.num_virtual_functions) data->pci_dev.flags |= ...;
Agreed. The function now only returns -1 or 0.
That is - we can leave the 'else' out since we are doing 'goto'. Likewise, '> 0' can be left out because pciGetVirtualFunctions() sets nonzero value there.
--- src/node_device/node_device_hal.c | 11 ++++++++--- src/node_device/node_device_udev.c | 11 ++++++++--- src/util/virpci.c | 36 +++++++++++++++++++++++------------- 3 files changed, 39 insertions(+), 19 deletions(-)
diff --git a/src/node_device/node_device_hal.c b/src/node_device/node_device_hal.c index 1afa21c..d0c419d 100644 --- a/src/node_device/node_device_hal.c +++ b/src/node_device/node_device_hal.c @@ -151,10 +151,15 @@ static int gather_pci_cap(LibHalContext *ctx, const char *udi, if (!pciGetPhysicalFunction(sysfs_path,&d->pci_dev.physical_function)) d->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_PHYSICAL_FUNCTION;
- if (!pciGetVirtualFunctions(sysfs_path,&d->pci_dev.virtual_functions, -&d->pci_dev.num_virtual_functions) || - d->pci_dev.num_virtual_functions> 0) + int ret = pciGetVirtualFunctions(sysfs_path, +&d->pci_dev.virtual_functions, +&d->pci_dev.num_virtual_functions); + if (ret< 0) { + VIR_FREE(sysfs_path); + return -1; + } else if (!ret&& (d->pci_dev.num_virtual_functions> 0)) { d->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION; + }
VIR_FREE(sysfs_path); } diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index 87f1000..47ac4f4 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -416,6 +416,7 @@ static int udevProcessPCI(struct udev_device *device, union _virNodeDevCapData *data =&def->caps->data; int ret = -1; char *p; + int rc;
syspath = udev_device_get_syspath(device);
@@ -484,9 +485,13 @@ static int udevProcessPCI(struct udev_device *device, if (!pciGetPhysicalFunction(syspath,&data->pci_dev.physical_function)) data->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_PHYSICAL_FUNCTION;
- if (!pciGetVirtualFunctions(syspath,&data->pci_dev.virtual_functions, -&data->pci_dev.num_virtual_functions) || - data->pci_dev.num_virtual_functions> 0) + rc = pciGetVirtualFunctions(syspath, +&data->pci_dev.virtual_functions, +&data->pci_dev.num_virtual_functions); + /* Out of memory */ + if (rc< 0) + goto out; + else if (!rc&& (data->pci_dev.num_virtual_functions> 0)) data->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION;
ret = 0; diff --git a/src/util/virpci.c b/src/util/virpci.c index 0fb9923..9f4f3c7 100644 --- a/src/util/virpci.c +++ b/src/util/virpci.c @@ -1968,6 +1968,7 @@ pciGetVirtualFunctions(const char *sysfs_path, unsigned int *num_virtual_functions) { int ret = -1; + int i; DIR *dir = NULL; struct dirent *entry = NULL; char *device_link = NULL; @@ -1989,6 +1990,7 @@ pciGetVirtualFunctions(const char *sysfs_path, *num_virtual_functions = 0; while ((entry = readdir(dir))) { if (STRPREFIX(entry->d_name, "virtfn")) { + struct pci_config_address *config_addr = NULL;
if (virBuildPath(&device_link, sysfs_path, entry->d_name) == -1) { virReportOOMError(); @@ -1997,24 +1999,23 @@ pciGetVirtualFunctions(const char *sysfs_path,
VIR_DEBUG("Number of virtual functions: %d", *num_virtual_functions); - if (VIR_REALLOC_N(*virtual_functions, - (*num_virtual_functions) + 1) != 0) { - virReportOOMError(); - VIR_FREE(device_link); - goto out; - }
if (pciGetPciConfigAddressFromSysfsDeviceLink(device_link, -&((*virtual_functions)[*num_virtual_functions])) != +&config_addr) != SRIOV_FOUND) { - /* We should not get back SRIOV_NOT_FOUND in this - * case, so if we do, it's an error. */ - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Failed to get SR IOV function from device " - "link '%s'"), device_link); + VIR_WARN("Failed to get SRIOV function from device " + "link '%s'", device_link); VIR_FREE(device_link); - goto out; + continue;
This 'continue' causes immediate jump to the beginning of the next iteration, so the 'else' is a bit overkill. But I can live with that.
Okay. This is improved in v2.
} else { + if (VIR_ALLOC_N(*virtual_functions, + *num_virtual_functions + 1)< 0) { + virReportOOMError(); + VIR_FREE(config_addr); + goto out; + } + + (*virtual_functions)[*num_virtual_functions] = config_addr; (*num_virtual_functions)++; } VIR_FREE(device_link); @@ -2022,8 +2023,17 @@ pciGetVirtualFunctions(const char *sysfs_path, }
ret = 0; + goto cleanup;
out: + if (*virtual_functions) { + for (i = 0; i< *num_virtual_functions; i++) + VIR_FREE((*virtual_functions)[i]); + VIR_FREE(*virtual_functions); + } + +cleanup: + VIR_FREE(device_link); if (dir) closedir(dir);
Re: these 'out' and 'cleanup' labels. I think the preferred way is:
...(some code)...
ret = 0; cleanup: VIR_FREE(some_var); return ret;
error: VIR_FREE(other_var); <either return val< 0 or goto cleanup>
Agreed, this is the convention we use in libvirt.
}
So if you can spare us the 'out' label which is there a while, that'd be great.
Michal

pciGetVirtualFunctions returns 0 even if there is no "virtfn" entry under the device sysfs path. And pciGetVirtualFunctions returns -1 when it fails to get the PCI config space of one VF, however, with keeping the the VFs already detected. That's why udevProcessPCI and gather_pci_cap use logic like: if (!pciGetVirtualFunctions(syspath, &data->pci_dev.virtual_functions, &data->pci_dev.num_virtual_functions) || data->pci_dev.num_virtual_functions > 0) data->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION; to tag the PCI device with "virtual_function" cap. However, this results in a VF will aslo get "virtual_function" cap. This patch fixes it by: * Ignoring the VF which has failure of getting PCI config space (given that the successfully detected VFs are kept , it makes sense to not give up on the failure of one VF too) with a warning, so pciGetVirtualFunctions will not return -1 except out of memory. * Free the allocated *virtual_functions when out of memory And thus the logic can be changed to: /* Out of memory */ int ret = pciGetVirtualFunctions(syspath, &data->pci_dev.virtual_functions, &data->pci_dev.num_virtual_functions); if (ret < 0 ) goto out; if (data->pci_dev.num_virtual_functions > 0) data->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION; --- src/node_device/node_device_hal.c | 12 +++++++-- src/node_device/node_device_udev.c | 11 ++++++-- src/util/virpci.c | 46 +++++++++++++++++++++--------------- 3 files changed, 44 insertions(+), 25 deletions(-) diff --git a/src/node_device/node_device_hal.c b/src/node_device/node_device_hal.c index e64d6ac..6da5873 100644 --- a/src/node_device/node_device_hal.c +++ b/src/node_device/node_device_hal.c @@ -151,9 +151,15 @@ static int gather_pci_cap(LibHalContext *ctx, const char *udi, if (!pciGetPhysicalFunction(sysfs_path, &d->pci_dev.physical_function)) d->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_PHYSICAL_FUNCTION; - if (!pciGetVirtualFunctions(sysfs_path, &d->pci_dev.virtual_functions, - &d->pci_dev.num_virtual_functions) || - d->pci_dev.num_virtual_functions > 0) + int ret = pciGetVirtualFunctions(sysfs_path, + &d->pci_dev.virtual_functions, + &d->pci_dev.num_virtual_functions); + if (ret < 0) { + VIR_FREE(sysfs_path); + return -1; + } + + if (d->pci_dev.num_virtual_functions > 0) d->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION; VIR_FREE(sysfs_path); diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index 62f6320..a90217d 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -416,6 +416,7 @@ static int udevProcessPCI(struct udev_device *device, union _virNodeDevCapData *data = &def->caps->data; int ret = -1; char *p; + int rc; syspath = udev_device_get_syspath(device); @@ -484,9 +485,13 @@ static int udevProcessPCI(struct udev_device *device, if (!pciGetPhysicalFunction(syspath, &data->pci_dev.physical_function)) data->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_PHYSICAL_FUNCTION; - if (!pciGetVirtualFunctions(syspath, &data->pci_dev.virtual_functions, - &data->pci_dev.num_virtual_functions) || - data->pci_dev.num_virtual_functions > 0) + rc = pciGetVirtualFunctions(syspath, + &data->pci_dev.virtual_functions, + &data->pci_dev.num_virtual_functions); + /* Out of memory */ + if (rc < 0) + goto out; + else if (!rc && (data->pci_dev.num_virtual_functions > 0)) data->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION; ret = 0; diff --git a/src/util/virpci.c b/src/util/virpci.c index 0fb9923..ee4b3a8 100644 --- a/src/util/virpci.c +++ b/src/util/virpci.c @@ -1968,6 +1968,7 @@ pciGetVirtualFunctions(const char *sysfs_path, unsigned int *num_virtual_functions) { int ret = -1; + int i; DIR *dir = NULL; struct dirent *entry = NULL; char *device_link = NULL; @@ -1989,45 +1990,52 @@ pciGetVirtualFunctions(const char *sysfs_path, *num_virtual_functions = 0; while ((entry = readdir(dir))) { if (STRPREFIX(entry->d_name, "virtfn")) { + struct pci_config_address *config_addr = NULL; if (virBuildPath(&device_link, sysfs_path, entry->d_name) == -1) { virReportOOMError(); - goto out; + goto error; } VIR_DEBUG("Number of virtual functions: %d", *num_virtual_functions); - if (VIR_REALLOC_N(*virtual_functions, - (*num_virtual_functions) + 1) != 0) { - virReportOOMError(); - VIR_FREE(device_link); - goto out; - } if (pciGetPciConfigAddressFromSysfsDeviceLink(device_link, - &((*virtual_functions)[*num_virtual_functions])) != + &config_addr) != SRIOV_FOUND) { - /* We should not get back SRIOV_NOT_FOUND in this - * case, so if we do, it's an error. */ - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Failed to get SR IOV function from device " - "link '%s'"), device_link); + VIR_WARN("Failed to get SRIOV function from device " + "link '%s'", device_link); VIR_FREE(device_link); - goto out; - } else { - (*num_virtual_functions)++; + continue; } + + if (VIR_ALLOC_N(*virtual_functions, + *num_virtual_functions + 1) < 0) { + virReportOOMError(); + VIR_FREE(config_addr); + goto error; + } + + (*virtual_functions)[*num_virtual_functions] = config_addr; + (*num_virtual_functions)++; VIR_FREE(device_link); } } ret = 0; - -out: +cleanup: + VIR_FREE(device_link); if (dir) closedir(dir); - return ret; + +error: + if (*virtual_functions) { + for (i = 0; i < *num_virtual_functions; i++) + VIR_FREE((*virtual_functions)[i]); + VIR_FREE(*virtual_functions); + } + goto cleanup; } /* -- 1.7.7.6

On 2013年01月15日 17:56, Osier Yang wrote:
pciGetVirtualFunctions returns 0 even if there is no "virtfn" entry under the device sysfs path.
And pciGetVirtualFunctions returns -1 when it fails to get the PCI config space of one VF, however, with keeping the the VFs already detected.
That's why udevProcessPCI and gather_pci_cap use logic like:
if (!pciGetVirtualFunctions(syspath, &data->pci_dev.virtual_functions, &data->pci_dev.num_virtual_functions) || data->pci_dev.num_virtual_functions> 0) data->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION;
to tag the PCI device with "virtual_function" cap.
However, this results in a VF will aslo get "virtual_function" cap.
This patch fixes it by: * Ignoring the VF which has failure of getting PCI config space (given that the successfully detected VFs are kept , it makes sense to not give up on the failure of one VF too) with a warning, so pciGetVirtualFunctions will not return -1 except out of memory.
* Free the allocated *virtual_functions when out of memory
And thus the logic can be changed to:
/* Out of memory */ int ret = pciGetVirtualFunctions(syspath, &data->pci_dev.virtual_functions, &data->pci_dev.num_virtual_functions);
if (ret< 0 ) goto out; if (data->pci_dev.num_virtual_functions> 0) data->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION; --- src/node_device/node_device_hal.c | 12 +++++++-- src/node_device/node_device_udev.c | 11 ++++++-- src/util/virpci.c | 46 +++++++++++++++++++++--------------- 3 files changed, 44 insertions(+), 25 deletions(-)
diff --git a/src/node_device/node_device_hal.c b/src/node_device/node_device_hal.c index e64d6ac..6da5873 100644 --- a/src/node_device/node_device_hal.c +++ b/src/node_device/node_device_hal.c @@ -151,9 +151,15 @@ static int gather_pci_cap(LibHalContext *ctx, const char *udi, if (!pciGetPhysicalFunction(sysfs_path,&d->pci_dev.physical_function)) d->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_PHYSICAL_FUNCTION;
- if (!pciGetVirtualFunctions(sysfs_path,&d->pci_dev.virtual_functions, -&d->pci_dev.num_virtual_functions) || - d->pci_dev.num_virtual_functions> 0) + int ret = pciGetVirtualFunctions(sysfs_path, +&d->pci_dev.virtual_functions, +&d->pci_dev.num_virtual_functions); + if (ret< 0) { + VIR_FREE(sysfs_path); + return -1; + } + + if (d->pci_dev.num_virtual_functions> 0) d->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION;
VIR_FREE(sysfs_path); diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index 62f6320..a90217d 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -416,6 +416,7 @@ static int udevProcessPCI(struct udev_device *device, union _virNodeDevCapData *data =&def->caps->data; int ret = -1; char *p; + int rc;
syspath = udev_device_get_syspath(device);
@@ -484,9 +485,13 @@ static int udevProcessPCI(struct udev_device *device, if (!pciGetPhysicalFunction(syspath,&data->pci_dev.physical_function)) data->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_PHYSICAL_FUNCTION;
- if (!pciGetVirtualFunctions(syspath,&data->pci_dev.virtual_functions, -&data->pci_dev.num_virtual_functions) || - data->pci_dev.num_virtual_functions> 0) + rc = pciGetVirtualFunctions(syspath, +&data->pci_dev.virtual_functions, +&data->pci_dev.num_virtual_functions); + /* Out of memory */ + if (rc< 0) + goto out; + else if (!rc&& (data->pci_dev.num_virtual_functions> 0)) data->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION;
ret = 0; diff --git a/src/util/virpci.c b/src/util/virpci.c index 0fb9923..ee4b3a8 100644 --- a/src/util/virpci.c +++ b/src/util/virpci.c @@ -1968,6 +1968,7 @@ pciGetVirtualFunctions(const char *sysfs_path, unsigned int *num_virtual_functions) { int ret = -1; + int i; DIR *dir = NULL; struct dirent *entry = NULL; char *device_link = NULL; @@ -1989,45 +1990,52 @@ pciGetVirtualFunctions(const char *sysfs_path, *num_virtual_functions = 0; while ((entry = readdir(dir))) { if (STRPREFIX(entry->d_name, "virtfn")) { + struct pci_config_address *config_addr = NULL;
if (virBuildPath(&device_link, sysfs_path, entry->d_name) == -1) { virReportOOMError(); - goto out; + goto error; }
VIR_DEBUG("Number of virtual functions: %d", *num_virtual_functions); - if (VIR_REALLOC_N(*virtual_functions, - (*num_virtual_functions) + 1) != 0) { - virReportOOMError(); - VIR_FREE(device_link); - goto out; - }
if (pciGetPciConfigAddressFromSysfsDeviceLink(device_link, -&((*virtual_functions)[*num_virtual_functions])) != +&config_addr) != SRIOV_FOUND) { - /* We should not get back SRIOV_NOT_FOUND in this - * case, so if we do, it's an error. */ - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Failed to get SR IOV function from device " - "link '%s'"), device_link); + VIR_WARN("Failed to get SRIOV function from device " + "link '%s'", device_link); VIR_FREE(device_link); - goto out; - } else { - (*num_virtual_functions)++; + continue; } + + if (VIR_ALLOC_N(*virtual_functions, + *num_virtual_functions + 1)< 0) { + virReportOOMError(); + VIR_FREE(config_addr); + goto error; + } + + (*virtual_functions)[*num_virtual_functions] = config_addr; + (*num_virtual_functions)++; VIR_FREE(device_link); } }
ret = 0; - -out: +cleanup: + VIR_FREE(device_link); if (dir) closedir(dir); - return ret; + +error: + if (*virtual_functions) { + for (i = 0; i< *num_virtual_functions; i++) + VIR_FREE((*virtual_functions)[i]); + VIR_FREE(*virtual_functions); + } + goto cleanup; }
/*
Eh, changes on *_udev.c was lost when committing. With the following diff squashed in:

On 15.01.2013 10:56, Osier Yang wrote:
pciGetVirtualFunctions returns 0 even if there is no "virtfn" entry under the device sysfs path.
And pciGetVirtualFunctions returns -1 when it fails to get the PCI config space of one VF, however, with keeping the the VFs already detected.
That's why udevProcessPCI and gather_pci_cap use logic like:
if (!pciGetVirtualFunctions(syspath, &data->pci_dev.virtual_functions, &data->pci_dev.num_virtual_functions) || data->pci_dev.num_virtual_functions > 0) data->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION;
to tag the PCI device with "virtual_function" cap.
However, this results in a VF will aslo get "virtual_function" cap.
This patch fixes it by: * Ignoring the VF which has failure of getting PCI config space (given that the successfully detected VFs are kept , it makes sense to not give up on the failure of one VF too) with a warning, so pciGetVirtualFunctions will not return -1 except out of memory.
* Free the allocated *virtual_functions when out of memory
And thus the logic can be changed to:
/* Out of memory */ int ret = pciGetVirtualFunctions(syspath, &data->pci_dev.virtual_functions, &data->pci_dev.num_virtual_functions);
if (ret < 0 ) goto out; if (data->pci_dev.num_virtual_functions > 0) data->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION; --- src/node_device/node_device_hal.c | 12 +++++++-- src/node_device/node_device_udev.c | 11 ++++++-- src/util/virpci.c | 46 +++++++++++++++++++++--------------- 3 files changed, 44 insertions(+), 25 deletions(-)
diff --git a/src/node_device/node_device_hal.c b/src/node_device/node_device_hal.c index e64d6ac..6da5873 100644 --- a/src/node_device/node_device_hal.c +++ b/src/node_device/node_device_hal.c @@ -151,9 +151,15 @@ static int gather_pci_cap(LibHalContext *ctx, const char *udi, if (!pciGetPhysicalFunction(sysfs_path, &d->pci_dev.physical_function)) d->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_PHYSICAL_FUNCTION;
- if (!pciGetVirtualFunctions(sysfs_path, &d->pci_dev.virtual_functions, - &d->pci_dev.num_virtual_functions) || - d->pci_dev.num_virtual_functions > 0) + int ret = pciGetVirtualFunctions(sysfs_path, + &d->pci_dev.virtual_functions, + &d->pci_dev.num_virtual_functions); + if (ret < 0) { + VIR_FREE(sysfs_path); + return -1; + } + + if (d->pci_dev.num_virtual_functions > 0) d->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION;
VIR_FREE(sysfs_path); diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index 62f6320..a90217d 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -416,6 +416,7 @@ static int udevProcessPCI(struct udev_device *device, union _virNodeDevCapData *data = &def->caps->data; int ret = -1; char *p; + int rc;
syspath = udev_device_get_syspath(device);
@@ -484,9 +485,13 @@ static int udevProcessPCI(struct udev_device *device, if (!pciGetPhysicalFunction(syspath, &data->pci_dev.physical_function)) data->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_PHYSICAL_FUNCTION;
- if (!pciGetVirtualFunctions(syspath, &data->pci_dev.virtual_functions, - &data->pci_dev.num_virtual_functions) || - data->pci_dev.num_virtual_functions > 0) + rc = pciGetVirtualFunctions(syspath, + &data->pci_dev.virtual_functions, + &data->pci_dev.num_virtual_functions); + /* Out of memory */ + if (rc < 0) + goto out; + else if (!rc && (data->pci_dev.num_virtual_functions > 0)) data->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION;
ret = 0; diff --git a/src/util/virpci.c b/src/util/virpci.c index 0fb9923..ee4b3a8 100644 --- a/src/util/virpci.c +++ b/src/util/virpci.c @@ -1968,6 +1968,7 @@ pciGetVirtualFunctions(const char *sysfs_path, unsigned int *num_virtual_functions) { int ret = -1; + int i; DIR *dir = NULL; struct dirent *entry = NULL; char *device_link = NULL; @@ -1989,45 +1990,52 @@ pciGetVirtualFunctions(const char *sysfs_path, *num_virtual_functions = 0; while ((entry = readdir(dir))) { if (STRPREFIX(entry->d_name, "virtfn")) { + struct pci_config_address *config_addr = NULL;
if (virBuildPath(&device_link, sysfs_path, entry->d_name) == -1) { virReportOOMError(); - goto out; + goto error; }
VIR_DEBUG("Number of virtual functions: %d", *num_virtual_functions); - if (VIR_REALLOC_N(*virtual_functions, - (*num_virtual_functions) + 1) != 0) { - virReportOOMError(); - VIR_FREE(device_link); - goto out; - }
if (pciGetPciConfigAddressFromSysfsDeviceLink(device_link, - &((*virtual_functions)[*num_virtual_functions])) != + &config_addr) != SRIOV_FOUND) { - /* We should not get back SRIOV_NOT_FOUND in this - * case, so if we do, it's an error. */ - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Failed to get SR IOV function from device " - "link '%s'"), device_link); + VIR_WARN("Failed to get SRIOV function from device " + "link '%s'", device_link); VIR_FREE(device_link); - goto out; - } else { - (*num_virtual_functions)++; + continue; } + + if (VIR_ALLOC_N(*virtual_functions, + *num_virtual_functions + 1) < 0) { + virReportOOMError(); + VIR_FREE(config_addr); + goto error; + } + + (*virtual_functions)[*num_virtual_functions] = config_addr; + (*num_virtual_functions)++; VIR_FREE(device_link); } }
ret = 0; - -out: +cleanup: + VIR_FREE(device_link); if (dir) closedir(dir); - return ret; + +error: + if (*virtual_functions) { + for (i = 0; i < *num_virtual_functions; i++) + VIR_FREE((*virtual_functions)[i]); + VIR_FREE(*virtual_functions); + } + goto cleanup; }
/*
ACK to this and te follow up *_udev.c patch squashed in. Michal

This abstracts nodeDeviceVportCreateDelete as an util function virManageVport, which can be further used by later storage patches (to support persistent vHBA, I don't want to create the vHBA using the public API, which is not good). --- src/libvirt_private.syms | 1 + src/node_device/node_device_driver.c | 101 +++------------------------------- src/node_device/node_device_driver.h | 5 -- src/util/virutil.c | 84 ++++++++++++++++++++++++++++ src/util/virutil.h | 11 ++++ 5 files changed, 104 insertions(+), 98 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 2945917..146e57e 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1298,6 +1298,7 @@ virIndexToDiskName; virIsCapableFCHost; virIsCapableVport; virIsDevMapperDevice; +virManageVport; virParseNumber; virParseVersionString; virPipeReadUntilEOF; diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index 050ea62..0d0afe0 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -408,91 +408,6 @@ cleanup: return ret; } - -static int -nodeDeviceVportCreateDelete(const int parent_host, - const char *wwpn, - const char *wwnn, - int operation) -{ - int retval = 0; - char *operation_path = NULL, *vport_name = NULL; - const char *operation_file = NULL; - - 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: - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Invalid vport operation (%d)"), operation); - retval = -1; - goto cleanup; - break; - } - - if (virAsprintf(&operation_path, - "%shost%d%s", - LINUX_SYSFS_FC_HOST_PREFIX, - parent_host, - operation_file) < 0) { - - virReportOOMError(); - retval = -1; - goto cleanup; - } - - if (!virFileExists(operation_path)) { - VIR_FREE(operation_path); - if (virAsprintf(&operation_path, - "%shost%d%s", - LINUX_SYSFS_SCSI_HOST_PREFIX, - parent_host, - operation_file) < 0) { - virReportOOMError(); - retval = -1; - goto cleanup; - } - - if (!virFileExists(operation_path)) { - VIR_ERROR(_("No vport operation path found for host%d"), - parent_host); - retval = -1; - goto cleanup; - } - } - - VIR_DEBUG("Vport operation path is '%s'", operation_path); - - if (virAsprintf(&vport_name, - "%s:%s", - wwpn, - wwnn) < 0) { - - virReportOOMError(); - retval = -1; - goto cleanup; - } - - if (virFileWriteStr(operation_path, vport_name, 0) == -1) { - virReportSystemError(errno, - _("Write of '%s' to '%s' during " - "vport create/delete failed"), - vport_name, operation_path); - retval = -1; - } - -cleanup: - VIR_FREE(vport_name); - VIR_FREE(operation_path); - VIR_DEBUG("%s", _("Vport operation complete")); - return retval; -} - - static int get_time(time_t *t) { @@ -594,10 +509,10 @@ nodeDeviceCreateXML(virConnectPtr conn, goto cleanup; } - if (nodeDeviceVportCreateDelete(parent_host, - wwpn, - wwnn, - VPORT_CREATE) == -1) { + if (virManageVport(parent_host, + wwpn, + wwnn, + VPORT_CREATE) == -1) { goto cleanup; } @@ -661,10 +576,10 @@ nodeDeviceDestroy(virNodeDevicePtr dev) goto out; } - if (nodeDeviceVportCreateDelete(parent_host, - wwpn, - wwnn, - VPORT_DELETE) == -1) { + if (virManageVport(parent_host, + wwpn, + wwnn, + VPORT_DELETE) == -1) { goto out; } diff --git a/src/node_device/node_device_driver.h b/src/node_device/node_device_driver.h index 7775a59..093cdab 100644 --- a/src/node_device/node_device_driver.h +++ b/src/node_device/node_device_driver.h @@ -32,11 +32,6 @@ # define LINUX_SYSFS_SCSI_HOST_POSTFIX "device" # define LINUX_SYSFS_FC_HOST_PREFIX "/sys/class/fc_host/" -# define VPORT_CREATE 0 -# define VPORT_DELETE 1 -# define LINUX_SYSFS_VPORT_CREATE_POSTFIX "/vport_create" -# define LINUX_SYSFS_VPORT_DELETE_POSTFIX "/vport_delete" - # define LINUX_NEW_DEVICE_WAIT_TIME 60 # ifdef HAVE_HAL diff --git a/src/util/virutil.c b/src/util/virutil.c index 8b74afb..be80397 100644 --- a/src/util/virutil.c +++ b/src/util/virutil.c @@ -3374,6 +3374,79 @@ cleanup: VIR_FREE(scsi_host_path); return ret; } + +int +virManageVport(const int parent_host, + const char *wwpn, + const char *wwnn, + int operation) +{ + int ret = -1; + char *operation_path = NULL, *vport_name = NULL; + const char *operation_file = NULL; + + switch (operation) { + case VPORT_CREATE: + operation_file = "vport_create"; + break; + case VPORT_DELETE: + operation_file = "vport_delete"; + break; + default: + virReportError(VIR_ERR_OPERATION_INVALID, + _("Invalid vport operation (%d)"), operation); + goto cleanup; + } + + if (virAsprintf(&operation_path, + "%shost%d/%s", + SYSFS_FC_HOST_PATH, + parent_host, + operation_file) < 0) { + virReportOOMError(); + goto cleanup; + } + + if (!virFileExists(operation_path)) { + VIR_FREE(operation_path); + if (virAsprintf(&operation_path, + "%shost%d/%s", + SYSFS_SCSI_HOST_PATH, + parent_host, + operation_file) < 0) { + virReportOOMError(); + goto cleanup; + } + + if (!virFileExists(operation_path)) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("vport operation '%s' is not supported for host%d"), + operation_file, parent_host); + goto cleanup; + } + } + + if (virAsprintf(&vport_name, + "%s:%s", + wwpn, + wwnn) < 0) { + virReportOOMError(); + goto cleanup; + } + + if (virFileWriteStr(operation_path, vport_name, 0) == 0) + ret = 0; + else + virReportSystemError(errno, + _("Write of '%s' to '%s' during " + "vport create/delete failed"), + vport_name, operation_path); + +cleanup: + VIR_FREE(vport_name); + VIR_FREE(operation_path); + return ret; +} #else int virReadFCHost(const char *sysfs_prefix ATTRIBUTE_UNUSED, @@ -3399,4 +3472,15 @@ virIsCapbleVport(const char *sysfs_prefix ATTRIBUTE_UNUSED, virReportSystemError(ENOSYS, "%s", _("Not supported on this platform")); return -1; } + +int +virManageVport(const int parent_host ATTRIBUTE_UNUSED, + const char *wwpn ATTRIBUTE_UNUSED, + const char *wwnn ATTRIBUTE_UNUSED, + int operation ATTRIBUTE_UNUSED) +{ + virReportSystemError(ENOSYS, "%s", _("Not supported on this platform")); + return -1; +} + #endif /* __linux__ */ diff --git a/src/util/virutil.h b/src/util/virutil.h index d87aa92..c07417c 100644 --- a/src/util/virutil.h +++ b/src/util/virutil.h @@ -302,4 +302,15 @@ int virReadFCHost(const char *sysfs_prefix, int virIsCapableFCHost(const char *sysfs_prefix, int host); int virIsCapableVport(const char *sysfs_prefix, int host); +enum { + VPORT_CREATE, + VPORT_DELETE, +}; + +int virManageVport(const int parent_host, + const char *wwpn, + const char *wwnn, + int operation) + ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); + #endif /* __VIR_UTIL_H__ */ -- 1.7.7.6

On 14.01.2013 15:34, Osier Yang wrote:
This abstracts nodeDeviceVportCreateDelete as an util function virManageVport, which can be further used by later storage patches (to support persistent vHBA, I don't want to create the vHBA using the public API, which is not good). --- src/libvirt_private.syms | 1 + src/node_device/node_device_driver.c | 101 +++------------------------------- src/node_device/node_device_driver.h | 5 -- src/util/virutil.c | 84 ++++++++++++++++++++++++++++ src/util/virutil.h | 11 ++++ 5 files changed, 104 insertions(+), 98 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 2945917..146e57e 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1298,6 +1298,7 @@ virIndexToDiskName; virIsCapableFCHost; virIsCapableVport; virIsDevMapperDevice; +virManageVport; virParseNumber; virParseVersionString; virPipeReadUntilEOF; diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index 050ea62..0d0afe0 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -408,91 +408,6 @@ cleanup: return ret; }
- -static int -nodeDeviceVportCreateDelete(const int parent_host, - const char *wwpn, - const char *wwnn, - int operation) -{ - int retval = 0; - char *operation_path = NULL, *vport_name = NULL; - const char *operation_file = NULL; - - 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: - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Invalid vport operation (%d)"), operation); - retval = -1; - goto cleanup; - break; - } - - if (virAsprintf(&operation_path, - "%shost%d%s", - LINUX_SYSFS_FC_HOST_PREFIX, - parent_host, - operation_file) < 0) { - - virReportOOMError(); - retval = -1; - goto cleanup; - } - - if (!virFileExists(operation_path)) { - VIR_FREE(operation_path); - if (virAsprintf(&operation_path, - "%shost%d%s", - LINUX_SYSFS_SCSI_HOST_PREFIX, - parent_host, - operation_file) < 0) { - virReportOOMError(); - retval = -1; - goto cleanup; - } - - if (!virFileExists(operation_path)) { - VIR_ERROR(_("No vport operation path found for host%d"), - parent_host); - retval = -1; - goto cleanup; - } - } - - VIR_DEBUG("Vport operation path is '%s'", operation_path); - - if (virAsprintf(&vport_name, - "%s:%s", - wwpn, - wwnn) < 0) { - - virReportOOMError(); - retval = -1; - goto cleanup; - } - - if (virFileWriteStr(operation_path, vport_name, 0) == -1) { - virReportSystemError(errno, - _("Write of '%s' to '%s' during " - "vport create/delete failed"), - vport_name, operation_path); - retval = -1; - } - -cleanup: - VIR_FREE(vport_name); - VIR_FREE(operation_path); - VIR_DEBUG("%s", _("Vport operation complete")); - return retval; -} - - static int get_time(time_t *t) { @@ -594,10 +509,10 @@ nodeDeviceCreateXML(virConnectPtr conn, goto cleanup; }
- if (nodeDeviceVportCreateDelete(parent_host, - wwpn, - wwnn, - VPORT_CREATE) == -1) { + if (virManageVport(parent_host, + wwpn, + wwnn, + VPORT_CREATE) == -1) { goto cleanup; }
@@ -661,10 +576,10 @@ nodeDeviceDestroy(virNodeDevicePtr dev) goto out; }
- if (nodeDeviceVportCreateDelete(parent_host, - wwpn, - wwnn, - VPORT_DELETE) == -1) { + if (virManageVport(parent_host, + wwpn, + wwnn, + VPORT_DELETE) == -1) { goto out; }
diff --git a/src/node_device/node_device_driver.h b/src/node_device/node_device_driver.h index 7775a59..093cdab 100644 --- a/src/node_device/node_device_driver.h +++ b/src/node_device/node_device_driver.h @@ -32,11 +32,6 @@ # define LINUX_SYSFS_SCSI_HOST_POSTFIX "device" # define LINUX_SYSFS_FC_HOST_PREFIX "/sys/class/fc_host/"
-# define VPORT_CREATE 0 -# define VPORT_DELETE 1 -# define LINUX_SYSFS_VPORT_CREATE_POSTFIX "/vport_create" -# define LINUX_SYSFS_VPORT_DELETE_POSTFIX "/vport_delete" - # define LINUX_NEW_DEVICE_WAIT_TIME 60
# ifdef HAVE_HAL diff --git a/src/util/virutil.c b/src/util/virutil.c index 8b74afb..be80397 100644 --- a/src/util/virutil.c +++ b/src/util/virutil.c @@ -3374,6 +3374,79 @@ cleanup: VIR_FREE(scsi_host_path); return ret; } + +int +virManageVport(const int parent_host, + const char *wwpn, + const char *wwnn, + int operation) +{ + int ret = -1; + char *operation_path = NULL, *vport_name = NULL; + const char *operation_file = NULL; + + switch (operation) { + case VPORT_CREATE: + operation_file = "vport_create"; + break; + case VPORT_DELETE: + operation_file = "vport_delete"; + break; + default: + virReportError(VIR_ERR_OPERATION_INVALID, + _("Invalid vport operation (%d)"), operation); + goto cleanup; + } + + if (virAsprintf(&operation_path, + "%shost%d/%s", + SYSFS_FC_HOST_PATH, + parent_host, + operation_file) < 0) { + virReportOOMError(); + goto cleanup; + } + + if (!virFileExists(operation_path)) { + VIR_FREE(operation_path); + if (virAsprintf(&operation_path, + "%shost%d/%s", + SYSFS_SCSI_HOST_PATH, + parent_host, + operation_file) < 0) { + virReportOOMError(); + goto cleanup; + } + + if (!virFileExists(operation_path)) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("vport operation '%s' is not supported for host%d"), + operation_file, parent_host);
Nice change from VIR_ERROR() to virReportError(). ACK Michal

This fixes the header names (all util files are named with "vir" prefix now. Some of them are not util files, but the name has been changed), and sort the whole libvirt_private.syms by the header name after the name fixes. --- src/libvirt_private.syms | 1164 +++++++++++++++++++++++----------------------- 1 files changed, 583 insertions(+), 581 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 146e57e..21df094 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -5,50 +5,7 @@ # Keep this file sorted by header name, then by symbols with each header. # -# bitmap.h -virBitmapClearAll; -virBitmapClearBit; -virBitmapCopy; -virBitmapCountBits; -virBitmapEqual; -virBitmapFormat; -virBitmapFree; -virBitmapGetBit; -virBitmapIsAllSet; -virBitmapNew; -virBitmapNewCopy; -virBitmapNewData; -virBitmapNextSetBit; -virBitmapParse; -virBitmapSetAll; -virBitmapSetBit; -virBitmapSize; -virBitmapString; -virBitmapToData; - - -# buf.h -virBufferAdd; -virBufferAddChar; -virBufferAdjustIndent; -virBufferAsprintf; -virBufferContentAndReset; -virBufferCurrentContent; -virBufferError; -virBufferEscape; -virBufferEscapeSexpr; -virBufferEscapeShell; -virBufferEscapeString; -virBufferFreeAndReset; -virBufferGetIndent; -virBufferStrcat; -virBufferTrim; -virBufferURIEncodeString; -virBufferUse; -virBufferVasprintf; - - -# caps.h +# capabilities.h virCapabilitiesAddGuest; virCapabilitiesAddGuestDomain; virCapabilitiesAddGuestFeature; @@ -71,118 +28,6 @@ virCapabilitiesSetHostCPU; virCapabilitiesSetMacPrefix; -# cgroup.h -virCgroupAddTask; -virCgroupAddTaskController; -virCgroupAllowDevice; -virCgroupAllowDeviceMajor; -virCgroupAllowDevicePath; -virCgroupControllerTypeFromString; -virCgroupControllerTypeToString; -virCgroupDenyAllDevices; -virCgroupDenyDevice; -virCgroupDenyDeviceMajor; -virCgroupDenyDevicePath; -virCgroupForDomain; -virCgroupForDriver; -virCgroupForEmulator; -virCgroupForVcpu; -virCgroupFree; -virCgroupGetAppRoot; -virCgroupGetBlkioWeight; -virCgroupGetCpuacctPercpuUsage; -virCgroupGetCpuacctStat; -virCgroupGetCpuacctUsage; -virCgroupGetCpuCfsPeriod; -virCgroupGetCpuCfsQuota; -virCgroupGetCpusetCpus; -virCgroupGetCpusetMems; -virCgroupGetCpuShares; -virCgroupGetFreezerState; -virCgroupGetMemoryHardLimit; -virCgroupGetMemorySoftLimit; -virCgroupGetMemoryUsage; -virCgroupGetMemSwapHardLimit; -virCgroupGetMemSwapUsage; -virCgroupKill; -virCgroupKillPainfully; -virCgroupKillRecursive; -virCgroupMounted; -virCgroupMoveTask; -virCgroupPathOfController; -virCgroupRemove; -virCgroupSetBlkioDeviceWeight; -virCgroupSetBlkioWeight; -virCgroupSetCpuCfsPeriod; -virCgroupSetCpuCfsQuota; -virCgroupSetCpusetCpus; -virCgroupSetCpusetMems; -virCgroupSetCpuShares; -virCgroupSetFreezerState; -virCgroupSetMemory; -virCgroupSetMemoryHardLimit; -virCgroupSetMemorySoftLimit; -virCgroupSetMemSwapHardLimit; - - -# command.h -virCommandAbort; -virCommandAddArg; -virCommandAddArgBuffer; -virCommandAddArgFormat; -virCommandAddArgList; -virCommandAddArgPair; -virCommandAddArgSet; -virCommandAddEnvBuffer; -virCommandAddEnvFormat; -virCommandAddEnvPair; -virCommandAddEnvPass; -virCommandAddEnvPassCommon; -virCommandAddEnvString; -virCommandAllowCap; -virCommandClearCaps; -virCommandDaemonize; -virCommandExec; -virCommandFree; -virCommandHandshakeNotify; -virCommandHandshakeWait; -virCommandNew; -virCommandNewArgList; -virCommandNewArgs; -virCommandNonblockingFDs; -virCommandPreserveFD; -virCommandRequireHandshake; -virCommandRun; -virCommandRunAsync; -virCommandSetErrorBuffer; -virCommandSetErrorFD; -virCommandSetInputBuffer; -virCommandSetInputFD; -virCommandSetOutputBuffer; -virCommandSetOutputFD; -virCommandSetPidFile; -virCommandSetPreExecHook; -virCommandSetWorkingDirectory; -virCommandToString; -virCommandTransferFD; -virCommandWait; -virCommandWriteArgLog; -virFork; -virRun; - - -# conf.h -virConfFree; -virConfFreeValue; -virConfGetValue; -virConfNew; -virConfReadFile; -virConfReadMem; -virConfSetValue; -virConfWriteFile; -virConfWriteMem; - - # cpu.h cpuBaseline; cpuBaselineXML; @@ -243,22 +88,6 @@ virDevicePCIAddressFormat; virDevicePCIAddressIsValid; virDevicePCIAddressParseXML; -# dnsmasq.h -dnsmasqAddDhcpHost; -dnsmasqAddHost; -dnsmasqCapsGet; -dnsmasqCapsGetBinaryPath; -dnsmasqCapsGetVersion; -dnsmasqCapsNewFromBinary; -dnsmasqCapsNewFromBuffer; -dnsmasqCapsNewFromFile; -dnsmasqCapsRefresh; -dnsmasqContextFree; -dnsmasqContextNew; -dnsmasqDelete; -dnsmasqReload; -dnsmasqSave; - # domain_audit.h virDomainAuditCgroup; @@ -616,27 +445,6 @@ virDomainConfNWFilterTeardown; virDomainConfVMNWFilterTeardown; -# ebtables.h -ebtablesAddForwardAllowIn; -ebtablesAddForwardPolicyReject; -ebtablesContextFree; -ebtablesContextNew; -ebtablesRemoveForwardAllowIn; - - -# event_poll.h -virEventPollAddHandle; -virEventPollAddTimeout; -virEventPollFromNativeEvents; -virEventPollInit; -virEventPollRemoveHandle; -virEventPollRemoveTimeout; -virEventPollRunOnce; -virEventPollToNativeEvents; -virEventPollUpdateHandle; -virEventPollUpdateTimeout; - - # fdstream.h virFDStreamConnectUNIX; virFDStreamCreateFile; @@ -644,30 +452,6 @@ virFDStreamOpen; virFDStreamOpenFile; -# hash.h -virHashAddEntry; -virHashCreate; -virHashEqual; -virHashForEach; -virHashFree; -virHashGetItems; -virHashLookup; -virHashRemoveAll; -virHashRemoveEntry; -virHashRemoveSet; -virHashSearch; -virHashSize; -virHashSteal; -virHashTableSize; -virHashUpdateEntry; - - -# hooks.h -virHookCall; -virHookInitialize; -virHookPresent; - - # interface_conf.h virInterfaceAssignDef; virInterfaceDefFormat; @@ -684,105 +468,30 @@ virInterfaceObjUnlock; virInterfaceRemove; -# iptables.h -iptablesAddForwardAllowCross; -iptablesAddForwardAllowIn; -iptablesAddForwardAllowOut; -iptablesAddForwardAllowRelatedIn; -iptablesAddForwardMasquerade; -iptablesAddForwardRejectIn; -iptablesAddForwardRejectOut; -iptablesAddOutputFixUdpChecksum; -iptablesAddTcpInput; -iptablesAddUdpInput; -iptablesContextFree; -iptablesContextNew; -iptablesRemoveForwardAllowCross; -iptablesRemoveForwardAllowIn; -iptablesRemoveForwardAllowOut; -iptablesRemoveForwardAllowRelatedIn; -iptablesRemoveForwardMasquerade; -iptablesRemoveForwardRejectIn; -iptablesRemoveForwardRejectOut; -iptablesRemoveOutputFixUdpChecksum; -iptablesRemoveTcpInput; -iptablesRemoveUdpInput; +# libvirt_internal.h +virDomainMigrateBegin3; +virDomainMigrateConfirm3; +virDomainMigrateFinish; +virDomainMigrateFinish2; +virDomainMigrateFinish3; +virDomainMigratePerform; +virDomainMigratePerform3; +virDomainMigratePrepare; +virDomainMigratePrepare2; +virDomainMigratePrepare3; +virDomainMigratePrepareTunnel; +virDomainMigratePrepareTunnel3; +virDrvSupportsFeature; +virRegisterDeviceMonitor; +virRegisterDriver; +virRegisterInterfaceDriver; +virRegisterNetworkDriver; +virRegisterNWFilterDriver; +virRegisterSecretDriver; +virRegisterStorageDriver; -# json.h -virJSONValueArrayAppend; -virJSONValueArrayGet; -virJSONValueArraySize; -virJSONValueFree; -virJSONValueFromString; -virJSONValueGetBoolean; -virJSONValueGetNumberDouble; -virJSONValueGetNumberInt; -virJSONValueGetNumberLong; -virJSONValueGetNumberUint; -virJSONValueGetNumberUlong; -virJSONValueGetString; -virJSONValueIsNull; -virJSONValueNewArray; -virJSONValueNewBoolean; -virJSONValueNewNull; -virJSONValueNewNumberDouble; -virJSONValueNewNumberInt; -virJSONValueNewNumberLong; -virJSONValueNewNumberUint; -virJSONValueNewNumberUlong; -virJSONValueNewObject; -virJSONValueNewString; -virJSONValueNewStringLen; -virJSONValueObjectAppend; -virJSONValueObjectAppendBoolean; -virJSONValueObjectAppendNull; -virJSONValueObjectAppendNumberDouble; -virJSONValueObjectAppendNumberInt; -virJSONValueObjectAppendNumberLong; -virJSONValueObjectAppendNumberUint; -virJSONValueObjectAppendNumberUlong; -virJSONValueObjectAppendString; -virJSONValueObjectGet; -virJSONValueObjectGetBoolean; -virJSONValueObjectGetKey; -virJSONValueObjectGetNumberDouble; -virJSONValueObjectGetNumberInt; -virJSONValueObjectGetNumberLong; -virJSONValueObjectGetNumberUint; -virJSONValueObjectGetNumberUlong; -virJSONValueObjectGetString; -virJSONValueObjectGetValue; -virJSONValueObjectHasKey; -virJSONValueObjectIsNull; -virJSONValueObjectKeysNumber; -virJSONValueToString; - - -# libvirt_internal.h -virDomainMigrateBegin3; -virDomainMigrateConfirm3; -virDomainMigrateFinish; -virDomainMigrateFinish2; -virDomainMigrateFinish3; -virDomainMigratePerform; -virDomainMigratePerform3; -virDomainMigratePrepare; -virDomainMigratePrepare2; -virDomainMigratePrepare3; -virDomainMigratePrepareTunnel; -virDomainMigratePrepareTunnel3; -virDrvSupportsFeature; -virRegisterDeviceMonitor; -virRegisterDriver; -virRegisterInterfaceDriver; -virRegisterNetworkDriver; -virRegisterNWFilterDriver; -virRegisterSecretDriver; -virRegisterStorageDriver; - - -# locking.h +# lock_manager.h virLockManagerAcquire; virLockManagerAddResource; virLockManagerFree; @@ -797,46 +506,12 @@ virLockManagerRelease; virLockManagerSetPluginDir; -# logging.h -virLogDefineFilter; -virLogDefineOutput; -virLogEmergencyDumpAll; -virLogGetDefaultPriority; -virLogGetFilters; -virLogGetNbFilters; -virLogGetNbOutputs; -virLogGetOutputs; -virLogLock; -virLogMessage; -virLogParseDefaultPriority; -virLogParseFilters; -virLogParseOutputs; -virLogReset; -virLogSetBufferSize; -virLogSetDefaultPriority; -virLogSetFromEnv; -virLogUnlock; - - -# memory.h -virAlloc; -virAllocN; -virAllocVar; -virDeleteElementsN; -virExpandN; -virFree; -virInsertElementsN; -virReallocN; -virResizeN; -virShrinkN; - - # netdev_bandwidth_conf.h virNetDevBandwidthFormat; virNetDevBandwidthParse; -#netdev_vlan_conf.h +# netdev_vlan_conf.h virNetDevVlanFormat; virNetDevVlanParse; @@ -956,7 +631,7 @@ virNWFilterUnlockFilterUpdates; virNWFilterUnRegisterCallbackDriver; -# nwfilter_ipaddrmap +# nwfilter_ipaddrmap.h virNWFilterIPAddrMapAddIPAddr; virNWFilterIPAddrMapDelIPAddr; virNWFilterIPAddrMapGetIPAddr; @@ -990,46 +665,6 @@ virNWFilterVarValueGetNthValue; virNWFilterVarValueGetSimple; -# pci.h -pciConfigAddressToSysfsFile; -pciDettachDevice; -pciDeviceFileIterate; -pciDeviceGetManaged; -pciDeviceGetName; -pciDeviceGetRemoveSlot; -pciDeviceGetReprobe; -pciDeviceGetUnbindFromStub; -pciDeviceGetUsedBy; -pciDeviceGetVirtualFunctionInfo; -pciDeviceIsAssignable; -pciDeviceIsVirtualFunction; -pciDeviceListAdd; -pciDeviceListCount; -pciDeviceListDel; -pciDeviceListFind; -pciDeviceListFindIndex; -pciDeviceListFree; -pciDeviceListGet; -pciDeviceListNew; -pciDeviceListSteal; -pciDeviceListStealIndex; -pciDeviceNetName; -pciDeviceReAttachInit; -pciDeviceSetManaged; -pciDeviceSetRemoveSlot; -pciDeviceSetReprobe; -pciDeviceSetUnbindFromStub; -pciDeviceSetUsedBy; -pciFreeDevice; -pciGetDevice; -pciGetPhysicalFunction; -pciGetVirtualFunctionIndex; -pciGetVirtualFunctions; -pciReAttachDevice; -pciResetDevice; -pciWaitForDeviceCleanup; - - # secret_conf.h virSecretDefFormat; virSecretDefFree; @@ -1165,205 +800,143 @@ virStorageEncryptionParseNode; virStorageGenerateQcowPassphrase; -# storage_file.h -virStorageFileChainLookup; -virStorageFileFormatTypeFromString; -virStorageFileFormatTypeToString; -virStorageFileFreeMetadata; -virStorageFileGetLVMKey; -virStorageFileGetMetadata; -virStorageFileGetMetadataFromFD; -virStorageFileGetSCSIKey; -virStorageFileIsClusterFS; -virStorageFileIsSharedFS; -virStorageFileIsSharedFSType; -virStorageFileProbeFormat; -virStorageFileProbeFormatFromFD; -virStorageFileResize; +# viralloc.h +virAlloc; +virAllocN; +virAllocVar; +virDeleteElementsN; +virExpandN; +virFree; +virInsertElementsN; +virReallocN; +virResizeN; +virShrinkN; -# sysinfo.h -virSysinfoDefFree; -virSysinfoFormat; -virSysinfoRead; -virSysinfoSetup; +# virarch.h +virArchFromHost; +virArchFromString; +virArchGetEndian; +virArchGetWordSize; +virArchToString; -# threadpool.h -virThreadPoolFree; -virThreadPoolGetMaxWorkers; -virThreadPoolGetMinWorkers; -virThreadPoolGetPriorityWorkers; -virThreadPoolNew; -virThreadPoolSendJob; +# viraudit.h +virAuditClose; +virAuditEncode; +virAuditLog; +virAuditOpen; +virAuditSend; -# threads.h -virCondBroadcast; -virCondDestroy; -virCondInit; -virCondSignal; -virCondWait; -virCondWaitUntil; -virMutexDestroy; -virMutexInit; -virMutexInitRecursive; -virMutexLock; -virMutexUnlock; -virOnce; -virThreadCreate; -virThreadID; -virThreadInitialize; -virThreadIsSelf; -virThreadJoin; -virThreadSelf; -virThreadSelfID; +# virauth.h +virAuthGetConfigFilePath; +virAuthGetPassword; +virAuthGetUsername; -# usb.h -usbDeviceFileIterate; -usbDeviceGetBus; -usbDeviceGetDevno; -usbDeviceGetName; -usbDeviceGetUsedBy; -usbDeviceListAdd; -usbDeviceListCount; -usbDeviceListDel; -usbDeviceListFind; -usbDeviceListFree; -usbDeviceListGet; -usbDeviceListNew; -usbDeviceListSteal; -usbDeviceSetUsedBy; -usbFindDevice; -usbFindDeviceByBus; -usbFindDeviceByVendor; -usbFreeDevice; -usbGetDevice; - -# util.h -saferead; -safewrite; -safezero; -virArgvToString; -virAsprintf; -virBuildPathInternal; -virDirCreate; -virDoubleToStr; -virEnumFromString; -virEnumToString; -virFileAbsPath; -virFileAccessibleAs; -virFileBuildPath; -virFileExists; -virFileFindMountPoint; -virFileHasSuffix; -virFileIsAbsPath; -virFileIsDir; -virFileIsExecutable; -virFileIsLink; -virFileLinkPointsTo; -virFileLock; -virFileMakePath; -virFileMakePathWithMode; -virFileMatchesNameSuffix; -virFileOpenAs; -virFileOpenTty; -virFileReadAll; -virFileReadLimFD; -virFileResolveAllLinks; -virFileResolveLink; -virFileSanitizePath; -virFileSkipRoot; -virFileStripSuffix; -virFileUnlock; -virFileWaitForDevices; -virFileWriteStr; -virFindFileInPath; -virFormatIntDecimal; -virGetDeviceID; -virGetDeviceUnprivSGIO; -virGetGroupID; -virGetGroupName; -virGetHostname; -virGetUnprivSGIOSysfsPath; -virGetUserCacheDirectory; -virGetUserConfigDirectory; -virGetUserDirectory; -virGetUserID; -virGetUserName; -virGetUserRuntimeDirectory; -virHexToBin; -virIndexToDiskName; -virIsCapableFCHost; -virIsCapableVport; -virIsDevMapperDevice; -virManageVport; -virParseNumber; -virParseVersionString; -virPipeReadUntilEOF; -virReadFCHost; -virScaleInteger; -virSetBlocking; -virSetCloseExec; -virSetDeviceUnprivSGIO; -virSetInherit; -virSetNonBlock; -virSetUIDGID; -virSkipSpaces; -virSkipSpacesAndBackslash; -virSkipSpacesBackwards; -virStrcpy; -virStrIsPrint; -virStrncpy; -virStrToDouble; -virStrToLong_i; -virStrToLong_l; -virStrToLong_ll; -virStrToLong_ui; -virStrToLong_ul; -virStrToLong_ull; -virTrimSpaces; -virValidateWWN; -virVasprintf; - - -# uuid.h -virGetHostUUID; -virSetHostUUIDStr; -virUUIDFormat; -virUUIDGenerate; -virUUIDIsValid; -virUUIDParse; +# virauthconfig.h +virAuthConfigFree; +virAuthConfigLookup; +virAuthConfigNew; +virAuthConfigNewData; -# virauth.h -virAuthGetConfigFilePath; -virAuthGetPassword; -virAuthGetUsername; - - -# virauthconfig.h -virAuthConfigFree; -virAuthConfigLookup; -virAuthConfigNew; -virAuthConfigNewData; +# virbitmap.h +virBitmapClearAll; +virBitmapClearBit; +virBitmapCopy; +virBitmapCountBits; +virBitmapEqual; +virBitmapFormat; +virBitmapFree; +virBitmapGetBit; +virBitmapIsAllSet; +virBitmapNew; +virBitmapNewCopy; +virBitmapNewData; +virBitmapNextSetBit; +virBitmapParse; +virBitmapSetAll; +virBitmapSetBit; +virBitmapSize; +virBitmapString; +virBitmapToData; -# viraudit.h -virAuditClose; -virAuditEncode; -virAuditLog; -virAuditOpen; -virAuditSend; +# virbuffer.h +virBufferAdd; +virBufferAddChar; +virBufferAdjustIndent; +virBufferAsprintf; +virBufferContentAndReset; +virBufferCurrentContent; +virBufferError; +virBufferEscape; +virBufferEscapeSexpr; +virBufferEscapeShell; +virBufferEscapeString; +virBufferFreeAndReset; +virBufferGetIndent; +virBufferStrcat; +virBufferTrim; +virBufferURIEncodeString; +virBufferUse; +virBufferVasprintf; -# virarch.h -virArchFromHost; -virArchFromString; -virArchGetEndian; -virArchGetWordSize; -virArchToString; +# vircgroup.h +virCgroupAddTask; +virCgroupAddTaskController; +virCgroupAllowDevice; +virCgroupAllowDeviceMajor; +virCgroupAllowDevicePath; +virCgroupControllerTypeFromString; +virCgroupControllerTypeToString; +virCgroupDenyAllDevices; +virCgroupDenyDevice; +virCgroupDenyDeviceMajor; +virCgroupDenyDevicePath; +virCgroupForDomain; +virCgroupForDriver; +virCgroupForEmulator; +virCgroupForVcpu; +virCgroupFree; +virCgroupGetAppRoot; +virCgroupGetBlkioWeight; +virCgroupGetCpuacctPercpuUsage; +virCgroupGetCpuacctStat; +virCgroupGetCpuacctUsage; +virCgroupGetCpuCfsPeriod; +virCgroupGetCpuCfsQuota; +virCgroupGetCpusetCpus; +virCgroupGetCpusetMems; +virCgroupGetCpuShares; +virCgroupGetFreezerState; +virCgroupGetMemoryHardLimit; +virCgroupGetMemorySoftLimit; +virCgroupGetMemoryUsage; +virCgroupGetMemSwapHardLimit; +virCgroupGetMemSwapUsage; +virCgroupKill; +virCgroupKillPainfully; +virCgroupKillRecursive; +virCgroupMounted; +virCgroupMoveTask; +virCgroupPathOfController; +virCgroupRemove; +virCgroupSetBlkioDeviceWeight; +virCgroupSetBlkioWeight; +virCgroupSetCpuCfsPeriod; +virCgroupSetCpuCfsQuota; +virCgroupSetCpusetCpus; +virCgroupSetCpusetMems; +virCgroupSetCpuShares; +virCgroupSetFreezerState; +virCgroupSetMemory; +virCgroupSetMemoryHardLimit; +virCgroupSetMemorySoftLimit; +virCgroupSetMemSwapHardLimit; # virchrdev.h @@ -1372,11 +945,107 @@ virChrdevFree; virChrdevOpen; +# vircommand.h +virCommandAbort; +virCommandAddArg; +virCommandAddArgBuffer; +virCommandAddArgFormat; +virCommandAddArgList; +virCommandAddArgPair; +virCommandAddArgSet; +virCommandAddEnvBuffer; +virCommandAddEnvFormat; +virCommandAddEnvPair; +virCommandAddEnvPass; +virCommandAddEnvPassCommon; +virCommandAddEnvString; +virCommandAllowCap; +virCommandClearCaps; +virCommandDaemonize; +virCommandExec; +virCommandFree; +virCommandHandshakeNotify; +virCommandHandshakeWait; +virCommandNew; +virCommandNewArgList; +virCommandNewArgs; +virCommandNonblockingFDs; +virCommandPreserveFD; +virCommandRequireHandshake; +virCommandRun; +virCommandRunAsync; +virCommandSetErrorBuffer; +virCommandSetErrorFD; +virCommandSetInputBuffer; +virCommandSetInputFD; +virCommandSetOutputBuffer; +virCommandSetOutputFD; +virCommandSetPidFile; +virCommandSetPreExecHook; +virCommandSetWorkingDirectory; +virCommandToString; +virCommandTransferFD; +virCommandWait; +virCommandWriteArgLog; +virFork; +virRun; + + +# virconf.h +virConfFree; +virConfFreeValue; +virConfGetValue; +virConfNew; +virConfReadFile; +virConfReadMem; +virConfSetValue; +virConfWriteFile; +virConfWriteMem; + + # virdbus.h virDBusGetSessionBus; virDBusGetSystemBus; +# virdnsmasq.h +dnsmasqAddDhcpHost; +dnsmasqAddHost; +dnsmasqCapsGet; +dnsmasqCapsGetBinaryPath; +dnsmasqCapsGetVersion; +dnsmasqCapsNewFromBinary; +dnsmasqCapsNewFromBuffer; +dnsmasqCapsNewFromFile; +dnsmasqCapsRefresh; +dnsmasqContextFree; +dnsmasqContextNew; +dnsmasqDelete; +dnsmasqReload; +dnsmasqSave; + + +# virebtables.h +ebtablesAddForwardAllowIn; +ebtablesAddForwardPolicyReject; +ebtablesContextFree; +ebtablesContextNew; +ebtablesRemoveForwardAllowIn; + + +# vireventpoll.h +virEventPollAddHandle; +virEventPollAddTimeout; +virEventPollFromNativeEvents; +virEventPollInit; +virEventPollRemoveHandle; +virEventPollRemoveTimeout; +virEventPollRunOnce; +virEventPollToNativeEvents; +virEventPollUpdateHandle; +virEventPollUpdateTimeout; + + # virfile.h virFileClose; virFileDirectFdFlag; @@ -1392,10 +1061,109 @@ virFileWrapperFdFree; virFileWrapperFdNew; +# virhash.h +virHashAddEntry; +virHashCreate; +virHashEqual; +virHashForEach; +virHashFree; +virHashGetItems; +virHashLookup; +virHashRemoveAll; +virHashRemoveEntry; +virHashRemoveSet; +virHashSearch; +virHashSize; +virHashSteal; +virHashTableSize; +virHashUpdateEntry; + + +# virhooks.h +virHookCall; +virHookInitialize; +virHookPresent; + + # virinitctl.h virInitctlSetRunLevel; +# viriptables.h +iptablesAddForwardAllowCross; +iptablesAddForwardAllowIn; +iptablesAddForwardAllowOut; +iptablesAddForwardAllowRelatedIn; +iptablesAddForwardMasquerade; +iptablesAddForwardRejectIn; +iptablesAddForwardRejectOut; +iptablesAddOutputFixUdpChecksum; +iptablesAddTcpInput; +iptablesAddUdpInput; +iptablesContextFree; +iptablesContextNew; +iptablesRemoveForwardAllowCross; +iptablesRemoveForwardAllowIn; +iptablesRemoveForwardAllowOut; +iptablesRemoveForwardAllowRelatedIn; +iptablesRemoveForwardMasquerade; +iptablesRemoveForwardRejectIn; +iptablesRemoveForwardRejectOut; +iptablesRemoveOutputFixUdpChecksum; +iptablesRemoveTcpInput; +iptablesRemoveUdpInput; + + +# virjson.h +virJSONValueArrayAppend; +virJSONValueArrayGet; +virJSONValueArraySize; +virJSONValueFree; +virJSONValueFromString; +virJSONValueGetBoolean; +virJSONValueGetNumberDouble; +virJSONValueGetNumberInt; +virJSONValueGetNumberLong; +virJSONValueGetNumberUint; +virJSONValueGetNumberUlong; +virJSONValueGetString; +virJSONValueIsNull; +virJSONValueNewArray; +virJSONValueNewBoolean; +virJSONValueNewNull; +virJSONValueNewNumberDouble; +virJSONValueNewNumberInt; +virJSONValueNewNumberLong; +virJSONValueNewNumberUint; +virJSONValueNewNumberUlong; +virJSONValueNewObject; +virJSONValueNewString; +virJSONValueNewStringLen; +virJSONValueObjectAppend; +virJSONValueObjectAppendBoolean; +virJSONValueObjectAppendNull; +virJSONValueObjectAppendNumberDouble; +virJSONValueObjectAppendNumberInt; +virJSONValueObjectAppendNumberLong; +virJSONValueObjectAppendNumberUint; +virJSONValueObjectAppendNumberUlong; +virJSONValueObjectAppendString; +virJSONValueObjectGet; +virJSONValueObjectGetBoolean; +virJSONValueObjectGetKey; +virJSONValueObjectGetNumberDouble; +virJSONValueObjectGetNumberInt; +virJSONValueObjectGetNumberLong; +virJSONValueObjectGetNumberUint; +virJSONValueObjectGetNumberUlong; +virJSONValueObjectGetString; +virJSONValueObjectGetValue; +virJSONValueObjectHasKey; +virJSONValueObjectIsNull; +virJSONValueObjectKeysNumber; +virJSONValueToString; + + # virkeycode.h virKeycodeSetTypeFromString; virKeycodeSetTypeToString; @@ -1426,6 +1194,27 @@ virLockSpaceReleaseResource; virLockSpaceReleaseResourcesForOwner; +# virlog.h +virLogDefineFilter; +virLogDefineOutput; +virLogEmergencyDumpAll; +virLogGetDefaultPriority; +virLogGetFilters; +virLogGetNbFilters; +virLogGetNbOutputs; +virLogGetOutputs; +virLogLock; +virLogMessage; +virLogParseDefaultPriority; +virLogParseFilters; +virLogParseOutputs; +virLogReset; +virLogSetBufferSize; +virLogSetDefaultPriority; +virLogSetFromEnv; +virLogUnlock; + + # virmacaddr.h virMacAddrCmp; virMacAddrCmpRaw; @@ -1590,7 +1379,7 @@ virNetDevVPortProfileOpTypeFromString; virNetDevVPortProfileOpTypeToString; -#virnetlink.h +# virnetlink.h virNetlinkCommand; virNetlinkEventAddClient; virNetlinkEventRemoveClient; @@ -1782,6 +1571,46 @@ virObjectRef; virObjectUnref; +# virpci.h +pciConfigAddressToSysfsFile; +pciDettachDevice; +pciDeviceFileIterate; +pciDeviceGetManaged; +pciDeviceGetName; +pciDeviceGetRemoveSlot; +pciDeviceGetReprobe; +pciDeviceGetUnbindFromStub; +pciDeviceGetUsedBy; +pciDeviceGetVirtualFunctionInfo; +pciDeviceIsAssignable; +pciDeviceIsVirtualFunction; +pciDeviceListAdd; +pciDeviceListCount; +pciDeviceListDel; +pciDeviceListFind; +pciDeviceListFindIndex; +pciDeviceListFree; +pciDeviceListGet; +pciDeviceListNew; +pciDeviceListSteal; +pciDeviceListStealIndex; +pciDeviceNetName; +pciDeviceReAttachInit; +pciDeviceSetManaged; +pciDeviceSetRemoveSlot; +pciDeviceSetReprobe; +pciDeviceSetUnbindFromStub; +pciDeviceSetUsedBy; +pciFreeDevice; +pciGetDevice; +pciGetPhysicalFunction; +pciGetVirtualFunctionIndex; +pciGetVirtualFunctions; +pciReAttachDevice; +pciResetDevice; +pciWaitForDeviceCleanup; + + # virpidfile.h virPidFileAcquire; virPidFileAcquirePath; @@ -1838,6 +1667,36 @@ virSocketAddrSetIPv4Addr; virSocketAddrSetPort; +# virstoragefile.h +virStorageFileChainLookup; +virStorageFileFormatTypeFromString; +virStorageFileFormatTypeToString; +virStorageFileFreeMetadata; +virStorageFileGetLVMKey; +virStorageFileGetMetadata; +virStorageFileGetMetadataFromFD; +virStorageFileGetSCSIKey; +virStorageFileIsClusterFS; +virStorageFileIsSharedFS; +virStorageFileIsSharedFSType; +virStorageFileProbeFormat; +virStorageFileProbeFormatFromFD; +virStorageFileResize; + + +# virstring.h +virStringFreeList; +virStringJoin; +virStringSplit; + + +# virsysinfo.h +virSysinfoDefFree; +virSysinfoFormat; +virSysinfoRead; +virSysinfoSetup; + + # virterror_internal.h virDispatchError; virErrorInitialize; @@ -1850,10 +1709,35 @@ virSetErrorLogPriorityFunc; virStrerror; -# virstring.h -virStringFreeList; -virStringJoin; -virStringSplit; +# virthread.h +virCondBroadcast; +virCondDestroy; +virCondInit; +virCondSignal; +virCondWait; +virCondWaitUntil; +virMutexDestroy; +virMutexInit; +virMutexInitRecursive; +virMutexLock; +virMutexUnlock; +virOnce; +virThreadCreate; +virThreadID; +virThreadInitialize; +virThreadIsSelf; +virThreadJoin; +virThreadSelf; +virThreadSelfID; + + +# virthreadpool.h +virThreadPoolFree; +virThreadPoolGetMaxWorkers; +virThreadPoolGetMinWorkers; +virThreadPoolGetPriorityWorkers; +virThreadPoolNew; +virThreadPoolSendJob; # virtime.h @@ -1883,7 +1767,125 @@ virURIFree; virURIParse; -# xml.h +# virusb.h +usbDeviceFileIterate; +usbDeviceGetBus; +usbDeviceGetDevno; +usbDeviceGetName; +usbDeviceGetUsedBy; +usbDeviceListAdd; +usbDeviceListCount; +usbDeviceListDel; +usbDeviceListFind; +usbDeviceListFree; +usbDeviceListGet; +usbDeviceListNew; +usbDeviceListSteal; +usbDeviceSetUsedBy; +usbFindDevice; +usbFindDeviceByBus; +usbFindDeviceByVendor; +usbFreeDevice; +usbGetDevice; + + +# virutil.h +saferead; +safewrite; +safezero; +virArgvToString; +virAsprintf; +virBuildPathInternal; +virDirCreate; +virDoubleToStr; +virEnumFromString; +virEnumToString; +virFileAbsPath; +virFileAccessibleAs; +virFileBuildPath; +virFileExists; +virFileFindMountPoint; +virFileHasSuffix; +virFileIsAbsPath; +virFileIsDir; +virFileIsExecutable; +virFileIsLink; +virFileLinkPointsTo; +virFileLock; +virFileMakePath; +virFileMakePathWithMode; +virFileMatchesNameSuffix; +virFileOpenAs; +virFileOpenTty; +virFileReadAll; +virFileReadLimFD; +virFileResolveAllLinks; +virFileResolveLink; +virFileSanitizePath; +virFileSkipRoot; +virFileStripSuffix; +virFileUnlock; +virFileWaitForDevices; +virFileWriteStr; +virFindFileInPath; +virFormatIntDecimal; +virGetDeviceID; +virGetDeviceUnprivSGIO; +virGetGroupID; +virGetGroupName; +virGetHostname; +virGetUnprivSGIOSysfsPath; +virGetUserCacheDirectory; +virGetUserConfigDirectory; +virGetUserDirectory; +virGetUserID; +virGetUserName; +virGetUserRuntimeDirectory; +virHexToBin; +virIndexToDiskName; +virIsCapableFCHost; +virIsCapableVport; +virIsDevMapperDevice; +virManageVport; +virParseNumber; +virParseVersionString; +virPipeReadUntilEOF; +virReadFCHost; +virScaleInteger; +virSetBlocking; +virSetCloseExec; +virSetDeviceUnprivSGIO; +virSetInherit; +virSetNonBlock; +virSetUIDGID; +virSkipSpaces; +virSkipSpacesAndBackslash; +virSkipSpacesBackwards; +virStrcpy; +virStrIsPrint; +virStrncpy; +virStrToDouble; +virStrToLong_i; +virStrToLong_l; +virStrToLong_ll; +virStrToLong_ui; +virStrToLong_ul; +virStrToLong_ull; +virTrimSpaces; +virValidateWWN; +virVasprintf; + + +# viruuid.h +virGetHostUUID; +virSetHostUUIDStr; +virUUIDFormat; +virUUIDGenerate; +virUUIDIsValid; +virUUIDParse; + + +# virxml.h virXMLChildElementCount; virXMLParseHelper; virXMLPickShellSafeComment; -- 1.7.7.6

On 14.01.2013 15:34, Osier Yang wrote:
This fixes the header names (all util files are named with "vir" prefix now. Some of them are not util files, but the name has been changed), and sort the whole libvirt_private.syms by the header name after the name fixes. --- src/libvirt_private.syms | 1164 +++++++++++++++++++++++----------------------- 1 files changed, 583 insertions(+), 581 deletions(-)
Wow. slightly bigger change, but splitting it into slices doesn't make any sense. I did compile check and it passed. So you have my ACK, but I would give others some time to make deeper review if they want to. Michal

On 01/14/2013 12:42 PM, Michal Privoznik wrote:
On 14.01.2013 15:34, Osier Yang wrote:
This fixes the header names (all util files are named with "vir" prefix now. Some of them are not util files, but the name has been changed), and sort the whole libvirt_private.syms by the header name after the name fixes. --- src/libvirt_private.syms | 1164 +++++++++++++++++++++++----------------------- 1 files changed, 583 insertions(+), 581 deletions(-)
Wow. slightly bigger change, but splitting it into slices doesn't make any sense. I did compile check and it passed. So you have my ACK, but I would give others some time to make deeper review if they want to.
Looked good in my quick read through as well. No complaints if you push it, since 'make check' validates that we aren't exposing too many or missing any names. ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 2013年01月14日 22:34, Osier Yang wrote:
This is the second part of [1]. Some of them are preparations for persistent vHBA support in storage pool, some are fixes, some are improvements.
[1] https://www.redhat.com/archives/libvir-list/2013-January/msg00328.html
v1 - v2: * One more patch: 9/9
Osier Yang (9): nodedev: Remove the unused enum nodedev: Introduce two new flags for listAll API util: Add one helper virReadFCHost to read the value of fc_host entry nodedev: Use access instead of stat nodedev: Refactor the helpers nodedev: Dump max vports and vports in use for HBA's XML nodedev: Fix the improper logic when enumerating SRIOV VF nodedev: Abstract nodeDeviceVportCreateDelete as util function cleanup libvirt_private.syms
docs/formatnode.html.in | 10 +- docs/schemas/nodedev.rng | 6 + include/libvirt/libvirt.h.in | 20 +- src/conf/node_device_conf.c | 41 +- src/conf/node_device_conf.h | 16 +- src/libvirt.c | 2 + src/libvirt_private.syms | 1160 +++++++++++++++-------------- src/node_device/node_device_driver.c | 110 +--- src/node_device/node_device_driver.h | 19 +- src/node_device/node_device_hal.c | 15 +- src/node_device/node_device_linux_sysfs.c | 245 ++---- src/node_device/node_device_udev.c | 14 +- src/util/virpci.c | 36 +- src/util/virutil.c | 224 ++++++ src/util/virutil.h | 19 + tools/virsh-nodedev.c | 6 + tools/virsh.pod | 7 +- 17 files changed, 1034 insertions(+), 916 deletions(-)
Regards, Osier
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
participants (3)
-
Eric Blake
-
Michal Privoznik
-
Osier Yang