A Mountable is passed from the library to the daemon as a string. The daemon stub parses it into a mountable_t, which it passes to the implementation. Update all implementations which now take a mountable_t. --- daemon/blkid.c | 33 +++++++++++++++++++++++----- daemon/daemon.h | 41 ++++++++++++++++++++++++++++++++++ daemon/ext2.c | 15 ++++++++++--- daemon/guestfsd.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++--- daemon/labels.c | 16 ++++++++++++-- daemon/mount.c | 49 +++++++++++++++++++++++++++++++++-------- generator/c.ml | 12 +++++++++- generator/daemon.ml | 11 +++++++--- 8 files changed, 213 insertions(+), 27 deletions(-) diff --git a/daemon/blkid.c b/daemon/blkid.c index 64919dd..d54ac22 100644 --- a/daemon/blkid.c +++ b/daemon/blkid.c @@ -67,21 +67,42 @@ get_blkid_tag (const char *device, const char *tag) } char * -do_vfs_type (const char *device) +do_vfs_type (const mountable_t *mountable) { - return get_blkid_tag (device, "TYPE"); + switch (mountable->type) { + case MOUNTABLE_DEVICE: + case MOUNTABLE_BTRFSVOL: + return get_blkid_tag (mountable->device, "TYPE"); + default: + reply_with_error ("Invalid mountable type: %i", mountable->type); + return NULL; + } } char * -do_vfs_label (const char *device) +do_vfs_label (const mountable_t *mountable) { - return get_blkid_tag (device, "LABEL"); + switch (mountable->type) { + case MOUNTABLE_DEVICE: + case MOUNTABLE_BTRFSVOL: + return get_blkid_tag (mountable->device, "LABEL"); + default: + reply_with_error ("Invalid mountable type: %i", mountable->type); + return NULL; + } } char * -do_vfs_uuid (const char *device) +do_vfs_uuid (const mountable_t *mountable) { - return get_blkid_tag (device, "UUID"); + switch (mountable->type) { + case MOUNTABLE_DEVICE: + case MOUNTABLE_BTRFSVOL: + return get_blkid_tag (mountable->device, "UUID"); + default: + reply_with_error ("Invalid mountable type: %i", mountable->type); + return NULL; + } } /* RHEL5 blkid doesn't have the -p (low-level probing) option and the diff --git a/daemon/daemon.h b/daemon/daemon.h index 44fb9ab..502e0eb 100644 --- a/daemon/daemon.h +++ b/daemon/daemon.h @@ -49,6 +49,19 @@ extern int xwrite (int sock, const void *buf, size_t len) extern int xread (int sock, void *buf, size_t len) __attribute__((__warn_unused_result__)); +/* Mountables */ + +typedef enum { + MOUNTABLE_DEVICE, + MOUNTABLE_BTRFSVOL +} mountable_type_t; + +typedef struct { + mountable_type_t type; + const char *device; + const char *volume; +} mountable_t; + /* Growable strings buffer. */ struct stringsbuf { char **argv; @@ -116,6 +129,8 @@ extern void trim (char *str); extern int device_name_translation (char *device); +extern int parse_btrfsvol (char *desc, mountable_t *mountable); + extern int prog_exists (const char *prog); extern void udev_settle (void); @@ -330,6 +345,32 @@ is_zero (const char *buffer, size_t size) } \ } while (0) +/* All functions that take a mountable argument must call this macro. + * It parses the mountable into a mountable_t, ensures any + * underlying device exists, and does device name translation + * (described in the guestfs(3) manpage). + * + * Note that the "string" argument may be modified. + */ +#define RESOLVE_MOUNTABLE(string,mountable,cancel_stmt,fail_stmt) \ + do { \ + if (strncmp ((string), "btrfsvol:", strlen ("btrfsvol:")) == 0) { \ + if (parse_btrfsvol ((string) + strlen ("btrfsvol:"), &(mountable)) == -1)\ + { \ + cancel_stmt; \ + reply_with_error ("%s: %s: expecting a btrfs volume", \ + __func__, (string)); \ + fail_stmt; \ + } \ + } \ + \ + else { \ + (mountable).type = MOUNTABLE_DEVICE; \ + (mountable).device = (string); \ + RESOLVE_DEVICE((string), cancel_stmt, fail_stmt); \ + } \ + } while (0) + /* Helper for functions which need either an absolute path in the * mounted filesystem, OR a /dev/ device which exists. * diff --git a/daemon/ext2.c b/daemon/ext2.c index e4548d6..e10facd 100644 --- a/daemon/ext2.c +++ b/daemon/ext2.c @@ -116,13 +116,19 @@ do_tune2fs_l (const char *device) int do_set_e2label (const char *device, const char *label) { - return do_set_label (device, label); + mountable_t mountable; + mountable.type = MOUNTABLE_DEVICE; + mountable.device = device; + return do_set_label (&mountable, label); } char * do_get_e2label (const char *device) { - return do_vfs_label (device); + mountable_t mountable; + mountable.type = MOUNTABLE_DEVICE; + mountable.device = device; + return do_vfs_label (&mountable); } int @@ -143,7 +149,10 @@ do_set_e2uuid (const char *device, const char *uuid) char * do_get_e2uuid (const char *device) { - return do_vfs_uuid (device); + mountable_t mountable; + mountable.type = MOUNTABLE_DEVICE; + mountable.device = device; + return do_vfs_uuid (&mountable); } /* If the filesystem is not mounted, run e2fsck -f on it unconditionally. */ diff --git a/daemon/guestfsd.c b/daemon/guestfsd.c index 70abbea..02454e3 100644 --- a/daemon/guestfsd.c +++ b/daemon/guestfsd.c @@ -358,6 +358,13 @@ read_cmdline (void) /* Return true iff device is the root device (and therefore should be * ignored from the point of view of user calls). */ +static int +is_root_device_stat (struct stat *statbuf) +{ + if (statbuf->st_rdev == root_device) return 1; + return 0; +} + int is_root_device (const char *device) { @@ -366,9 +373,8 @@ is_root_device (const char *device) perror (device); return 0; } - if (statbuf.st_rdev == root_device) - return 1; - return 0; + + return is_root_device_stat (&statbuf); } /* Turn "/path" into "/sysroot/path". @@ -1164,6 +1170,57 @@ device_name_translation (char *device) return -1; } +/* Parse the mountable descriptor for a btrfs subvolume. Don't call this + * directly - use the RESOLVE_MOUNTABLE macro. + * + * A btrfs subvolume is given as: + * + * btrfsvol:/dev/sda3/root + * + * where /dev/sda3 is a block device containing a btrfs filesystem, and root is + * the name of a subvolume on it. This function is passed the string following + * 'btrfsvol:'. + */ +int +parse_btrfsvol (char *desc, mountable_t *mountable) +{ + mountable->type = MOUNTABLE_BTRFSVOL; + + char *device = desc; + + if (strncmp (device, "/dev/", strlen("/dev/")) == -1) + return -1; + + char *volume = NULL; + char *slash = device + strlen("/dev/") - 1; + while ((slash = strchr (slash + 1, '/'))) { + *slash = '\0'; + + struct stat statbuf; + if (stat (device, &statbuf) == -1) { + perror (device); + return -1; + } + + if (!S_ISDIR (statbuf.st_mode) && + !is_root_device_stat(&statbuf) && + device_name_translation (device) == 0) + { + volume = slash + 1; + break; + } + + *slash = '/'; + } + + if (!volume) return -1; + + mountable->device = device; + mountable->volume = volume; + + return 0; +} + /* Check program exists and is executable on $PATH. Actually, we * just assume PATH contains the default entries (see main() above). */ diff --git a/daemon/labels.c b/daemon/labels.c index 2fda354..6e21bd8 100644 --- a/daemon/labels.c +++ b/daemon/labels.c @@ -71,15 +71,27 @@ ntfslabel (const char *device, const char *label) } int -do_set_label (const char *device, const char *label) +do_set_label (const mountable_t *mountable, const char *label) { int r; /* How we set the label depends on the filesystem type. */ - CLEANUP_FREE char *vfs_type = do_vfs_type (device); + CLEANUP_FREE char *vfs_type = do_vfs_type (mountable); if (vfs_type == NULL) return -1; + const char *device; + switch (mountable->type) { + case MOUNTABLE_DEVICE: + case MOUNTABLE_BTRFSVOL: + device = mountable->device; + break; + + default: + reply_with_error ("Invalid mountable type: %i", mountable->type); + return -1; + } + if (STREQ (vfs_type, "ext2") || STREQ (vfs_type, "ext3") || STREQ (vfs_type, "ext4")) r = e2label (device, label); diff --git a/daemon/mount.c b/daemon/mount.c index af92834..484e45c 100644 --- a/daemon/mount.c +++ b/daemon/mount.c @@ -124,7 +124,7 @@ is_device_mounted (const char *device) int do_mount_vfs (const char *options, const char *vfstype, - const char *device, const char *mountpoint) + const mountable_t *mountable, const char *mountpoint) { int r; CLEANUP_FREE char *mp = NULL; @@ -149,12 +149,43 @@ do_mount_vfs (const char *options, const char *vfstype, return -1; } + CLEANUP_FREE char *options_plus = NULL; + const char *device = mountable->device; + switch (mountable->type) { + case MOUNTABLE_DEVICE: + break; + + case MOUNTABLE_BTRFSVOL: + if (options && strlen (options) > 0) { + if (asprintf (&options_plus, "subvol=%s,%s", + mountable->volume, options) == -1) + { + reply_with_perror ("asprintf"); + return -1; + } + } + + else { + if (asprintf (&options_plus, "subvol=%s", mountable->volume) == -1) { + reply_with_perror ("asprintf"); + return -1; + } + } + break; + + default: + reply_with_error ("Invalid mountable type: %i", mountable->type); + return -1; + } + if (vfstype) r = command (NULL, &error, - str_mount, "-o", options, "-t", vfstype, device, mp, NULL); + str_mount, "-o", options_plus ? options_plus : options, + "-t", vfstype, device, mp, NULL); else r = command (NULL, &error, - str_mount, "-o", options, device, mp, NULL); + str_mount, "-o", options_plus ? options_plus : options, + device, mp, NULL); if (r == -1) { reply_with_error ("%s on %s (options: '%s'): %s", device, mountpoint, options, error); @@ -165,22 +196,22 @@ do_mount_vfs (const char *options, const char *vfstype, } int -do_mount (const char *device, const char *mountpoint) +do_mount (const mountable_t *mountable, const char *mountpoint) { - return do_mount_vfs ("", NULL, device, mountpoint); + return do_mount_vfs ("", NULL, mountable, mountpoint); } int -do_mount_ro (const char *device, const char *mountpoint) +do_mount_ro (const mountable_t *mountable, const char *mountpoint) { - return do_mount_vfs ("ro", NULL, device, mountpoint); + return do_mount_vfs ("ro", NULL, mountable, mountpoint); } int -do_mount_options (const char *options, const char *device, +do_mount_options (const char *options, const mountable_t *mountable, const char *mountpoint) { - return do_mount_vfs (options, NULL, device, mountpoint); + return do_mount_vfs (options, NULL, mountable, mountpoint); } /* Takes optional arguments, consult optargs_bitmask. */ diff --git a/generator/c.ml b/generator/c.ml index afa81ed..6d26868 100644 --- a/generator/c.ml +++ b/generator/c.ml @@ -121,12 +121,18 @@ let rec generate_prototype ?(extern = true) ?(static = false) List.iter ( function | Pathname n - | Device n | Mountable n | Dev_or_Path n + | Device n | Dev_or_Path n | String n | OptString n | Key n -> next (); pr "const char *%s" n + | Mountable n -> + next(); + if in_daemon then + pr "const mountable_t *%s" n + else + pr "const char *%s" n | StringList n | DeviceList n -> next (); pr "char *const *%s" n @@ -160,6 +166,7 @@ let rec generate_prototype ?(extern = true) ?(static = false) (* Generate C call arguments, eg "(handle, foo, bar)" *) and generate_c_call_args ?handle ?(implicit_size_ptr = "&size") + ?(in_daemon = false) (ret, args, optargs) = pr "("; let comma = ref false in @@ -176,6 +183,9 @@ and generate_c_call_args ?handle ?(implicit_size_ptr = "&size") | BufferIn n -> next (); pr "%s, %s_size" n n + | Mountable n -> + next (); + pr (if in_daemon then "&%s" else "%s") n | arg -> next (); pr "%s" (name_of_argt arg) diff --git a/generator/daemon.ml b/generator/daemon.ml index a075bdc..d81af93 100644 --- a/generator/daemon.ml +++ b/generator/daemon.ml @@ -38,6 +38,7 @@ let generate_daemon_actions_h () = pr "\n"; pr "#include \"guestfs_protocol.h\"\n"; + pr "#include \"daemon.h\"\n"; pr "\n"; List.iter ( @@ -111,11 +112,12 @@ and generate_daemon_actions () = pr " struct guestfs_%s_args args;\n" name; List.iter ( function - | Device n | Mountable n | Dev_or_Path n + | Device n | Dev_or_Path n | Pathname n | String n | Key n | OptString n -> pr " char *%s;\n" n + | Mountable n -> pr " mountable_t %s;\n" n | StringList n | DeviceList n -> pr " char **%s;\n" n | Bool n -> pr " int %s;\n" n | Int n -> pr " int %s;\n" n @@ -205,10 +207,13 @@ and generate_daemon_actions () = pr_args n; pr " ABS_PATH (%s, %s, goto done);\n" n (if is_filein then "cancel_receive ()" else ""); - | Device n | Mountable n -> + | Device n -> pr_args n; pr " RESOLVE_DEVICE (%s, %s, goto done);\n" n (if is_filein then "cancel_receive ()" else ""); + | Mountable n -> + pr " RESOLVE_MOUNTABLE(args.%s, %s, %s, goto done);\n" + n n (if is_filein then "cancel_receive ()" else ""); | Dev_or_Path n -> pr_args n; pr " REQUIRE_ROOT_OR_RESOLVE_DEVICE (%s, %s, goto done);\n" @@ -257,7 +262,7 @@ and generate_daemon_actions () = (function FileIn _ | FileOut _ -> false | _ -> true) args in let style = ret, args' @ args_of_optargs optargs, [] in pr " r = do_%s " name; - generate_c_call_args style; + generate_c_call_args ~in_daemon:true style; pr ";\n" in (match ret with -- 1.8.1

On Fri, 2013-02-08 at 11:56 +0000, Richard W.M. Jones wrote: They're certainly belt and braces. I'm happy to take them out. I don't think so. I've was pretty careful about errors on error paths (see extensive use of fprintf). ACK. I don't know either. I think I was copying other usage, tbh. I'm happy to change this or not. No, I added in_daemon to generate_c_call_args specifically for mountable support. The reason is that in the library the call is passed a string, but in the daemon it's passed a parsed mountable, so it needs to differentiate. Matt

--- generator/bindtests.ml | 2 +- generator/c.ml | 9 +++++---- generator/csharp.ml | 6 ++++-- generator/daemon.ml | 4 ++-- generator/erlang.ml | 5 +++-- generator/fish.ml | 11 ++++++----- generator/gobject.ml | 8 +++++--- generator/haskell.ml | 12 ++++++++---- generator/java.ml | 10 +++++----- generator/lua.ml | 9 ++++++--- generator/ocaml.ml | 8 +++++--- generator/perl.ml | 11 ++++++++--- generator/php.ml | 15 ++++++++++----- generator/python.ml | 18 ++++++++++++------ generator/ruby.ml | 6 ++++-- generator/tests_c_api.ml | 5 +++-- generator/types.ml | 1 + generator/utils.ml | 3 ++- generator/xdr.ml | 3 ++- 19 files changed, 92 insertions(+), 54 deletions(-) diff --git a/generator/bindtests.ml b/generator/bindtests.ml index 0ca5af9..4f99021 100644 --- a/generator/bindtests.ml +++ b/generator/bindtests.ml @@ -127,7 +127,7 @@ print_strings (guestfs_h *g, char *const *argv) List.iter ( function | Pathname n - | Device n | Mountable n | Dev_or_Path n + | Device n | Mountable n | Dev_or_Path n | Mountable_or_Path n | String n | FileIn n | FileOut n diff --git a/generator/c.ml b/generator/c.ml index 6d26868..d42b5b1 100644 --- a/generator/c.ml +++ b/generator/c.ml @@ -121,7 +121,7 @@ let rec generate_prototype ?(extern = true) ?(static = false) List.iter ( function | Pathname n - | Device n | Dev_or_Path n + | Device n | Dev_or_Path n | Mountable_or_Path n | String n | OptString n | Key n -> @@ -864,7 +864,7 @@ and generate_client_actions hash () = | Device n | Mountable n | Pathname n - | Dev_or_Path n + | Dev_or_Path n | Mountable_or_Path n | FileIn n | FileOut n | BufferIn n @@ -982,7 +982,7 @@ and generate_client_actions hash () = | Device n | Mountable n | Pathname n - | Dev_or_Path n + | Dev_or_Path n | Mountable_or_Path n | FileIn n | FileOut n -> (* guestfish doesn't support string escaping, so neither do we *) @@ -1296,7 +1296,8 @@ and generate_client_actions hash () = ) else ( List.iter ( function - | Pathname n | Device n | Mountable n | Dev_or_Path n | String n + | Pathname n | Device n | Mountable n | Dev_or_Path n + | Mountable_or_Path n | String n | Key n -> pr " args.%s = (char *) %s;\n" n n | OptString n -> diff --git a/generator/csharp.ml b/generator/csharp.ml index e2fcb42..04ef94c 100644 --- a/generator/csharp.ml +++ b/generator/csharp.ml @@ -187,7 +187,8 @@ namespace Guestfs (c_return_type ()) c_function; List.iter ( function - | Pathname n | Device n | Mountable n | Dev_or_Path n | String n + | Pathname n | Device n | Mountable n + | Dev_or_Path n | Mountable_or_Path n | String n | OptString n | FileIn n | FileOut n | Key n @@ -214,7 +215,8 @@ namespace Guestfs in List.iter ( function - | Pathname n | Device n | Mountable n | Dev_or_Path n | String n + | Pathname n | Device n | Mountable n + | Dev_or_Path n | Mountable_or_Path n | String n | OptString n | FileIn n | FileOut n | Key n diff --git a/generator/daemon.ml b/generator/daemon.ml index d81af93..1d2f9fb 100644 --- a/generator/daemon.ml +++ b/generator/daemon.ml @@ -112,7 +112,7 @@ and generate_daemon_actions () = pr " struct guestfs_%s_args args;\n" name; List.iter ( function - | Device n | Dev_or_Path n + | Device n | Dev_or_Path n | Mountable_or_Path n | Pathname n | String n | Key n @@ -214,7 +214,7 @@ and generate_daemon_actions () = | Mountable n -> pr " RESOLVE_MOUNTABLE(args.%s, %s, %s, goto done);\n" n n (if is_filein then "cancel_receive ()" else ""); - | Dev_or_Path n -> + | Dev_or_Path n | Mountable_or_Path n -> pr_args n; pr " REQUIRE_ROOT_OR_RESOLVE_DEVICE (%s, %s, goto done);\n" n (if is_filein then "cancel_receive ()" else ""); diff --git a/generator/erlang.ml b/generator/erlang.ml index 004aee4..6242338 100644 --- a/generator/erlang.ml +++ b/generator/erlang.ml @@ -293,7 +293,7 @@ extern void free_strings (char **r); fun i -> function | Pathname n - | Device n | Mountable n | Dev_or_Path n + | Device n | Mountable n | Dev_or_Path n | Mountable_or_Path n | String n | FileIn n | FileOut n @@ -386,7 +386,8 @@ extern void free_strings (char **r); (* Free strings if we copied them above. *) List.iter ( function - | Pathname n | Device n | Mountable n | Dev_or_Path n | String n + | Pathname n | Device n | Mountable n + | Dev_or_Path n | Mountable_or_Path n | String n | OptString n | FileIn n | FileOut n | Key n -> pr " free (%s);\n" n | StringList n | DeviceList n -> diff --git a/generator/fish.ml b/generator/fish.ml index eacca37..f766170 100644 --- a/generator/fish.ml +++ b/generator/fish.ml @@ -328,7 +328,7 @@ Guestfish will prompt for these separately." | String n | OptString n -> pr " const char *%s;\n" n | Pathname n - | Dev_or_Path n + | Dev_or_Path n | Mountable_or_Path n | FileIn n | FileOut n | Key n -> pr " char *%s;\n" n @@ -411,7 +411,7 @@ Guestfish will prompt for these separately." | String name -> pr " %s = argv[i++];\n" name | Pathname name - | Dev_or_Path name -> + | Dev_or_Path name | Mountable_or_Path name -> pr " %s = win_prefix (argv[i++]); /* process \"win:\" prefix */\n" name; pr " if (%s == NULL) goto out_%s;\n" name name | OptString name -> @@ -625,8 +625,8 @@ Guestfish will prompt for these separately." | Bool name | Int name | Int64 name -> pr " out_%s:\n" name - | Pathname name | Dev_or_Path name | FileOut name - | Key name -> + | Pathname name | Dev_or_Path name | Mountable_or_Path name + | FileOut name | Key name -> pr " free (%s);\n" name; pr " out_%s:\n" name | FileIn name -> @@ -852,7 +852,8 @@ and generate_fish_actions_pod () = pr " %s" name; List.iter ( function - | Pathname n | Device n | Mountable n | Dev_or_Path n | String n -> + | Pathname n | Device n | Mountable n + | Dev_or_Path n | Mountable_or_Path n | String n -> pr " %s" n | OptString n -> pr " %s" n | StringList n | DeviceList n -> pr " '%s ...'" n diff --git a/generator/gobject.ml b/generator/gobject.ml index 5a36bfc..801ffb4 100644 --- a/generator/gobject.ml +++ b/generator/gobject.ml @@ -77,7 +77,7 @@ let generate_gobject_proto name ?(single_line = true) | String n | Device n | Mountable n | Pathname n - | Dev_or_Path n + | Dev_or_Path n | Mountable_or_Path n | OptString n | Key n | FileIn n @@ -1033,7 +1033,8 @@ guestfs_session_close(GuestfsSession *session, GError **err) pr " (transfer none) (type utf8):" | OptString _ -> pr " (transfer none) (type utf8) (allow-none):" - | Device _ | Mountable _ | Pathname _ | Dev_or_Path _ + | Device _ | Mountable _ | Pathname _ + | Dev_or_Path _ | Mountable_or_Path _ | FileIn _ | FileOut _ -> pr " (transfer none) (type filename):" | StringList _ -> @@ -1185,7 +1186,8 @@ guestfs_session_close(GuestfsSession *session, GError **err) | BufferIn n -> pr "%s, %s_size" n n | Bool n | Int n | Int64 n | String n | Device n | Mountable n - | Pathname n | Dev_or_Path n | OptString n | StringList n + | Pathname n | Dev_or_Path n | Mountable_or_Path n + | OptString n | StringList n | DeviceList n | Key n | FileIn n | FileOut n -> pr "%s" n | Pointer _ -> diff --git a/generator/haskell.ml b/generator/haskell.ml index 2033dbf..0bf8551 100644 --- a/generator/haskell.ml +++ b/generator/haskell.ml @@ -140,7 +140,8 @@ assocListOfHashtable (a:b:rest) = (a,b) : assocListOfHashtable rest function | FileIn n | FileOut n - | Pathname n | Device n | Mountable n | Dev_or_Path n | String n + | Pathname n | Device n | Mountable n + | Dev_or_Path n | Mountable_or_Path n | String n | Key n -> pr "withCString %s $ \\%s -> " n n | BufferIn n -> @@ -157,7 +158,8 @@ assocListOfHashtable (a:b:rest) = (a,b) : assocListOfHashtable rest | Int n -> sprintf "(fromIntegral %s)" n | Int64 n | Pointer (_, n) -> sprintf "(fromIntegral %s)" n | FileIn n | FileOut n - | Pathname n | Device n | Mountable n | Dev_or_Path n + | Pathname n | Device n | Mountable n + | Dev_or_Path n | Mountable_or_Path n | String n | OptString n | StringList n | DeviceList n | Key n -> n @@ -214,7 +216,8 @@ and generate_haskell_prototype ~handle ?(hs = false) (ret, args, optargs) = List.iter ( fun arg -> (match arg with - | Pathname _ | Device _ | Mountable _ | Dev_or_Path _ | String _ + | Pathname _ | Device _ | Mountable _ + | Dev_or_Path _ | Mountable_or_Path _ | String _ | Key _ -> pr "CString" | BufferIn _ -> @@ -256,7 +259,8 @@ and generate_haskell_prototype ~handle ?(hs = false) (ret, args, optargs) = List.iter ( fun arg -> (match arg with - | Pathname _ | Device _ | Mountable _ | Dev_or_Path _ | String _ + | Pathname _ | Device _ | Mountable _ + | Dev_or_Path _ | Mountable_or_Path _ | String _ | Key _ -> pr "String" | BufferIn _ -> diff --git a/generator/java.ml b/generator/java.ml index ad53ba3..660d37a 100644 --- a/generator/java.ml +++ b/generator/java.ml @@ -463,7 +463,7 @@ and generate_java_prototype ?(public=false) ?(privat=false) ?(native=false) match arg with | Pathname n - | Device n | Mountable n | Dev_or_Path n + | Device n | Mountable n | Dev_or_Path n | Mountable_or_Path n | String n | OptString n | FileIn n @@ -798,7 +798,7 @@ get_all_event_callbacks (guestfs_h *g, size_t *len_rtn) List.iter ( function | Pathname n - | Device n | Mountable n | Dev_or_Path n + | Device n | Mountable n | Dev_or_Path n | Mountable_or_Path n | String n | OptString n | FileIn n @@ -867,7 +867,7 @@ get_all_event_callbacks (guestfs_h *g, size_t *len_rtn) List.iter ( function | Pathname n - | Device n | Mountable n | Dev_or_Path n + | Device n | Mountable n | Dev_or_Path n | Mountable_or_Path n | String n | OptString n | FileIn n @@ -924,7 +924,7 @@ get_all_event_callbacks (guestfs_h *g, size_t *len_rtn) List.iter ( function | Pathname n - | Device n | Mountable n | Dev_or_Path n + | Device n | Mountable n | Dev_or_Path n | Mountable_or_Path n | String n | FileIn n | FileOut n @@ -991,7 +991,7 @@ get_all_event_callbacks (guestfs_h *g, size_t *len_rtn) List.iter ( function | Pathname n - | Device n | Mountable n | Dev_or_Path n + | Device n | Mountable n | Dev_or_Path n | Mountable_or_Path n | String n | FileIn n | FileOut n diff --git a/generator/lua.ml b/generator/lua.ml index 04a3941..1878844 100644 --- a/generator/lua.ml +++ b/generator/lua.ml @@ -470,7 +470,8 @@ guestfs_lua_delete_event_callback (lua_State *L) List.iter ( function - | Pathname n | Device n | Mountable n | Dev_or_Path n | String n + | Pathname n | Device n | Mountable n + | Dev_or_Path n | Mountable_or_Path n | String n | FileIn n | FileOut n | Key n -> pr " const char *%s;\n" n | BufferIn n -> @@ -500,7 +501,8 @@ guestfs_lua_delete_event_callback (lua_State *L) fun i -> let i = i+2 in (* Lua indexes from 1(!), plus the handle. *) function - | Pathname n | Device n | Mountable n | Dev_or_Path n | String n + | Pathname n | Device n | Mountable n + | Dev_or_Path n | Mountable_or_Path n | String n | FileIn n | FileOut n | Key n -> pr " %s = luaL_checkstring (L, %d);\n" n i | BufferIn n -> @@ -560,7 +562,8 @@ guestfs_lua_delete_event_callback (lua_State *L) (* Free temporary data. *) List.iter ( function - | Pathname _ | Device _ | Mountable _ | Dev_or_Path _ | String _ + | Pathname _ | Device _ | Mountable _ + | Dev_or_Path _ | Mountable_or_Path _ | String _ | FileIn _ | FileOut _ | Key _ | BufferIn _ | OptString _ | Bool _ | Int _ | Int64 _ diff --git a/generator/ocaml.ml b/generator/ocaml.ml index 6302298..315d33d 100644 --- a/generator/ocaml.ml +++ b/generator/ocaml.ml @@ -497,7 +497,7 @@ copy_table (char * const * argv) List.iter ( function | Pathname n - | Device n | Mountable n | Dev_or_Path n + | Device n | Mountable n | Dev_or_Path n | Mountable_or_Path n | String n | FileIn n | FileOut n @@ -583,7 +583,8 @@ copy_table (char * const * argv) (* Free strings if we copied them above. *) List.iter ( function - | Pathname n | Device n | Mountable n | Dev_or_Path n | String n + | Pathname n | Device n | Mountable n + | Dev_or_Path n | Mountable_or_Path n | String n | OptString n | FileIn n | FileOut n | BufferIn n | Key n -> pr " free (%s);\n" n | StringList n | DeviceList n -> @@ -712,7 +713,8 @@ and generate_ocaml_function_type ?(extra_unit = false) (ret, args, optargs) = ) optargs; List.iter ( function - | Pathname _ | Device _ | Mountable _ | Dev_or_Path _ | String _ + | Pathname _ | Device _ | Mountable _ + | Dev_or_Path _ | Mountable_or_Path _ | String _ | FileIn _ | FileOut _ | BufferIn _ | Key _ -> pr "string -> " | OptString _ -> pr "string option -> " | StringList _ | DeviceList _ -> pr "string array -> " diff --git a/generator/perl.ml b/generator/perl.ml index 3b4108a..bdfdb5b 100644 --- a/generator/perl.ml +++ b/generator/perl.ml @@ -342,7 +342,8 @@ user_cancel (g) iteri ( fun i -> function - | Pathname n | Device n | Mountable n | Dev_or_Path n | String n + | Pathname n | Device n | Mountable n + | Dev_or_Path n | Mountable_or_Path n | String n | FileIn n | FileOut n | Key n -> pr " char *%s;\n" n | BufferIn n -> @@ -491,7 +492,8 @@ user_cancel (g) (* Cleanup any arguments. *) List.iter ( function - | Pathname _ | Device _ | Mountable _ | Dev_or_Path _ | String _ + | Pathname _ | Device _ | Mountable _ + | Dev_or_Path _ | Mountable_or_Path _ | String _ | OptString _ | Bool _ | Int _ | Int64 _ | FileIn _ | FileOut _ | BufferIn _ | Key _ | Pointer _ -> () @@ -922,6 +924,8 @@ handlers and threads. | Device n -> pr "[ '%s', 'string(device)', %d ]" n i | Mountable n -> pr "[ '%s', 'string(mountable)', %d ]" n i | Dev_or_Path n -> pr "[ '%s', 'string(dev_or_path)', %d ]" n i + | Mountable_or_Path n -> + pr "[ '%s', 'string(mountable_or_path)', %d ]" n i | String n -> pr "[ '%s', 'string', %d ]" n i | FileIn n -> pr "[ '%s', 'string(filename)', %d ]" n i | FileOut n -> pr "[ '%s', 'string(filename)', %d ]" n i @@ -1093,7 +1097,8 @@ and generate_perl_prototype name (ret, args, optargs) = if !comma then pr ", "; comma := true; match arg with - | Pathname n | Device n | Mountable n | Dev_or_Path n | String n + | Pathname n | Device n | Mountable n + | Dev_or_Path n | Mountable_or_Path n | String n | OptString n | Bool n | Int n | Int64 n | FileIn n | FileOut n | BufferIn n | Key n | Pointer (_, n) -> pr "$%s" n diff --git a/generator/php.ml b/generator/php.ml index cf23863..a4c4536 100644 --- a/generator/php.ml +++ b/generator/php.ml @@ -188,7 +188,8 @@ PHP_FUNCTION (guestfs_last_error) List.iter ( function - | String n | Device n | Mountable n | Pathname n | Dev_or_Path n + | String n | Device n | Mountable n | Pathname n + | Dev_or_Path n | Mountable_or_Path n | FileIn n | FileOut n | Key n | OptString n | BufferIn n -> @@ -232,7 +233,8 @@ PHP_FUNCTION (guestfs_last_error) let param_string = String.concat "" ( List.map ( function - | String n | Device n | Mountable n | Pathname n | Dev_or_Path n + | String n | Device n | Mountable n | Pathname n + | Dev_or_Path n | Mountable_or_Path n | FileIn n | FileOut n | BufferIn n | Key n -> "s" | OptString n -> "s!" | StringList n | DeviceList n -> "a" @@ -260,7 +262,8 @@ PHP_FUNCTION (guestfs_last_error) pr " &z_g"; List.iter ( function - | String n | Device n | Mountable n | Pathname n | Dev_or_Path n + | String n | Device n | Mountable n | Pathname n + | Dev_or_Path n | Mountable_or_Path n | FileIn n | FileOut n | BufferIn n | Key n | OptString n -> pr ", &%s, &%s_size" n n @@ -293,7 +296,8 @@ PHP_FUNCTION (guestfs_last_error) List.iter ( function - | String n | Device n | Mountable n | Pathname n | Dev_or_Path n + | String n | Device n | Mountable n | Pathname n + | Dev_or_Path n | Mountable_or_Path n | FileIn n | FileOut n | Key n | OptString n -> (* Just need to check the string doesn't contain any ASCII @@ -420,7 +424,8 @@ PHP_FUNCTION (guestfs_last_error) (* Free up parameters. *) List.iter ( function - | String n | Device n | Mountable n | Pathname n | Dev_or_Path n + | String n | Device n | Mountable n | Pathname n + | Dev_or_Path n | Mountable_or_Path n | FileIn n | FileOut n | Key n | OptString n -> () | BufferIn n -> () diff --git a/generator/python.ml b/generator/python.ml index cff3049..1b2e755 100644 --- a/generator/python.ml +++ b/generator/python.ml @@ -289,7 +289,8 @@ free_strings (char **argv) List.iter ( function - | Pathname n | Device n | Mountable n | Dev_or_Path n | String n | Key n + | Pathname n | Device n | Mountable n + | Dev_or_Path n | Mountable_or_Path n | String n | Key n | FileIn n | FileOut n -> pr " const char *%s;\n" n | OptString n -> pr " const char *%s;\n" n @@ -326,7 +327,8 @@ free_strings (char **argv) pr " if (!PyArg_ParseTuple (args, (char *) \"O"; List.iter ( function - | Pathname _ | Device _ | Mountable _ | Dev_or_Path _ | String _ | Key _ + | Pathname _ | Device _ | Mountable _ + | Dev_or_Path _ | Mountable_or_Path _ | String _ | Key _ | FileIn _ | FileOut _ -> pr "s" | OptString _ -> pr "z" | StringList _ | DeviceList _ -> pr "O" @@ -347,7 +349,8 @@ free_strings (char **argv) pr " &py_g"; List.iter ( function - | Pathname n | Device n | Mountable n | Dev_or_Path n | String n | Key n + | Pathname n | Device n | Mountable n + | Dev_or_Path n | Mountable_or_Path n | String n | Key n | FileIn n | FileOut n -> pr ", &%s" n | OptString n -> pr ", &%s" n | StringList n | DeviceList n -> pr ", &py_%s" n @@ -369,7 +372,8 @@ free_strings (char **argv) pr " g = get_handle (py_g);\n"; List.iter ( function - | Pathname _ | Device _ | Mountable _ | Dev_or_Path _ | String _ | Key _ + | Pathname _ | Device _ | Mountable _ + | Dev_or_Path _ | Mountable_or_Path _ | String _ | Key _ | FileIn _ | FileOut _ | OptString _ | Bool _ | Int _ | Int64 _ | BufferIn _ -> () | StringList n | DeviceList n -> @@ -505,7 +509,8 @@ free_strings (char **argv) List.iter ( function - | Pathname _ | Device _ | Mountable _ | Dev_or_Path _ | String _ | Key _ + | Pathname _ | Device _ | Mountable _ + | Dev_or_Path _ | Mountable_or_Path _ | String _ | Key _ | FileIn _ | FileOut _ | OptString _ | Bool _ | Int _ | Int64 _ | BufferIn _ | Pointer _ -> () | StringList n | DeviceList n -> @@ -773,7 +778,8 @@ class GuestFS(object): *) List.iter ( function - | Pathname _ | Device _ | Mountable _ | Dev_or_Path _ | String _ | Key _ + | Pathname _ | Device _ | Mountable _ + | Dev_or_Path _ | Mountable_or_Path _ | String _ | Key _ | FileIn _ | FileOut _ | OptString _ | Bool _ | Int _ | Int64 _ | BufferIn _ | Pointer _ -> () | StringList n | DeviceList n -> diff --git a/generator/ruby.ml b/generator/ruby.ml index b98c7ff..4f2c60b 100644 --- a/generator/ruby.ml +++ b/generator/ruby.ml @@ -505,7 +505,8 @@ ruby_user_cancel (VALUE gv) List.iter ( function - | Pathname n | Device n | Mountable n | Dev_or_Path n | String n | Key n + | Pathname n | Device n | Mountable n + | Dev_or_Path n | Mountable_or_Path n | String n | Key n | FileIn n | FileOut n -> pr " const char *%s = StringValueCStr (%sv);\n" n n; | BufferIn n -> @@ -607,7 +608,8 @@ ruby_user_cancel (VALUE gv) List.iter ( function - | Pathname _ | Device _ | Mountable _ | Dev_or_Path _ | String _ | Key _ + | Pathname _ | Device _ | Mountable _ + | Dev_or_Path _ | Mountable_or_Path _ | String _ | Key _ | FileIn _ | FileOut _ | OptString _ | Bool _ | Int _ | Int64 _ | BufferIn _ | Pointer _ -> () | StringList n | DeviceList n -> diff --git a/generator/tests_c_api.ml b/generator/tests_c_api.ml index 9cc047c..a55b196 100644 --- a/generator/tests_c_api.ml +++ b/generator/tests_c_api.ml @@ -794,7 +794,7 @@ and generate_test_command_call ?(expect_error = false) ?test test_name cmd = | Pathname n, arg | Device n, arg | Mountable n, arg - | Dev_or_Path n, arg + | Dev_or_Path n, arg | Mountable_or_Path n, arg | String n, arg | OptString n, arg | Key n, arg -> @@ -903,7 +903,8 @@ and generate_test_command_call ?(expect_error = false) ?test test_name cmd = function | OptString _, "NULL" -> pr ", NULL" | Pathname n, _ - | Device n, _ | Mountable n, _ | Dev_or_Path n, _ + | Device n, _ | Mountable n, _ + | Dev_or_Path n, _ | Mountable_or_Path n, _ | String n, _ | OptString n, _ | Key n, _ -> diff --git a/generator/types.ml b/generator/types.ml index d928724..0198eac 100644 --- a/generator/types.ml +++ b/generator/types.ml @@ -135,6 +135,7 @@ and argt = | Mountable of string (* location of mountable filesystem, cannot be NULL *) | Pathname of string (* file name, cannot be NULL *) | Dev_or_Path of string (* /dev device name or Pathname, cannot be NULL *) + | Mountable_or_Path of string (* mount or Pathname, cannot be NULL *) | OptString of string (* const char *name, may be NULL *) | StringList of string(* list of strings (each string cannot be NULL) *) | DeviceList of string(* list of Device names (each cannot be NULL) *) diff --git a/generator/utils.ml b/generator/utils.ml index 6a94009..6bc039f 100644 --- a/generator/utils.ml +++ b/generator/utils.ml @@ -250,7 +250,8 @@ let map_chars f str = List.map f (explode str) let name_of_argt = function - | Pathname n | Device n | Mountable n | Dev_or_Path n | String n | OptString n + | Pathname n | Device n | Mountable n | Dev_or_Path n + | Mountable_or_Path n | String n | OptString n | StringList n | DeviceList n | Bool n | Int n | Int64 n | FileIn n | FileOut n | BufferIn n | Key n | Pointer (_, n) -> n diff --git a/generator/xdr.ml b/generator/xdr.ml index 976a958..e2161cd 100644 --- a/generator/xdr.ml +++ b/generator/xdr.ml @@ -106,7 +106,8 @@ let generate_xdr () = pr "struct %s_args {\n" name; List.iter ( function - | Pathname n | Device n | Mountable n | Dev_or_Path n | String n + | Pathname n | Device n | Mountable n | Dev_or_Path n + | Mountable_or_Path n | String n | Key n -> pr " string %s<>;\n" n | OptString n -> pr " guestfs_str *%s;\n" n -- 1.8.1

A Mountable_or_Path argument is passed as a mountable_t. A new type is added to mountable_t to handle already mounted paths. --- daemon/daemon.h | 45 +++++++++++++++++++++++++++++++++------------ daemon/guestfsd.c | 26 ++++++++++++++++++++++++++ generator/c.ml | 6 +++--- generator/daemon.ml | 14 +++++++------- 4 files changed, 69 insertions(+), 22 deletions(-) diff --git a/daemon/daemon.h b/daemon/daemon.h index 502e0eb..a94c338 100644 --- a/daemon/daemon.h +++ b/daemon/daemon.h @@ -32,6 +32,20 @@ #include "guestfs-internal-all.h" +/* Mountables */ + +typedef enum { + MOUNTABLE_DEVICE, /* A bare device */ + MOUNTABLE_BTRFSVOL, /* A btrfs subvolume: device + volume */ + MOUNTABLE_PATH /* An already mounted path: device = path */ +} mountable_type_t; + +typedef struct { + mountable_type_t type; + const char *device; + const char *volume; +} mountable_t; + /*-- in guestfsd.c --*/ extern int verbose; @@ -49,18 +63,7 @@ extern int xwrite (int sock, const void *buf, size_t len) extern int xread (int sock, void *buf, size_t len) __attribute__((__warn_unused_result__)); -/* Mountables */ - -typedef enum { - MOUNTABLE_DEVICE, - MOUNTABLE_BTRFSVOL -} mountable_type_t; - -typedef struct { - mountable_type_t type; - const char *device; - const char *volume; -} mountable_t; +extern char *mountable_to_string (const mountable_t *mountable); /* Growable strings buffer. */ struct stringsbuf { @@ -391,6 +394,24 @@ is_zero (const char *buffer, size_t size) } \ } while (0) +/* Helper for functions which need either an absolute path in the + * mounted filesystem, OR a valid mountable description. + */ +#define REQUIRE_ROOT_OR_RESOLVE_MOUNTABLE(string, mountable, \ + cancel_stmt, fail_stmt) \ + do { \ + if (STREQLEN ((string), "/dev/", strlen ("/dev/")) || (string)[0] != '/') {\ + RESOLVE_MOUNTABLE (string, mountable, cancel_stmt, fail_stmt); \ + } \ + \ + else { \ + NEED_ROOT (cancel_stmt, fail_stmt); \ + (mountable).type = MOUNTABLE_PATH; \ + (mountable).device = (string); \ + } \ + } while (0) \ + + /* NB: * (1) You must match CHROOT_IN and CHROOT_OUT even along error paths. * (2) You must not change directory! cwd must always be "/", otherwise diff --git a/daemon/guestfsd.c b/daemon/guestfsd.c index 02454e3..74c7dfc 100644 --- a/daemon/guestfsd.c +++ b/daemon/guestfsd.c @@ -1221,6 +1221,32 @@ parse_btrfsvol (char *desc, mountable_t *mountable) return 0; } +/* Convert a mountable_t back to its string representation + * + * This function can be used in an error path, and must not call + * reply_with_error(). + */ +char * +mountable_to_string (const mountable_t *mountable) +{ + char *desc; + + switch (mountable->type) { + case MOUNTABLE_DEVICE: + case MOUNTABLE_PATH: + return strdup (mountable->device); + + case MOUNTABLE_BTRFSVOL: + if (asprintf(&desc, "btrfsvol:%s/%s", + mountable->device, mountable->volume) == -1) + return NULL; + return desc; + + default: + return NULL; + } +} + /* Check program exists and is executable on $PATH. Actually, we * just assume PATH contains the default entries (see main() above). */ diff --git a/generator/c.ml b/generator/c.ml index d42b5b1..ad4ee44 100644 --- a/generator/c.ml +++ b/generator/c.ml @@ -121,13 +121,13 @@ let rec generate_prototype ?(extern = true) ?(static = false) List.iter ( function | Pathname n - | Device n | Dev_or_Path n | Mountable_or_Path n + | Device n | Dev_or_Path n | String n | OptString n | Key n -> next (); pr "const char *%s" n - | Mountable n -> + | Mountable n | Mountable_or_Path n -> next(); if in_daemon then pr "const mountable_t *%s" n @@ -183,7 +183,7 @@ and generate_c_call_args ?handle ?(implicit_size_ptr = "&size") | BufferIn n -> next (); pr "%s, %s_size" n n - | Mountable n -> + | Mountable n | Mountable_or_Path n -> next (); pr (if in_daemon then "&%s" else "%s") n | arg -> diff --git a/generator/daemon.ml b/generator/daemon.ml index 1d2f9fb..704deb6 100644 --- a/generator/daemon.ml +++ b/generator/daemon.ml @@ -112,12 +112,9 @@ and generate_daemon_actions () = pr " struct guestfs_%s_args args;\n" name; List.iter ( function - | Device n | Dev_or_Path n | Mountable_or_Path n - | Pathname n - | String n - | Key n - | OptString n -> pr " char *%s;\n" n - | Mountable n -> pr " mountable_t %s;\n" n + | Device n | Dev_or_Path n + | Pathname n | String n | Key n | OptString n -> pr " char *%s;\n" n + | Mountable n | Mountable_or_Path n -> pr " mountable_t %s;\n" n | StringList n | DeviceList n -> pr " char **%s;\n" n | Bool n -> pr " int %s;\n" n | Int n -> pr " int %s;\n" n @@ -214,10 +211,13 @@ and generate_daemon_actions () = | Mountable n -> pr " RESOLVE_MOUNTABLE(args.%s, %s, %s, goto done);\n" n n (if is_filein then "cancel_receive ()" else ""); - | Dev_or_Path n | Mountable_or_Path n -> + | Dev_or_Path n -> pr_args n; pr " REQUIRE_ROOT_OR_RESOLVE_DEVICE (%s, %s, goto done);\n" n (if is_filein then "cancel_receive ()" else ""); + | Mountable_or_Path n -> + pr " REQUIRE_ROOT_OR_RESOLVE_MOUNTABLE(args.%s, %s, %s, goto done);\n" + n n (if is_filein then "cancel_receive ()" else ""); | String n | Key n -> pr_args n | OptString n -> pr " %s = args.%s ? *args.%s : NULL;\n" n n n | StringList n -> -- 1.8.1

These 2 internal functions allow mounting and unmounting of mountables outside /sysroot. --- daemon/daemon.h | 8 +++++ daemon/mount.c | 103 +++++++++++++++++++++++++++++++++++++++----------------- 2 files changed, 81 insertions(+), 30 deletions(-) diff --git a/daemon/daemon.h b/daemon/daemon.h index a94c338..78591a0 100644 --- a/daemon/daemon.h +++ b/daemon/daemon.h @@ -65,6 +65,14 @@ extern int xread (int sock, void *buf, size_t len) extern char *mountable_to_string (const mountable_t *mountable); +/*-- in mount.c --*/ + +extern int mount_vfs_internal (const char *options, const char *vfstype, + const mountable_t *mountable, + const char *mp, const char *user_mp, char **err); +extern int umount_internal (const char *pathordevice, + int force, int lazyunmount, char **err); + /* Growable strings buffer. */ struct stringsbuf { char **argv; diff --git a/daemon/mount.c b/daemon/mount.c index 484e45c..34510bd 100644 --- a/daemon/mount.c +++ b/daemon/mount.c @@ -126,31 +126,44 @@ int do_mount_vfs (const char *options, const char *vfstype, const mountable_t *mountable, const char *mountpoint) { - int r; - CLEANUP_FREE char *mp = NULL; - CLEANUP_FREE char *error = NULL; - struct stat statbuf; - ABS_PATH (mountpoint, , return -1); - mp = sysroot_path (mountpoint); + CLEANUP_FREE char *mp = sysroot_path (mountpoint); if (!mp) { reply_with_perror ("malloc"); return -1; } /* Check the mountpoint exists and is a directory. */ + struct stat statbuf; if (stat (mp, &statbuf) == -1) { reply_with_perror ("mount: %s", mountpoint); return -1; } + if (!S_ISDIR (statbuf.st_mode)) { reply_with_perror ("mount: %s: mount point is not a directory", mountpoint); return -1; } + CLEANUP_FREE char *err = NULL; + int r = mount_vfs_internal (options, vfstype, mountable, + mp, mountpoint, &err); + + if (r == -1) { + reply_with_error ("%s", err ? err : "malloc"); + } + + return r; +} + +int +mount_vfs_internal (const char *options, const char *vfstype, + const mountable_t *mountable, + const char *mp, const char *user_mp, + char **err) +{ CLEANUP_FREE char *options_plus = NULL; - const char *device = mountable->device; switch (mountable->type) { case MOUNTABLE_DEVICE: break; @@ -160,24 +173,34 @@ do_mount_vfs (const char *options, const char *vfstype, if (asprintf (&options_plus, "subvol=%s,%s", mountable->volume, options) == -1) { - reply_with_perror ("asprintf"); + /* No point trying to allocate more memory for an error message */ + fprintf (stderr, "asprintf: %m\n"); return -1; } } else { if (asprintf (&options_plus, "subvol=%s", mountable->volume) == -1) { - reply_with_perror ("asprintf"); + /* No point trying to allocate more memory for an error message */ + fprintf (stderr, "asprintf: %m\n"); return -1; } } break; default: - reply_with_error ("Invalid mountable type: %i", mountable->type); + if (asprintf (err, "Invalid mountable type: %i", mountable->type) == -1) + { + /* No point trying to allocate more memory for an error message */ + fprintf (stderr, "Invalid mountable type: %i", mountable->type); + fprintf (stderr, "asprintf: %m\n"); + } return -1; } + CLEANUP_FREE char *error = NULL; + const char *device = mountable->device; + int r; if (vfstype) r = command (NULL, &error, str_mount, "-o", options_plus ? options_plus : options, @@ -186,9 +209,17 @@ do_mount_vfs (const char *options, const char *vfstype, r = command (NULL, &error, str_mount, "-o", options_plus ? options_plus : options, device, mp, NULL); + if (r == -1) { - reply_with_error ("%s on %s (options: '%s'): %s", - device, mountpoint, options, error); + if (asprintf (err, "%s on %s (options: '%s'): %s", + device, user_mp, options_plus ? options_plus : options, + error) == -1) + { + /* No point trying to allocate more memory for an error message */ + fprintf (stderr, "%s on %s (options: '%s'): %s", + device, user_mp, options_plus ? options_plus : options, error); + fprintf (stderr, "asprintf: %m\n"); + } return -1; } @@ -216,28 +247,35 @@ do_mount_options (const char *options, const mountable_t *mountable, /* Takes optional arguments, consult optargs_bitmask. */ int -do_umount (const char *pathordevice, - int force, int lazyunmount) +do_umount (const char *pathordevice, int force, int lazyunmount) { - int r; - CLEANUP_FREE char *err = NULL; - CLEANUP_FREE char *buf = NULL; - int is_dev; - const char *argv[MAX_ARGS]; - size_t i = 0; + CLEANUP_FREE char *buf = STREQLEN (pathordevice, "/dev/", 5) ? + strdup (pathordevice) : sysroot_path (pathordevice); - is_dev = STREQLEN (pathordevice, "/dev/", 5); - buf = is_dev ? strdup (pathordevice) - : sysroot_path (pathordevice); if (buf == NULL) { reply_with_perror ("malloc"); return -1; } - if (!(optargs_bitmask & GUESTFS_UMOUNT_FORCE_BITMASK)) - force = 0; - if (!(optargs_bitmask & GUESTFS_UMOUNT_LAZYUNMOUNT_BITMASK)) - lazyunmount = 0; + if (!(optargs_bitmask & GUESTFS_UMOUNT_FORCE_BITMASK)) force = 0; + if (!(optargs_bitmask & GUESTFS_UMOUNT_LAZYUNMOUNT_BITMASK)) lazyunmount = 0; + + CLEANUP_FREE char *err = NULL; + int r = umount_internal (buf, force, lazyunmount, &err); + + if (r == -1) { + reply_with_error ("%s", err ? err : "malloc"); + } + + return r; +} + +int +umount_internal (const char *pathordevice, int force, int lazyunmount, + char **err) +{ + const char *argv[MAX_ARGS]; + size_t i = 0; /* Use the external /bin/umount program, so that /etc/mtab is kept * updated. @@ -249,13 +287,18 @@ do_umount (const char *pathordevice, if (lazyunmount) ADD_ARG (argv, i, "-l"); - ADD_ARG (argv, i, buf); + ADD_ARG (argv, i, pathordevice); ADD_ARG (argv, i, NULL); - r = commandv (NULL, &err, argv); + CLEANUP_FREE char *errout = NULL; + int r = commandv (NULL, &errout, argv); if (r == -1) { - reply_with_error ("%s: %s", pathordevice, err); + if (asprintf (err, "%s: %s", pathordevice, errout) == -1) { + /* No point trying to allocate more memory for an error message */ + fprintf (stderr, "%s: %s\n", pathordevice, errout); + fprintf (stderr, "asprintf: %m\n"); + } return -1; } -- 1.8.1

btrfs_subvolume_list can now take either the path of a mounted btrfs filesystem, or a mountable describing the location of a btrfs filesystem, or one of its volumes. In the latter case, the filesystem will be automatically mounted outside of /sysroot before running the btrfs tool, and unmounted afterwards. --- daemon/btrfs.c | 72 ++++++++++++++++++++++++++++++++++++++++++++-------- generator/actions.ml | 2 +- 2 files changed, 62 insertions(+), 12 deletions(-) diff --git a/daemon/btrfs.c b/daemon/btrfs.c index 81ce5f5..2c30926 100644 --- a/daemon/btrfs.c +++ b/daemon/btrfs.c @@ -307,16 +307,49 @@ do_btrfs_subvolume_create (const char *dest) } guestfs_int_btrfssubvolume_list * -do_btrfs_subvolume_list (const char *fs) +do_btrfs_subvolume_list (const mountable_t *fs) { char **lines; /* Execute 'btrfs subvolume list <fs>', and split the output into lines */ { - CLEANUP_FREE char *fs_buf = sysroot_path (fs); - if (fs_buf == NULL) { - reply_with_perror ("malloc"); - return NULL; + CLEANUP_FREE char *fs_buf = NULL; + + if (fs->type == MOUNTABLE_PATH) { + fs_buf = sysroot_path (fs->device); + if (fs_buf == NULL) { + reply_with_perror ("malloc"); + + cmderror: + if (fs->type != MOUNTABLE_PATH && fs_buf) { + CLEANUP_FREE char *err = NULL; + if (umount_internal (fs_buf, 1, 0, &err) == -1) + fprintf (stderr, "%s\n", err); + + if (rmdir (fs_buf) == -1 && errno != ENOENT) + fprintf (stderr, "rmdir: %m\n"); + } + return NULL; + } + } + + else { + fs_buf = strdup ("/tmp/btrfs.XXXXXX"); + if (fs_buf == NULL) { + reply_with_perror ("strdup"); + goto cmderror; + } + + if (mkdtemp (fs_buf) == NULL) { + reply_with_perror ("mkdtemp"); + goto cmderror; + } + + CLEANUP_FREE char *err = NULL; + if (mount_vfs_internal ("", NULL, fs, fs_buf, "<internal>", &err) == -1) { + reply_with_error ("%s", err ? err : "malloc"); + goto cmderror; + } } size_t i = 0; @@ -328,16 +361,33 @@ do_btrfs_subvolume_list (const char *fs) ADD_ARG (argv, i, fs_buf); ADD_ARG (argv, i, NULL); - CLEANUP_FREE char *out = NULL, *err = NULL; - int r = commandv (&out, &err, argv); + CLEANUP_FREE char *out = NULL, *errout = NULL; + int r = commandv (&out, &errout, argv); + + if (fs->type != MOUNTABLE_PATH) { + CLEANUP_FREE char *err = NULL; + if (umount_internal (fs_buf, 1, 0, &err) == -1) { + reply_with_error ("%s", err ? err : "malloc"); + goto cmderror; + } + + if (rmdir (fs_buf) == -1 && errno != ENOENT) { + reply_with_error ("rmdir: %m\n"); + goto cmderror; + } + } + if (r == -1) { - reply_with_error ("%s: %s", fs, err); - return NULL; + CLEANUP_FREE char *fs_desc = mountable_to_string (fs); + if (fs_desc == NULL) { + fprintf (stderr, "malloc: %m"); + } + reply_with_error ("%s: %s", fs_desc ? fs_desc : "malloc", errout); + goto cmderror; } lines = split_lines (out); - if (!lines) - return NULL; + if (!lines) return NULL; } /* Output is: diff --git a/generator/actions.ml b/generator/actions.ml index 4f613db..86f58ea 100644 --- a/generator/actions.ml +++ b/generator/actions.ml @@ -9514,7 +9514,7 @@ directory and the name of the snapshot, in the form C</path/to/dest/name>." }; { defaults with name = "btrfs_subvolume_list"; - style = RStructList ("subvolumes", "btrfssubvolume"), [Pathname "fs"], []; + style = RStructList ("subvolumes", "btrfssubvolume"), [Mountable_or_Path "fs"], []; proc_nr = Some 325; optional = Some "btrfs"; camel_name = "BTRFSSubvolumeList"; tests = [] (* tested in tests/btrfs *); -- 1.8.1

--- src/listfs.c | 37 ++++++++++++++++++++++++++++++------- 1 file changed, 30 insertions(+), 7 deletions(-) diff --git a/src/listfs.c b/src/listfs.c index 4bcdf51..3e4f28e 100644 --- a/src/listfs.c +++ b/src/listfs.c @@ -136,6 +136,20 @@ remove_from_list (char **list, const char *item) } } +static void +add_vfs(guestfs_h *g, char *mountable, char *vfs_type, + char ***ret, size_t *ret_size) +{ + /* Extend the return array. */ + size_t i = *ret_size; + *ret_size += 2; + *ret = safe_realloc (g, *ret, (*ret_size + 1) * sizeof (char *)); + + (*ret)[i] = mountable; + (*ret)[i+1] = vfs_type; + (*ret)[i+2] = NULL; +} + /* Use vfs-type to look for a filesystem of some sort on 'dev'. * Apart from some types which we ignore, add the result to the * 'ret' string list. @@ -157,6 +171,21 @@ check_with_vfs_type (guestfs_h *g, const char *device, v = safe_strdup (g, "unknown"); free (vfs_type); } + else if (STREQ (vfs_type, "btrfs")) { + struct guestfs_btrfssubvolume_list *vols = + guestfs_btrfs_subvolume_list (g, device); + + for (size_t i = 0; i < vols->len; i++) { + struct guestfs_btrfssubvolume *this = &vols->val[i]; + char *mountable = safe_asprintf (g, "btrfsvol:%s/%s", + device, this->btrfssubvolume_path); + add_vfs (g, mountable, safe_strdup (g, "btrfs"), ret, ret_size); + } + + guestfs_free_btrfssubvolume_list (vols); + + v = safe_strdup (g, "btrfs"); + } else { /* Ignore all "*_member" strings. In libblkid these are returned * for things which are members of some RAID or LVM set, most @@ -177,13 +206,7 @@ check_with_vfs_type (guestfs_h *g, const char *device, v = vfs_type; } - /* Extend the return array. */ - size_t i = *ret_size; - *ret_size += 2; - *ret = safe_realloc (g, *ret, (*ret_size + 1) * sizeof (char *)); - (*ret)[i] = safe_strdup (g, device); - (*ret)[i+1] = v; - (*ret)[i+2] = NULL; + add_vfs (g, safe_strdup (g, device), v, ret, ret_size); } /* We should ignore partitions that have MBR type byte 0x42, because -- 1.8.1

On Fri, 2013-02-08 at 13:22 +0000, Richard W.M. Jones wrote: Heh. It's a simple pattern. You just have to recognise it for what it is, then you don't have to think about it ;) Matt

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(-) diff --git a/src/guestfs-internal.h b/src/guestfs-internal.h index cf298f2..bec4889 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 ef2999b..1a45b9a 100644 --- a/src/inspect-fs-cd.c +++ b/src/inspect-fs-cd.c @@ -504,7 +504,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 0069dc6..0833245 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) { int r; @@ -96,24 +97,30 @@ 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); - CLEANUP_FREE char *vfs_type = guestfs_vfs_type (g, device); + CLEANUP_FREE char *vfs_type = guestfs_vfs_type (g, mountable); guestfs_pop_error_handler (g); 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 (vfs_type && STREQ (vfs_type, "swap")) { if (extend_fses (g) == -1) return -1; struct inspect_fs *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) { @@ -121,7 +128,7 @@ guestfs___check_for_filesystem_on (guestfs_h *g, const char *device) return -1; struct inspect_fs *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; @@ -137,19 +144,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) @@ -158,27 +165,23 @@ 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) { - partnum = guestfs_part_to_partnum (g, device); + if (!whole_device && m->type == MOUNTABLE_DEVICE) { + partnum = guestfs_part_to_partnum (g, m->device); /* If this returns an error it just means it's not a partition */ } 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; @@ -199,7 +202,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; @@ -215,7 +218,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

--- tests/mountable/Makefile.am | 2 +- tests/mountable/test-inspect.sh | 57 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 1 deletion(-) create mode 100755 tests/mountable/test-inspect.sh diff --git a/tests/mountable/Makefile.am b/tests/mountable/Makefile.am index d50947f..f4b6e01 100644 --- a/tests/mountable/Makefile.am +++ b/tests/mountable/Makefile.am @@ -19,7 +19,7 @@ include $(top_srcdir)/subdir-rules.mk TESTS_ENVIRONMENT = $(top_builddir)/run --test -TESTS=test-internal_parse_mountable +TESTS=test-internal_parse_mountable test-inspect.sh check_PROGRAMS = test-internal_parse_mountable test_internal_parse_mountable_SOURCES = test-internal_parse_mountable.c diff --git a/tests/mountable/test-inspect.sh b/tests/mountable/test-inspect.sh new file mode 100755 index 0000000..2ad3c1b --- /dev/null +++ b/tests/mountable/test-inspect.sh @@ -0,0 +1,57 @@ +#!/bin/bash - +# libguestfs +# Copyright (C) 2013 Red Hat Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + +set -e +export LANG=C + +guestfish=../../fish/guestfish +canonical="sed s,/dev/vd,/dev/sd,g" + +rm -f test.qcow2 test.output + +# Start with the regular (good) fedora image, modify /etc/fstab +# and then inspect it. +qemu-img create -F raw -b ../guests/fedora-btrfs.img -f qcow2 test.qcow2 + +# Test that basic inspection works and the expected filesystems are +# found +$guestfish -a test.qcow2 -i <<'EOF' | sort | $canonical > test.output + inspect-get-mountpoints btrfsvol:/dev/sda2/root +EOF + +if [ "$(cat test.output)" != "/: btrfsvol:/dev/sda2/root +/boot: /dev/sda1 +/home: btrfsvol:/dev/sda2/home" ]; then + echo "$0: error #1: unexpected output from inspect-get-mountpoints" + cat test.output + exit 1 +fi + +# Additional sanity check: did we get the release name right? +$guestfish -a test.qcow2 -i <<'EOF' > test.output + inspect-get-product-name btrfsvol:/dev/sda2/root +EOF + +if [ "$(cat test.output)" != "Fedora release 14 (Phony)"]; then + echo "$0: error #2: unexpected output from inspect-get-product-name" + cat test.output + exit 1 +fi + +rm test.qcow2 +rm test.output -- 1.8.1