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

[Richard W.M. Jones]

On Thu, Nov 10, 2011 at 02:53:48PM +0000, 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).

Apparently glob doesn't necessarily set errno.  Can we capture the
errno and at least dump the message out to stderr?  A simple callback
that just calls perror [not reply_with_perror] should be enough.

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

These error messages are still fairly grotty.  It would be some
consistent to have something like this:

+    if (err == GLOB_NOSPACE) {
+        reply_with_error ("glob: returned GLOB_NOSPACE: rerun with LIBGUESTFS_DEBUG=1");
+        goto error;
+    } else if (err == GLOB_ABORTED) {
+        reply_with_error ("glob: returned GLOB_ABORTED: rerun with LIBGUESTFS_DEBUG=1");
+        goto error;

That should be enough for us to go and track the problem back to the
code if it ever did occur.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://et.redhat.com/~rjones/virt-df/