[Libguestfs] [PATCH 5/7] inspect: Update inspect_os to use mountables
Richard W.M. Jones
rjones at redhat.com
Tue Feb 12 12:25:18 UTC 2013
On Tue, Feb 12, 2013 at 11:04:24AM +0000, Matthew Booth wrote:
> This fixes inspection of guests which use btrfs subvolumes.
> ---
> src/guestfs-internal.h | 7 +--
> src/inspect-fs-cd.c | 2 +-
> src/inspect-fs-unix.c | 144 +++++++++++++++++++++++++++----------------------
> src/inspect-fs.c | 53 +++++++++---------
> src/inspect.c | 12 ++---
> 5 files changed, 118 insertions(+), 100 deletions(-)
Difficult to review this patch. There seem to be at least
two patches in this:
(1) A patch that just changes device -> mountable. Easy
to review.
(2) A patch that does the other changes, sans unrelated variable-
movement.
Rich.
> diff --git a/src/guestfs-internal.h b/src/guestfs-internal.h
> index bd7c802..c52b9a5 100644
> --- a/src/guestfs-internal.h
> +++ b/src/guestfs-internal.h
> @@ -391,7 +391,7 @@ enum inspect_os_package_management {
>
> struct inspect_fs {
> int is_root;
> - char *device;
> + char *mountable;
> enum inspect_os_type type;
> enum inspect_os_distro distro;
> enum inspect_os_package_format package_format;
> @@ -414,7 +414,7 @@ struct inspect_fs {
> };
>
> struct inspect_fstab_entry {
> - char *device;
> + char *mountable;
> char *mountpoint;
> };
>
> @@ -524,7 +524,8 @@ extern struct inspect_fs *guestfs___search_for_root (guestfs_h *g, const char *r
> /* inspect-fs.c */
> extern int guestfs___is_file_nocase (guestfs_h *g, const char *);
> extern int guestfs___is_dir_nocase (guestfs_h *g, const char *);
> -extern int guestfs___check_for_filesystem_on (guestfs_h *g, const char *device);
> +extern int guestfs___check_for_filesystem_on (guestfs_h *g,
> + const char *mountable);
> extern int guestfs___parse_unsigned_int (guestfs_h *g, const char *str);
> extern int guestfs___parse_unsigned_int_ignore_trailing (guestfs_h *g, const char *str);
> extern int guestfs___parse_major_minor (guestfs_h *g, struct inspect_fs *fs);
> diff --git a/src/inspect-fs-cd.c b/src/inspect-fs-cd.c
> index e4f257f..407e4f8 100644
> --- a/src/inspect-fs-cd.c
> +++ b/src/inspect-fs-cd.c
> @@ -503,7 +503,7 @@ guestfs___check_installer_iso (guestfs_h *g, struct inspect_fs *fs,
> return 0;
>
> /* Otherwise we matched an ISO, so fill in the fs fields. */
> - fs->device = safe_strdup (g, device);
> + fs->mountable = safe_strdup (g, device);
> fs->is_root = 1;
> fs->format = OS_FORMAT_INSTALLER;
> fs->type = osinfo->type;
> diff --git a/src/inspect-fs-unix.c b/src/inspect-fs-unix.c
> index 40f797d..26c7579 100644
> --- a/src/inspect-fs-unix.c
> +++ b/src/inspect-fs-unix.c
> @@ -160,8 +160,7 @@ 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,
> - Hash_table *md_map);
> + const char *mountable, const char *mp);
> 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 *));
> @@ -870,17 +869,13 @@ check_hostname_freebsd (guestfs_h *g, struct inspect_fs *fs)
> static int
> check_fstab (guestfs_h *g, struct inspect_fs *fs)
> {
> - CLEANUP_FREE_STRING_LIST char **entries = NULL;
> - char **entry;
> - char augpath[256];
> - int r;
> - CLEANUP_HASH_FREE Hash_table *md_map;
> -
> /* Generate a map of MD device paths listed in /etc/mdadm.conf to MD device
> * paths in the guestfs appliance */
> + CLEANUP_HASH_FREE Hash_table *md_map = NULL;
> if (map_md_devices (g, &md_map) == -1) return -1;
>
> - entries = guestfs_aug_match (g, "/files/etc/fstab/*[label() != '#comment']");
> + CLEANUP_FREE_STRING_LIST char **entries =
> + guestfs_aug_match (g, "/files/etc/fstab/*[label() != '#comment']");
> if (entries == NULL)
> return -1;
>
> @@ -889,20 +884,81 @@ check_fstab (guestfs_h *g, struct inspect_fs *fs)
> return -1;
> }
>
> - for (entry = entries; *entry != NULL; entry++) {
> - snprintf (augpath, sizeof augpath, "%s/spec", *entry);
> - CLEANUP_FREE char *spec = guestfs_aug_get (g, augpath);
> - if (spec == NULL)
> - return -1;
> + for (char **entry = entries; *entry != NULL; entry++) {
> + char augpath[256];
>
> snprintf (augpath, sizeof augpath, "%s/file", *entry);
> CLEANUP_FREE char *mp = guestfs_aug_get (g, augpath);
> - if (mp == NULL)
> - return -1;
> + if (mp == NULL) return -1;
> +
> + /* Ignore certain mountpoints. */
> + if (STRPREFIX (mp, "/dev/") ||
> + STREQ (mp, "/dev") ||
> + STRPREFIX (mp, "/media/") ||
> + STRPREFIX (mp, "/proc/") ||
> + STREQ (mp, "/proc") ||
> + STRPREFIX (mp, "/selinux/") ||
> + STREQ (mp, "/selinux") ||
> + STRPREFIX (mp, "/sys/") ||
> + STREQ (mp, "/sys"))
> + continue;
>
> - r = add_fstab_entry (g, fs, spec, mp, md_map);
> - if (r == -1)
> - return -1;
> + snprintf (augpath, sizeof augpath, "%s/spec", *entry);
> + CLEANUP_FREE char *spec = guestfs_aug_get (g, augpath);
> + if (spec == NULL) return -1;
> +
> + /* Ignore /dev/fd (floppy disks) (RHBZ#642929) and CD-ROM drives. */
> + if ((STRPREFIX (spec, "/dev/fd") && c_isdigit (spec[7])) ||
> + STREQ (spec, "/dev/floppy") ||
> + STREQ (spec, "/dev/cdrom"))
> + continue;
> +
> + /* Resolve UUID= and LABEL= to the actual device. */
> + CLEANUP_FREE char *mountable = NULL;
> + if (STRPREFIX (spec, "UUID="))
> + mountable = guestfs_findfs_uuid (g, &spec[5]);
> + else if (STRPREFIX (spec, "LABEL="))
> + mountable = guestfs_findfs_label (g, &spec[6]);
> + /* Ignore "/.swap" (Pardus) and pseudo-devices like "tmpfs". */
> + else if (STREQ (spec, "/dev/root"))
> + /* Resolve /dev/root to the current device. */
> + mountable = safe_strdup (g, fs->mountable);
> + else if (STRPREFIX (spec, "/dev/"))
> + /* Resolve guest block device names. */
> + mountable = resolve_fstab_device (g, spec, md_map);
> +
> + /* If we haven't resolved the device successfully by this point,
> + * we don't care, just ignore it.
> + */
> + if (mountable == NULL)
> + continue;
> +
> + snprintf (augpath, sizeof augpath, "%s/vfstype", *entry);
> + CLEANUP_FREE char *vfstype = guestfs_aug_get (g, augpath);
> + if (vfstype == NULL) return -1;
> +
> + if (STREQ (vfstype, "btrfs")) {
> + snprintf (augpath, sizeof augpath, "%s/opt", *entry);
> + CLEANUP_FREE_STRING_LIST char **opts = guestfs_aug_match (g, augpath);
> + if (opts == NULL) return -1;
> +
> + for (char **opt = opts; *opt; opt++) {
> + CLEANUP_FREE char *optname = guestfs_aug_get (g, augpath);
> + if (optname == NULL) return -1;
> +
> + if (STREQ (optname, "subvol")) {
> + snprintf (augpath, sizeof augpath, "%s/value", *opt);
> + CLEANUP_FREE char *subvol = guestfs_aug_get (g, augpath);
> + if (subvol == NULL) return -1;
> +
> + char *new = safe_asprintf (g, "btrfsvol:%s/%s", mountable, subvol);
> + free (mountable);
> + mountable = new;
> + }
> + }
> + }
> +
> + if (add_fstab_entry (g, fs, mountable, mp) == -1) return -1;
> }
>
> return 0;
> @@ -918,48 +974,8 @@ 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, Hash_table *md_map)
> + const char *mountable, const char *mountpoint)
> {
> - /* Ignore certain mountpoints. */
> - if (STRPREFIX (mp, "/dev/") ||
> - STREQ (mp, "/dev") ||
> - STRPREFIX (mp, "/media/") ||
> - STRPREFIX (mp, "/proc/") ||
> - STREQ (mp, "/proc") ||
> - STRPREFIX (mp, "/selinux/") ||
> - STREQ (mp, "/selinux") ||
> - STRPREFIX (mp, "/sys/") ||
> - STREQ (mp, "/sys"))
> - return 0;
> -
> - /* Ignore /dev/fd (floppy disks) (RHBZ#642929) and CD-ROM drives. */
> - if ((STRPREFIX (spec, "/dev/fd") && c_isdigit (spec[7])) ||
> - STREQ (spec, "/dev/floppy") ||
> - STREQ (spec, "/dev/cdrom"))
> - return 0;
> -
> - /* Resolve UUID= and LABEL= to the actual device. */
> - char *device = NULL;
> - if (STRPREFIX (spec, "UUID="))
> - device = guestfs_findfs_uuid (g, &spec[5]);
> - else if (STRPREFIX (spec, "LABEL="))
> - device = guestfs_findfs_label (g, &spec[6]);
> - /* Ignore "/.swap" (Pardus) and pseudo-devices like "tmpfs". */
> - else if (STREQ (spec, "/dev/root"))
> - /* Resolve /dev/root to the current device. */
> - device = safe_strdup (g, fs->device);
> - else if (STRPREFIX (spec, "/dev/"))
> - /* Resolve guest block device names. */
> - device = resolve_fstab_device (g, spec, md_map);
> -
> - /* If we haven't resolved the device successfully by this point,
> - * we don't care, just ignore it.
> - */
> - if (device == NULL)
> - return 0;
> -
> - char *mountpoint = safe_strdup (g, mp);
> -
> /* Add this to the fstab entry in 'fs'.
> * Note these are further filtered by guestfs_inspect_get_mountpoints
> * and guestfs_inspect_get_filesystems.
> @@ -970,8 +986,6 @@ add_fstab_entry (guestfs_h *g, struct inspect_fs *fs,
> p = realloc (fs->fstab, n * sizeof (struct inspect_fstab_entry));
> if (p == NULL) {
> perrorf (g, "realloc");
> - free (device);
> - free (mountpoint);
> return -1;
> }
>
> @@ -979,10 +993,10 @@ add_fstab_entry (guestfs_h *g, struct inspect_fs *fs,
> fs->nr_fstab = n;
>
> /* These are owned by the handle and freed by guestfs___free_inspect_info. */
> - fs->fstab[n-1].device = device;
> - fs->fstab[n-1].mountpoint = mountpoint;
> + fs->fstab[n-1].mountable = safe_strdup (g, mountable);
> + fs->fstab[n-1].mountpoint = safe_strdup (g, mountpoint);
>
> - debug (g, "fstab: device=%s mountpoint=%s", device, mountpoint);
> + debug (g, "fstab: mountable=%s mountpoint=%s", mountable, mountpoint);
>
> return 0;
> }
> diff --git a/src/inspect-fs.c b/src/inspect-fs.c
> index be22eb0..8a88822 100644
> --- a/src/inspect-fs.c
> +++ b/src/inspect-fs.c
> @@ -79,7 +79,8 @@ free_regexps (void)
> pcre_free (re_major_minor);
> }
>
> -static int check_filesystem (guestfs_h *g, const char *device,
> +static int check_filesystem (guestfs_h *g, const char *mountable,
> + const struct guestfs_mountable_internal *m,
> int whole_device);
> static int extend_fses (guestfs_h *g);
>
> @@ -87,7 +88,7 @@ static int extend_fses (guestfs_h *g);
> * another entry in g->fses.
> */
> int
> -guestfs___check_for_filesystem_on (guestfs_h *g, const char *device)
> +guestfs___check_for_filesystem_on (guestfs_h *g, const char *mountable)
> {
> CLEANUP_FREE char *vfs_type = NULL;
> int is_swap, r;
> @@ -98,25 +99,31 @@ guestfs___check_for_filesystem_on (guestfs_h *g, const char *device)
> * temporarily replace the error handler with a null one.
> */
> guestfs_push_error_handler (g, NULL, NULL);
> - vfs_type = guestfs_vfs_type (g, device);
> + vfs_type = guestfs_vfs_type (g, mountable);
> guestfs_pop_error_handler (g);
>
> is_swap = vfs_type && STREQ (vfs_type, "swap");
> debug (g, "check_for_filesystem_on: %s (%s)",
> - device, vfs_type ? vfs_type : "failed to get vfs type");
> + mountable, vfs_type ? vfs_type : "failed to get vfs type");
>
> if (is_swap) {
> if (extend_fses (g) == -1)
> return -1;
> fs = &g->fses[g->nr_fses-1];
> - fs->device = safe_strdup (g, device);
> + fs->mountable = safe_strdup (g, mountable);
> return 0;
> }
>
> + struct guestfs_mountable_internal *m =
> + guestfs_internal_parse_mountable (g, mountable);
> +
> /* If it's a whole device, see if it is an install ISO. */
> - int whole_device = guestfs_is_whole_device (g, device);
> - if (whole_device == -1) {
> - return -1;
> + int whole_device = 0;
> + if (m->type == MOUNTABLE_DEVICE) {
> + whole_device = guestfs_is_whole_device (g, m->device);
> + if (whole_device == -1) {
> + return -1;
> + }
> }
>
> if (whole_device) {
> @@ -124,7 +131,7 @@ guestfs___check_for_filesystem_on (guestfs_h *g, const char *device)
> return -1;
> fs = &g->fses[g->nr_fses-1];
>
> - r = guestfs___check_installer_iso (g, fs, device);
> + r = guestfs___check_installer_iso (g, fs, m->device);
> if (r == -1) { /* Fatal error. */
> g->nr_fses--;
> return -1;
> @@ -140,19 +147,19 @@ guestfs___check_for_filesystem_on (guestfs_h *g, const char *device)
> guestfs_push_error_handler (g, NULL, NULL);
> if (vfs_type && STREQ (vfs_type, "ufs")) { /* Hack for the *BSDs. */
> /* FreeBSD fs is a variant of ufs called ufs2 ... */
> - r = guestfs_mount_vfs (g, "ro,ufstype=ufs2", "ufs", device, "/");
> + r = guestfs_mount_vfs (g, "ro,ufstype=ufs2", "ufs", mountable, "/");
> if (r == -1)
> /* while NetBSD and OpenBSD use another variant labeled 44bsd */
> - r = guestfs_mount_vfs (g, "ro,ufstype=44bsd", "ufs", device, "/");
> + r = guestfs_mount_vfs (g, "ro,ufstype=44bsd", "ufs", mountable, "/");
> } else {
> - r = guestfs_mount_ro (g, device, "/");
> + r = guestfs_mount_ro (g, mountable, "/");
> }
> guestfs_pop_error_handler (g);
> if (r == -1)
> return 0;
>
> /* Do the rest of the checks. */
> - r = check_filesystem (g, device, whole_device);
> + r = check_filesystem (g, mountable, m, whole_device);
>
> /* Unmount the filesystem. */
> if (guestfs_umount_all (g) == -1)
> @@ -161,28 +168,24 @@ guestfs___check_for_filesystem_on (guestfs_h *g, const char *device)
> return r;
> }
>
> -/* is_block and is_partnum are just hints: is_block is true if the
> - * filesystem is a whole block device (eg. /dev/sda). is_partnum
> - * is > 0 if the filesystem is a direct partition, and in this case
> - * it is the partition number counting from 1
> - * (eg. /dev/sda1 => is_partnum == 1).
> - */
> static int
> -check_filesystem (guestfs_h *g, const char *device, int whole_device)
> +check_filesystem (guestfs_h *g, const char *mountable,
> + const struct guestfs_mountable_internal *m,
> + int whole_device)
> {
> if (extend_fses (g) == -1)
> return -1;
>
> int partnum = -1;
> - if (!whole_device) {
> + if (!whole_device && m->type == MOUNTABLE_DEVICE) {
> guestfs_push_error_handler (g, NULL, NULL);
> - partnum = guestfs_part_to_partnum (g, device);
> + partnum = guestfs_part_to_partnum (g, m->device);
> guestfs_pop_error_handler (g);
> }
>
> struct inspect_fs *fs = &g->fses[g->nr_fses-1];
>
> - fs->device = safe_strdup (g, device);
> + fs->mountable = safe_strdup (g, mountable);
>
> /* Optimize some of the tests by avoiding multiple tests of the same thing. */
> int is_dir_etc = guestfs_is_dir (g, "/etc") > 0;
> @@ -203,7 +206,7 @@ check_filesystem (guestfs_h *g, const char *device, int whole_device)
> * that is probably /dev/sda5 (see:
> * http://www.freebsd.org/doc/handbook/disk-organization.html)
> */
> - if (match (g, device, re_first_partition))
> + if (m->type == MOUNTABLE_DEVICE && match (g, m->device, re_first_partition))
> return 0;
>
> fs->is_root = 1;
> @@ -219,7 +222,7 @@ check_filesystem (guestfs_h *g, const char *device, int whole_device)
> * that is probably /dev/sda5 (see:
> * http://www.freebsd.org/doc/handbook/disk-organization.html)
> */
> - if (match (g, device, re_first_partition))
> + if (m->type == MOUNTABLE_DEVICE && match (g, m->device, re_first_partition))
> return 0;
>
> fs->is_root = 1;
> diff --git a/src/inspect.c b/src/inspect.c
> index ae746cb..f031434 100644
> --- a/src/inspect.c
> +++ b/src/inspect.c
> @@ -107,7 +107,7 @@ guestfs__inspect_get_roots (guestfs_h *g)
> count = 0;
> for (i = 0; i < g->nr_fses; ++i) {
> if (g->fses[i].is_root) {
> - ret[count] = safe_strdup (g, g->fses[i].device);
> + ret[count] = safe_strdup (g, g->fses[i].mountable);
> count++;
> }
> }
> @@ -347,7 +347,7 @@ guestfs__inspect_get_mountpoints (guestfs_h *g, const char *root)
> for (i = 0; i < nr; ++i)
> if (CRITERION (fs, i)) {
> ret[2*count] = safe_strdup (g, fs->fstab[i].mountpoint);
> - ret[2*count+1] = safe_strdup (g, fs->fstab[i].device);
> + ret[2*count+1] = safe_strdup (g, fs->fstab[i].mountable);
> count++;
> }
> #undef CRITERION
> @@ -379,7 +379,7 @@ guestfs__inspect_get_filesystems (guestfs_h *g, const char *root)
> }
>
> for (i = 0; i < nr; ++i)
> - ret[i] = safe_strdup (g, fs->fstab[i].device);
> + ret[i] = safe_strdup (g, fs->fstab[i].mountable);
>
> return ret;
> }
> @@ -485,7 +485,7 @@ guestfs___free_inspect_info (guestfs_h *g)
> {
> size_t i;
> for (i = 0; i < g->nr_fses; ++i) {
> - free (g->fses[i].device);
> + free (g->fses[i].mountable);
> free (g->fses[i].product_name);
> free (g->fses[i].product_variant);
> free (g->fses[i].arch);
> @@ -494,7 +494,7 @@ guestfs___free_inspect_info (guestfs_h *g)
> free (g->fses[i].windows_current_control_set);
> size_t j;
> for (j = 0; j < g->fses[i].nr_fstab; ++j) {
> - free (g->fses[i].fstab[j].device);
> + free (g->fses[i].fstab[j].mountable);
> free (g->fses[i].fstab[j].mountpoint);
> }
> free (g->fses[i].fstab);
> @@ -608,7 +608,7 @@ guestfs___search_for_root (guestfs_h *g, const char *root)
> struct inspect_fs *fs;
> for (i = 0; i < g->nr_fses; ++i) {
> fs = &g->fses[i];
> - if (fs->is_root && STREQ (root, fs->device))
> + if (fs->is_root && STREQ (root, fs->mountable))
> return fs;
> }
>
> --
> 1.8.1.2
>
> _______________________________________________
> Libguestfs mailing list
> Libguestfs at redhat.com
> https://www.redhat.com/mailman/listinfo/libguestfs
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine. Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/
More information about the Libguestfs
mailing list