[Libguestfs] [PATCH 1/2] New API: list_md_devices

Matthew Booth mbooth at redhat.com
Thu Nov 10 15:07:13 UTC 2011


On 10/11/11 14:53, Richard W.M. Jones wrote:
> 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).

glob has a callback (which I'm not using) for returning an error for 
each individual failure it hits. It's not actually documented to set 
errno at all. A glob failure here is pretty unlikely, so writing a bunch 
of code to collate errors and report them all nicely seems a waste. 
Simply catching an error seems good enough.

>> +    } else if (err == GLOB_NOMATCH) {
>> +        /* This is fine */
>> +    }
>
> I think this branch should just be dropped.

Heh, yeah. It used to have code in it :)

>> +    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.

Ok.

>> +# 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?

Maybe another day :)

>> +# Ensure list-md-devices now returns the newly create md device
>
> "created"
>
> Generally looking good.

Updated patch to follow.

Matt
-- 
Matthew Booth, RHCA, RHCSS
Red Hat Engineering, Virtualisation Team

GPG ID:  D33C3490
GPG FPR: 3733 612D 2D05 5458 8A8A 1600 3441 EA19 D33C 3490




More information about the Libguestfs mailing list