Richard W.M. Jones
2011-Nov-09 18:23 UTC
[Libguestfs] [PATCH] Add tune2fs support to libguestfs.
At the moment OpenStack uses kpartx and nbd to resize filesystems and inject files to guests. I sincerely hope they don't allow untrusted users to upload guest images / AMIs :-( To fix this I'm looking into adding libguestfs support as an optional backend in OpenStack. The only missing feature in libguestfs is the ability to call tune2fs on a filesystem. This patch series adds tune2fs support. This also reveals a few bugs in the generator when you start to have calls with lots of required and optional parameters. Rich.
Richard W.M. Jones
2011-Nov-09 18:23 UTC
[Libguestfs] [PATCH 1/5] tests: Fix bitmask parameter when testing optional arguments.
From: "Richard W.M. Jones" <rjones at redhat.com> The bitmask was being constructed backwards(!) As a result, any test which tested optional arguments didn't work. There are very few such tests and they happened not to be affected by this. --- generator/generator_capitests.ml | 13 +++++++------ 1 files changed, 7 insertions(+), 6 deletions(-) diff --git a/generator/generator_capitests.ml b/generator/generator_capitests.ml index 2cad2ae..7bfd87c 100644 --- a/generator/generator_capitests.ml +++ b/generator/generator_capitests.ml @@ -776,8 +776,8 @@ and generate_test_command_call ?(expect_error = false) ?test test_name cmd if optargs <> [] then ( pr " struct guestfs_%s_argv optargs;\n" name; - let bitmask = List.fold_left ( - fun bitmask optarg -> + let _, bitmask = List.fold_left ( + fun (shift, bitmask) optarg -> let is_set match optarg with | Bool n, "" -> false @@ -803,10 +803,11 @@ and generate_test_command_call ?(expect_error = false) ?test test_name cmd | String n, arg -> pr " optargs.%s = \"%s\";\n" n (c_quote arg); true | _ -> assert false in - let bitmask = Int64.shift_left bitmask 1 in - let bitmask = if is_set then Int64.succ bitmask else bitmask in - bitmask - ) 0L optargs in + let bit = if is_set then Int64.shift_left 1L shift else 0L in + let bitmask = Int64.logor bitmask bit in + let shift = shift + 1 in + (shift, bitmask) + ) (0, 0L) optargs in pr " optargs.bitmask = UINT64_C(0x%Lx);\n" bitmask; ); -- 1.7.6
Richard W.M. Jones
2011-Nov-09 18:23 UTC
[Libguestfs] [PATCH 2/5] ocaml: Fix bindings when a function takes more than 10 parameters.
From: "Richard W.M. Jones" <rjones at redhat.com> If any function had more than 10 required + optional parameters, OCaml bindings could not be generated. Currently there are no such functions. --- generator/generator_ocaml.ml | 18 ++++++++++++++---- 1 files changed, 14 insertions(+), 4 deletions(-) diff --git a/generator/generator_ocaml.ml b/generator/generator_ocaml.ml index 51bc3d6..4f7548c 100644 --- a/generator/generator_ocaml.ml +++ b/generator/generator_ocaml.ml @@ -440,13 +440,23 @@ copy_table (char * const * argv) pr ")\n"; pr "{\n"; + (* CAMLparam<N> can only take up to 5 parameters. Further parameters + * have to be passed in groups of 5 to CAMLxparam<N> calls. + *) (match params with - | [p1; p2; p3; p4; p5] -> - pr " CAMLparam5 (%s);\n" (String.concat ", " params) | p1 :: p2 :: p3 :: p4 :: p5 :: rest -> pr " CAMLparam5 (%s);\n" (String.concat ", " [p1; p2; p3; p4; p5]); - pr " CAMLxparam%d (%s);\n" - (List.length rest) (String.concat ", " rest) + let rec loop = function + | [] -> () + | p1 :: p2 :: p3 :: p4 :: p5 :: rest -> + pr " CAMLxparam5 (%s);\n" + (String.concat ", " [p1; p2; p3; p4; p5]); + loop rest + | rest -> + pr " CAMLxparam%d (%s);\n" + (List.length rest) (String.concat ", " rest) + in + loop rest | ps -> pr " CAMLparam%d (%s);\n" (List.length ps) (String.concat ", " ps) ); -- 1.7.6
Richard W.M. Jones
2011-Nov-09 18:23 UTC
[Libguestfs] [PATCH 3/5] ocaml: Catch EVENT_ENTER case in test.
From: "Richard W.M. Jones" <rjones at redhat.com> For some reason we are not compiling the tests with -warn-error so this problem was not noticed before. This fixes commit 9420eaf44ec4067c3740b91b0be0fede08a0c515. --- ocaml/t/guestfs_400_events.ml | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/ocaml/t/guestfs_400_events.ml b/ocaml/t/guestfs_400_events.ml index 66dc3c3..4585a09 100644 --- a/ocaml/t/guestfs_400_events.ml +++ b/ocaml/t/guestfs_400_events.ml @@ -27,7 +27,8 @@ let log g ev eh buf array | Guestfs.EVENT_PROGRESS -> "progress" | Guestfs.EVENT_APPLIANCE -> "appliance" | Guestfs.EVENT_LIBRARY -> "library" - | Guestfs.EVENT_TRACE -> "trace" in + | Guestfs.EVENT_TRACE -> "trace" + | Guestfs.EVENT_ENTER -> "enter" in let eh : int = Obj.magic eh in -- 1.7.6
Richard W.M. Jones
2011-Nov-09 18:23 UTC
[Libguestfs] [PATCH 4/5] ocaml: Compile OCaml bindings and tests with -warn-error.
From: "Richard W.M. Jones" <rjones at redhat.com> --- ocaml/Makefile.am | 27 +++++++++++++++------------ 1 files changed, 15 insertions(+), 12 deletions(-) diff --git a/ocaml/Makefile.am b/ocaml/Makefile.am index e2fdf43..5287035 100644 --- a/ocaml/Makefile.am +++ b/ocaml/Makefile.am @@ -41,6 +41,9 @@ AM_CPPFLAGS = -I$(top_builddir) -I$(OCAMLLIB) -I$(top_srcdir)/ocaml \ if HAVE_OCAML +OCAMLCFLAGS = -g -warn-error CDEFLMPSUVYZX +OCAMLOPTFLAGS = $(OCAMLCFLAGS) + noinst_DATA = mlguestfs.cma mlguestfs.cmxa META OBJS = guestfs_c.o guestfs_c_actions.o guestfs.cmo @@ -90,46 +93,46 @@ noinst_DATA += bindtests \ bindtests: bindtests.cmx mlguestfs.cmxa mkdir -p t - $(OCAMLFIND) ocamlopt -cclib -L$(top_builddir)/src/.libs -I . -package unix -linkpkg mlguestfs.cmxa $< -o $@ + $(OCAMLFIND) ocamlopt $(OCAMLOPTFLAGS) -cclib -L$(top_builddir)/src/.libs -I . -package unix -linkpkg mlguestfs.cmxa $< -o $@ t/guestfs_005_load: t/guestfs_005_load.cmx mlguestfs.cmxa mkdir -p t - $(OCAMLFIND) ocamlopt -cclib -L$(top_builddir)/src/.libs -I . -package unix -linkpkg mlguestfs.cmxa $< -o $@ + $(OCAMLFIND) ocamlopt $(OCAMLOPTFLAGS) -cclib -L$(top_builddir)/src/.libs -I . -package unix -linkpkg mlguestfs.cmxa $< -o $@ t/guestfs_010_basic: t/guestfs_010_basic.cmx mlguestfs.cmxa mkdir -p t - $(OCAMLFIND) ocamlopt -cclib -L$(top_builddir)/src/.libs -I . -package unix -linkpkg mlguestfs.cmxa $< -o $@ + $(OCAMLFIND) ocamlopt $(OCAMLOPTFLAGS) -cclib -L$(top_builddir)/src/.libs -I . -package unix -linkpkg mlguestfs.cmxa $< -o $@ t/guestfs_070_threads: t/guestfs_070_threads.cmx mlguestfs.cmxa mkdir -p t - $(OCAMLFIND) ocamlopt -cclib -L$(top_builddir)/src/.libs -I . -package unix,threads -thread -linkpkg mlguestfs.cmxa $< -o $@ + $(OCAMLFIND) ocamlopt $(OCAMLOPTFLAGS) -cclib -L$(top_builddir)/src/.libs -I . -package unix,threads -thread -linkpkg mlguestfs.cmxa $< -o $@ t/guestfs_080_optargs: t/guestfs_080_optargs.cmx mlguestfs.cmxa mkdir -p t - $(OCAMLFIND) ocamlopt -cclib -L$(top_builddir)/src/.libs -I . -package unix -linkpkg mlguestfs.cmxa $< -o $@ + $(OCAMLFIND) ocamlopt $(OCAMLOPTFLAGS) -cclib -L$(top_builddir)/src/.libs -I . -package unix -linkpkg mlguestfs.cmxa $< -o $@ t/guestfs_400_events: t/guestfs_400_events.cmx mlguestfs.cmxa mkdir -p t - $(OCAMLFIND) ocamlopt -cclib -L$(top_builddir)/src/.libs -I . -package unix -linkpkg mlguestfs.cmxa $< -o $@ + $(OCAMLFIND) ocamlopt $(OCAMLOPTFLAGS) -cclib -L$(top_builddir)/src/.libs -I . -package unix -linkpkg mlguestfs.cmxa $< -o $@ t/guestfs_400_progress: t/guestfs_400_progress.cmx mlguestfs.cmxa mkdir -p t - $(OCAMLFIND) ocamlopt -cclib -L$(top_builddir)/src/.libs -I . -package unix -linkpkg mlguestfs.cmxa $< -o $@ + $(OCAMLFIND) ocamlopt $(OCAMLOPTFLAGS) -cclib -L$(top_builddir)/src/.libs -I . -package unix -linkpkg mlguestfs.cmxa $< -o $@ # Need to rebuild the tests from source if the main library has # changed at all, otherwise we get inconsistent assumptions. t/guestfs_070_threads.cmx: t/guestfs_070_threads.ml mlguestfs.cmxa - $(OCAMLFIND) ocamlopt -package unix,threads -thread -linkpkg -c $< -o $@ + $(OCAMLFIND) ocamlopt $(OCAMLOPTFLAGS) -package unix,threads -thread -linkpkg -c $< -o $@ t/%.cmx: t/%.ml mlguestfs.cmxa - $(OCAMLFIND) ocamlopt -package unix -linkpkg -c $< -o $(builddir)/$@ + $(OCAMLFIND) ocamlopt $(OCAMLOPTFLAGS) -package unix -linkpkg -c $< -o $(builddir)/$@ %.cmi: %.mli - $(OCAMLFIND) ocamlc -package unix -c $< -o $(builddir)/$@ + $(OCAMLFIND) ocamlc $(OCAMLCFLAGS) -package unix -c $< -o $(builddir)/$@ %.cmo: %.ml - $(OCAMLFIND) ocamlc -package unix -c $< -o $(builddir)/$@ + $(OCAMLFIND) ocamlc $(OCAMLCFLAGS) -package unix -c $< -o $(builddir)/$@ %.cmx: %.ml - $(OCAMLFIND) ocamlopt -package unix -c $< -o $(builddir)/$@ + $(OCAMLFIND) ocamlopt $(OCAMLOPTFLAGS) -package unix -c $< -o $(builddir)/$@ depend: .depend -- 1.7.6
Richard W.M. Jones
2011-Nov-09 18:23 UTC
[Libguestfs] [PATCH 5/5] New API: Bind the tune2fs command.
From: "Richard W.M. Jones" <rjones at redhat.com> Previously we bound the 'tune2fs -l' command so that we could list out the tunables of an ext2/3/4 filesystem. Also commands like set_e2label and set_e2uuid used tune2fs. This commit binds many of the tunables that can be set using tune2fs. The coverage is not complete, but we can add more later because this uses optional parameters so the call is extensible without breaking ABI. The current change gives us enough for using libguestfs within OpenStack. --- daemon/ext2.c | 192 ++++++++++++++++++++++++++++++++++++++++ generator/generator_actions.ml | 84 +++++++++++++++++ src/MAX_PROC_NR | 2 +- 3 files changed, 277 insertions(+), 1 deletions(-) diff --git a/daemon/ext2.c b/daemon/ext2.c index f7889da..9bc3581 100644 --- a/daemon/ext2.c +++ b/daemon/ext2.c @@ -498,3 +498,195 @@ do_mke2fs_JU (const char *fstype, int blocksize, const char *device, free (err); return 0; } + +/* Takes optional arguments, consult optargs_bitmask. */ +int +do_tune2fs (const char *device, /* only required parameter */ + int force, + int maxmountcount, + int mountcount, + const char *errorbehavior, + int64_t group, + int intervalbetweenchecksdays, + int intervalbetweenchecksweeks, + int intervalbetweenchecksmonths, + int reservedblockspercentage, + const char *lastmounteddirectory, + int64_t reservedblockscount, + int64_t user) +{ + const size_t max_args = 64; + const char *argv[max_args]; + size_t i = 0; + int r; + char *err; + char prog[] = "tune2fs"; + char maxmountcount_s[64]; + char mountcount_s[64]; + char group_s[64]; + char intervalbetweenchecks_s[64]; + char reservedblockspercentage_s[64]; + char reservedblockscount_s[64]; + char user_s[64]; + size_t ibc_count = 0; + + if (e2prog (prog) == -1) + return -1; + + argv[i++] = prog; + + if (optargs_bitmask & GUESTFS_TUNE2FS_FORCE_BITMASK) { + if (force) + argv[i++] = "-f"; + } + + if (optargs_bitmask & GUESTFS_TUNE2FS_MAXMOUNTCOUNT_BITMASK) { + if (maxmountcount < 0) { + reply_with_error ("maxmountcount cannot be negative"); + return -1; + } + argv[i++] = "-c"; + snprintf (maxmountcount_s, sizeof maxmountcount_s, "%d", maxmountcount); + argv[i++] = maxmountcount_s; + } + + if (optargs_bitmask & GUESTFS_TUNE2FS_MOUNTCOUNT_BITMASK) { + if (mountcount < 0) { + reply_with_error ("mountcount cannot be negative"); + return -1; + } + argv[i++] = "-C"; + snprintf (mountcount_s, sizeof mountcount_s, "%d", mountcount); + argv[i++] = mountcount_s; + } + + if (optargs_bitmask & GUESTFS_TUNE2FS_ERRORBEHAVIOR_BITMASK) { + if (STRNEQ (errorbehavior, "continue") && + STRNEQ (errorbehavior, "remount-ro") && + STRNEQ (errorbehavior, "panic")) { + reply_with_error ("invalid errorbehavior parameter: %s", errorbehavior); + return -1; + } + argv[i++] = "-e"; + argv[i++] = errorbehavior; + } + + if (optargs_bitmask & GUESTFS_TUNE2FS_GROUP_BITMASK) { + if (group < 0) { + reply_with_error ("group cannot be negative"); + return -1; + } + argv[i++] = "-g"; + snprintf (group_s, sizeof group_s, "%" PRIi64, group); + argv[i++] = group_s; + } + + if (optargs_bitmask & GUESTFS_TUNE2FS_INTERVALBETWEENCHECKSDAYS_BITMASK) { + if (intervalbetweenchecksdays < 0) { + reply_with_error ("intervalbetweenchecksdays cannot be negative"); + return -1; + } + argv[i++] = "-i"; + if (intervalbetweenchecksdays > 0) { + snprintf (intervalbetweenchecks_s, sizeof intervalbetweenchecks_s, + "%dd", intervalbetweenchecksdays); + argv[i++] = intervalbetweenchecks_s; + } + else + argv[i++] = "0"; + + ibc_count++; + } + + if (optargs_bitmask & GUESTFS_TUNE2FS_INTERVALBETWEENCHECKSWEEKS_BITMASK) { + if (intervalbetweenchecksweeks < 0) { + reply_with_error ("intervalbetweenchecksweeks cannot be negative"); + return -1; + } + argv[i++] = "-i"; + if (intervalbetweenchecksweeks > 0) { + snprintf (intervalbetweenchecks_s, sizeof intervalbetweenchecks_s, + "%dw", intervalbetweenchecksweeks); + argv[i++] = intervalbetweenchecks_s; + } + else + argv[i++] = "0"; + + ibc_count++; + } + + if (optargs_bitmask & GUESTFS_TUNE2FS_INTERVALBETWEENCHECKSMONTHS_BITMASK) { + if (intervalbetweenchecksmonths < 0) { + reply_with_error ("intervalbetweenchecksmonths cannot be negative"); + return -1; + } + argv[i++] = "-i"; + if (intervalbetweenchecksmonths > 0) { + snprintf (intervalbetweenchecks_s, sizeof intervalbetweenchecks_s, + "%dm", intervalbetweenchecksmonths); + argv[i++] = intervalbetweenchecks_s; + } + else + argv[i++] = "0"; + + ibc_count++; + } + + if (ibc_count > 1) { + reply_with_error ("intervalbetweendays/weeks/months: only one of these optional parameters can be specified"); + return -1; + } + + if (optargs_bitmask & GUESTFS_TUNE2FS_RESERVEDBLOCKSPERCENTAGE_BITMASK) { + if (reservedblockspercentage < 0) { + reply_with_error ("reservedblockspercentage cannot be negative"); + return -1; + } + argv[i++] = "-m"; + snprintf (reservedblockspercentage_s, sizeof reservedblockspercentage_s, + "%d", reservedblockspercentage); + argv[i++] = reservedblockspercentage_s; + } + + if (optargs_bitmask & GUESTFS_TUNE2FS_LASTMOUNTEDDIRECTORY_BITMASK) { + argv[i++] = "-M"; + argv[i++] = lastmounteddirectory; + } + + if (optargs_bitmask & GUESTFS_TUNE2FS_RESERVEDBLOCKSCOUNT_BITMASK) { + if (reservedblockscount < 0) { + reply_with_error ("reservedblockscount cannot be negative"); + return -1; + } + argv[i++] = "-r"; + snprintf (reservedblockscount_s, sizeof reservedblockscount_s, + "%" PRIi64, reservedblockscount); + argv[i++] = reservedblockscount_s; + } + + if (optargs_bitmask & GUESTFS_TUNE2FS_USER_BITMASK) { + if (user < 0) { + reply_with_error ("user cannot be negative"); + return -1; + } + argv[i++] = "-u"; + snprintf (user_s, sizeof user_s, "%" PRIi64, user); + argv[i++] = user_s; + } + + argv[i++] = device; + argv[i++] = NULL; + + if (i > max_args) + abort (); + + r = commandv (NULL, &err, argv); + if (r == -1) { + reply_with_error ("%s: %s: %s", prog, device, err); + free (err); + return -1; + } + + free (err); + return 0; +} diff --git a/generator/generator_actions.ml b/generator/generator_actions.ml index d3fa3e0..1b2c36b 100644 --- a/generator/generator_actions.ml +++ b/generator/generator_actions.ml @@ -6332,6 +6332,90 @@ is for copying blocks within existing files. See C<guestfs_cp>, C<guestfs_cp_a> and C<guestfs_mv> for general file copying and moving functions."); + ("tune2fs", (RErr, [Device "device"], [Bool "force"; Int "maxmountcount"; Int "mountcount"; String "errorbehavior"; Int64 "group"; Int "intervalbetweenchecksdays"; Int "intervalbetweenchecksweeks"; Int "intervalbetweenchecksmonths"; Int "reservedblockspercentage"; String "lastmounteddirectory"; Int64 "reservedblockscount"; Int64 "user"]), 298, [], + [InitBasicFS, Always, TestRun ( + [["tune2fs"; "/dev/sda1"; "false"; "0"; ""; "NOARG"; ""; "0"; ""; ""; ""; "NOARG"; ""; ""]])], + "adjust ext2/ext3/ext4 filesystem parameters", + "\ +This call allows you to adjust various filesystem parameters of +an ext2/ext3/ext4 filesystem called C<device>. + +The optional parameters are: + +=over 4 + +=item C<force> + +Force tune2fs to complete the operation even in the face of errors. +This is the same as the tune2fs C<-f> option. + +=item C<maxmountcount> + +Set the number of mounts after which the filesystem is checked +by L<e2fsck(8)>. If this is C<0> then the number of mounts is +disregarded. This is the same as the tune2fs C<-c> option. + +=item C<mountcount> + +Set the number of times the filesystem has been mounted. +This is the same as the tune2fs C<-C> option. + +=item C<errorbehavior> + +Change the behavior of the kernel code when errors are detected. +Possible values currently are: C<continue>, C<remount-ro>, C<panic>. +In practice these options don't really make any difference, +particularly for write errors. + +This is the same as the tune2fs C<-e> option. + +=item C<group> + +Set the group which can use reserved filesystem blocks. +This is the same as the tune2fs C<-g> option except that it +can only be specified as a number. + +=item C<intervalbetweenchecksdays> + +=item C<intervalbetweenchecksweeks> + +=item C<intervalbetweenchecksmonths> + +Adjust the maximal time between two filesystem checks +(in days, weeks or months respectively). Only a maximum of +one of these optional parameters may be passed to the call. +If the option is passed as C<0> then time-dependent checking +is disabled. + +This is the same as the tune2fs C<-i> option. + +=item C<reservedblockspercentage> + +Set the percentage of the filesystem which may only be allocated +by privileged processes. +This is the same as the tune2fs C<-m> option. + +=item C<lastmounteddirectory> + +Set the last mounted directory. +This is the same as the tune2fs C<-M> option. + +=item C<reservedblockscount> +Set the number of reserved filesystem blocks. +This is the same as the tune2fs C<-r> option. + +=item C<user> + +Set the user who can use the reserved filesystem blocks. +This is the same as the tune2fs C<-u> option except that it +can only be specified as a number. + +=back + +To get the current values of filesystem parameters, see +C<guestfs_tune2fs_l>. For precise details of how tune2fs +works, see the L<tune2fs(8)> man page."); + ] let all_functions = non_daemon_functions @ daemon_functions diff --git a/src/MAX_PROC_NR b/src/MAX_PROC_NR index 95de1ee..a1f7f63 100644 --- a/src/MAX_PROC_NR +++ b/src/MAX_PROC_NR @@ -1 +1 @@ -297 +298 -- 1.7.6
Mark McLoughlin
2011-Nov-10 12:16 UTC
[Libguestfs] [PATCH] Add tune2fs support to libguestfs.
Hey Rich, On Wed, 2011-11-09 at 18:23 +0000, Richard W.M. Jones wrote:> At the moment OpenStack uses kpartx and nbd to resize filesystems and > inject files to guests. I sincerely hope they don't allow untrusted > users to upload guest images / AMIs :-(I'm not saying the current situation is ideal, but could you talk me through exactly what the concerns are with what OpenStack is currently doing with potentially untrusted images? Is it this one? http://libguestfs.org/guestfs.3.html#security_of_mounting_filesystems "there are very many filesystem drivers in the kernel, and many of them are infrequently used and not much developer attention has been paid to the code. Linux userspace helps potential crackers by detecting the filesystem type and automatically choosing the right VFS driver, even if that filesystem type is obscure or unexpected for the administrator." I guess passing e.g. '-t ext2,ext3' to the mount command would mitigate this? Any other glaring issues with what it's doing?> To fix this I'm looking into adding libguestfs support as an optional > backend in OpenStack.Awesome!> The only missing feature in libguestfs is the ability to call tune2fs > on a filesystem. This patch series adds tune2fs support. This also > reveals a few bugs in the generator when you start to have calls with > lots of required and optional parameters.Cool stuff. Cheers, Mark.