[Libguestfs] [PATCH 1/2] inspection: Handle MD devices in fstab

[Matthew Booth]

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.

Matt
-- 
Matthew Booth, RHCA, RHCSS
Red Hat Engineering, Virtualisation Team

GPG ID:  D33C3490
GPG FPR: 3733 612D 2D05 5458 8A8A 1600 3441 EA19 D33C 3490