On 01/02/2017 09:51 AM, Ján Tomko wrote:
On Fri, Nov 18, 2016 at 09:26:36AM -0500, John Ferlan wrote:
Create a utility routine in order to read the scsi_host fabric_name files looking for a match to a passed fabric_name
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/libvirt_private.syms | 1 + src/util/virutil.c | 86 ++++++++++++++++++++++++++++++++++++++++-------- src/util/virutil.h | 4 +++ 3 files changed, 78 insertions(+), 13 deletions(-)
diff --git a/src/util/virutil.c b/src/util/virutil.c index a135819..fb72f2d 100644 --- a/src/util/virutil.c +++ b/src/util/virutil.c @@ -2166,6 +2166,18 @@ virManageVport(const int parent_host, return ret; }
+# define READ_WWN(wwn_path, buf) \
This macro either jumps to a label or alters a third variable. I don't think its scope should extend one function.
+ do { \
+ if (virFileReadAll(wwn_path, 1024, &buf) < 0) \ + goto cleanup; \
These two lines can be split out and with the STR{,N}EQ included, this macro can be COMPARE_WWN, removing the need for changing p. Or turned into a more complex function like virUSBSysReadFile.
Not quite sure why it's felt 'p' wouldn't need changing. On my test system the "port_name", "node_name", and "fabric_wwn" files each have "0x######\n", but the stored/XML format will be ###### Maybe it's just late and I'm overthinking it.
+ if ((p = strchr(buf, '\n'))) \ + *p = '\0'; \ + if (STRPREFIX(buf, "0x")) \ + p = buf + strlen("0x"); \ + else \ + p = buf; \ + } while (0) + /* virGetFCHostNameByWWN: * * Iterate over the sysfs tree to get FC host name (e.g. host5)
The rest looks good to me.
So I went with a function: /* virReadCompareWWN * @prefix: path to the wwn file * @d_name: name of the current directory * @f_name: file name to read * * Read/compare the on-disk file with the passed wwn value. * * Returns: * -1 : Error * 0 : No match * 1 : Match */ static int virReadCompareWWN(const char *prefix, const char *d_name, const char *f_name, const char *wwn) { char *path; char *buf = NULL; char *p; int ret = -1; if (virAsprintf(&path, "%s/%s/%s", prefix, d_name, f_name) < 0) return -1; if (!virFileExists(path)) { ret = 0; goto cleanup; } if (virFileReadAll(path, 1024, &buf) < 0) goto cleanup; if ((p = strchr(buf, '\n'))) *p = '\0'; if (STRPREFIX(buf, "0x")) p = buf + strlen("0x"); else p = buf; if (STRNEQ(wwn, p)) ret = 0; else ret = 1; cleanup: VIR_FREE(path); VIR_FREE(buf); return ret; } It should be obvious what the callers do, but again I can post an updated series as long as you're generally fine with this. Tks - John