Re: [Libguestfs] [PATCH 1/2] New API: list_md_devices
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/