10:31 a.m.
This patch fixes inspection when fstab contains devices md devices
specified as /dev/mdN. The appliance creates these devices without reference to
the guest's mdadm.conf so, for e.g. /dev/md0 in the guest will often be created
as /dev/md127 in the appliance. With this patch, we match the uuids of detected
md devices against uuids specified in mdadm.conf, and map them appropriately
when we encounter them in fstab.
The test in this patch also checks that devices specified by their md name, e.g.
/dev/md/boot, work correctly, although the patch isn't required for devices
specified this way.
---
regressions/Makefile.am | 1 +
regressions/test-inspect-fstab-md.sh | 65 ++++++
src/inspect.c | 22 ++-
src/inspect_fs_unix.c | 411 ++++++++++++++++++++++++++++++----
4 files changed, 456 insertions(+), 43 deletions(-)
create mode 100755 regressions/test-inspect-fstab-md.sh
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
2:30 a.m.
On 11/23/2011 12:31 AM, Matthew Booth wrote:
This patch fixes inspection when fstab contains devices md devices
specified as /dev/mdN. The appliance creates these devices without reference to
the guest's mdadm.conf so, for e.g. /dev/md0 in the guest will often be created
as /dev/md127 in the appliance. With this patch, we match the uuids of detected
md devices against uuids specified in mdadm.conf, and map them appropriately
when we encounter them in fstab.
The test in this patch also checks that devices specified by their md name, e.g.
/dev/md/boot, work correctly, although the patch isn't required for devices
specified this way.
Sorry to say that I can't apply this big patch to do a test.
Thanks
-Wanlong Gao
---
regressions/Makefile.am | 1 +
regressions/test-inspect-fstab-md.sh | 65 ++++++
src/inspect.c | 22 ++-
src/inspect_fs_unix.c | 411 ++++++++++++++++++++++++++++++----
4 files changed, 456 insertions(+), 43 deletions(-)
create mode 100755 regressions/test-inspect-fstab-md.sh
_______________________________________________
Libguestfs mailing list
Libguestfs(a)redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs
4747
days inactive
4748
days old
2 comments
3 participants
participants (3)
-
Matthew Booth -
Richard W.M. Jones -
Wanlong Gao