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/
_______________________________________________
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs
