Sign Up
Log In
Log In
or
Sign Up
Places
All Projects
Status Monitor
Collapse sidebar
SUSE:SLE-15-SP2:GA
efivar
efivar-bsc1181967-fix-nvme-parsing.patch
Overview
Repositories
Revisions
Requests
Users
Attributes
Meta
File efivar-bsc1181967-fix-nvme-parsing.patch of Package efivar
From 180f2b162ef8748924a50775af82d5f0c0af0c49 Mon Sep 17 00:00:00 2001 From: Peter Jones <pjones@redhat.com> Date: Wed, 2 Oct 2019 17:01:00 -0400 Subject: [PATCH 01/10] Fix the error path in set_disk_and_part_name() Signed-off-by: Peter Jones <pjones@redhat.com> (cherry picked from commit 22bc0866e941cbfe57de6522db51e9cf2c6b3ff1) --- src/linux.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/linux.c b/src/linux.c index 6d405af..5ee4f99 100644 --- a/src/linux.c +++ b/src/linux.c @@ -1,6 +1,6 @@ /* * libefiboot - library for the manipulation of EFI boot variables - * Copyright 2012-2015 Red Hat, Inc. + * Copyright 2012-2019 Red Hat, Inc. * Copyright (C) 2001 Dell Computer Corporation <Matt_Domsch@dell.com> * * This library is free software; you can redistribute it and/or @@ -169,6 +169,8 @@ set_disk_name(struct device *dev, const char * const fmt, ...) int HIDDEN set_disk_and_part_name(struct device *dev) { + int rc = -1; + /* * results are like such: * maj:min -> ../../devices/pci$PCI_STUFF/$BLOCKDEV_STUFF/block/$DISK/$PART @@ -200,6 +202,7 @@ set_disk_and_part_name(struct device *dev) set_disk_name(dev, "%s", penultimate); set_part_name(dev, "%s", ultimate); debug("disk:%s part:%s", penultimate, ultimate); + rc = 0; } else if (ultimate && approximate && !strcmp(approximate, "nvme")) { /* * 259:0 -> ../../devices/pci0000:00/0000:00:1d.0/0000:05:00.0/nvme/nvme0/nvme0n1 @@ -207,6 +210,7 @@ set_disk_and_part_name(struct device *dev) set_disk_name(dev, "%s", ultimate); set_part_name(dev, "%sp%d", ultimate, dev->part); debug("disk:%s part:%sp%d", ultimate, ultimate, dev->part); + rc = 0; } else if (ultimate && penultimate && !strcmp(penultimate, "block")) { /* * 253:0 -> ../../devices/virtual/block/dm-0 (... I guess) @@ -220,15 +224,19 @@ set_disk_and_part_name(struct device *dev) set_disk_name(dev, "%s", ultimate); set_part_name(dev, "%s%d", ultimate, dev->part); debug("disk:%s part:%s%d", ultimate, ultimate, dev->part); + rc = 0; } else if (ultimate && approximate && !strcmp(approximate, "mtd")) { /* * 31:0 -> ../../devices/platform/1e000000.palmbus/1e000b00.spi/spi_master/spi32766/spi32766.0/mtd/mtd0/mtdblock0 */ set_disk_name(dev, "%s", ultimate); debug("disk:%s", ultimate); + rc = 0; } - return 0; + if (rc < 0) + efi_error("Could not parse disk name:\"%s\"", dev->link); + return rc; } static struct dev_probe *dev_probes[] = { -- 2.29.2 From 381cd7236498f4759a5d14a2ddc2651fd05839b9 Mon Sep 17 00:00:00 2001 From: Peter Jones <pjones@redhat.com> Date: Thu, 21 Feb 2019 15:26:23 -0500 Subject: [PATCH 02/10] Get rid of the arrows in our debug messages. They're not *that* useful, and the code is clever and problematic. Resolves github issue #124 Signed-off-by: Peter Jones <pjones@redhat.com> (cherry picked from commit e39fb5b9e590c6616459c15b5d95fbc09fb077d6) --- src/linux-acpi-root.c | 7 ------- src/linux-emmc.c | 9 --------- src/linux-md.c | 8 -------- src/linux-nvme.c | 9 --------- src/linux-pci-root.c | 7 ------- src/linux-pci.c | 8 -------- src/linux-sata.c | 11 ----------- src/linux-scsi.c | 24 ------------------------ src/linux-soc-root.c | 7 ------- src/linux-virtblk.c | 8 -------- src/util.h | 1 - 11 files changed, 99 deletions(-) diff --git a/src/linux-acpi-root.c b/src/linux-acpi-root.c index 06e69ee..30728de 100644 --- a/src/linux-acpi-root.c +++ b/src/linux-acpi-root.c @@ -51,13 +51,6 @@ parse_acpi_root(struct device *dev, const char *current, const char *root UNUSED char *colon; const char *devpart = current; - char *spaces; - - pos = strlen(current); - spaces = alloca(pos+1); - memset(spaces, ' ', pos+1); - spaces[pos] = '\0'; - pos = 0; debug("entry"); diff --git a/src/linux-emmc.c b/src/linux-emmc.c index 87e9247..b290ed0 100644 --- a/src/linux-emmc.c +++ b/src/linux-emmc.c @@ -50,13 +50,6 @@ parse_emmc(struct device *dev, const char *current, const char *root UNUSED) int rc; int32_t tosser0, tosser1, tosser2, tosser3, slot_id, partition; int pos0 = 0, pos1 = 0; - char *spaces; - - pos0 = strlen(current); - spaces = alloca(pos0+1); - memset(spaces, ' ', pos0+1); - spaces[pos0] = '\0'; - pos0 = 0; debug("entry"); @@ -65,8 +58,6 @@ parse_emmc(struct device *dev, const char *current, const char *root UNUSED) &tosser0, &tosser1, &tosser2, &slot_id, &pos0, &tosser3, &partition, &pos1); debug("current:\"%s\" rc:%d pos0:%d pos1:%d\n", current, rc, pos0, pos1); - arrow(LOG_DEBUG, spaces, 9, pos0, rc, 4); - arrow(LOG_DEBUG, spaces, 9, pos1, rc, 6); /* * If it isn't of that form, it's not one of our emmc devices. */ diff --git a/src/linux-md.c b/src/linux-md.c index 0a5c1cd..cb584c9 100644 --- a/src/linux-md.c +++ b/src/linux-md.c @@ -44,13 +44,6 @@ parse_md(struct device *dev, const char *current, const char *root UNUSED) int rc; int32_t md, tosser0, part; int pos0 = 0, pos1 = 0; - char *spaces; - - pos0 = strlen(current); - spaces = alloca(pos0+1); - memset(spaces, ' ', pos0+1); - spaces[pos0] = '\0'; - pos0 = 0; debug("entry"); @@ -58,7 +51,6 @@ parse_md(struct device *dev, const char *current, const char *root UNUSED) rc = sscanf(current, "md%d/%nmd%dp%d%n", &md, &pos0, &tosser0, &part, &pos1); debug("current:\"%s\" rc:%d pos0:%d pos1:%d\n", current, rc, pos0, pos1); - arrow(LOG_DEBUG, spaces, 9, pos0, rc, 3); /* * If it isn't of that form, it's not one of our partitioned md devices. */ diff --git a/src/linux-nvme.c b/src/linux-nvme.c index d68d11a..1d8fc65 100644 --- a/src/linux-nvme.c +++ b/src/linux-nvme.c @@ -54,13 +54,6 @@ parse_nvme(struct device *dev, const char *current, const char *root UNUSED) int32_t tosser0, tosser1, tosser2, ctrl_id, ns_id, partition; uint8_t *filebuf = NULL; int pos0 = 0, pos1 = 0; - char *spaces; - - pos0 = strlen(current); - spaces = alloca(pos0+1); - memset(spaces, ' ', pos0+1); - spaces[pos0] = '\0'; - pos0 = 0; debug("entry"); @@ -69,8 +62,6 @@ parse_nvme(struct device *dev, const char *current, const char *root UNUSED) &tosser0, &ctrl_id, &ns_id, &pos0, &tosser1, &tosser2, &partition, &pos1); debug("current:\"%s\" rc:%d pos0:%d pos1:%d\n", current, rc, pos0, pos1); - arrow(LOG_DEBUG, spaces, 9, pos0, rc, 3); - arrow(LOG_DEBUG, spaces, 9, pos1, rc, 6); /* * If it isn't of that form, it's not one of our nvme devices. */ diff --git a/src/linux-pci-root.c b/src/linux-pci-root.c index a2d9fb0..5d0e471 100644 --- a/src/linux-pci-root.c +++ b/src/linux-pci-root.c @@ -48,13 +48,6 @@ parse_pci_root(struct device *dev, const char *current, const char *root UNUSED) uint16_t root_domain; uint8_t root_bus; const char *devpart = current; - char *spaces; - - pos = strlen(current); - spaces = alloca(pos+1); - memset(spaces, ' ', pos+1); - spaces[pos] = '\0'; - pos = 0; debug("entry"); diff --git a/src/linux-pci.c b/src/linux-pci.c index f63f591..64aaefb 100644 --- a/src/linux-pci.c +++ b/src/linux-pci.c @@ -48,13 +48,6 @@ parse_pci(struct device *dev, const char *current, const char *root) int rc; int pos; const char *devpart = current; - char *spaces; - - pos = strlen(current); - spaces = alloca(pos+1); - memset(spaces, ' ', pos+1); - spaces[pos] = '\0'; - pos = 0; debug("entry"); @@ -75,7 +68,6 @@ parse_pci(struct device *dev, const char *current, const char *root) rc = sscanf(devpart, "%hx:%hhx:%hhx.%hhx/%n", &domain, &bus, &device, &function, &pos); debug("current:\"%s\" rc:%d pos:%d", devpart, rc, pos); - arrow(LOG_DEBUG, spaces, 9, pos, rc, 3); if (rc != 4) break; devpart += pos; diff --git a/src/linux-sata.c b/src/linux-sata.c index 8526502..3564117 100644 --- a/src/linux-sata.c +++ b/src/linux-sata.c @@ -148,13 +148,6 @@ parse_sata(struct device *dev, const char *devlink, const char *root UNUSED) uint64_t scsi_lun, tosser3; int pos = 0; int rc; - char *spaces; - - pos = strlen(current); - spaces = alloca(pos+1); - memset(spaces, ' ', pos+1); - spaces[pos] = '\0'; - pos = 0; debug("entry"); if (is_pata(dev)) { @@ -169,7 +162,6 @@ parse_sata(struct device *dev, const char *devlink, const char *root UNUSED) debug("searching for ata1/"); rc = sscanf(current, "ata%"PRIu32"/%n", &print_id, &pos); debug("current:\"%s\" rc:%d pos:%d\n", current, rc, pos); - arrow(LOG_DEBUG, spaces, 9, pos, rc, 1); /* * If we don't find this one, it isn't an ata device, so return 0 not * error. Later errors mean it is an ata device, but we can't parse @@ -183,7 +175,6 @@ parse_sata(struct device *dev, const char *devlink, const char *root UNUSED) debug("searching for host0/"); rc = sscanf(current, "host%"PRIu32"/%n", &scsi_bus, &pos); debug("current:\"%s\" rc:%d pos:%d\n", current, rc, pos); - arrow(LOG_DEBUG, spaces, 9, pos, rc, 1); if (rc != 1) return -1; current += pos; @@ -193,7 +184,6 @@ parse_sata(struct device *dev, const char *devlink, const char *root UNUSED) rc = sscanf(current, "target%"PRIu32":%"PRIu32":%"PRIu64"/%n", &scsi_device, &scsi_target, &scsi_lun, &pos); debug("current:\"%s\" rc:%d pos:%d\n", current, rc, pos); - arrow(LOG_DEBUG, spaces, 9, pos, rc, 3); if (rc != 3) return -1; current += pos; @@ -203,7 +193,6 @@ parse_sata(struct device *dev, const char *devlink, const char *root UNUSED) rc = sscanf(current, "%"PRIu32":%"PRIu32":%"PRIu32":%"PRIu64"/%n", &tosser0, &tosser1, &tosser2, &tosser3, &pos); debug("current:\"%s\" rc:%d pos:%d\n", current, rc, pos); - arrow(LOG_DEBUG, spaces, 9, pos, rc, 4); if (rc != 4) return -1; current += pos; diff --git a/src/linux-scsi.c b/src/linux-scsi.c index a5e81cf..04892f0 100644 --- a/src/linux-scsi.c +++ b/src/linux-scsi.c @@ -45,13 +45,6 @@ parse_scsi_link(const char *current, uint32_t *scsi_host, int rc; int sz = 0; int pos0 = 0, pos1 = 0; - char *spaces; - - sz = strlen(current); - spaces = alloca(sz+1); - memset(spaces, ' ', sz+1); - spaces[sz] = '\0'; - sz = 0; debug("entry"); /* @@ -108,7 +101,6 @@ parse_scsi_link(const char *current, uint32_t *scsi_host, debug("searching for host4/"); rc = sscanf(current, "host%d/%n", scsi_host, &pos0); debug("current:\"%s\" rc:%d pos0:%d\n", current+sz, rc, pos0); - arrow(LOG_DEBUG, spaces, 9, pos0, rc, 1); if (rc != 1) return -1; sz += pos0; @@ -126,8 +118,6 @@ parse_scsi_link(const char *current, uint32_t *scsi_host, rc = sscanf(current+sz, "port-%d:%d%n:%d%n", &tosser0, &tosser1, &pos0, &tosser2, &pos1); debug("current:\"%s\" rc:%d pos0:%d pos1:%d\n", current+sz, rc, pos0, pos1); - arrow(LOG_DEBUG, spaces, 9, pos0, rc, 2); - arrow(LOG_DEBUG, spaces, 9, pos1, rc, 3); if (rc == 2 || rc == 3) { sz += pos0; pos0 = 0; @@ -153,7 +143,6 @@ parse_scsi_link(const char *current, uint32_t *scsi_host, debug("searching for expander-4:0/"); rc = sscanf(current+sz, "expander-%d:%d/%n", &tosser0, &tosser1, &pos0); debug("current:\"%s\" rc:%d pos0:%d\n", current+sz, rc, pos0); - arrow(LOG_DEBUG, spaces, 9, pos0, rc, 2); if (rc == 2) { if (!remote_target_id) { efi_error("Device is PHY is a remote target, but remote_target_id is NULL"); @@ -169,7 +158,6 @@ parse_scsi_link(const char *current, uint32_t *scsi_host, debug("searching for port-2:0:2/"); rc = sscanf(current+sz, "port-%d:%d:%d/%n", &tosser0, &tosser1, &tosser2, &pos0); debug("current:\"%s\" rc:%d pos0:%d\n", current+sz, rc, pos0); - arrow(LOG_DEBUG, spaces, 9, pos0, rc, 3); if (rc != 3) { efi_error("Couldn't parse port expander port string"); return -1; @@ -192,8 +180,6 @@ parse_scsi_link(const char *current, uint32_t *scsi_host, pos1 = 0; rc = sscanf(current + sz + pos0, ":%d%n", &tosser2, &pos1); - arrow(LOG_DEBUG, spaces, 9, pos0, rc + 2, 2); - arrow(LOG_DEBUG, spaces, 9, pos0 + pos1, rc + 2, 3); if (rc != 0 && rc != 1) return -1; if (remote_port_id && rc == 1) @@ -217,7 +203,6 @@ parse_scsi_link(const char *current, uint32_t *scsi_host, rc = sscanf(current + sz, "target%d:%d:%"PRIu64"/%n", &tosser0, &tosser1, &tosser3, &pos0); debug("current:\"%s\" rc:%d pos0:%d\n", current+sz, rc, pos0); - arrow(LOG_DEBUG, spaces, 9, pos0, rc, 3); if (rc != 3) return -1; sz += pos0; @@ -230,7 +215,6 @@ parse_scsi_link(const char *current, uint32_t *scsi_host, rc = sscanf(current + sz, "%d:%d:%d:%"PRIu64"/%n", scsi_bus, scsi_device, scsi_target, scsi_lun, &pos0); debug("current:\"%s\" rc:%d pos0:%d\n", current+sz, rc, pos0); - arrow(LOG_DEBUG, spaces, 9, pos0, rc, 4); if (rc != 4) return -1; sz += pos0; @@ -247,13 +231,6 @@ parse_scsi(struct device *dev, const char *current, const char *root UNUSED) ssize_t sz; int pos; int rc; - char *spaces; - - pos = strlen(current); - spaces = alloca(pos+1); - memset(spaces, ' ', pos+1); - spaces[pos] = '\0'; - pos = 0; debug("entry"); @@ -265,7 +242,6 @@ parse_scsi(struct device *dev, const char *current, const char *root UNUSED) &dev->scsi_info.scsi_lun, &pos); debug("current:\"%s\" rc:%d pos:%d\n", dev->device, rc, pos); - arrow(LOG_DEBUG, spaces, 9, pos, rc, 3); if (rc != 4) return 0; diff --git a/src/linux-soc-root.c b/src/linux-soc-root.c index 394f496..373cd59 100644 --- a/src/linux-soc-root.c +++ b/src/linux-soc-root.c @@ -43,13 +43,6 @@ parse_soc_root(struct device *dev UNUSED, const char *current, const char *root int rc; int pos; const char *devpart = current; - char *spaces; - - pos = strlen(current); - spaces = alloca(pos+1); - memset(spaces, ' ', pos+1); - spaces[pos] = '\0'; - pos = 0; debug("entry"); diff --git a/src/linux-virtblk.c b/src/linux-virtblk.c index c54a813..2e9889d 100644 --- a/src/linux-virtblk.c +++ b/src/linux-virtblk.c @@ -50,20 +50,12 @@ parse_virtblk(struct device *dev, const char *current, const char *root UNUSED) uint32_t tosser; int pos; int rc; - char *spaces; - - pos = strlen(current); - spaces = alloca(pos+1); - memset(spaces, ' ', pos+1); - spaces[pos] = '\0'; - pos = 0; debug("entry"); debug("searching for virtio0/"); rc = sscanf(current, "virtio%x/%n", &tosser, &pos); debug("current:\"%s\" rc:%d pos:%d\n", current, rc, pos); - arrow(LOG_DEBUG, spaces, 9, pos, rc, 1); /* * If we couldn't find virtioX/ then it isn't a virtio device. */ diff --git a/src/util.h b/src/util.h index f63a890..53f6547 100644 --- a/src/util.h +++ b/src/util.h @@ -379,7 +379,6 @@ swizzle_guid_to_uuid(efi_guid_t *guid) #undef log #endif #define log(level, fmt, args...) log_(__FILE__, __LINE__, __func__, level, fmt, ## args) -#define arrow(l,b,o,p,n,m) ({if(n==m){char c_=b[p+1]; b[o]='^'; b[p+o]='^';b[p+o+1]='\0';log(l,"%s",b);b[o]=' ';b[p+o]=' ';b[p+o+1]=c_;}}) #define debug(fmt, args...) log(LOG_DEBUG, fmt, ## args) #endif /* EFIVAR_UTIL_H */ -- 2.29.2 From dcac5a192c76599084153b8f7f0d60f8a77264ff Mon Sep 17 00:00:00 2001 From: Peter Jones <pjones@redhat.com> Date: Wed, 13 Mar 2019 11:02:01 -0400 Subject: [PATCH 03/10] Add more hexdump logging functions. Signed-off-by: Peter Jones <pjones@redhat.com> (cherry picked from commit 48910a0cd39d7c08dab555fb464f9765163ad37b) --- src/hexdump.h | 30 ++++++++++++++++++++++++++++-- src/util.h | 10 ++++++++++ 2 files changed, 38 insertions(+), 2 deletions(-) diff --git a/src/hexdump.h b/src/hexdump.h index 4c45cb3..f8c32fa 100644 --- a/src/hexdump.h +++ b/src/hexdump.h @@ -63,8 +63,12 @@ prepare_text(uint8_t *data, unsigned long size, char *buf) buf[offset] = '\0'; } +/* + * variadic fhexdump formatted + * think of it as: fprintf(f, %s%s\n", vformat(fmt, ap), hexdump(data,size)); + */ static inline void UNUSED -hexdump(uint8_t *data, unsigned long size) +vfhexdumpf(FILE *f, const char * const fmt, uint8_t *data, unsigned long size, va_list ap) { unsigned long display_offset = (unsigned long)data & 0xffffffff; unsigned long offset = 0; @@ -80,11 +84,33 @@ hexdump(uint8_t *data, unsigned long size) return; prepare_text(data+offset, size-offset, txtbuf); - printf("%016lx %s %s\n", display_offset, hexbuf, txtbuf); + vfprintf(f, fmt, ap); + fprintf(f, "%016lx %s %s\n", display_offset, hexbuf, txtbuf); display_offset += sz; offset += sz; } + fflush(f); +} + +/* + * fhexdump formatted + * think of it as: fprintf(f, %s%s\n", format(fmt, ...), hexdump(data,size)); + */ +static inline void UNUSED +fhexdumpf(FILE *f, const char * const fmt, uint8_t *data, unsigned long size, ...) +{ + va_list ap; + + va_start(ap, size); + vfhexdumpf(f, fmt, data, size, ap); + va_end(ap); +} + +static inline void UNUSED +hexdump(uint8_t *data, unsigned long size) +{ + fhexdumpf(stdout, "", data, size); } #endif /* STATIC_HEXDUMP_H */ diff --git a/src/util.h b/src/util.h index 53f6547..fe9755b 100644 --- a/src/util.h +++ b/src/util.h @@ -380,5 +380,15 @@ swizzle_guid_to_uuid(efi_guid_t *guid) #endif #define log(level, fmt, args...) log_(__FILE__, __LINE__, __func__, level, fmt, ## args) #define debug(fmt, args...) log(LOG_DEBUG, fmt, ## args) +#define log_hex_(file, line, func, level, buf, size) \ + ({ \ + if (efi_get_verbose() >= level) { \ + fhexdumpf(efi_get_logfile(), "%s:%d %s(): ", \ + (uint8_t *)buf, size, \ + file, line, func); \ + } \ + }) +#define log_hex(level, buf, size) log_hex_(__FILE__, __LINE__, __func__, level, buf, size) +#define debug_hex(buf, size) log_hex(LOG_DEBUG, buf, size) #endif /* EFIVAR_UTIL_H */ -- 2.29.2 From 65e8ff7d8fb2549117b357c42a4388779f2b71d4 Mon Sep 17 00:00:00 2001 From: Peter Jones <pjones@redhat.com> Date: Wed, 2 Oct 2019 16:59:14 -0400 Subject: [PATCH 04/10] sysfs parsing: add some more debugging output This adds highlights under the things we've found when searching, like: linux-pci.c:66 parse_pci(): searching for 0000:00:00.0/ linux-pci.c:69 parse_pci(): current:'0000:00:1d.4/0000:6e:00.0/nvme/nvme0/nvme0n1' rc:4 pos0:0 pos1:13 linux-pci.c:70 parse_pci(): ^^^^^^^^^^^^^ Signed-off-by: Peter Jones <pjones@redhat.com> (cherry picked from commit 097ff1033c6ee258b47720d079e27350f7189bbd) NOTE: Replace efi_set_loglevel() with efi_get_verbose() --- src/linux-acpi-root.c | 5 ++++- src/linux-emmc.c | 3 ++- src/linux-md.c | 3 ++- src/linux-nvme.c | 3 ++- src/linux-pci-root.c | 4 +++- src/linux-pci.c | 3 ++- src/linux-pmem.c | 4 +++- src/linux-sata.c | 6 +++++- src/linux-scsi.c | 10 +++++++++- src/linux-soc-root.c | 4 +++- src/linux-virtblk.c | 3 ++- src/util.h | 36 ++++++++++++++++++++++++++++++++++++ 12 files changed, 73 insertions(+), 11 deletions(-) diff --git a/src/linux-acpi-root.c b/src/linux-acpi-root.c index 30728de..e6f18e4 100644 --- a/src/linux-acpi-root.c +++ b/src/linux-acpi-root.c @@ -1,6 +1,6 @@ /* * libefiboot - library for the manipulation of EFI boot variables - * Copyright 2012-2018 Red Hat, Inc. + * Copyright 2012-2019 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 License as @@ -63,6 +63,7 @@ parse_acpi_root(struct device *dev, const char *current, const char *root UNUSED */ rc = sscanf(devpart, "../../devices/platform/%n", &pos); debug("devpart:\"%s\" rc:%d pos:%d", devpart, rc, pos); + dbgmk(" ", pos); if (rc != 0 || pos < 1) return 0; devpart += pos; @@ -97,6 +98,7 @@ parse_acpi_root(struct device *dev, const char *current, const char *root UNUSED pos -= 4; debug("devpart:\"%s\" rc:%d pos:%d", devpart, rc, pos); + dbgmk(" ", pos); acpi_header = strndupa(devpart, pos); if (!acpi_header) return 0; @@ -114,6 +116,7 @@ parse_acpi_root(struct device *dev, const char *current, const char *root UNUSED } debug("devpart:\"%s\" parsed:%04hx:%02hhx pos:%d rc:%d", devpart, pad0, pad1, pos, rc); + dbgmk(" ", pos); devpart += pos; diff --git a/src/linux-emmc.c b/src/linux-emmc.c index b290ed0..c49a296 100644 --- a/src/linux-emmc.c +++ b/src/linux-emmc.c @@ -1,6 +1,6 @@ /* * libefiboot - library for the manipulation of EFI boot variables - * Copyright 2012-2018 Red Hat, Inc. + * Copyright 2012-2019 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 License as @@ -58,6 +58,7 @@ parse_emmc(struct device *dev, const char *current, const char *root UNUSED) &tosser0, &tosser1, &tosser2, &slot_id, &pos0, &tosser3, &partition, &pos1); debug("current:\"%s\" rc:%d pos0:%d pos1:%d\n", current, rc, pos0, pos1); + dbgmk(" ", pos0, pos1); /* * If it isn't of that form, it's not one of our emmc devices. */ diff --git a/src/linux-md.c b/src/linux-md.c index cb584c9..4714456 100644 --- a/src/linux-md.c +++ b/src/linux-md.c @@ -1,6 +1,6 @@ /* * libefiboot - library for the manipulation of EFI boot variables - * Copyright 2012-2018 Red Hat, Inc. + * Copyright 2012-2019 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 License as @@ -51,6 +51,7 @@ parse_md(struct device *dev, const char *current, const char *root UNUSED) rc = sscanf(current, "md%d/%nmd%dp%d%n", &md, &pos0, &tosser0, &part, &pos1); debug("current:\"%s\" rc:%d pos0:%d pos1:%d\n", current, rc, pos0, pos1); + dbgmk(" ", pos0, pos1); /* * If it isn't of that form, it's not one of our partitioned md devices. */ diff --git a/src/linux-nvme.c b/src/linux-nvme.c index 1d8fc65..f8077e7 100644 --- a/src/linux-nvme.c +++ b/src/linux-nvme.c @@ -1,6 +1,6 @@ /* * libefiboot - library for the manipulation of EFI boot variables - * Copyright 2012-2018 Red Hat, Inc. + * Copyright 2012-2019 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 License as @@ -62,6 +62,7 @@ parse_nvme(struct device *dev, const char *current, const char *root UNUSED) &tosser0, &ctrl_id, &ns_id, &pos0, &tosser1, &tosser2, &partition, &pos1); debug("current:\"%s\" rc:%d pos0:%d pos1:%d\n", current, rc, pos0, pos1); + dbgmk(" ", pos0, pos1); /* * If it isn't of that form, it's not one of our nvme devices. */ diff --git a/src/linux-pci-root.c b/src/linux-pci-root.c index 5d0e471..83711b8 100644 --- a/src/linux-pci-root.c +++ b/src/linux-pci-root.c @@ -1,6 +1,6 @@ /* * libefiboot - library for the manipulation of EFI boot variables - * Copyright 2012-2018 Red Hat, Inc. + * Copyright 2012-2019 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 License as @@ -57,6 +57,8 @@ parse_pci_root(struct device *dev, const char *current, const char *root UNUSED) * ^d ^p */ rc = sscanf(devpart, "../../devices/pci%hx:%hhx/%n", &root_domain, &root_bus, &pos); + debug("current:\"%s\" rc:%d pos:%d", devpart, rc, pos); + dbgmk(" ", pos); /* * If we can't find that, it's not a PCI device. */ diff --git a/src/linux-pci.c b/src/linux-pci.c index 64aaefb..989dd2b 100644 --- a/src/linux-pci.c +++ b/src/linux-pci.c @@ -1,6 +1,6 @@ /* * libefiboot - library for the manipulation of EFI boot variables - * Copyright 2012-2018 Red Hat, Inc. + * Copyright 2012-2019 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 License as @@ -68,6 +68,7 @@ parse_pci(struct device *dev, const char *current, const char *root) rc = sscanf(devpart, "%hx:%hhx:%hhx.%hhx/%n", &domain, &bus, &device, &function, &pos); debug("current:\"%s\" rc:%d pos:%d", devpart, rc, pos); + dbgmk(" ", pos); if (rc != 4) break; devpart += pos; diff --git a/src/linux-pmem.c b/src/linux-pmem.c index 4d981fc..f55d41f 100644 --- a/src/linux-pmem.c +++ b/src/linux-pmem.c @@ -1,6 +1,6 @@ /* * libefiboot - library for the manipulation of EFI boot variables - * Copyright 2012-2018 Red Hat, Inc. + * Copyright 2012-2019 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 License as @@ -105,6 +105,8 @@ parse_pmem(struct device *dev, const char *current, const char *root UNUSED) "../../devices/LNXSYSTM:%hhx/LNXSYBUS:%hhx/ACPI%hx:%hhx/ndbus%d/region%d/btt%d.%d/%n", &system, &sysbus, &pnp_id, &acpi_id, &ndbus, ®ion, &btt_region_id, &btt_id, &pos); + debug("current:\"%s\" rc:%d pos:%d", current, rc, pos); + dbgmk(" ", pos); if (rc < 8) return 0; diff --git a/src/linux-sata.c b/src/linux-sata.c index 3564117..036f495 100644 --- a/src/linux-sata.c +++ b/src/linux-sata.c @@ -1,6 +1,6 @@ /* * libefiboot - library for the manipulation of EFI boot variables - * Copyright 2012-2018 Red Hat, Inc. + * Copyright 2012-2019 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 License as @@ -162,6 +162,7 @@ parse_sata(struct device *dev, const char *devlink, const char *root UNUSED) debug("searching for ata1/"); rc = sscanf(current, "ata%"PRIu32"/%n", &print_id, &pos); debug("current:\"%s\" rc:%d pos:%d\n", current, rc, pos); + dbgmk(" ", pos); /* * If we don't find this one, it isn't an ata device, so return 0 not * error. Later errors mean it is an ata device, but we can't parse @@ -175,6 +176,7 @@ parse_sata(struct device *dev, const char *devlink, const char *root UNUSED) debug("searching for host0/"); rc = sscanf(current, "host%"PRIu32"/%n", &scsi_bus, &pos); debug("current:\"%s\" rc:%d pos:%d\n", current, rc, pos); + dbgmk(" ", pos); if (rc != 1) return -1; current += pos; @@ -184,6 +186,7 @@ parse_sata(struct device *dev, const char *devlink, const char *root UNUSED) rc = sscanf(current, "target%"PRIu32":%"PRIu32":%"PRIu64"/%n", &scsi_device, &scsi_target, &scsi_lun, &pos); debug("current:\"%s\" rc:%d pos:%d\n", current, rc, pos); + dbgmk(" ", pos); if (rc != 3) return -1; current += pos; @@ -193,6 +196,7 @@ parse_sata(struct device *dev, const char *devlink, const char *root UNUSED) rc = sscanf(current, "%"PRIu32":%"PRIu32":%"PRIu32":%"PRIu64"/%n", &tosser0, &tosser1, &tosser2, &tosser3, &pos); debug("current:\"%s\" rc:%d pos:%d\n", current, rc, pos); + dbgmk(" ", pos); if (rc != 4) return -1; current += pos; diff --git a/src/linux-scsi.c b/src/linux-scsi.c index 04892f0..06e41f3 100644 --- a/src/linux-scsi.c +++ b/src/linux-scsi.c @@ -1,6 +1,6 @@ /* * libefiboot - library for the manipulation of EFI boot variables - * Copyright 2012-2018 Red Hat, Inc. + * Copyright 2012-2019 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 License as @@ -101,6 +101,7 @@ parse_scsi_link(const char *current, uint32_t *scsi_host, debug("searching for host4/"); rc = sscanf(current, "host%d/%n", scsi_host, &pos0); debug("current:\"%s\" rc:%d pos0:%d\n", current+sz, rc, pos0); + dbgmk(" ", pos0); if (rc != 1) return -1; sz += pos0; @@ -118,6 +119,7 @@ parse_scsi_link(const char *current, uint32_t *scsi_host, rc = sscanf(current+sz, "port-%d:%d%n:%d%n", &tosser0, &tosser1, &pos0, &tosser2, &pos1); debug("current:\"%s\" rc:%d pos0:%d pos1:%d\n", current+sz, rc, pos0, pos1); + dbgmk(" ", pos0, pos1); if (rc == 2 || rc == 3) { sz += pos0; pos0 = 0; @@ -143,6 +145,7 @@ parse_scsi_link(const char *current, uint32_t *scsi_host, debug("searching for expander-4:0/"); rc = sscanf(current+sz, "expander-%d:%d/%n", &tosser0, &tosser1, &pos0); debug("current:\"%s\" rc:%d pos0:%d\n", current+sz, rc, pos0); + dbgmk(" ", pos0); if (rc == 2) { if (!remote_target_id) { efi_error("Device is PHY is a remote target, but remote_target_id is NULL"); @@ -158,6 +161,7 @@ parse_scsi_link(const char *current, uint32_t *scsi_host, debug("searching for port-2:0:2/"); rc = sscanf(current+sz, "port-%d:%d:%d/%n", &tosser0, &tosser1, &tosser2, &pos0); debug("current:\"%s\" rc:%d pos0:%d\n", current+sz, rc, pos0); + dbgmk(" ", pos0); if (rc != 3) { efi_error("Couldn't parse port expander port string"); return -1; @@ -182,6 +186,7 @@ parse_scsi_link(const char *current, uint32_t *scsi_host, rc = sscanf(current + sz + pos0, ":%d%n", &tosser2, &pos1); if (rc != 0 && rc != 1) return -1; + dbgmk(" ", pos0, pos0+pos1); if (remote_port_id && rc == 1) *remote_port_id = tosser2; if (local_port_id && rc == 0) @@ -203,6 +208,7 @@ parse_scsi_link(const char *current, uint32_t *scsi_host, rc = sscanf(current + sz, "target%d:%d:%"PRIu64"/%n", &tosser0, &tosser1, &tosser3, &pos0); debug("current:\"%s\" rc:%d pos0:%d\n", current+sz, rc, pos0); + dbgmk(" ", pos0); if (rc != 3) return -1; sz += pos0; @@ -215,6 +221,7 @@ parse_scsi_link(const char *current, uint32_t *scsi_host, rc = sscanf(current + sz, "%d:%d:%d:%"PRIu64"/%n", scsi_bus, scsi_device, scsi_target, scsi_lun, &pos0); debug("current:\"%s\" rc:%d pos0:%d\n", current+sz, rc, pos0); + dbgmk(" ", pos0); if (rc != 4) return -1; sz += pos0; @@ -242,6 +249,7 @@ parse_scsi(struct device *dev, const char *current, const char *root UNUSED) &dev->scsi_info.scsi_lun, &pos); debug("current:\"%s\" rc:%d pos:%d\n", dev->device, rc, pos); + dbgmk(" ", pos); if (rc != 4) return 0; diff --git a/src/linux-soc-root.c b/src/linux-soc-root.c index 373cd59..dc2a305 100644 --- a/src/linux-soc-root.c +++ b/src/linux-soc-root.c @@ -1,6 +1,6 @@ /* * libefiboot - library for the manipulation of EFI boot variables - * Copyright 2012-2018 Red Hat, Inc. + * Copyright 2012-2019 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 License as @@ -47,6 +47,8 @@ parse_soc_root(struct device *dev UNUSED, const char *current, const char *root debug("entry"); rc = sscanf(devpart, "../../devices/platform/soc/%*[^/]/%n", &pos); + debug("current:\"%s\" rc:%d pos:%d", current, rc, pos); + dbgmk(" ", pos); if (rc != 0) return 0; devpart += pos; diff --git a/src/linux-virtblk.c b/src/linux-virtblk.c index 2e9889d..3c9c690 100644 --- a/src/linux-virtblk.c +++ b/src/linux-virtblk.c @@ -1,6 +1,6 @@ /* * libefiboot - library for the manipulation of EFI boot variables - * Copyright 2012-2018 Red Hat, Inc. + * Copyright 2012-2019 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 License as @@ -56,6 +56,7 @@ parse_virtblk(struct device *dev, const char *current, const char *root UNUSED) debug("searching for virtio0/"); rc = sscanf(current, "virtio%x/%n", &tosser, &pos); debug("current:\"%s\" rc:%d pos:%d\n", current, rc, pos); + dbgmk(" ", pos); /* * If we couldn't find virtioX/ then it isn't a virtio device. */ diff --git a/src/util.h b/src/util.h index fe9755b..587a72b 100644 --- a/src/util.h +++ b/src/util.h @@ -360,6 +360,41 @@ swizzle_guid_to_uuid(efi_guid_t *guid) u16[1] = __builtin_bswap16(u16[1]); } +static inline void UNUSED +debug_markers_(const char * const file, int line, + const char * const func, int level, + const char * const prefix, ...) +{ + FILE *logfile; + va_list ap; + int pos; + int n = 0; + bool on = false; + + va_start(ap, prefix); + for (n = 0, pos = va_arg(ap, int); pos >= 0; pos = va_arg(ap, int), n++) + ; + va_end(ap); + if (n < 2) + return; + n = 0; + + if (efi_get_verbose() < level) + return; + logfile = efi_get_logfile(); + fprintf(logfile, "%s:%d %s(): %s", file, line, func, prefix ? prefix : ""); + va_start(ap, prefix); + while ((pos = va_arg(ap, int)) >= 0) { + for (; n <= pos; n++) { + if (n == pos) + on = !on; + fprintf(logfile, "%c", on ? '^' : ' '); + } + } + fprintf(logfile, "\n"); + va_end(ap); +} + #define log_(file, line, func, level, fmt, args...) \ ({ \ if (efi_get_verbose() >= level) { \ @@ -390,5 +425,6 @@ swizzle_guid_to_uuid(efi_guid_t *guid) }) #define log_hex(level, buf, size) log_hex_(__FILE__, __LINE__, __func__, level, buf, size) #define debug_hex(buf, size) log_hex(LOG_DEBUG, buf, size) +#define dbgmk(prefix, args...) debug_markers_(__FILE__, __LINE__, __func__, LOG_DEBUG, prefix, ## args, -1) #endif /* EFIVAR_UTIL_H */ -- 2.29.2 From 9974732f38c5afa46d9033e1c6782b1c0185918a Mon Sep 17 00:00:00 2001 From: Peter Jones <pjones@redhat.com> Date: Tue, 15 Oct 2019 16:53:27 -0400 Subject: [PATCH 05/10] sysfs parsers: make all the /sys/block link parsers work the same way Apparently I wrote some of these one way and some the other, and the one special case where everything was "current+sz" instead of some form of "current += pos; sz += pos; ...; return sz;" or the subtraction version. Make them all the same, where possible. Signed-off-by: Peter Jones <pjones@redhat.com> (cherry picked from commit 4718694094647b981b74453445d000b0296b6544) --- src/linux-acpi-root.c | 93 +++++++++++++----------- src/linux-ata.c | 10 +-- src/linux-emmc.c | 22 +++--- src/linux-nvme.c | 24 ++++--- src/linux-pci-root.c | 17 ++--- src/linux-pci.c | 28 ++++---- src/linux-pmem.c | 22 +++--- src/linux-sas.c | 8 ++- src/linux-sata.c | 51 +++++++------- src/linux-scsi.c | 159 ++++++++++++++++++++++-------------------- src/linux-soc-root.c | 24 +++---- src/linux-virtblk.c | 15 ++-- 12 files changed, 256 insertions(+), 217 deletions(-) diff --git a/src/linux-acpi-root.c b/src/linux-acpi-root.c index e6f18e4..d2f012d 100644 --- a/src/linux-acpi-root.c +++ b/src/linux-acpi-root.c @@ -41,17 +41,16 @@ * ^ root hub ^pci dev ^pci dev ^ blockdev stuff */ static ssize_t -parse_acpi_root(struct device *dev, const char *current, const char *root UNUSED) +parse_acpi_root(struct device *dev, const char *path, const char *root UNUSED) { + const char *current = path; int rc; - int pos; + int pos0 = -1, pos1 = -1, pos2 = -1; uint16_t pad0; uint8_t pad1; char *acpi_header = NULL; char *colon; - const char *devpart = current; - debug("entry"); /* @@ -61,23 +60,25 @@ parse_acpi_root(struct device *dev, const char *current, const char *root UNUSED * This is annoying because "/%04ms%h:%hhx/" won't bind from the right * side in sscanf. */ - rc = sscanf(devpart, "../../devices/platform/%n", &pos); - debug("devpart:\"%s\" rc:%d pos:%d", devpart, rc, pos); - dbgmk(" ", pos); - if (rc != 0 || pos < 1) + rc = sscanf(current, "../../devices/%nplatform/%n", &pos0, &pos1); + debug("current:'%s' rc:%d pos0:%d pos1:%d", current, rc, pos0, pos1); + dbgmk(" ", pos0, pos1); + if (rc != 0 || pos0 == -1 || pos1 == -1) return 0; - devpart += pos; + current += pos1; + debug("searching for an ACPI string like A0000:00 or ACPI0000:00"); + pos0 = 0; /* * If it's too short to be A0000:00, it's not an ACPI string */ - if (strlen(devpart) < 8) + if (strlen(current) < 8) return 0; - colon = strchr(devpart, ':'); + colon = strchr(current, ':'); if (!colon) return 0; - pos = colon - devpart; + pos1 = colon - current; /* * If colon doesn't point at something between one of these: @@ -85,61 +86,71 @@ parse_acpi_root(struct device *dev, const char *current, const char *root UNUSED * ^ 5 ^ 8 * Then it's not an ACPI string. */ - if (pos < 5 || pos > 8) + if (pos1 < 5 || pos1 > 8) return 0; - dev->acpi_root.acpi_hid_str = strndup(devpart, pos + 1); + debug("current:'%s' rc:%d pos0:%d pos1:%d", current, rc, pos0, pos1); + dbgmk(" ", pos0, pos1); + + dev->acpi_root.acpi_hid_str = strndup(current, pos1 + 1); if (!dev->acpi_root.acpi_hid_str) { efi_error("Could not allocate memory"); return -1; } - dev->acpi_root.acpi_hid_str[pos] = 0; - debug("acpi_hid_str:\"%s\"", dev->acpi_root.acpi_hid_str); - - pos -= 4; - debug("devpart:\"%s\" rc:%d pos:%d", devpart, rc, pos); - dbgmk(" ", pos); - acpi_header = strndupa(devpart, pos); + dev->acpi_root.acpi_hid_str[pos1] = 0; + debug("acpi_hid_str:'%s'", dev->acpi_root.acpi_hid_str); + + /* + * The string is like ACPI0000:00. + * ^^^^ + * Here we're saying only this bit has been parsed, though we've + * partially parsed up to the colon. + */ + pos1 -= 4; + acpi_header = strndupa(current, pos1); if (!acpi_header) return 0; - acpi_header[pos] = 0; - debug("devpart:\"%s\" acpi_header:\"%s\"", devpart, acpi_header); - devpart += pos; + acpi_header[pos1] = 0; + debug("acpi_header:'%s'", acpi_header); /* * If we can't find these numbers, it's not an ACPI string */ - rc = sscanf(devpart, "%hx:%hhx/%n", &pad0, &pad1, &pos); + rc = sscanf(current+pos1, "%hx:%hhx/%n", &pad0, &pad1, &pos2); if (rc != 2) { - efi_error("Could not parse ACPI path \"%s\"", devpart); + efi_error("Could not parse ACPI path \"%s\"", current); return 0; } - debug("devpart:\"%s\" parsed:%04hx:%02hhx pos:%d rc:%d", - devpart, pad0, pad1, pos, rc); - dbgmk(" ", pos); - - devpart += pos; - - rc = parse_acpi_hid_uid(dev, "devices/platform/%s%04hX:%02hhX", - acpi_header, pad0, pad1); - debug("rc:%d acpi_header:%s pad0:%04hX pad1:%02hhX", - rc, acpi_header, pad0, pad1); - if (rc < 0 && errno == ENOENT) { - rc = parse_acpi_hid_uid(dev, "devices/platform/%s%04hx:%02hhx", + debug("current:'%s' rc:%d pos0:%d pos1:%d pos2:%d", + current, rc, pos0, pos1, pos2); + dbgmk(" ", pos0, pos2); + current += pos2; + + const char * const formats[] = { + "devices/platform/%s%04hX:%02hhX", + "devices/platform/%s%04hx:%02hhx", + NULL + }; + + for (unsigned int i = 0; formats[i]; i++) { + rc = parse_acpi_hid_uid(dev, formats[i], acpi_header, pad0, pad1); debug("rc:%d acpi_header:%s pad0:%04hx pad1:%02hhx", rc, acpi_header, pad0, pad1); - } - if (rc < 0) { + if (rc >= 0) + break; + if (errno != ENOENT) { efi_error("Could not parse hid/uid"); return rc; } + } debug("Parsed HID:0x%08x UID:0x%"PRIx64" uidstr:\"%s\" path:\"%s\"", dev->acpi_root.acpi_hid, dev->acpi_root.acpi_uid, dev->acpi_root.acpi_uid_str, dev->acpi_root.acpi_cid_str); - return devpart - current; + debug("current:'%s' sz:%zd", current, current - path); + return current - path; } static ssize_t diff --git a/src/linux-ata.c b/src/linux-ata.c index 43e5f4c..6e86ba2 100644 --- a/src/linux-ata.c +++ b/src/linux-ata.c @@ -1,6 +1,6 @@ /* * libefiboot - library for the manipulation of EFI boot variables - * Copyright 2012-2018 Red Hat, Inc. + * Copyright 2012-2019 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 License as @@ -58,7 +58,7 @@ is_pata(struct device *dev) * 11:0 -> ../../devices/pci0000:00/0000:00:11.5/ata3/host2/target2:0:0/2:0:0:0/block/sr0 */ static ssize_t -parse_ata(struct device *dev, const char *current, const char *root UNUSED) +parse_ata(struct device *dev, const char *path, const char *root UNUSED) { uint32_t scsi_host, scsi_bus, scsi_device, scsi_target; uint64_t scsi_lun; @@ -108,7 +108,7 @@ parse_ata(struct device *dev, const char *current, const char *root UNUSED) return 0; } - char *host = strstr(current, "/host"); + char *host = strstr(path, "/host"); if (!host) return -1; @@ -125,10 +125,10 @@ parse_ata(struct device *dev, const char *current, const char *root UNUSED) dev->ata_info.scsi_target = scsi_target; dev->ata_info.scsi_lun = scsi_lun; - char *block = strstr(current, "/block/"); + char *block = strstr(path, "/block/"); if (!block) return -1; - return block + 1 - current; + return block + 1 - path; } static ssize_t diff --git a/src/linux-emmc.c b/src/linux-emmc.c index c49a296..bafa9cd 100644 --- a/src/linux-emmc.c +++ b/src/linux-emmc.c @@ -24,6 +24,7 @@ #include <fcntl.h> #include <inttypes.h> #include <stdint.h> +#include <sys/param.h> #include <unistd.h> #include "efiboot.h" @@ -45,20 +46,21 @@ */ static ssize_t -parse_emmc(struct device *dev, const char *current, const char *root UNUSED) +parse_emmc(struct device *dev, const char *path, const char *root UNUSED) { + const char * current = path; int rc; int32_t tosser0, tosser1, tosser2, tosser3, slot_id, partition; - int pos0 = 0, pos1 = 0; + int pos0 = -1, pos1 = -1, pos2 = -1; debug("entry"); debug("searching for mmc_host/mmc0/mmc0:0001/block/mmcblk0 or mmc_host/mmc0/mmc0:0001/block/mmcblk0/mmcblk0p1"); - rc = sscanf(current, "mmc_host/mmc%d/mmc%d:%d/block/mmcblk%d%n/mmcblk%dp%d%n", - &tosser0, &tosser1, &tosser2, &slot_id, - &pos0, &tosser3, &partition, &pos1); - debug("current:\"%s\" rc:%d pos0:%d pos1:%d\n", current, rc, pos0, pos1); - dbgmk(" ", pos0, pos1); + rc = sscanf(current, "%nmmc_host/mmc%d/mmc%d:%d/block/mmcblk%d%n/mmcblk%dp%d%n", + &pos0, &tosser0, &tosser1, &tosser2, &slot_id, + &pos1, &tosser3, &partition, &pos2); + debug("current:\"%s\" rc:%d pos0:%d pos1:%d pos2:%d\n", current, rc, pos0, pos1, pos2); + dbgmk(" ", pos0, MAX(pos1,pos2)); /* * If it isn't of that form, it's not one of our emmc devices. */ @@ -72,10 +74,12 @@ parse_emmc(struct device *dev, const char *current, const char *root UNUSED) if (dev->part == -1) dev->part = partition; - pos0 = pos1; + pos2 = pos1; } + current += pos2; - return pos0; + debug("current:'%s' sz:%zd", current, current - path); + return current - path; } static ssize_t diff --git a/src/linux-nvme.c b/src/linux-nvme.c index f8077e7..648e6c3 100644 --- a/src/linux-nvme.c +++ b/src/linux-nvme.c @@ -24,6 +24,7 @@ #include <fcntl.h> #include <inttypes.h> #include <stdint.h> +#include <sys/param.h> #include <unistd.h> #include "efiboot.h" @@ -48,26 +49,29 @@ */ static ssize_t -parse_nvme(struct device *dev, const char *current, const char *root UNUSED) +parse_nvme(struct device *dev, const char *path, const char *root UNUSED) { + const char *current = path; int rc; int32_t tosser0, tosser1, tosser2, ctrl_id, ns_id, partition; uint8_t *filebuf = NULL; - int pos0 = 0, pos1 = 0; + int pos0 = -1, pos1 = -1, pos2 = -1; debug("entry"); debug("searching for nvme/nvme0/nvme0n1 or nvme/nvme0/nvme0n1/nvme0n1p1"); - rc = sscanf(current, "nvme/nvme%d/nvme%dn%d%n/nvme%dn%dp%d%n", - &tosser0, &ctrl_id, &ns_id, &pos0, - &tosser1, &tosser2, &partition, &pos1); - debug("current:\"%s\" rc:%d pos0:%d pos1:%d\n", current, rc, pos0, pos1); - dbgmk(" ", pos0, pos1); + rc = sscanf(current, "%nnvme/nvme%d/nvme%dn%d%n/nvme%dn%dp%d%n", + &pos0, &tosser0, &ctrl_id, &ns_id, + &pos1, &tosser1, &tosser2, &partition, &pos2); + debug("current:\"%s\" rc:%d pos0:%d pos1:%d pos2:%d\n", current, rc, pos0, pos1, pos2); + dbgmk(" ", pos0, MAX(pos1,pos2)); /* * If it isn't of that form, it's not one of our nvme devices. */ if (rc != 3 && rc != 6) return 0; + if (rc == 3) + pos2 = pos1; dev->nvme_info.ctrl_id = ctrl_id; dev->nvme_info.ns_id = ns_id; @@ -78,8 +82,9 @@ parse_nvme(struct device *dev, const char *current, const char *root UNUSED) if (dev->part == -1) dev->part = partition; - pos0 = pos1; + pos1 = pos2; } + current += pos1; /* * now fish the eui out of sysfs is there is one... @@ -111,7 +116,8 @@ parse_nvme(struct device *dev, const char *current, const char *root UNUSED) memcpy(dev->nvme_info.eui, eui, sizeof(eui)); } - return pos0; + debug("current:'%s' sz:%zd", current, current - path); + return current - path; } static ssize_t diff --git a/src/linux-pci-root.c b/src/linux-pci-root.c index 83711b8..c105477 100644 --- a/src/linux-pci-root.c +++ b/src/linux-pci-root.c @@ -41,13 +41,13 @@ * */ static ssize_t -parse_pci_root(struct device *dev, const char *current, const char *root UNUSED) +parse_pci_root(struct device *dev, const char *path, const char *root UNUSED) { + const char * current = path; int rc; - int pos; + int pos0 = -1, pos1 = -1; uint16_t root_domain; uint8_t root_bus; - const char *devpart = current; debug("entry"); @@ -56,15 +56,15 @@ parse_pci_root(struct device *dev, const char *current, const char *root UNUSED) * pci0000:00/ * ^d ^p */ - rc = sscanf(devpart, "../../devices/pci%hx:%hhx/%n", &root_domain, &root_bus, &pos); - debug("current:\"%s\" rc:%d pos:%d", devpart, rc, pos); - dbgmk(" ", pos); + rc = sscanf(current, "%n../../devices/pci%hx:%hhx/%n", &pos0, &root_domain, &root_bus, &pos1); + debug("current:'%s' rc:%d pos0:%d pos1:%d", current, rc, pos0, pos1); + dbgmk(" ", pos0, pos1); /* * If we can't find that, it's not a PCI device. */ if (rc != 2) return 0; - devpart += pos; + current += pos1; dev->pci_root.pci_domain = root_domain; dev->pci_root.pci_bus = root_bus; @@ -75,7 +75,8 @@ parse_pci_root(struct device *dev, const char *current, const char *root UNUSED) return -1; errno = 0; - return devpart - current; + debug("current:'%s' sz:%zd\n", current, current - path); + return current - path; } static ssize_t diff --git a/src/linux-pci.c b/src/linux-pci.c index 989dd2b..eed0e1b 100644 --- a/src/linux-pci.c +++ b/src/linux-pci.c @@ -43,11 +43,10 @@ * */ static ssize_t -parse_pci(struct device *dev, const char *current, const char *root) +parse_pci(struct device *dev, const char *path, const char *root) { + const char *current = path; int rc; - int pos; - const char *devpart = current; debug("entry"); @@ -55,23 +54,22 @@ parse_pci(struct device *dev, const char *current, const char *root) * 0000:00:01.0/0000:01:00.0/ * ^d ^b ^d ^f (of the last one in the series) */ - while (*devpart) { + while (*current) { uint16_t domain; uint8_t bus, device, function; struct pci_dev_info *pci_dev; unsigned int i = dev->n_pci_devs; struct stat statbuf; + int pos0 = -1, pos1 = -1; - debug("devpart is \"%s\"", devpart); - pos = 0; debug("searching for 0000:00:00.0/"); - rc = sscanf(devpart, "%hx:%hhx:%hhx.%hhx/%n", - &domain, &bus, &device, &function, &pos); - debug("current:\"%s\" rc:%d pos:%d", devpart, rc, pos); - dbgmk(" ", pos); + rc = sscanf(current, "%n%hx:%hhx:%hhx.%hhx/%n", + &pos0, &domain, &bus, &device, &function, &pos1); + debug("current:'%s' rc:%d pos0:%d pos1:%d", current, rc, pos0, pos1); + dbgmk(" ", pos0, pos1); if (rc != 4) break; - devpart += pos; + current += pos1; debug("found pci domain %04hx:%02hhx:%02hhx.%02hhx", domain, bus, device, function); @@ -88,13 +86,13 @@ parse_pci(struct device *dev, const char *current, const char *root) dev->pci_dev[i].pci_bus = bus; dev->pci_dev[i].pci_device = device; dev->pci_dev[i].pci_function = function; - char *tmp = strndup(root, devpart-root+1); + char *tmp = strndup(root, current-root+1); char *linkbuf = NULL; if (!tmp) { efi_error("could not allocate memory"); return -1; } - tmp[devpart - root] = '\0'; + tmp[current - root] = '\0'; rc = sysfs_stat(&statbuf, "class/block/%s/driver", tmp); if (rc < 0 && errno == ENOENT) { debug("No driver link for /sys/class/block/%s", tmp); @@ -115,8 +113,8 @@ parse_pci(struct device *dev, const char *current, const char *root) dev->n_pci_devs += 1; } - debug("next:\"%s\"", devpart); - return devpart - current; + debug("current:'%s' sz:%zd\n", current, current - path); + return current - path; } static ssize_t diff --git a/src/linux-pmem.c b/src/linux-pmem.c index f55d41f..5327658 100644 --- a/src/linux-pmem.c +++ b/src/linux-pmem.c @@ -70,12 +70,14 @@ */ static ssize_t -parse_pmem(struct device *dev, const char *current, const char *root UNUSED) +parse_pmem(struct device *dev, const char *path, const char *root UNUSED) { + const char *current = path; uint8_t *filebuf = NULL; uint8_t system, sysbus, acpi_id; uint16_t pnp_id; - int ndbus, region, btt_region_id, btt_id, rc, pos; + int ndbus, region, btt_region_id, btt_id, rc; + int pos0 = -1, pos1 = -1; char *namespace = NULL; debug("entry"); @@ -95,20 +97,21 @@ parse_pmem(struct device *dev, const char *current, const char *root UNUSED) } /* - * We're not actually using any of the values here except pos (our + * We're not actually using any of the values here except pos1 (our * return value), but rather just being paranoid that this is the sort * of device we care about. * * 259:0 -> ../../devices/LNXSYSTM:00/LNXSYBUS:00/ACPI0012:00/ndbus0/region12/btt12.1/block/pmem12s */ rc = sscanf(current, - "../../devices/LNXSYSTM:%hhx/LNXSYBUS:%hhx/ACPI%hx:%hhx/ndbus%d/region%d/btt%d.%d/%n", - &system, &sysbus, &pnp_id, &acpi_id, &ndbus, ®ion, - &btt_region_id, &btt_id, &pos); - debug("current:\"%s\" rc:%d pos:%d", current, rc, pos); - dbgmk(" ", pos); + "../../devices/%nLNXSYSTM:%hhx/LNXSYBUS:%hhx/ACPI%hx:%hhx/ndbus%d/region%d/btt%d.%d/%n", + &pos0, &system, &sysbus, &pnp_id, &acpi_id, &ndbus, + ®ion, &btt_region_id, &btt_id, &pos1); + debug("current:\"%s\" rc:%d pos0:%d pos1:%d", current, rc, pos0, pos1); + dbgmk(" ", pos0, pos1); if (rc < 8) return 0; + current += pos1; /* * but the UUID we really do need to have. @@ -158,7 +161,8 @@ parse_pmem(struct device *dev, const char *current, const char *root UNUSED) dev->interface_type = nd_pmem; - return pos; + debug("current:'%s' sz:%zd", current, current - path); + return current - path; } static ssize_t diff --git a/src/linux-sas.c b/src/linux-sas.c index bb04fe8..3ae4ebb 100644 --- a/src/linux-sas.c +++ b/src/linux-sas.c @@ -148,8 +148,9 @@ get_local_sas_address(uint64_t *sas_address, struct device *dev) * anywhere. */ static ssize_t -parse_sas(struct device *dev, const char *current, const char *root UNUSED) +parse_sas(struct device *dev, const char *path, const char *root UNUSED) { + const char *current = path; struct stat statbuf = { 0, }; int rc; uint32_t scsi_host, scsi_bus, scsi_device, scsi_target; @@ -172,6 +173,7 @@ parse_sas(struct device *dev, const char *current, const char *root UNUSED) */ if (pos < 0) return 0; + current += pos; /* * Make sure it has the actual /SAS/ bits before we continue @@ -236,7 +238,9 @@ parse_sas(struct device *dev, const char *current, const char *root UNUSED) dev->scsi_info.scsi_target = scsi_target; dev->scsi_info.scsi_lun = scsi_lun; dev->interface_type = sas; - return pos; + + debug("current:'%s' sz:%zd", current, current - path); + return current - path; } static ssize_t diff --git a/src/linux-sata.c b/src/linux-sata.c index 036f495..c30a3e6 100644 --- a/src/linux-sata.c +++ b/src/linux-sata.c @@ -138,15 +138,15 @@ sysfs_sata_get_port_info(uint32_t print_id, struct device *dev) } static ssize_t -parse_sata(struct device *dev, const char *devlink, const char *root UNUSED) +parse_sata(struct device *dev, const char *path, const char *root UNUSED) { - const char *current = devlink; + const char *current = path; uint32_t print_id; uint32_t scsi_bus, tosser0; uint32_t scsi_device, tosser1; uint32_t scsi_target, tosser2; uint64_t scsi_lun, tosser3; - int pos = 0; + int pos0 = -1, pos1 = -1; int rc; debug("entry"); @@ -160,9 +160,9 @@ parse_sata(struct device *dev, const char *devlink, const char *root UNUSED) * ^dev ^host x y z */ debug("searching for ata1/"); - rc = sscanf(current, "ata%"PRIu32"/%n", &print_id, &pos); - debug("current:\"%s\" rc:%d pos:%d\n", current, rc, pos); - dbgmk(" ", pos); + rc = sscanf(current, "%nata%"PRIu32"/%n", &pos0, &print_id, &pos1); + debug("current:'%s' rc:%d pos0:%d pos1:%d\n", current, rc, pos0, pos1); + dbgmk(" ", pos0, pos1); /* * If we don't find this one, it isn't an ata device, so return 0 not * error. Later errors mean it is an ata device, but we can't parse @@ -170,36 +170,36 @@ parse_sata(struct device *dev, const char *devlink, const char *root UNUSED) */ if (rc != 1) return 0; - current += pos; - pos = 0; + current += pos1; + pos0 = pos1 = -1; debug("searching for host0/"); - rc = sscanf(current, "host%"PRIu32"/%n", &scsi_bus, &pos); - debug("current:\"%s\" rc:%d pos:%d\n", current, rc, pos); - dbgmk(" ", pos); + rc = sscanf(current, "%nhost%"PRIu32"/%n", &pos0, &scsi_bus, &pos1); + debug("current:'%s' rc:%d pos0:%d pos1:%d\n", current, rc, pos0, pos1); + dbgmk(" ", pos0, pos1); if (rc != 1) return -1; - current += pos; - pos = 0; + current += pos1; + pos0 = pos1 = -1; debug("searching for target0:0:0:0/"); - rc = sscanf(current, "target%"PRIu32":%"PRIu32":%"PRIu64"/%n", - &scsi_device, &scsi_target, &scsi_lun, &pos); - debug("current:\"%s\" rc:%d pos:%d\n", current, rc, pos); - dbgmk(" ", pos); + rc = sscanf(current, "%ntarget%"PRIu32":%"PRIu32":%"PRIu64"/%n", + &pos0, &scsi_device, &scsi_target, &scsi_lun, &pos1); + debug("current:'%s' rc:%d pos0:%d pos1:%d\n", current, rc, pos0, pos1); + dbgmk(" ", pos0, pos1); if (rc != 3) return -1; - current += pos; - pos = 0; + current += pos1; + pos0 = pos1 = -1; debug("searching for 0:0:0:0/"); - rc = sscanf(current, "%"PRIu32":%"PRIu32":%"PRIu32":%"PRIu64"/%n", - &tosser0, &tosser1, &tosser2, &tosser3, &pos); - debug("current:\"%s\" rc:%d pos:%d\n", current, rc, pos); - dbgmk(" ", pos); + rc = sscanf(current, "%n%"PRIu32":%"PRIu32":%"PRIu32":%"PRIu64"/%n", + &pos0, &tosser0, &tosser1, &tosser2, &tosser3, &pos1); + debug("current:'%s' rc:%d pos0:%d pos1:%d\n", current, rc, pos0, pos1); + dbgmk(" ", pos0, pos1); if (rc != 4) return -1; - current += pos; + current += pos1; rc = sysfs_sata_get_port_info(print_id, dev); if (rc < 0) @@ -213,7 +213,8 @@ parse_sata(struct device *dev, const char *devlink, const char *root UNUSED) if (dev->interface_type == unknown) dev->interface_type = sata; - return current - devlink; + debug("current:'%s' sz:%zd\n", current, current - path); + return current - path; } static ssize_t diff --git a/src/linux-scsi.c b/src/linux-scsi.c index 06e41f3..218b5a3 100644 --- a/src/linux-scsi.c +++ b/src/linux-scsi.c @@ -24,6 +24,7 @@ #include <fcntl.h> #include <inttypes.h> #include <stdint.h> +#include <sys/param.h> #include <unistd.h> #include "efiboot.h" @@ -36,15 +37,15 @@ * helper for scsi formats... */ ssize_t HIDDEN -parse_scsi_link(const char *current, uint32_t *scsi_host, +parse_scsi_link(const char *path, uint32_t *scsi_host, uint32_t *scsi_bus, uint32_t *scsi_device, uint32_t *scsi_target, uint64_t *scsi_lun, uint32_t *local_port_id, uint32_t *remote_port_id, uint32_t *remote_target_id) { + const char *current = path; int rc; - int sz = 0; - int pos0 = 0, pos1 = 0; + int pos0 = -1, pos1 = -1, pos2 = -1; debug("entry"); /* @@ -99,13 +100,13 @@ parse_scsi_link(const char *current, uint32_t *scsi_host, * or host4/port-4:0:0 */ debug("searching for host4/"); - rc = sscanf(current, "host%d/%n", scsi_host, &pos0); - debug("current:\"%s\" rc:%d pos0:%d\n", current+sz, rc, pos0); - dbgmk(" ", pos0); + rc = sscanf(current, "%nhost%d/%n", scsi_host, &pos0, &pos1); + debug("current:'%s' rc:%d pos0:%d pos1:%d\n", current, rc, pos0, pos1); + dbgmk(" ", pos0, pos1); if (rc != 1) return -1; - sz += pos0; - pos0 = 0; + current += pos1; + pos0 = pos1 = -1; /* * We might have this next: @@ -116,20 +117,27 @@ parse_scsi_link(const char *current, uint32_t *scsi_host, * port-2:0:2/end_device-2:0:2/target2:0:0/2:0:0:0/block/sda/sda1 */ debug("searching for port-4:0 or port-4:0:0"); - rc = sscanf(current+sz, "port-%d:%d%n:%d%n", &tosser0, - &tosser1, &pos0, &tosser2, &pos1); - debug("current:\"%s\" rc:%d pos0:%d pos1:%d\n", current+sz, rc, pos0, pos1); - dbgmk(" ", pos0, pos1); - if (rc == 2 || rc == 3) { - sz += pos0; - pos0 = 0; - if (local_port_id && rc == 2) - *local_port_id = tosser1; - if (remote_port_id && rc == 3) + rc = sscanf(current, "%nport-%d:%d%n:%d%n", + &pos0, &tosser0, &tosser1, &pos1, &tosser2, &pos2); + debug("current:'%s' rc:%d pos0:%d pos1:%d pos2:%d\n", current, rc, pos0, pos1, pos2); + dbgmk(" ", pos0, MAX(pos1, pos2)); + if (rc == 3) { + if (remote_port_id) *remote_port_id = tosser2; + pos1 = pos2; + } else if (rc == 2) { + if (local_port_id) + *local_port_id = tosser1; + } else if (rc != 0) { + return -1; + } else { + pos1 = 0; + } + current += pos1; - if (current[sz] == '/') - sz += 1; + if (current[0] == '/') + current += 1; + pos0 = pos1 = pos2 = -1; /* * We might have this next: @@ -143,32 +151,32 @@ parse_scsi_link(const char *current, uint32_t *scsi_host, * to get past it. */ debug("searching for expander-4:0/"); - rc = sscanf(current+sz, "expander-%d:%d/%n", &tosser0, &tosser1, &pos0); - debug("current:\"%s\" rc:%d pos0:%d\n", current+sz, rc, pos0); - dbgmk(" ", pos0); + rc = sscanf(current, "%nexpander-%d:%d/%n", &pos0, &tosser0, &tosser1, &pos1); + debug("current:'%s' rc:%d pos0:%d pos1:%d\n", current, rc, pos0, pos1); + dbgmk(" ", pos0, pos1); if (rc == 2) { if (!remote_target_id) { efi_error("Device is PHY is a remote target, but remote_target_id is NULL"); return -1; } *remote_target_id = tosser1; - sz += pos0; - pos0 = 0; + current += pos1; + pos0 = pos1 = -1; /* * if we have that, we should have a 3-part port next */ debug("searching for port-2:0:2/"); - rc = sscanf(current+sz, "port-%d:%d:%d/%n", &tosser0, &tosser1, &tosser2, &pos0); - debug("current:\"%s\" rc:%d pos0:%d\n", current+sz, rc, pos0); - dbgmk(" ", pos0); + rc = sscanf(current, "%nport-%d:%d:%d/%n", &pos0, &tosser0, &tosser1, &tosser2, &pos1); + debug("current:'%s' rc:%d pos0:%d pos1:%d\n", current, rc, pos0, pos1); + dbgmk(" ", pos0, pos1); if (rc != 3) { efi_error("Couldn't parse port expander port string"); return -1; } - sz += pos0; + current += pos1; } - pos0 = 0; + pos0 = pos1 = -1; /* next: * /end_device-4:0 @@ -177,88 +185,86 @@ parse_scsi_link(const char *current, uint32_t *scsi_host, * but we don't care for now about any of them anyway. */ debug("searching for end_device-4:0/ or end_device-4:0:0/"); - rc = sscanf(current + sz, "end_device-%d:%d%n", &tosser0, &tosser1, &pos0); - debug("current:\"%s\" rc:%d pos0:%d\n", current+sz, rc, pos0); - if (rc != 2) - return -1; - - pos1 = 0; - rc = sscanf(current + sz + pos0, ":%d%n", &tosser2, &pos1); - if (rc != 0 && rc != 1) - return -1; - dbgmk(" ", pos0, pos0+pos1); - if (remote_port_id && rc == 1) + rc = sscanf(current, "%nend_device-%d:%d%n:%d%n", + &pos0, &tosser0, &tosser1, &pos1, &tosser2, &pos2); + debug("current:'%s' rc:%d pos0:%d\n", current, rc, pos0); + dbgmk(" ", pos0, MAX(pos1, pos2)); + if (rc == 3) { + if (remote_port_id) *remote_port_id = tosser2; - if (local_port_id && rc == 0) + pos1 = pos2; + } else if (rc == 2) { + if (local_port_id) *local_port_id = tosser1; - sz += pos0 + pos1; - pos0 = pos1 = 0; - - if (current[sz] == '/') - sz += 1; - } else if (rc != 0) { - return -1; + } else { + pos1 = 0; } + current += pos1; + pos0 = pos1 = pos2 = -1; + + if (current[0] == '/') + current += 1; /* now: * /target4:0:0/ */ uint64_t tosser3; debug("searching for target4:0:0/"); - rc = sscanf(current + sz, "target%d:%d:%"PRIu64"/%n", &tosser0, &tosser1, - &tosser3, &pos0); - debug("current:\"%s\" rc:%d pos0:%d\n", current+sz, rc, pos0); - dbgmk(" ", pos0); + rc = sscanf(current, "%ntarget%d:%d:%"PRIu64"/%n", + &pos0, &tosser0, &tosser1, &tosser3, &pos1); + debug("current:'%s' rc:%d pos0:%d pos1:%d\n", current, rc, pos0, pos1); + dbgmk(" ", pos0, pos1); if (rc != 3) return -1; - sz += pos0; - pos0 = 0; + current += pos1; + pos0 = pos1 = -1; /* now: * %d:%d:%d:%llu/ */ debug("searching for 4:0:0:0/"); - rc = sscanf(current + sz, "%d:%d:%d:%"PRIu64"/%n", - scsi_bus, scsi_device, scsi_target, scsi_lun, &pos0); - debug("current:\"%s\" rc:%d pos0:%d\n", current+sz, rc, pos0); - dbgmk(" ", pos0); + rc = sscanf(current, "%n%d:%d:%d:%"PRIu64"/%n", + &pos0, scsi_bus, scsi_device, scsi_target, scsi_lun, &pos1); + debug("current:'%s' rc:%d pos0:%d pos1:%d\n", current, rc, pos0, pos1); + dbgmk(" ", pos0, pos1); if (rc != 4) return -1; - sz += pos0; + current += pos1; - debug("returning %d", sz); - return sz; + debug("current:'%s' sz:%zd", current, current - path); + return current - path; } static ssize_t -parse_scsi(struct device *dev, const char *current, const char *root UNUSED) +parse_scsi(struct device *dev, const char *path, const char *root UNUSED) { + const char *current = path; uint32_t scsi_host, scsi_bus, scsi_device, scsi_target; uint64_t scsi_lun; - ssize_t sz; - int pos; + int pos0, pos1; int rc; debug("entry"); - debug("searching for ../../../0:0:0:0"); - rc = sscanf(dev->device, "../../../%d:%d:%d:%"PRIu64"%n", + debug("searching device for ../../../0:0:0:0"); + pos0 = pos1 = -1; + rc = sscanf(dev->device, "../../../%n%d:%d:%d:%"PRIu64"%n", + &pos0, &dev->scsi_info.scsi_bus, &dev->scsi_info.scsi_device, &dev->scsi_info.scsi_target, &dev->scsi_info.scsi_lun, - &pos); - debug("current:\"%s\" rc:%d pos:%d\n", dev->device, rc, pos); - dbgmk(" ", pos); + &pos1); + debug("device:'%s' rc:%d pos0:%d pos1:%d\n", dev->device, rc, pos0, pos1); + dbgmk(" ", pos0, pos1); if (rc != 4) return 0; - sz = parse_scsi_link(current, &scsi_host, - &scsi_bus, &scsi_device, - &scsi_target, &scsi_lun, - NULL, NULL, NULL); - if (sz < 0) + pos0 = parse_scsi_link(current, &scsi_host, &scsi_bus, &scsi_device, + &scsi_target, &scsi_lun, NULL, NULL, NULL); + if (pos0 < 0) return 0; + current += pos0; /* * SCSI disks can have up to 16 partitions, or 4 bits worth @@ -281,7 +287,8 @@ parse_scsi(struct device *dev, const char *current, const char *root UNUSED) return -1; } - return sz; + debug("current:'%s' sz:%zd\n", current, current - path); + return current - path; } static ssize_t diff --git a/src/linux-soc-root.c b/src/linux-soc-root.c index dc2a305..efef7bb 100644 --- a/src/linux-soc-root.c +++ b/src/linux-soc-root.c @@ -38,23 +38,23 @@ * I don't *think* the devicetree nodes stack. */ static ssize_t -parse_soc_root(struct device *dev UNUSED, const char *current, const char *root UNUSED) +parse_soc_root(struct device *dev UNUSED, const char *path, const char *root UNUSED) { + const char *current = path; int rc; - int pos; - const char *devpart = current; + int pos0 = -1, pos1 = -1; - debug("entry"); + debug("entry"); - rc = sscanf(devpart, "../../devices/platform/soc/%*[^/]/%n", &pos); - debug("current:\"%s\" rc:%d pos:%d", current, rc, pos); - dbgmk(" ", pos); - if (rc != 0) - return 0; - devpart += pos; - debug("new position is \"%s\"", devpart); + rc = sscanf(current, "../../devices/%nplatform/soc/%*[^/]/%n", &pos0, &pos1); + if (rc != 0 || pos0 == -1 || pos1 == -1) + return 0; + debug("current:'%s' rc:%d pos0:%d pos1:%d", current, rc, pos0, pos1); + dbgmk(" ", pos0, pos1); + current += pos1; - return devpart - current; + debug("current:'%s' sz:%zd\n", current, current - path); + return current - path; } enum interface_type soc_root_iftypes[] = { soc_root, unknown }; diff --git a/src/linux-virtblk.c b/src/linux-virtblk.c index 3c9c690..50afab8 100644 --- a/src/linux-virtblk.c +++ b/src/linux-virtblk.c @@ -45,18 +45,19 @@ * But usually we just write the HD() entry, of course. */ static ssize_t -parse_virtblk(struct device *dev, const char *current, const char *root UNUSED) +parse_virtblk(struct device *dev, const char *path, const char *root UNUSED) { + const char *current = path; uint32_t tosser; - int pos; + int pos0 = -1, pos1 = -1; int rc; debug("entry"); debug("searching for virtio0/"); - rc = sscanf(current, "virtio%x/%n", &tosser, &pos); - debug("current:\"%s\" rc:%d pos:%d\n", current, rc, pos); - dbgmk(" ", pos); + rc = sscanf(current, "%nvirtio%x/%n", &pos0, &tosser, &pos1); + debug("current:'%s' rc:%d pos0:%d pos1:%d\n", current, rc, pos0, pos1); + dbgmk(" ", pos0, pos1); /* * If we couldn't find virtioX/ then it isn't a virtio device. */ @@ -64,8 +65,10 @@ parse_virtblk(struct device *dev, const char *current, const char *root UNUSED) return 0; dev->interface_type = virtblk; + current += pos1; - return pos; + debug("current:'%s' sz:%zd\n", current, current - path); + return current - path; } enum interface_type virtblk_iftypes[] = { virtblk, unknown }; -- 2.29.2 From cdbd8dc71ed0c86195475cd765daa7d273ea28e6 Mon Sep 17 00:00:00 2001 From: Peter Jones <pjones@redhat.com> Date: Tue, 15 Oct 2019 16:26:30 -0400 Subject: [PATCH 06/10] Improve consistency of debug prints This changes debug prints in a couple of ways: - always calls the path we're parsing "current" in the output - always use ' not " for quoting in the debug output, so tools that escape strings won't change the lenghts - everything that parses "current" has a debug print after each parse attempt and before returning. Signed-off-by: Peter Jones <pjones@redhat.com> (cherry picked from commit d153ca75d0323eb125e525ebdf4ac5c982941d15) --- src/dp-acpi.c | 8 ++++---- src/linux-acpi-root.c | 4 ++-- src/linux-acpi.c | 6 +++--- src/linux-ata.c | 11 +++++++---- src/linux-i2o.c | 10 +++++----- src/linux-md.c | 4 ++-- src/linux-pci-root.c | 2 +- src/linux-pmem.c | 5 +++-- src/linux.c | 27 +++++++++++++++++---------- 9 files changed, 44 insertions(+), 33 deletions(-) diff --git a/src/dp-acpi.c b/src/dp-acpi.c index 3a80ba9..4ce5c12 100644 --- a/src/dp-acpi.c +++ b/src/dp-acpi.c @@ -1,6 +1,6 @@ /* * libefivar - library for the manipulation of EFI variables - * Copyright 2012-2015 Red Hat, Inc. + * Copyright 2012-2019 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 License as @@ -51,9 +51,9 @@ _format_acpi_hid_ex(char *buf, size_t size, const char *dp_type UNUSED, { ssize_t off = 0; - debug("hid:0x%08x hidstr:\"%s\"", dp->acpi_hid_ex.hid, hidstr); - debug("cid:0x%08x cidstr:\"%s\"", dp->acpi_hid_ex.cid, cidstr); - debug("uid:0x%08x uidstr:\"%s\"", dp->acpi_hid_ex.uid, uidstr); + debug("hid:0x%08x hidstr:'%s'", dp->acpi_hid_ex.hid, hidstr); + debug("cid:0x%08x cidstr:'%s'", dp->acpi_hid_ex.cid, cidstr); + debug("uid:0x%08x uidstr:'%s'", dp->acpi_hid_ex.uid, uidstr); if (!hidstr && !cidstr && (uidstr || dp->acpi_hid_ex.uid)) { format(buf, size, off, "AcpiExp", diff --git a/src/linux-acpi-root.c b/src/linux-acpi-root.c index d2f012d..259c431 100644 --- a/src/linux-acpi-root.c +++ b/src/linux-acpi-root.c @@ -144,7 +144,7 @@ parse_acpi_root(struct device *dev, const char *path, const char *root UNUSED) return rc; } } - debug("Parsed HID:0x%08x UID:0x%"PRIx64" uidstr:\"%s\" path:\"%s\"", + debug("Parsed HID:0x%08x UID:0x%"PRIx64" uidstr:'%s' path:'%s'", dev->acpi_root.acpi_hid, dev->acpi_root.acpi_uid, dev->acpi_root.acpi_uid_str, dev->acpi_root.acpi_cid_str); @@ -162,7 +162,7 @@ dp_create_acpi_root(struct device *dev, debug("entry buf:%p size:%zd off:%zd", buf, size, off); if (dev->acpi_root.acpi_uid_str || dev->acpi_root.acpi_cid_str) { - debug("creating acpi_hid_ex dp hid:0x%08x uid:0x%"PRIx64" uidstr:\"%s\" cidstr:\"%s\"", + debug("creating acpi_hid_ex dp hid:0x%08x uid:0x%"PRIx64" uidstr:'%s' cidstr:'%s'", dev->acpi_root.acpi_hid, dev->acpi_root.acpi_uid, dev->acpi_root.acpi_uid_str, dev->acpi_root.acpi_cid_str); new = efidp_make_acpi_hid_ex(buf + off, size ? size - off : 0, diff --git a/src/linux-acpi.c b/src/linux-acpi.c index 346eba0..8f28306 100644 --- a/src/linux-acpi.c +++ b/src/linux-acpi.c @@ -1,6 +1,6 @@ /* * libefiboot - library for the manipulation of EFI boot variables - * Copyright 2012-2018 Red Hat, Inc. + * Copyright 2012-2019 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 License as @@ -54,7 +54,7 @@ parse_acpi_hid_uid(struct device *dev, const char *fmt, ...) if (l > 1) { fbuf[l-1] = 0; dev->acpi_root.acpi_cid_str = strdup(fbuf); - debug("Setting ACPI root path to \"%s\"", fbuf); + debug("Setting ACPI root path to '%s'", fbuf); } } @@ -111,7 +111,7 @@ hid_err: } } } - debug("acpi root UID:0x%"PRIx64" uidstr:\"%s\"", + debug("acpi root UID:0x%"PRIx64" uidstr:'%s'", dev->acpi_root.acpi_uid, dev->acpi_root.acpi_uid_str); errno = 0; diff --git a/src/linux-ata.c b/src/linux-ata.c index 6e86ba2..c7b744e 100644 --- a/src/linux-ata.c +++ b/src/linux-ata.c @@ -60,6 +60,7 @@ is_pata(struct device *dev) static ssize_t parse_ata(struct device *dev, const char *path, const char *root UNUSED) { + const char *current = path; uint32_t scsi_host, scsi_bus, scsi_device, scsi_target; uint64_t scsi_lun; int pos; @@ -118,6 +119,7 @@ parse_ata(struct device *dev, const char *path, const char *root UNUSED) NULL, NULL, NULL); if (pos < 0) return -1; + current += pos; dev->ata_info.scsi_host = scsi_host; dev->ata_info.scsi_bus = scsi_bus; @@ -125,10 +127,11 @@ parse_ata(struct device *dev, const char *path, const char *root UNUSED) dev->ata_info.scsi_target = scsi_target; dev->ata_info.scsi_lun = scsi_lun; - char *block = strstr(path, "/block/"); - if (!block) - return -1; - return block + 1 - path; + char *block = strstr(current, "/block/"); + if (block) + current += block + 1 - current; + debug("current:'%s' sz:%zd", current, current - path); + return current - path; } static ssize_t diff --git a/src/linux-i2o.c b/src/linux-i2o.c index 3ce25b9..fe11a68 100644 --- a/src/linux-i2o.c +++ b/src/linux-i2o.c @@ -1,6 +1,6 @@ /* * libefiboot - library for the manipulation of EFI boot variables - * Copyright 2012-2018 Red Hat, Inc. + * Copyright 2012-2019 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 License as @@ -33,7 +33,7 @@ * ... probably doesn't work. */ static ssize_t -parse_i2o(struct device *dev, const char *current UNUSED, const char *root UNUSED) +parse_i2o(struct device *dev, const char *current, const char *root UNUSED) { debug("entry"); /* I2O disks can have up to 16 partitions, or 4 bits worth. */ @@ -47,9 +47,9 @@ parse_i2o(struct device *dev, const char *current UNUSED, const char *root UNUSE } char *block = strstr(current, "/block/"); - if (!block) - return -1; - return block + 1 - current; + ssize_t sz = block ? block + 1 - current : -1; + debug("current:'%s' sz:%zd", current, sz); + return sz; } enum interface_type i2o_iftypes[] = { i2o, unknown }; diff --git a/src/linux-md.c b/src/linux-md.c index 4714456..333f614 100644 --- a/src/linux-md.c +++ b/src/linux-md.c @@ -50,7 +50,7 @@ parse_md(struct device *dev, const char *current, const char *root UNUSED) debug("searching for mdM/mdMpN"); rc = sscanf(current, "md%d/%nmd%dp%d%n", &md, &pos0, &tosser0, &part, &pos1); - debug("current:\"%s\" rc:%d pos0:%d pos1:%d\n", current, rc, pos0, pos1); + debug("current:'%s' rc:%d pos0:%d pos1:%d\n", current, rc, pos0, pos1); dbgmk(" ", pos0, pos1); /* * If it isn't of that form, it's not one of our partitioned md devices. @@ -63,10 +63,10 @@ parse_md(struct device *dev, const char *current, const char *root UNUSED) if (dev->part == -1) dev->part = part; + debug("current:'%s' sz:%d\n", current, pos1); return pos1; } - static char * make_part_name(struct device *dev) { diff --git a/src/linux-pci-root.c b/src/linux-pci-root.c index c105477..f67f941 100644 --- a/src/linux-pci-root.c +++ b/src/linux-pci-root.c @@ -87,7 +87,7 @@ dp_create_pci_root(struct device *dev UNUSED, debug("entry buf:%p size:%zd off:%zd", buf, size, off); debug("returning 0"); if (dev->acpi_root.acpi_uid_str) { - debug("creating acpi_hid_ex dp hid:0x%08x uid:\"%s\"", + debug("creating acpi_hid_ex dp hid:0x%08x uid:'%s'", dev->acpi_root.acpi_hid, dev->acpi_root.acpi_uid_str); new = efidp_make_acpi_hid_ex(buf + off, size ? size - off : 0, diff --git a/src/linux-pmem.c b/src/linux-pmem.c index 5327658..e2915d5 100644 --- a/src/linux-pmem.c +++ b/src/linux-pmem.c @@ -103,11 +103,12 @@ parse_pmem(struct device *dev, const char *path, const char *root UNUSED) * * 259:0 -> ../../devices/LNXSYSTM:00/LNXSYBUS:00/ACPI0012:00/ndbus0/region12/btt12.1/block/pmem12s */ + pos0 = pos1 = -1; rc = sscanf(current, "../../devices/%nLNXSYSTM:%hhx/LNXSYBUS:%hhx/ACPI%hx:%hhx/ndbus%d/region%d/btt%d.%d/%n", &pos0, &system, &sysbus, &pnp_id, &acpi_id, &ndbus, ®ion, &btt_region_id, &btt_id, &pos1); - debug("current:\"%s\" rc:%d pos0:%d pos1:%d", current, rc, pos0, pos1); + debug("current:'%s' rc:%d pos0:%d pos1:%d", current, rc, pos0, pos1); dbgmk(" ", pos0, pos1); if (rc < 8) return 0; @@ -126,7 +127,7 @@ parse_pmem(struct device *dev, const char *path, const char *root UNUSED) return -1; filebuf = NULL; - debug("nvdimm namespace is \"%s\"", namespace); + debug("nvdimm namespace is '%s'", namespace); rc = read_sysfs_file(&filebuf, "bus/nd/devices/%s/uuid", namespace); free(namespace); if (rc < 0 || filebuf == NULL) diff --git a/src/linux.c b/src/linux.c index 5ee4f99..ea40236 100644 --- a/src/linux.c +++ b/src/linux.c @@ -184,10 +184,10 @@ set_disk_and_part_name(struct device *dev) errno = 0; debug("dev->disk_name:%p dev->part_name:%p", dev->disk_name, dev->part_name); debug("dev->part:%d", dev->part); - debug("ultimate:\"%s\"", ultimate ? : ""); - debug("penultimate:\"%s\"", penultimate ? : ""); - debug("approximate:\"%s\"", approximate ? : ""); - debug("proximate:\"%s\"", proximate ? : ""); + debug("ultimate:'%s'", ultimate ? : ""); + debug("penultimate:'%s'", penultimate ? : ""); + debug("approximate:'%s'", approximate ? : ""); + debug("proximate:'%s'", proximate ? : ""); if (ultimate && penultimate && ((proximate && !strcmp(proximate, "nvme")) || @@ -470,7 +470,11 @@ struct device HIDDEN efi_error("parsing %s failed", probe->name); goto err; } else if (pos > 0) { - debug("%s matched %s", probe->name, current); + char match[pos+1]; + + strncpy(match, current, pos); + match[pos] = '\0'; + debug("%s matched '%s'", probe->name, match); dev->flags |= probe->flags; if (probe->flags & DEV_PROVIDES_HD || @@ -480,7 +484,10 @@ struct device HIDDEN dev->probes[n++] = dev_probes[i]; current += pos; - debug("current:%s", current); + if (current[0] == '\0') + debug("finished"); + else + debug("current:'%s'", current); last_successful_probe = i; if (!*current || !strncmp(current, "block/", 6)) @@ -489,8 +496,8 @@ struct device HIDDEN continue; } - debug("dev_probes[i+1]: %p dev->interface_type: %d\n", - dev_probes[i+1], dev->interface_type); + debug("dev_probes[%d]: %p dev->interface_type: %d\n", + i+1, dev_probes[i+1], dev->interface_type); if (dev_probes[i+1] == NULL && dev->interface_type == unknown) { pos = 0; rc = sscanf(current, "%*[^/]/%n", &pos); @@ -506,8 +513,8 @@ slash_err: if (!current[pos]) goto slash_err; - debug("Cannot parse device link segment \"%s\"", current); - debug("Skipping to \"%s\"", current + pos); + debug("Cannot parse device link segment '%s'", current); + debug("Skipping to '%s'", current + pos); debug("This means we can only create abbreviated paths"); dev->flags |= DEV_ABBREV_ONLY; i = last_successful_probe; -- 2.29.2 From 068e3cf7ce405d83bb81771ae777adbd42dc8521 Mon Sep 17 00:00:00 2001 From: Peter Jones <pjones@redhat.com> Date: Fri, 11 Oct 2019 14:20:54 -0400 Subject: [PATCH 07/10] Try even harder to find disk device symlinks in sysfs. Today's realization is that the thing encoded into the structure of sysfs is, in the best case, the dependency graph of the makefile targets to build a device driver. In the case of nvme-fabric, or really wherever the kernel has class_create() and device_create() in the same function, there's an extra level of indirection. Anyway, in this patch we stop pretending sysfs isn't completely absurd, and just try adding "/device" in the middle of the driver symlink path, until we actually either get ENOENT on the device symlink or find a device symlink that actually has a driver symlink under it. Signed-off-by: Peter Jones <pjones@redhat.com> (cherry picked from commit c9c1424d0e09bf33b747d37c43177c63367b1290) NOTE: Removed efi_error_pop() --- src/linux-nvme.c | 13 +++++---- src/linux.c | 40 +++++++++++++++++----------- src/linux.h | 69 +++++++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 98 insertions(+), 24 deletions(-) diff --git a/src/linux-nvme.c b/src/linux-nvme.c index 648e6c3..a0798b6 100644 --- a/src/linux-nvme.c +++ b/src/linux-nvme.c @@ -89,13 +89,12 @@ parse_nvme(struct device *dev, const char *path, const char *root UNUSED) /* * now fish the eui out of sysfs is there is one... */ - rc = read_sysfs_file(&filebuf, - "class/block/nvme%dn%d/eui", - ctrl_id, ns_id); - if ((rc < 0 && errno == ENOENT) || filebuf == NULL) { - rc = read_sysfs_file(&filebuf, - "class/block/nvme%dn%d/device/eui", - ctrl_id, ns_id); + char *euipath = NULL; + rc = read_sysfs_file(&filebuf, "class/block/nvme%dn%d/eui", ctrl_id, ns_id); + if (rc < 0 && (errno == ENOENT || errno == ENOTDIR)) { + rc = find_device_file(&euipath, "eui", "class/block/nvme%dn%d", ctrl_id, ns_id); + if (rc >= 0 && euipath != NULL) + rc = read_sysfs_file(&filebuf, "%s", euipath); } if (rc >= 0 && filebuf != NULL) { uint8_t eui[8]; diff --git a/src/linux.c b/src/linux.c index ea40236..fcb8684 100644 --- a/src/linux.c +++ b/src/linux.c @@ -410,25 +410,33 @@ struct device HIDDEN goto err; } - if (dev->device[0] != 0) { - rc = sysfs_readlink(&tmpbuf, "block/%s/device/driver", dev->disk_name); - if (rc < 0 || !tmpbuf) { - if (errno == ENOENT) { - /* - * nvme, for example, will have nvme0n1/device point - * at nvme0, and we need to look for device/driver - * there. - */ - rc = sysfs_readlink(&tmpbuf, - "block/%s/device/device/driver", - dev->disk_name); - } + /* + * So, on a normal disk, you get something like: + * /sys/block/sda/device -> ../../0:0:0:0 + * /sys/block/sda/device/driver -> ../../../../../../../bus/scsi/drivers/sd + * + * On a directly attached nvme device you get: + * /sys/block/nvme0n1/device -> ../../nvme0 + * /sys/block/nvme0n1/device/device -> ../../../0000:6e:00.0 + * /sys/block/nvme0n1/device/device/driver -> ../../../../bus/pci/drivers/nvme + * + * On a fabric-attached nvme device, you get something like: + * /sys/block/nvme0n1/device -> ../../nvme0 + * /sys/block/nvme0n1/device/device -> ../../ctl + * /sys/block/nvme0n1/device/device/device -> ../../../../../0000:6e:00.0 + * /sys/block/nvme0n1/device/device/device/driver -> ../../../../../../bus/pci/drivers/nvme-fabrics + * + * ... I think? I don't have one in front of me. + */ + + char *filepath = NULL; + rc = find_device_file(&filepath, "driver", "block/%s", dev->disk_name); + if (rc >= 0) { + rc = sysfs_readlink(&tmpbuf, "%s", filepath); if (rc < 0 || !tmpbuf) { - efi_error("readlink of /sys/block/%s/device/driver failed", - dev->disk_name); + efi_error("readlink of /sys/%s failed", filepath); goto err; } - } linkbuf = pathseg(tmpbuf, -1); if (!linkbuf) { diff --git a/src/linux.h b/src/linux.h index 43a9b78..401e3a0 100644 --- a/src/linux.h +++ b/src/linux.h @@ -1,6 +1,6 @@ /* * libefiboot - library for the manipulation of EFI boot variables - * Copyright 2012-2015 Red Hat, Inc. + * Copyright 2012-2019 Red Hat, Inc. * Copyright (C) 2001 Dell Computer Corporation <Matt_Domsch@dell.com> * * This library is free software; you can redistribute it and/or @@ -218,6 +218,22 @@ extern ssize_t HIDDEN make_mac_path(uint8_t *buf, ssize_t size, _rc; \ }) +#define sysfs_access(mode, fmt, args...) \ + ({ \ + int rc_; \ + char *pn_; \ + \ + rc_ = asprintfa(&pn_, "/sys/" fmt, ## args); \ + if (rc_ >= 0) { \ + rc_ = access(pn_, mode); \ + if (rc_ < 0) \ + efi_error("could not access %s", pn_); \ + } else { \ + efi_error("could not allocate memory"); \ + } \ + rc_; \ + }) + #define sysfs_stat(statbuf, fmt, args...) \ ({ \ int rc_; \ @@ -251,6 +267,57 @@ extern ssize_t HIDDEN make_mac_path(uint8_t *buf, ssize_t size, dir_; \ }) +/* + * Iterate a /sys/block directory looking for device/foo, device/device/foo, + * etc. I'm not proud of this method. + */ +#define find_device_file(result, name, fmt, args...) \ + ({ \ + int rc_ = 0; \ + debug("searching for %s from in %s", name, dev->disk_name); \ + for (unsigned int try_ = 0; true; try_++) { \ + char slashdev_[sizeof("device") \ + + try_ * strlen("/device")]; \ + \ + char *nul_ = stpcpy(slashdev_, "device"); \ + for (unsigned int i_ = 0; i_ < try_; i_++) \ + nul_ = stpcpy(nul_, "/device"); \ + \ + debug("trying /sys/" fmt "/%s/%s", \ + ## args, slashdev_, name); \ + \ + rc_ = sysfs_access(F_OK, fmt "/%s", ## args, slashdev_);\ + if (rc_ < 0) { \ + if (errno == ENOENT) { \ + break; \ + } \ + efi_error("cannot access /sys/"fmt"/%s: %m", \ + ## args, slashdev_); \ + goto find_device_link_err_; \ + } \ + \ + rc_ = sysfs_access(F_OK, fmt "/%s/%s", \ + ## args, slashdev_, name); \ + if (rc_ < 0) { \ + if (errno == ENOENT) { \ + break; \ + } \ + efi_error("cannot access /sys/"fmt"/%s/%s: %m", \ + ## args, slashdev_, name); \ + goto find_device_link_err_; \ + } \ + \ + rc_ = asprintfa(result, fmt "/%s/%s", \ + ## args, slashdev_, name); \ + if (rc_ < 0) { \ + efi_error("cannot allocate memory: %m"); \ + goto find_device_link_err_; \ + } \ + } \ +find_device_link_err_: \ + rc_; \ + }) + #define DEV_PROVIDES_ROOT 1 #define DEV_PROVIDES_HD 2 #define DEV_ABBREV_ONLY 4 -- 2.29.2 From 1135cdf7a817eae4cd217cd7774099543bb487ea Mon Sep 17 00:00:00 2001 From: Peter Jones <pjones@redhat.com> Date: Wed, 2 Oct 2019 17:04:12 -0400 Subject: [PATCH 08/10] Handle /sys/devices/virtual/{nvme-fabrics,nvme-subsystem} devices Signed-off-by: Peter Jones <pjones@redhat.com> (cherry picked from commit c41da0bdce04ab0f99b992d51ff6fd0ab55d040a) --- src/linux-nvme.c | 50 ++++++++++++++++++++--- src/linux-virtual-root.c | 88 ++++++++++++++++++++++++++++++++++++++++ src/linux.c | 43 +++++++++++++++++--- src/linux.h | 4 +- 4 files changed, 172 insertions(+), 13 deletions(-) create mode 100644 src/linux-virtual-root.c diff --git a/src/linux-nvme.c b/src/linux-nvme.c index a0798b6..74707e3 100644 --- a/src/linux-nvme.c +++ b/src/linux-nvme.c @@ -15,7 +15,6 @@ * You should have received a copy of the GNU Lesser General Public * License along with this library; if not, see * <http://www.gnu.org/licenses/>. - * */ #include "fix_coverity.h" @@ -35,6 +34,12 @@ * /sys/dev/block/$major:$minor looks like: * 259:0 -> ../../devices/pci0000:00/0000:00:1d.0/0000:05:00.0/nvme/nvme0/nvme0n1 * 259:1 -> ../../devices/pci0000:00/0000:00:1d.0/0000:05:00.0/nvme/nvme0/nvme0n1/nvme0n1p1 + * or: + * 259:0 ->../../devices/virtual/nvme-fabrics/ctl/nvme0/nvme0n1 + * 259:1 ->../../devices/virtual/nvme-fabrics/ctl/nvme0/nvme0n1/nvme0n1p1 + * or: + * 259:5 -> ../../devices/virtual/nvme-subsystem/nvme-subsys0/nvme0n1 + * 259:6 -> ../../devices/virtual/nvme-subsystem/nvme-subsys0/nvme0n1/nvme0n1p1 * * /sys/dev/block/259:0/device looks like: * device -> ../../nvme0 @@ -56,14 +61,42 @@ parse_nvme(struct device *dev, const char *path, const char *root UNUSED) int32_t tosser0, tosser1, tosser2, ctrl_id, ns_id, partition; uint8_t *filebuf = NULL; int pos0 = -1, pos1 = -1, pos2 = -1; + ssize_t sz = 0; + struct subdir { + const char * const name; + const char * const fmt; + int *pos0, *pos1; + } subdirs[] = { + {"nvme-subsysN/", "%nnvme-subsys%d/%n", &pos0, &pos2}, + {"ctl/", "%nctl/%n%n", &pos0, &pos1}, + {"nvme/", "%nnvme/%n%n", &pos0, &pos1}, + {NULL, } + }; debug("entry"); - debug("searching for nvme/nvme0/nvme0n1 or nvme/nvme0/nvme0n1/nvme0n1p1"); - rc = sscanf(current, "%nnvme/nvme%d/nvme%dn%d%n/nvme%dn%dp%d%n", - &pos0, &tosser0, &ctrl_id, &ns_id, - &pos1, &tosser1, &tosser2, &partition, &pos2); - debug("current:\"%s\" rc:%d pos0:%d pos1:%d pos2:%d\n", current, rc, pos0, pos1, pos2); + /* + * in this case, *any* of these is okay. + */ + for (int i = 0; subdirs[i].name; i++) { + debug("searching for %s", subdirs[i].name); + pos0 = tosser0 = pos1 = -1; + rc = sscanf(current, subdirs[i].fmt, &pos0, &pos1, &pos2); + debug("current:'%s' rc:%d pos0:%d pos1:%d\n", current, rc, + *subdirs[i].pos0, *subdirs[i].pos1); + dbgmk(" ", *subdirs[i].pos0, *subdirs[i].pos1); + if (*subdirs[i].pos0 >= 0 && *subdirs[i].pos1 >= *subdirs[i].pos0) { + sz += *subdirs[i].pos1; + current += *subdirs[i].pos1; + break; + } + } + + debug("searching for nvme0/nvme0n1 or nvme0/nvme0n1/nvme0n1p1"); + rc = sscanf(current, "%nnvme%d/nvme%dn%d%n/nvme%dn%dp%d%n", + &pos0, &tosser0, &ctrl_id, &ns_id, &pos1, + &tosser1, &tosser2, &partition, &pos2); + debug("current:'%s' rc:%d pos0:%d pos1:%d pos2:%d\n", current, rc, pos0, pos1, pos2); dbgmk(" ", pos0, MAX(pos1,pos2)); /* * If it isn't of that form, it's not one of our nvme devices. @@ -84,11 +117,13 @@ parse_nvme(struct device *dev, const char *path, const char *root UNUSED) pos1 = pos2; } + current += pos1; /* * now fish the eui out of sysfs is there is one... */ + debug("looking for the eui"); char *euipath = NULL; rc = read_sysfs_file(&filebuf, "class/block/nvme%dn%d/eui", ctrl_id, ns_id); if (rc < 0 && (errno == ENOENT || errno == ENOTDIR)) { @@ -111,6 +146,9 @@ parse_nvme(struct device *dev, const char *path, const char *root UNUSED) errno = EINVAL; return -1; } + debug("eui is %02hhx:%02hhx:%02hhx:%02hhx:%02hhx:%02hhx:%02hhx:%02hhx", + eui[0], eui[1], eui[2], eui[3], + eui[4], eui[5], eui[6], eui[7]); dev->nvme_info.has_eui = 1; memcpy(dev->nvme_info.eui, eui, sizeof(eui)); } diff --git a/src/linux-virtual-root.c b/src/linux-virtual-root.c new file mode 100644 index 0000000..75fbbfc --- /dev/null +++ b/src/linux-virtual-root.c @@ -0,0 +1,88 @@ +/* + * libefiboot - library for the manipulation of EFI boot variables + * Copyright 2012-2019 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 License as + * published by the Free Software Foundation; either version 2.1 of the + * License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, see + * <http://www.gnu.org/licenses/>. + */ + +#include "fix_coverity.h" + +#include <errno.h> +#include <fcntl.h> +#include <inttypes.h> +#include <stdint.h> +#include <unistd.h> + +#include "efiboot.h" + +/* + * Support virtually rooted devices (fibre+nvme, etc.) + * + * /sys/dev/block/$major:$minor looks like: + * 259:0 ->../../devices/virtual/nvme-fabrics/ctl/nvme0/nvme0n1 + * 259:1 ->../../devices/virtual/nvme-fabrics/ctl/nvme0/nvme0n1/nvme0n1p1 + * or: + * 259:5 -> ../../devices/virtual/nvme-subsystem/nvme-subsys0/nvme0n1 + * 259:6 -> ../../devices/virtual/nvme-subsystem/nvme-subsys0/nvme0n1/nvme0n1p1 + */ + +static ssize_t +parse_virtual_root(struct device *dev UNUSED, const char *current, const char *root UNUSED) +{ + int rc; + ssize_t sz; + int pos0 = 0, pos1 = 0; + struct subdir { + const char * const name; + const char * const fmt; + } subdirs[] = { + {"../../devices/virtual", "%n../../devices/virtual/%n"}, + {"nvme-subsystem/", "%nnvme-subsystem/%n"}, + {"nvme-fabrics/ctl/", "%nnvme-fabrics/ctl/%n"}, + {NULL, NULL} + }; + + debug("entry"); + + for (int i = 0; subdirs[i].name; i++) { + debug("searching for %s", subdirs[i].name); + pos0 = pos1 = -1; + rc = sscanf(current, subdirs[i].fmt, &pos0, &pos1); + debug("current:'%s' rc:%d pos0:%d pos1:%d\n", current, rc, pos0, pos1); + dbgmk(" ", pos0, pos1); + if (rc == 1) { + sz += pos1; + current += pos1; + if (i > 0) + goto found; + } + } + + sz = 0; +found: + debug("current:'%s' sz:%zd\n", current, sz); + return sz; +} + +static enum interface_type virtual_root_iftypes[] = { virtual_root, unknown }; + +struct dev_probe HIDDEN virtual_root_parser = { + .name = "virtual_root", + .iftypes = virtual_root_iftypes, + .flags = DEV_ABBREV_ONLY|DEV_PROVIDES_ROOT, + .parse = parse_virtual_root, +}; + +// vim:fenc=utf-8:tw=75:noet diff --git a/src/linux.c b/src/linux.c index fcb8684..72e4edf 100644 --- a/src/linux.c +++ b/src/linux.c @@ -170,16 +170,17 @@ int HIDDEN set_disk_and_part_name(struct device *dev) { int rc = -1; - - /* - * results are like such: - * maj:min -> ../../devices/pci$PCI_STUFF/$BLOCKDEV_STUFF/block/$DISK/$PART - */ - char *ultimate = pathseg(dev->link, -1); char *penultimate = pathseg(dev->link, -2); char *approximate = pathseg(dev->link, -3); char *proximate = pathseg(dev->link, -4); + char *psl5 = pathseg(dev->link, -5); + + + /* + * devlinks look something like: + * maj:min -> ../../devices/pci$PCI_STUFF/$BLOCKDEV_STUFF/block/$DISK/$PART + */ errno = 0; debug("dev->disk_name:%p dev->part_name:%p", dev->disk_name, dev->part_name); @@ -188,6 +189,7 @@ set_disk_and_part_name(struct device *dev) debug("penultimate:'%s'", penultimate ? : ""); debug("approximate:'%s'", approximate ? : ""); debug("proximate:'%s'", proximate ? : ""); + debug("psl5:'%s'", psl5 ? : ""); if (ultimate && penultimate && ((proximate && !strcmp(proximate, "nvme")) || @@ -232,6 +234,34 @@ set_disk_and_part_name(struct device *dev) set_disk_name(dev, "%s", ultimate); debug("disk:%s", ultimate); rc = 0; + } else if ((proximate && ultimate && !strcmp(proximate, "nvme-fabrics")) || + (approximate && ultimate && !strcmp(approximate, "nvme-subsystem"))) { + /* + * 259:0 ->../../devices/virtual/nvme-fabrics/ctl/nvme0/nvme0n1 + * ^ proximate ^ ultimate + * or + * 259:5 -> ../../devices/virtual/nvme-subsystem/nvme-subsys0/nvme0n1 + * ^ approximate ^ penultimate + * ultimate ^ + */ + set_disk_name(dev, "%s", ultimate); + debug("disk:%s", ultimate); + rc = 0; + } else if ((psl5 && penultimate && ultimate && !strcmp(psl5, "nvme-fabrics")) || + (proximate && penultimate && ultimate && !strcmp(proximate, "nvme-subsystem"))) { + /* + * 259:1 -> ../../devices/virtual/nvme-fabrics/ctl/nvme0/nvme0n1/nvme0n1p1 + * ^psl5 ^ penultimate + * ultimate ^ + * or + * 259:6 -> ../../devices/virtual/nvme-subsystem/nvme-subsys0/nvme0n1/nvme0n1p1 + * ^ proximate ^ penultimate + * ultimate ^ + */ + set_disk_name(dev, "%s", penultimate); + set_part_name(dev, "%s", ultimate); + debug("disk:%s part:%s", penultimate, ultimate); + rc = 0; } if (rc < 0) @@ -248,6 +278,7 @@ static struct dev_probe *dev_probes[] = { &acpi_root_parser, &pci_root_parser, &soc_root_parser, + &virtual_root_parser, &pci_parser, &virtblk_parser, &sas_parser, diff --git a/src/linux.h b/src/linux.h index 401e3a0..2bee8b4 100644 --- a/src/linux.h +++ b/src/linux.h @@ -99,7 +99,8 @@ struct emmc_info { enum interface_type { unknown, - isa, acpi_root, pci_root, soc_root, pci, network, + isa, acpi_root, pci_root, soc_root, virtual_root, + pci, network, ata, atapi, scsi, sata, sas, usb, i1394, fibre, i2o, md, virtblk, @@ -344,6 +345,7 @@ extern struct dev_probe pmem_parser; extern struct dev_probe pci_root_parser; extern struct dev_probe acpi_root_parser; extern struct dev_probe soc_root_parser; +extern struct dev_probe virtual_root_parser; extern struct dev_probe pci_parser; extern struct dev_probe sas_parser; extern struct dev_probe sata_parser; -- 2.29.2 From 7265b9240f468cc04246a987e75a478f34b7d687 Mon Sep 17 00:00:00 2001 From: Chih-Wei Huang <cwhuang@linux.org.tw> Date: Wed, 22 Jan 2020 12:16:12 +0800 Subject: [PATCH 09/10] Fix variable 'sz' uninitialized error To fix the error: external/efivar/src/linux-virtual-root.c:66:4: error: variable 'sz' is uninitialized when used here [-Werror,-Wuninitialized] sz += pos1; ^~ external/efivar/src/linux-virtual-root.c:45:12: note: initialize the variable 'sz' to silence this warning ssize_t sz; ^ = 0 1 error generated. Fixes: c41da0bd ("Handle /sys/devices/virtual/{nvme-fabrics,nvme-subsystem} devices") Signed-off-by: Chih-Wei Huang <cwhuang@linux.org.tw> (cherry picked from commit 9dc04c2fd88b6e0e0fe411885041925d52f71af3) --- src/linux-virtual-root.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/linux-virtual-root.c b/src/linux-virtual-root.c index 75fbbfc..2d39c28 100644 --- a/src/linux-virtual-root.c +++ b/src/linux-virtual-root.c @@ -42,7 +42,7 @@ static ssize_t parse_virtual_root(struct device *dev UNUSED, const char *current, const char *root UNUSED) { int rc; - ssize_t sz; + ssize_t sz = 0; int pos0 = 0, pos1 = 0; struct subdir { const char * const name; -- 2.29.2 From db3ba61c02e64bcf32a0d8acc94a10be03d557d4 Mon Sep 17 00:00:00 2001 From: dann frazier <dann.frazier@canonical.com> Date: Mon, 17 Aug 2020 13:11:15 -0600 Subject: [PATCH 10/10] Fix parsing for nvme-subsystem devices nvme-subsystem devices have a link that looks like: ../../devices/virtual/nvme-subsystem/nvme-subsys0/nvme0n1 However, the current code expects an additional /nvme0/ component, i.e.: ../../devices/virtual/nvme-subsystem/nvme-subsys0/nvme0/nvme0n1 Fix this by adding a separate branch for parsing nvme-subsystem devices. Resolves github issue #157. Signed-off-by: dann frazier <dann.frazier@canonical.com> (cherry picked from commit 4e12f997f8b6af76ef65e7045c232b7d642a1af4) --- src/linux-nvme.c | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/src/linux-nvme.c b/src/linux-nvme.c index 74707e3..16db03d 100644 --- a/src/linux-nvme.c +++ b/src/linux-nvme.c @@ -57,7 +57,7 @@ static ssize_t parse_nvme(struct device *dev, const char *path, const char *root UNUSED) { const char *current = path; - int rc; + int i, rc; int32_t tosser0, tosser1, tosser2, ctrl_id, ns_id, partition; uint8_t *filebuf = NULL; int pos0 = -1, pos1 = -1, pos2 = -1; @@ -78,7 +78,7 @@ parse_nvme(struct device *dev, const char *path, const char *root UNUSED) /* * in this case, *any* of these is okay. */ - for (int i = 0; subdirs[i].name; i++) { + for (i = 0; subdirs[i].name; i++) { debug("searching for %s", subdirs[i].name); pos0 = tosser0 = pos1 = -1; rc = sscanf(current, subdirs[i].fmt, &pos0, &pos1, &pos2); @@ -92,6 +92,19 @@ parse_nvme(struct device *dev, const char *path, const char *root UNUSED) } } + if (!subdirs[i].name) + return 0; + + debug("searching for nvme-subsysN/"); + if (!strncmp("nvme-subsysN/", subdirs[i].name, 13)) { + debug("searching for nvme0n1"); + rc = sscanf(current, "%nnvme%dn%d%n", + &pos0, &ctrl_id, &ns_id, &pos1); + debug("current:'%s' rc:%d pos0:%d pos1:%d\n", current, rc, pos0, pos1); + dbgmk(" ", pos0, pos1); + if (rc != 2) + return 0; + } else { debug("searching for nvme0/nvme0n1 or nvme0/nvme0n1/nvme0n1p1"); rc = sscanf(current, "%nnvme%d/nvme%dn%d%n/nvme%dn%dp%d%n", &pos0, &tosser0, &ctrl_id, &ns_id, &pos1, @@ -105,6 +118,7 @@ parse_nvme(struct device *dev, const char *path, const char *root UNUSED) return 0; if (rc == 3) pos2 = pos1; + } dev->nvme_info.ctrl_id = ctrl_id; dev->nvme_info.ns_id = ns_id; -- 2.29.2
Locations
Projects
Search
Status Monitor
Help
OpenBuildService.org
Documentation
API Documentation
Code of Conduct
Contact
Support
@OBShq
Terms
openSUSE Build Service is sponsored by
The Open Build Service is an
openSUSE project
.
Sign Up
Log In
Places
Places
All Projects
Status Monitor