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