1:41 p.m.
On Tue, Nov 22, 2011 at 04:31:50PM +0000, Matthew Booth wrote:
diff --git a/src/inspect.c b/src/inspect.c
index 1b41be5..f9b8298 100644
--- a/src/inspect.c
+++ b/src/inspect.c
@@ -61,8 +61,7 @@ guestfs__inspect_os (guestfs_h *g)
* information to the handle.
*/
/* Look to see if any devices directly contain filesystems (RHBZ#590167). */
- char **devices;
- devices = guestfs_list_devices (g);
+ char **devices = guestfs_list_devices (g);
if (devices == NULL)
return NULL;
@@ -77,8 +76,7 @@ guestfs__inspect_os (guestfs_h *g)
guestfs___free_string_list (devices);
/* Look at all partitions. */
- char **partitions;
- partitions = guestfs_list_partitions (g);
+ char **partitions = guestfs_list_partitions (g);
if (partitions == NULL) {
guestfs___free_inspect_info (g);
return NULL;
If there's not any need for these two hunks, then omit them. OTOH if
it's a good stylistic thing to have them, they should be in a separate
patch so that it can be cherry picked for the stable branch (since all
NFC code cleanups are candidates for cherry picking).
@@ -141,9 +146,23 @@ static int check_hostname_redhat (guestfs_h *g,
struct inspect_fs *fs);
static int check_hostname_freebsd (guestfs_h *g, struct inspect_fs *fs);
static int check_fstab (guestfs_h *g, struct inspect_fs *fs);
static int add_fstab_entry (guestfs_h *g, struct inspect_fs *fs,
- const char *spec, const char *mp);
-static char *resolve_fstab_device (guestfs_h *g, const char *spec);
-static int inspect_with_augeas (guestfs_h *g, struct inspect_fs *fs, const char
*filename, int (*f) (guestfs_h *, struct inspect_fs *));
+ const char *spec, const char *mp,
+ Hash_table *md_map);
+static char *resolve_fstab_device (guestfs_h *g, const char *spec,
+ Hash_table *md_map);
+static int inspect_with_augeas (guestfs_h *g, struct inspect_fs *fs, const char
**configfiles, int (*f) (guestfs_h *, struct inspect_fs *));
+static size_t uuid_hash(const void *x, size_t table_size);
+static bool uuid_cmp(const void *x, const void *y);
+static void md_uuid_free(void *x);
+static bool parse_uuid(const char *str, uint32_t *uuid);
+
+static size_t mdadm_app_hash(const void *x, size_t table_size);
+static bool mdadm_app_cmp(const void *x, const void *y);
+static void mdadm_app_free(void *x);
+
+static bool map_app_md_devices (guestfs_h *g, Hash_table **map);
+static bool map_md_devices(guestfs_h *g, Hash_table **map);
There's a mix of stylistic changes, extensions to core functions (such
as allowing inspect_with_augeas to take a list), and new functionality
(uuid_hash). To make this easy to backport that could be two or three
separate layers of patches.
@@ -684,14 +707,15 @@ check_hostname_freebsd (guestfs_h *g, struct
inspect_fs *fs)
static int
check_fstab (guestfs_h *g, struct inspect_fs *fs)
{
+ Hash_table *md_map;
+ if (!map_md_devices (g, &md_map)) return -1;
This doesn't call error/perrorf() on the error path.
It would be nice if we could enforce this invariant somehow (OCaml can
do this, for example). I don't know how to in C.
The hash could also do with a small comment to explain briefly what
md_map contains.
+ case 0:
+ /* Duplicate uuid in for md device is weird, but not fatal. */
+ warning(g, "inspect-os: md devices %s and %s have the same uuid",
+ ((md_uuid *)matched)->path, entry->path);
I would avoid using warning() -- it says as much in the comment above
the guestfs___warning function in src/guestfs.c. This one might be a
case for debug() instead.
That's all for now, I'll probably have more to say when I try it out.
Could all the md stuff be moved into another source file? Might be
cleaner ...
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming blog: http://rwmj.wordpress.com
Fedora now supports 80 OCaml packages (the OPEN alternative to F#)
http://cocan.org/getting_started_with_ocaml_on_red_hat_and_fedora