[Libguestfs] [PATCH 1/2] inspection: Handle MD devices in fstab
Richard W.M. Jones
rjones at redhat.com
Tue Nov 29 12:28:25 UTC 2011
On Tue, Nov 29, 2011 at 12:18:46PM +0000, Matthew Booth wrote:
> On 11/29/2011 08:58 AM, Richard W.M. Jones wrote:
> >On Fri, Nov 25, 2011 at 05:32:42PM +0000, Matthew Booth wrote:
> >> static int
> >> check_fstab (guestfs_h *g, struct inspect_fs *fs)
> >> {
> >>+ /* Generate a map of MD device paths listed in /etc/mdadm.conf to MD device
> >>+ * paths in the guestfs appliance */
> >>+ Hash_table *md_map;
> >>+ if (map_md_devices (g,&md_map) == -1) return -1;
> >
> >I'm pretty sure this is still wrong ...
> >
> >If /etc/mdadm.conf doesn't exist(?) or isn't parsable, map_md_devices
> >returns -1. It doesn't set error(), which could be bad, since it
> >returns -1 from check_fstab which requires you to set error().
>
> No, it doesn't. If augeas can't parse /etc/mdadm.conf, the match
> will return an empty list, not an error. In that case it follows
> this code path:
>
> if (matches[0] == NULL) {
> debug(g, "Appliance has MD devices, but augeas returned no array
> matches "
> "in mdadm.conf");
> guestfs___free_string_list(matches);
> return 0;
> }
>
> It returns 0, not -1, and the map has been explicitly set to NULL above.
>
> >What it _should_ do is not to call error(), and allow md_map to be
> >NULL, not returning from check_fstab, but continuing. Then the other
> >functions have to be changed to deal with md_map being NULL.
> >
> >Try creating some guests with /etc/mdadm.conf being
> >bogus/empty/missing and make sure that inspection can properly handle
> >them without an error.
>
> I already specifically tested that.
OK, in that case ACK.
I'm going to test it a bit here too.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-p2v converts physical machines to virtual machines. Boot with a
live CD or over the network (PXE) and turn machines into Xen guests.
http://et.redhat.com/~rjones/virt-p2v
More information about the Libguestfs
mailing list