search for: add_partitions

From: Pino Toscano <ptoscano@redhat.com> If the device name ends with a number, Linux uses partition names of the form <device>p<N>. Handle this case by knocking off the 'p' character. --- daemon/devsparts.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/daemon/devsparts.c b/daemon/devsparts.c index 5862ae2..b764f63 100644 --- a/daemon/devsparts.c +++

...MD devices in the + * list returned by guestfs_list_devices. This is because most + * API users expect that this list is effectively the same as + * the list of devices added by guestfs_add_drive. + */ + return foreach_block_device (add_device, false); } static int @@ -184,7 +192,7 @@ add_partitions (const char *device, struct stringsbuf *r) char ** do_list_partitions (void) { - return foreach_block_device (add_partitions); + return foreach_block_device (add_partitions, true); } char * -- 2.9.3

...ing (r, dev_path) == -1) { + if (asprintf (&dev_path, "/dev/%s", device) == -1) { + reply_with_perror ("asprintf"); return -1; } + if (add_string (r, dev_path) == -1) + return -1; + return 0; } @@ -153,10 +156,13 @@ do_list_devices (void) static int add_partitions (const char *device, struct stringsbuf *r) { - char devdir[256]; + CLEANUP_FREE char *devdir; /* Open the device's directory under /sys/block */ - snprintf (devdir, sizeof devdir, "/sys/block/%s", device); + if (asprintf (&devdir, "/sys/block/%s", device) == -1...

...same as the list of - * devices added by guestfs_add_drive. - * - * Also, MD devices are special devices - unlike the devices exposed - * by QEMU, and there is a special API for them, - * guestfs_list_md_devices. - */ - return foreach_block_device (add_device, false); -} - -static int -add_partitions (const char *device, struct stringsbuf *r) -{ - CLEANUP_FREE char *devdir = NULL; - - /* Open the device's directory under /sys/block */ - if (asprintf (&devdir, "/sys/block/%s", device) == -1) { - reply_with_perror ("asprintf"); - return -1; - } - - DIR *dir...

These two patches are probably not completely independent, but separating them is a lot of work. With *both* patches applied, all the tests and extra-tests pass. That's no guarantee however that there isn't a mistake, so I don't think this patch is a candidate for the 1.16 branch, until it's had a lot more testing in development. Rich.

...; + if (err) return NULL; - } /* Sort the devices. */ if (r.size > 0) @@ -116,7 +111,7 @@ foreach_block_device (block_dev_func_t func) return NULL; } - return r.argv; + return take_stringsbuf (&r); } /* Add a device to the list of devices */ @@ -150,7 +145,6 @@ add_partitions (const char *device, struct stringsbuf *r) DIR *dir = opendir (devdir); if (!dir) { reply_with_perror ("opendir: %s", devdir); - free_stringslen (r->argv, r->size); return -1; } @@ -174,7 +168,6 @@ add_partitions (const char *device, struct stringsbuf *r)...

If add_string_nodup fails free the passed string instead of leaking it, as that string would have been owned by the stringbuf. Adapt few places to this behaviour. --- daemon/btrfs.c | 4 +--- daemon/devsparts.c | 8 ++++---- daemon/guestfsd.c | 1 + 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/daemon/btrfs.c b/daemon/btrfs.c index 9b52aa8..d70565a 100644 ---

v1 -> v2: - Fixes as suggested by Pino. Rich.

..., dev_path) == -1) { + if (asprintf (&dev_path, "/dev/%s", device) == -1) { + reply_with_perror ("asprintf"); return -1; } + if (add_string_nodup (r, dev_path) == -1) + return -1; + return 0; } @@ -153,10 +156,13 @@ do_list_devices (void) static int add_partitions (const char *device, struct stringsbuf *r) { - char devdir[256]; + CLEANUP_FREE char *devdir = NULL; /* Open the device's directory under /sys/block */ - snprintf (devdir, sizeof devdir, "/sys/block/%s", device); + if (asprintf (&devdir, "/sys/block/%s", device...

...Array.to_list parts in + + (* Look in /sys/block/<device>/ for entries starting with + * <device>, eg. /sys/block/sda/sda1. + *) + List.exists (fun part -> String.is_prefix part device) parts + let rec list_partitions () = let partitions = map_block_devices ~return_md:true add_partitions in let partitions = List.flatten partitions in diff --git a/daemon/devsparts.mli b/daemon/devsparts.mli index 7b669c2..d3224a1 100644 --- a/daemon/devsparts.mli +++ b/daemon/devsparts.mli @@ -21,5 +21,6 @@ val list_partitions : unit -> string list val part_to_dev : string -> string val p...

...har *dev_path; + if (asprintf (&dev_path, "/dev/%s", d->d_name) == -1) { + reply_with_perror ("asprintf"); + closedir (dir); + return NULL; + } /* Ignore the root device. */ if (is_root_device (dev_path)) @@ -167,8 +171,12 @@ add_partitions (const char *device, struct stringsbuf *r) struct dirent *d; while ((d = readdir (dir)) != NULL) { if (STREQLEN (d->d_name, device, strlen (device))) { - char part[256]; - snprintf (part, sizeof part, "/dev/%s", d->d_name); + CLEANUP_FREE char *part; +...

...ting with <device>, eg. /sys/block/sda/sda1 + *) + let parts = Array.to_list (Sys.readdir dir) in + List.exists (fun part -> String.is_prefix part device) parts + with Sys_error (_) -> false + let rec list_partitions () = let partitions = map_block_devices ~return_md:true add_partitions in let partitions = List.flatten partitions in diff --git a/daemon/devsparts.mli b/daemon/devsparts.mli index 7b669c2..d3224a1 100644 --- a/daemon/devsparts.mli +++ b/daemon/devsparts.mli @@ -21,5 +21,6 @@ val list_partitions : unit -> string list val part_to_dev : string -> string val p...

...har *dev_path; + if (asprintf (&dev_path, "/dev/%s", d->d_name) == -1) { + reply_with_perror ("asprintf"); + closedir (dir); + return NULL; + } /* Ignore the root device. */ if (is_root_device (dev_path)) @@ -167,8 +171,12 @@ add_partitions (const char *device, struct stringsbuf *r) struct dirent *d; while ((d = readdir (dir)) != NULL) { if (STREQLEN (d->d_name, device, strlen (device))) { - char part[256]; - snprintf (part, sizeof part, "/dev/%s", d->d_name); + CLEANUP_FREE char *part; +...

v1 -> v2: - Use intprops macro suggested by danpb. Rich.

This happens with GCC 7.0.1. The errors were all of the form: qemu-speed-test.c: In function 'main': qemu-speed-test.c:153:7: error: this statement may fall through [-Werror=implicit-fallthrough=] usage (EXIT_SUCCESS); ^~~~~~~~~~~~~~~~~~~~ qemu-speed-test.c:155:5: note: here default: ^~~~~~~ --- builder/index-validate.c | 2 +-

Initial discussion is here: https://www.redhat.com/archives/libguestfs/2018-January/msg00188.html. v2 was posted here: https://www.redhat.com/archives/libguestfs/2018-January/msg00246.html. v3 comparing to v2 is just a rebase with slightly changed commits comments.

I ran the Coverity static analyzer[1] on libguestfs, and fixed many errors as a result. Coverity found some errors in gnulib, but it doesn't seem to be worth following those up since the version of gnulib we are using is so old. There are a couple more errors (possibly 1 false-positive) which I'm going to send in a separate email. BTW all the errors found by Coverity were in the daemon

Instead of parsing 'parted' output OCaml implementation relies on the following facts: 1. The function is applicable for MBR partitions only (as noted in documentation and as function name suggests). 2. An attempt to call the function for non-MBR partition fails with "part_get_mbr_part_type can only be used on MBR Partitions" error and NULL is returned. 3. MBR partition table

Before I rush to change something I request your comments on the subject. Let me know what do you think and if it does make sense. The issue: guesfs_list_filesystems is inconsistent in its output. For, example, it filters out partitioned physical devices but doesn't do the same for MD devices. More over, according to its name and API documentation guestfs_list_filesystem should return

Version 1 was here: https://www.redhat.com/archives/libguestfs/2017-June/msg00003.html This patch series reimplements a few more APIs in OCaml, including some very important core APIs like ?list_filesystems? and ?mount?. All the tests pass after this. The selection of APIs that I have moved may look a little random, but in fact they are all APIs consumed by the inspection code (and some more