Libguestfs - Nov 2011 - [PATCH 1/2] New API: list_md_devices

Return a list of Linux MD devices detected in the guest.
This API complements list_devices, list_partitions, list_lvs and
list_dm_devices.
---
 daemon/Makefile.am                  |    1 +
 daemon/daemon.h                     |    1 +
 daemon/guestfsd.c                   |   23 +++++++---
 daemon/md.c                         |   75 +++++++++++++++++++++++++++++++++++
 generator/generator_actions.ml      |   10 +++++
 po/POTFILES.in                      |    1 +
 regressions/Makefile.am             |    1 +
 regressions/test-list-md-devices.sh |   59 +++++++++++++++++++++++++++
 src/MAX_PROC_NR                     |    2 +-
 9 files changed, 165 insertions(+), 8 deletions(-)
 create mode 100644 daemon/md.c
 create mode 100755 regressions/test-list-md-devices.sh

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-New-API-list_md_devices.patch
Type: text/x-patch
Size: 7912 bytes
Desc: not available
URL:
<http://listman.redhat.com/archives/libguestfs/attachments/20111110/597a0235/attachment.bin>

---
 regressions/test-list-filesystems.sh |   77 ++++++++++++++++++++++++++++++++++
 src/listfs.c                         |   60 ++++++++++++++-------------
 2 files changed, 108 insertions(+), 29 deletions(-)
 create mode 100755 regressions/test-list-filesystems.sh

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-Update-list-filesystems-to-check-md-devices.patch
Type: text/x-patch
Size: 5471 bytes
Desc: not available
URL:
<http://listman.redhat.com/archives/libguestfs/attachments/20111110/8a40dcc6/attachment.bin>

On Thu, Nov 10, 2011 at 02:19:11PM +0000, Matthew Booth
wrote:
> +    /* Look for directories under /sys/block matching md[0-9]*
> +     * As an additional check, we also make sure they have a md
subdirectory */
> +    int err = glob(PREFIX "[0-9]*" SUFFIX, GLOB_ERR, NULL,
&mds);
> +    if (err == GLOB_NOSPACE) {
> +        reply_with_error("OOM searching for md devices");
> +        goto error;
> +    } else if (err == GLOB_ABORTED) {
> +        reply_with_error("Read error searching for md devices");
> +        goto error;
Since glob is supposed to set errno on error, I'd prefer:

  reply_with_perror ("glob");

for both of the branches above (combine them into a single branch
if you like).
> +    } else if (err == GLOB_NOMATCH) {
> +        /* This is fine */
> +    }
I think this branch should just be dropped.
> +    for (size_t i = 0; i < mds.gl_pathc; i++) {
> +        size_t len = strlen(mds.gl_pathv[i]) - strlen(PREFIX) -
strlen(SUFFIX);
> +
> +#define DEV "/dev/md"
> +        char *dev = malloc(strlen(DEV) + len  + 1);
Need to check the return value from malloc and reply_with_perror /
goto error on failure.
> +# Create a raid1 based on the 2 disks
> +debug sh "/usr/bin/yes | mdadm -C /dev/md/test --level=1
--raid-devices=2 /dev/vda /dev/vdb >/dev/null 2>&1"
I guessed we'd be running the debug backdoor here to create devices.
Is it not worth binding this call too?
> +# Ensure list-md-devices now returns the newly create md device
"created"

Generally looking good.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming blog: http://rwmj.wordpress.com
Fedora now supports 80 OCaml packages (the OPEN alternative to F#)
http://cocan.org/getting_started_with_ocaml_on_red_hat_and_fedora