[PATCH (efibootguard) 2/2] bg_printenv: Add -v option for verbosity

[Reichel Andreas]

Add -v option also to bg_printenv to get library messages.

Signed-off-by: Andreas Reichel <andreas.r...@siemens.com>
---
tools/bg_setenv.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/bg_setenv.c b/tools/bg_setenv.c
index 1a92748..06ce30a 100644
--- a/tools/bg_setenv.c
+++ b/tools/bg_setenv.c
@@ -32,7 +32,9 @@ static struct argp_option options_setenv[] = {
{"bootonce", 'b', 0, 0, "Simulate boot with update installed"},
{0}};

-static struct argp_option options_printenv[] = {{0}};
+static struct argp_option options_printenv[] = {
+ {"verbose", 'v', 0, 0, "Be verbose"},
+ {0}};

struct arguments {
bool output_to_file;
--
2.11.0

[Reichel Andreas]

The following features are added to the tools / library:

ebgparted.c:
Enhance block device scan algorithm:

* If the user manually changes the naming scheme of loopback devices,
detection of partitions breaks. The following gives an example case:

# Create a device node /dev/hdq equivalent to /dev/loop8
mknod /dev/hdq b 7 8
# Setup a loopback device with /home/user/testdisk.img
losetup /dev/hdq /home/user/testdisk.img

Some systems automatically create a /dev/loop8 device, others don't.
On systems, where /dev/loop8 is missing, ebgpart.c probes the file
/sys/block/loop8
and assumes, the file /dev/loop8 with the same basename exists, which
will fail.
The new algorithm also reads /sys/block/loop8/dev, which contains the
major and minor revision of the node and then looks inside /dev for
a matching node, no matter what its name is.

*Note*: It is not recommended to manually change the naming scheme,
since other tools also get into problems, for example 'lsblk', which
will not correctly print partitions.

bg_printenv:
Add the -v option according to bg_setenv to get info/debug messages
from the underlying libs.

Signed-off-by: Andreas Reichel <andreas.r...@siemens.com>

Reichel Andreas (2):
bg_utils: Fix detection for customized loop devs
bg_printenv: Add -v option for verbosity

tools/bg_setenv.c | 4 ++-
tools/ebgpart.c | 86 ++++++++++++++++++++++++++++++++++++++++++++++---------
tools/ebgpart.h | 3 +-
3 files changed, 77 insertions(+), 16 deletions(-)

--
2.11.0

[Reichel Andreas]

If the user manually creates a loopback device node
in /dev, for example /dev/NAME and uses it to setup a
loopback device, the resulting base name of /sys/block/loop*
does not match that of /dev/NAME. On some systems,
/dev/loop* gets created automatically by using losetup, on
others not.
The solution is to read the major and minor revision
out of /sys/block/NAME/dev and look for the same
in /dev. Thus, the correct block device node in /dev
can be found.

Signed-off-by: Andreas Reichel <andreas.r...@siemens.com>
---

tools/ebgpart.c | 86 +++++++++++++++++++++++++++++++++++++++++++++++----------
tools/ebgpart.h | 3 +-
2 files changed, 74 insertions(+), 15 deletions(-)

diff --git a/tools/ebgpart.c b/tools/ebgpart.c
index 07edb05..05cb1fb 100644
--- a/tools/ebgpart.c
+++ b/tools/ebgpart.c
@@ -355,25 +355,83 @@ bool check_partition_table(PedDevice *dev)

void ped_device_probe_all()
{
- struct dirent *devfile;
+ struct dirent *sysblockfile;
char fullname[256];

- DIR *devdir = opendir(DEVDIRNAME);
- if (!devdir) {
- VERBOSE(stderr, "Could not open %s\n", DEVDIRNAME);
+ DIR *sysblockdir = opendir(SYSBLOCKDIR);
+ if (!sysblockdir) {
+ VERBOSE(stderr, "Could not open %s\n", SYSBLOCKDIR);
return;
}

- /* get all files from devdir */
+ /* get all files from sysblockdir */
do {
- devfile = readdir(devdir);
- if (!devfile)
- break;
- if (strcmp(devfile->d_name, ".") == 0 ||
- strcmp(devfile->d_name, "..") == 0)
+ sysblockfile = readdir(sysblockdir);
+ if (!sysblockfile) break;
+ if (strcmp(sysblockfile->d_name, ".") == 0 ||
+ strcmp(sysblockfile->d_name, "..") == 0)
continue;
-
- snprintf(fullname, 255, "/dev/%s", devfile->d_name);
+ snprintf(fullname, 255, "/sys/block/%s/dev",
+ sysblockfile->d_name);
+ /* Get major and minor revision from /sys/block/sdX/dev */
+ FILE *fh = fopen(fullname, "r");
+ if (fh == 0) {
+ VERBOSE(
+ stderr,
+ "Error opening %s for read", fullname);
+ continue;
+ }
+ int fmajor, fminor;
+ if (fscanf(fh, "%u:%u", &fmajor, &fminor) < 2) {
+ VERBOSE(
+ stderr,
+ "Error reading major/minor of device entry. (%s)\n",
+ strerror(errno));
+ fclose(fh);
+ continue;
+ };
+ fclose(fh);
+ VERBOSE(stdout,
+ "Trying device with: Major = %d, Minor = %d, (%s)\n",
+ fmajor, fminor, fullname);
+ /* Check if this file is really in the dev directory */
+ snprintf(fullname, 255, "%s/%s", DEVDIR, sysblockfile->d_name);
+ struct stat fstat;
+ if (stat(fullname, &fstat) != -1) {
+ goto devnode_found;
+ }
+ /* Node with same name not found in /dev, thus search for node
+ * with identical Major and Minor revision */
+ DIR *devdir = opendir(DEVDIR);
+ if (!devdir) {
+ VERBOSE(stderr, "Failed to open %s\n", DEVDIR);
+ continue;
+ }
+ struct dirent *devfile;
+ do {
+ devfile = readdir(devdir);
+ if (!devfile) {
+ break;
+ }
+ snprintf(fullname, 255, "%s/%s", DEVDIR,
+ devfile->d_name);
+ if (stat(fullname, &fstat) == -1) {
+ VERBOSE(stderr, "stat failed on %s\n",
+ fullname);
+ break;
+ }
+ if (major(fstat.st_rdev) == fmajor &&
+ minor(fstat.st_rdev) == fminor) {
+ VERBOSE(stdout, "Node found: %s\n", fullname);
+ break;
+ }
+ fullname[0] = 0;
+ } while (devfile);
+ closedir(devdir);
+ devnode_found:
+ if (strlen(fullname) == 0) {
+ continue;
+ }
/* This is a block device, so add it to the list*/
PedDevice *dev = calloc(sizeof(PedDevice), 1);
asprintf(&dev->model, "N/A");
@@ -385,9 +443,9 @@ void ped_device_probe_all()
free(dev->path);
free(dev);
}
- } while (devfile);
+ } while (sysblockfile);

- closedir(devdir);
+ closedir(sysblockdir);
}

void ped_partition_destroy(PedPartition *p)
diff --git a/tools/ebgpart.h b/tools/ebgpart.h
index 76fee9d..679a1d7 100644
--- a/tools/ebgpart.h
+++ b/tools/ebgpart.h
@@ -41,7 +41,8 @@
#include <string.h>
#include <stdlib.h>

-#define DEVDIRNAME "/sys/block"
+#define SYSBLOCKDIR "/sys/block"
+#define DEVDIR "/dev"

#define LB_SIZE 512

--
2.11.0

[Claudius Heine]

If you want to be more secure, read one line first with read() (with
char limit) and use sscanf on this.

> + VERBOSE(
> + stderr,
> + "Error reading major/minor of device entry. (%s)\n",
> + strerror(errno));
> + fclose(fh);
> + continue;
> + };
> + fclose(fh);

Two close() for one file, could be done with goto, if you do it in an
additional function.

> + VERBOSE(stdout,
> + "Trying device with: Major = %d, Minor = %d, (%s)\n",
> + fmajor, fminor, fullname);
> + /* Check if this file is really in the dev directory */
> + snprintf(fullname, 255, "%s/%s", DEVDIR, sysblockfile->d_name);
> + struct stat fstat;
> + if (stat(fullname, &fstat) != -1) {
> + goto devnode_found;

No. Please rather use a function for this. A function that just returns
the path to the node in /dev with the specified minor/major.

gotos are ok for memory management, not for this.

Another reason for moving this into its own function. Then you don't
have two nested loops in one function.

Or even better if you just read once through those nodes and store them
somewhere. Then you have O(2n) instead of O(n^2).

[Reichel Andreas]

Add -v option also to bg_printenv to get library messages.

Signed-off-by: Andreas Reichel <andreas.r...@siemens.com>
---