Libguestfs - Dec 2010 - [PATCH 0/2] Add mkfs-opts API with optional arguments

This requires changing the generator so it can handle passing optional
arguments all thr way through to the daemon, changing the protocol
(see previous patch set), and implementing the new mkfs-opts call.

At the moment there is just an optional blocksize argument, thus
mimicking what you can already do with 'mkfs-b'.  But this change will
allow us in future to encode much of the rest of the /sbin/mkfs
command line.

Here is how it works from guestfish (with verbose & trace):
><fs> mkfs-opts ext2 /dev/vda1 blocksize:1024
mkfs_opts "ext2" "/dev/vda1"
"blocksize:1024"send_to_daemon: 0x2393bc0 g->state = 3, n = 72
proc 210 (part_disk) took 0.18 seconds
recv_from_daemon: 0x2393bc0 g->state = 3, size_rtn = 0x7fffd24c565c, buf_rtn
= 0x7fffd24c5650
mkfs -t ext2 -b 1024 /dev/vda1
mke2fs 1.41.12 (17-May-2010)
 = 0
><fs> mkfs-opts ext2 /dev/vda1 
mkfs_opts "ext2" "/dev/vda1"send_to_daemon: 0x2393bc0
g->state = 3, n = 72
proc 278 (mkfs_opts) took 0.19 seconds
recv_from_daemon: 0x2393bc0 g->state = 3, size_rtn = 0x7fffd24c565c, buf_rtn
= 0x7fffd24c5650
mkfs -t ext2 /dev/vda1
mke2fs 1.41.12 (17-May-2010)
 = 0
><fs> mkfs-opts ext2 /dev/vda1 blocksize:4096
mkfs_opts "ext2" "/dev/vda1"
"blocksize:4096"send_to_daemon: 0x2393bc0 g->state = 3, n = 72
proc 278 (mkfs_opts) took 0.30 seconds
recv_from_daemon: 0x2393bc0 g->state = 3, size_rtn = 0x7fffd24c565c, buf_rtn
= 0x7fffd24c5650
mkfs -t ext2 -b 4096 /dev/vda1
mke2fs 1.41.12 (17-May-2010)
 = 0
><fs> mkfs-opts ext2 /dev/vda1 blocksize:4096 blocksize:1024
mkfs-opts should have 2-3 parameter(s)
type 'help mkfs-opts' for help on mkfs-opts
><fs> mkfs-opts vfat /dev/vda1 blocksize:32768
mkfs_opts "vfat" "/dev/vda1"
"blocksize:32768"send_to_daemon: 0x2393bc0 g->state = 3, n = 72
proc 278 (mkfs_opts) took 0.30 seconds
recv_from_daemon: 0x2393bc0 g->state = 3, size_rtn = 0x7fffd24c565c, buf_rtn
= 0x7fffd24c5650
blockdev --getss /dev/vda1
mkfs -t vfat -s 64 /dev/vda1
 = 0

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://et.redhat.com/~rjones/libguestfs/
See what it can do: http://et.redhat.com/~rjones/libguestfs/recipes.html

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
New in Fedora 11: Fedora Windows cross-compiler. Compile Windows
programs, test, and build Windows installers. Over 70 libraries supprt'd
http://fedoraproject.org/wiki/MinGW http://www.annexia.org/fedora_mingw
-------------- next part --------------
>From 65f44b459070a1dbfba66c31e0be69588e49f4a8 Mon Sep 17 00:00:00 2001
From: Richard W.M. Jones <rjones at redhat.com>
Date: Thu, 2 Dec 2010 13:32:40 +0000
Subject: [PATCH 1/2] generator: Code to handle optional arguments in daemon
functions.

Previously we only supported optional arguments for library
functions (commit 14490c3e1aac61c6ac90f28828896683f64f0dc9).

This extends that work so that optional arguments can also be
passed through to the daemon.
---
 daemon/proto.c                   |    8 --
 generator/generator_c.ml         |  108 ++++++++++++--------
 generator/generator_capitests.ml |   66 +++++++++---
 generator/generator_daemon.ml    |  215 +++++++++++++++++++++-----------------
 generator/generator_xdr.ml       |   10 +-
 src/guestfs-internal.h           |    2 +-
 src/proto.c                      |    5 +-
 7 files changed, 244 insertions(+), 170 deletions(-)

diff --git a/daemon/proto.c b/daemon/proto.c
index 1fdb26c..00e3166 100644
--- a/daemon/proto.c
+++ b/daemon/proto.c
@@ -167,14 +167,6 @@ main_loop (int _sock)
       reply_with_error ("unexpected message status (%d)",
hdr.status);
       goto cont;
     }
-    /* This version of the daemon does not understand optional arguments
-     * at all.  When we fix this, we will remove the next conditional.
-     */
-    if (hdr.optargs_bitmask != 0) {
-      reply_with_error ("optargs_bitmask != 0 (%" PRIu64
")",
-                        hdr.optargs_bitmask);
-      goto cont;
-    }
 
     proc_nr = hdr.proc;
     serial = hdr.serial;
diff --git a/generator/generator_c.ml b/generator/generator_c.ml
index 24f4e2c..0b3c850 100644
--- a/generator/generator_c.ml
+++ b/generator/generator_c.ml
@@ -909,19 +909,17 @@ check_state (guestfs_h *g, const char *caller)
   (* Client-side stubs for each function. *)
   List.iter (
     fun (shortname, (ret, args, optargs as style), _, _, _, _, _) ->
-      if optargs <> [] then
-        failwithf "optargs not yet implemented for daemon functions";
-
       let name = "guestfs_" ^ shortname in
       let error_code = error_code_of ret in
 
       (* Generate the action stub. *)
       if optargs = [] then
         generate_prototype ~extern:false ~semicolon:false ~newline:true
-          ~handle:"g" name style
+          ~handle:"g" ~prefix:"guestfs_" shortname style
       else
         generate_prototype ~extern:false ~semicolon:false ~newline:true
-          ~handle:"g" ~suffix:"_argv" ~optarg_proto:Argv
name style;
+          ~handle:"g" ~prefix:"guestfs_"
~suffix:"_argv"
+          ~optarg_proto:Argv shortname style;
 
       pr "{\n";
 
@@ -999,45 +997,69 @@ check_state (guestfs_h *g, const char *caller)
       pr "\n";
 
       (* Send the main header and arguments. *)
-      (match args with
-       | [] ->
-           pr "  serial = guestfs___send (g, GUESTFS_PROC_%s,
progress_hint,\n"
-             (String.uppercase shortname);
-           pr "                           NULL, NULL);\n"
-       | args ->
-           List.iter (
-             function
-             | Pathname n | Device n | Dev_or_Path n | String n | Key n ->
-                 pr "  args.%s = (char *) %s;\n" n n
-             | OptString n ->
-                 pr "  args.%s = %s ? (char **) &%s : NULL;\n" n
n n
-             | StringList n | DeviceList n ->
-                 pr "  args.%s.%s_val = (char **) %s;\n" n n n;
-                 pr "  for (args.%s.%s_len = 0; %s[args.%s.%s_len];
args.%s.%s_len++) ;\n" n n n n n n n;
-             | Bool n ->
-                 pr "  args.%s = %s;\n" n n
-             | Int n ->
-                 pr "  args.%s = %s;\n" n n
+      if args = [] && optargs = [] then (
+        pr "  serial = guestfs___send (g, GUESTFS_PROC_%s, progress_hint,
0,\n"
+          (String.uppercase shortname);
+        pr "                           NULL, NULL);\n"
+      ) else (
+        List.iter (
+          function
+          | Pathname n | Device n | Dev_or_Path n | String n | Key n ->
+              pr "  args.%s = (char *) %s;\n" n n
+          | OptString n ->
+              pr "  args.%s = %s ? (char **) &%s : NULL;\n" n n n
+          | StringList n | DeviceList n ->
+              pr "  args.%s.%s_val = (char **) %s;\n" n n n;
+              pr "  for (args.%s.%s_len = 0; %s[args.%s.%s_len];
args.%s.%s_len++) ;\n" n n n n n n n;
+          | Bool n ->
+              pr "  args.%s = %s;\n" n n
+          | Int n ->
+              pr "  args.%s = %s;\n" n n
+          | Int64 n ->
+              pr "  args.%s = %s;\n" n n
+          | FileIn _ | FileOut _ -> ()
+          | BufferIn n ->
+              pr "  /* Just catch grossly large sizes. XDR encoding will
make this precise. */\n";
+              pr "  if (%s_size >= GUESTFS_MESSAGE_MAX) {\n" n;
+              trace_return_error ~indent:4 style;
+              pr "    error (g, \"%%s: size of input buffer too
large\", \"%s\");\n"
+                shortname;
+              pr "    guestfs___end_busy (g);\n";
+              pr "    return %s;\n" error_code;
+              pr "  }\n";
+              pr "  args.%s.%s_val = (char *) %s;\n" n n n;
+              pr "  args.%s.%s_len = %s_size;\n" n n n
+          | Pointer _ -> assert false
+        ) args;
+
+        List.iter (
+          fun argt ->
+            let n = name_of_argt argt in
+            let uc_shortname = String.uppercase shortname in
+            let uc_n = String.uppercase n in
+            pr "  if ((optargs->bitmask &
GUESTFS_%s_%s_BITMASK))\n"
+              uc_shortname uc_n;
+            (match argt with
+             | Bool n
+             | Int n
              | Int64 n ->
-                 pr "  args.%s = %s;\n" n n
-             | FileIn _ | FileOut _ -> ()
-             | BufferIn n ->
-                 pr "  /* Just catch grossly large sizes. XDR encoding
will make this precise. */\n";
-                 pr "  if (%s_size >= GUESTFS_MESSAGE_MAX) {\n" n;
-                 trace_return_error ~indent:4 style;
-                 pr "    error (g, \"%%s: size of input buffer too
large\", \"%s\");\n"
-                   shortname;
-                 pr "    guestfs___end_busy (g);\n";
-                 pr "    return %s;\n" error_code;
-                 pr "  }\n";
-                 pr "  args.%s.%s_val = (char *) %s;\n" n n n;
-                 pr "  args.%s.%s_len = %s_size;\n" n n n
-             | Pointer _ -> assert false
-           ) args;
-           pr "  serial = guestfs___send (g, GUESTFS_PROC_%s,
progress_hint,\n"
-             (String.uppercase shortname);
-           pr "        (xdrproc_t) xdr_%s_args, (char *)
&args);\n"
-             name;
+                 pr "    args.%s = optargs->%s;\n" n n;
+                 pr "  else\n";
+                 pr "    args.%s = 0;\n" n
+             | String n ->
+                 pr "    args.%s = (char *) %s;\n" n n;
+                 pr "  else\n";
+                 pr "    args.%s = (char *) \"\";\n" n
+             | _ -> assert false
+            )
+        ) optargs;
+
+        pr "  serial = guestfs___send (g, GUESTFS_PROC_%s,\n"
+          (String.uppercase shortname);
+        pr "                           progress_hint, %s,\n"
+          (if optargs <> [] then "optargs->bitmask" else
"0");
+        pr "                           (xdrproc_t) xdr_%s_args, (char *)
&args);\n"
+          name;
       );
       pr "  if (serial == -1) {\n";
       pr "    guestfs___end_busy (g);\n";
diff --git a/generator/generator_capitests.ml b/generator/generator_capitests.ml
index f5be3e7..5b40cc2 100644
--- a/generator/generator_capitests.ml
+++ b/generator/generator_capitests.ml
@@ -709,7 +709,7 @@ and generate_test_command_call ?(expect_error = false) ?test
test_name cmd    | [] -> assert false
   | name :: args ->
       (* Look up the command to find out what args/ret it has. *)
-      let style +      let style_ret, style_args, style_optargs          try
           let _, style, _, _, _, _, _              List.find (fun (n, _, _, _,
_, _, _) -> n = name) all_functions in
@@ -717,16 +717,24 @@ and generate_test_command_call ?(expect_error = false)
?test test_name cmd          with Not_found ->
           failwithf "%s: in test, command %s was not found" test_name
name in
 
-      (* If the call has optional args, fold them all together.  We cannot
-       * test partial optional args yet.
-       *)
-      let style -        let ret, args, optargs = style in
-        ret, args at optargs in
-
-      if List.length (snd style) <> List.length args then
-        failwithf "%s: in test, wrong number of args given to %s"
-          test_name name;
+      (* Match up the arguments strings and argument types. *)
+      let args, optargs +        let rec loop argts args +          match
argts, args with
+          | (t::ts), (s::ss) ->
+              let args, rest = loop ts ss in
+              ((t, s) :: args), rest
+          | [], ss -> [], ss
+          | ts, [] ->
+              failwithf "%s: in test, too few args given to function
%s"
+                test_name name
+        in
+        let args, optargs = loop style_args args in
+        let optargs, rest = loop style_optargs optargs in
+        if rest <> [] then
+          failwithf "%s: in test, too many args given to function %s"
+            test_name name;
+        args, optargs in
 
       pr "  {\n";
 
@@ -764,10 +772,28 @@ and generate_test_command_call ?(expect_error = false)
?test test_name cmd          | Pointer _, _ ->
             (* Difficult to make these pointers in order to run a test. *)
             assert false
-      ) (List.combine (snd style) args);
+      ) args;
+
+      (* Currently can only deal with a complete, in-order list of optargs. *)
+      if optargs <> [] then (
+        pr "    struct guestfs_%s_argv optargs;\n" name;
+        let len = List.length style_optargs in
+        let bitmask = Int64.pred (Int64.shift_left 1L len) in
+        pr "    optargs.bitmask = UINT64_C(0x%Lx);\n" bitmask;
+        List.iter (
+          function
+          | Bool n, arg
+          | Int n, arg
+          | Int64 n, arg ->
+              pr "    optargs.%s = %s;\n" n arg
+          | String n, arg ->
+              pr "    optargs.%s = \"%s\";\n" n (c_quote
arg);
+          | _ -> assert false
+        ) optargs;
+      );
 
       let error_code -        match fst style with
+        match style_ret with
         | RErr | RInt _ | RBool _ -> pr "    int r;\n";
"-1"
         | RInt64 _ -> pr "    int64_t r;\n"; "-1"
         | RConstString _ | RConstOptString _ ->
@@ -787,7 +813,10 @@ and generate_test_command_call ?(expect_error = false)
?test test_name cmd              "NULL" in
 
       pr "    suppress_error = %d;\n" (if expect_error then 1 else
0);
-      pr "    r = guestfs_%s (g" name;
+      if optargs = [] then
+        pr "    r = guestfs_%s (g" name
+      else
+        pr "    r = guestfs_%s_argv (g" name;
 
       (* Generate the parameters. *)
       List.iter (
@@ -820,13 +849,16 @@ and generate_test_command_call ?(expect_error = false)
?test test_name cmd          | Bool _, arg ->
             let b = bool_of_string arg in pr ", %d" (if b then 1 else
0)
         | Pointer _, _ -> assert false
-      ) (List.combine (snd style) args);
+      ) args;
 
-      (match fst style with
+      (match style_ret with
        | RBufferOut _ -> pr ", &size"
        | _ -> ()
       );
 
+      if optargs <> [] then
+        pr ", &optargs";
+
       pr ");\n";
 
       if not expect_error then
@@ -841,7 +873,7 @@ and generate_test_command_call ?(expect_error = false) ?test
test_name cmd         | Some f -> f ()
       );
 
-      (match fst style with
+      (match style_ret with
        | RErr | RInt _ | RInt64 _ | RBool _
        | RConstString _ | RConstOptString _ -> ()
        | RString _ | RBufferOut _ -> pr "    free (r);\n"
diff --git a/generator/generator_daemon.ml b/generator/generator_daemon.ml
index e3d87e5..f15ae61 100644
--- a/generator/generator_daemon.ml
+++ b/generator/generator_daemon.ml
@@ -37,7 +37,22 @@ let generate_daemon_actions_h ()    pr "\n";
 
   List.iter (
-    fun (name, style, _, _, _, _, _) ->
+    function
+    | shortname, (_, _, (_::_ as optargs)), _, _, _, _, _ ->
+        iteri (
+          fun i arg ->
+            let uc_shortname = String.uppercase shortname in
+            let n = name_of_argt arg in
+            let uc_n = String.uppercase n in
+            pr "#define GUESTFS_%s_%s_BITMASK
(UINT64_C(1)<<%d)\n"
+              uc_shortname uc_n i
+        ) optargs
+    | _ -> ()
+  ) daemon_functions;
+
+  List.iter (
+    fun (name, (ret, args, optargs), _, _, _, _, _) ->
+      let style = ret, args @ optargs, [] in
       generate_prototype
         ~single_line:true ~newline:true ~in_daemon:true ~prefix:"do_"
         name style;
@@ -64,9 +79,6 @@ and generate_daemon_actions ()  
   List.iter (
     fun (name, (ret, args, optargs), _, _, _, _, _) ->
-      if optargs <> [] then
-        failwithf "optional arguments not supported in the daemon
yet";
-
       (* Generate server-side stubs. *)
       pr "static void %s_stub (XDR *xdr_in)\n" name;
       pr "{\n";
@@ -86,98 +98,108 @@ and generate_daemon_actions ()              pr "  char
*r;\n";
             "NULL" in
 
-      (match args with
-       | [] -> ()
-       | args ->
-           pr "  struct guestfs_%s_args args;\n" name;
-           List.iter (
-             function
-             | Device n | Dev_or_Path n
-             | Pathname n
-             | String n
-             | Key n -> ()
-             | OptString n -> pr "  char *%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
-             | Int64 n -> pr "  int64_t %s;\n" n
-             | FileIn _ | FileOut _ -> ()
-             | BufferIn n ->
-                 pr "  const char *%s;\n" n;
-                 pr "  size_t %s_size;\n" n
-             | Pointer _ -> assert false
-           ) args
+      if args <> [] || optargs <> [] then (
+        pr "  struct guestfs_%s_args args;\n" name;
+        List.iter (
+          function
+          | Device n | Dev_or_Path n
+          | Pathname n
+          | String n
+          | Key n -> ()
+          | OptString n -> pr "  char *%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
+          | Int64 n -> pr "  int64_t %s;\n" n
+          | FileIn _ | FileOut _ -> ()
+          | BufferIn n ->
+              pr "  const char *%s;\n" n;
+              pr "  size_t %s_size;\n" n
+          | Pointer _ -> assert false
+        ) (args @ optargs)
       );
       pr "\n";
 
       let is_filein          List.exists (function FileIn _ -> true | _
-> false) args in
 
-      (match args with
-       | [] -> ()
-       | args ->
-           pr "  memset (&args, 0, sizeof args);\n";
-           pr "\n";
-           pr "  if (!xdr_guestfs_%s_args (xdr_in, &args)) {\n"
name;
-           if is_filein then
-             pr "    if (cancel_receive () != -2)\n";
-           pr "      reply_with_error (\"daemon failed to decode
procedure arguments\");\n";
-           pr "    goto done;\n";
-           pr "  }\n";
-           let pr_args n -             pr "  char *%s = args.%s;\n" n
n
-           in
-           let pr_list_handling_code n -             pr "  %s = realloc
(args.%s.%s_val,\n" n n n;
-             pr "                sizeof (char *) *
(args.%s.%s_len+1));\n" n n;
-             pr "  if (%s == NULL) {\n" n;
-             if is_filein then
-               pr "    if (cancel_receive () != -2)\n";
-             pr "      reply_with_perror (\"realloc\");\n";
-             pr "    goto done;\n";
-             pr "  }\n";
-             pr "  %s[args.%s.%s_len] = NULL;\n" n n n;
-             pr "  args.%s.%s_val = %s;\n" n n n;
-           in
-           List.iter (
-             function
-             | Pathname n ->
-                 pr_args n;
-                 pr "  ABS_PATH (%s, %s, goto done);\n"
-                   n (if is_filein then "cancel_receive ()" else
"0");
-             | Device n ->
-                 pr_args n;
-                 pr "  RESOLVE_DEVICE (%s, %s, goto done);\n"
-                   n (if is_filein then "cancel_receive ()" else
"0");
-             | 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
"0");
-             | String n | Key n -> pr_args n
-             | OptString n -> pr "  %s = args.%s ? *args.%s :
NULL;\n" n n n
-             | StringList n ->
-                 pr_list_handling_code n;
-             | DeviceList n ->
-                 pr_list_handling_code n;
-                 pr "  /* Ensure that each is a device,\n";
-                 pr "   * and perform device name translation.\n";
-                 pr "   */\n";
-                 pr "  {\n";
-                 pr "    size_t i;\n";
-                 pr "    for (i = 0; %s[i] != NULL; ++i)\n" n;
-                 pr "      RESOLVE_DEVICE (%s[i], %s, goto done);\n"
n
-                   (if is_filein then "cancel_receive ()" else
"0");
-                 pr "  }\n";
-             | Bool n -> pr "  %s = args.%s;\n" n n
-             | Int n -> pr "  %s = args.%s;\n" n n
-             | Int64 n -> pr "  %s = args.%s;\n" n n
-             | FileIn _ | FileOut _ -> ()
-             | BufferIn n ->
-                 pr "  %s = args.%s.%s_val;\n" n n n;
-                 pr "  %s_size = args.%s.%s_len;\n" n n n
-             | Pointer _ -> assert false
-           ) args;
-           pr "\n"
+      (* Reject unknown optional arguments. *)
+      if optargs <> [] then (
+        let len = List.length optargs in
+        let mask = Int64.lognot (Int64.pred (Int64.shift_left 1L len)) in
+        pr "  if (optargs_bitmask & UINT64_C(0x%Lx)) {\n" mask;
+        if is_filein then
+          pr "    if (cancel_receive () != -2)\n";
+        pr "    reply_with_error (\"unknown option in optional
arguments bitmask (this can happen if a program is compiled against a newer
version of libguestfs, then run against an older version of the
daemon)\");\n";
+        pr "    goto done;\n";
+        pr "  }\n";
+        pr "\n"
+      );
+
+      (* Decode arguments. *)
+      if args <> [] || optargs <> [] then (
+        pr "  memset (&args, 0, sizeof args);\n";
+        pr "\n";
+        pr "  if (!xdr_guestfs_%s_args (xdr_in, &args)) {\n"
name;
+        if is_filein then
+          pr "    if (cancel_receive () != -2)\n";
+        pr "      reply_with_error (\"daemon failed to decode
procedure arguments\");\n";
+        pr "    goto done;\n";
+        pr "  }\n";
+        let pr_args n +          pr "  char *%s = args.%s;\n" n n
+        in
+        let pr_list_handling_code n +          pr "  %s = realloc
(args.%s.%s_val,\n" n n n;
+          pr "                sizeof (char *) *
(args.%s.%s_len+1));\n" n n;
+          pr "  if (%s == NULL) {\n" n;
+          if is_filein then
+            pr "    if (cancel_receive () != -2)\n";
+          pr "      reply_with_perror (\"realloc\");\n";
+          pr "    goto done;\n";
+          pr "  }\n";
+          pr "  %s[args.%s.%s_len] = NULL;\n" n n n;
+          pr "  args.%s.%s_val = %s;\n" n n n;
+        in
+        List.iter (
+          function
+          | Pathname n ->
+              pr_args n;
+              pr "  ABS_PATH (%s, %s, goto done);\n"
+                n (if is_filein then "cancel_receive ()" else
"0");
+          | Device n ->
+              pr_args n;
+              pr "  RESOLVE_DEVICE (%s, %s, goto done);\n"
+                n (if is_filein then "cancel_receive ()" else
"0");
+          | 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
"0");
+          | String n | Key n -> pr_args n
+          | OptString n -> pr "  %s = args.%s ? *args.%s :
NULL;\n" n n n
+          | StringList n ->
+              pr_list_handling_code n;
+          | DeviceList n ->
+              pr_list_handling_code n;
+              pr "  /* Ensure that each is a device,\n";
+              pr "   * and perform device name translation.\n";
+              pr "   */\n";
+              pr "  {\n";
+              pr "    size_t i;\n";
+              pr "    for (i = 0; %s[i] != NULL; ++i)\n" n;
+              pr "      RESOLVE_DEVICE (%s[i], %s, goto done);\n" n
+                (if is_filein then "cancel_receive ()" else
"0");
+              pr "  }\n";
+          | Bool n -> pr "  %s = args.%s;\n" n n
+          | Int n -> pr "  %s = args.%s;\n" n n
+          | Int64 n -> pr "  %s = args.%s;\n" n n
+          | FileIn _ | FileOut _ -> ()
+          | BufferIn n ->
+              pr "  %s = args.%s.%s_val;\n" n n n;
+              pr "  %s_size = args.%s.%s_len;\n" n n n
+          | Pointer _ -> assert false
+        ) (args @ optargs);
+        pr "\n"
       );
 
       (* this is used at least for do_equal *)
@@ -191,11 +213,14 @@ and generate_daemon_actions ()        (* Don't want to
call the impl with any FileIn or FileOut
        * parameters, since these go "outside" the RPC protocol.
        *)
-      let args' -        List.filter (function FileIn _ | FileOut _ ->
false | _ -> true) args in
-      pr "  r = do_%s " name;
-      generate_c_call_args (ret, args', optargs);
-      pr ";\n";
+      let () +        let args' +          List.filter
+            (function FileIn _ | FileOut _ -> false | _ -> true) args in
+        let style = ret, args' @ optargs, [] in
+        pr "  r = do_%s " name;
+        generate_c_call_args style;
+        pr ";\n" in
 
       (match ret with
        | RErr | RInt _ | RInt64 _ | RBool _
diff --git a/generator/generator_xdr.ml b/generator/generator_xdr.ml
index ca114c5..5714c80 100644
--- a/generator/generator_xdr.ml
+++ b/generator/generator_xdr.ml
@@ -65,12 +65,14 @@ let generate_xdr ()  
   List.iter (
     fun (shortname, (ret, args, optargs), _, _, _, _, _) ->
-      if optargs <> [] then
-        failwithf "optional arguments not supported in XDR yet";
-
       let name = "guestfs_" ^ shortname in
 
-      (match args with
+      (* Ordinary arguments and optional arguments are concatenated
+       * together in the XDR args struct.  The optargs_bitmask field
+       * in the header controls which optional arguments are
+       * meaningful.
+       *)
+      (match args @ optargs with
        | [] -> ()
        | args ->
            pr "struct %s_args {\n" name;
diff --git a/src/guestfs-internal.h b/src/guestfs-internal.h
index e4e198b..bb68298 100644
--- a/src/guestfs-internal.h
+++ b/src/guestfs-internal.h
@@ -250,7 +250,7 @@ extern void guestfs___print_timestamped_message (guestfs_h
*g, const char *fs, .
 extern void guestfs___free_inspect_info (guestfs_h *g);
 extern int guestfs___set_busy (guestfs_h *g);
 extern int guestfs___end_busy (guestfs_h *g);
-extern int guestfs___send (guestfs_h *g, int proc_nr, uint64_t progress_hint,
xdrproc_t xdrp, char *args);
+extern int guestfs___send (guestfs_h *g, int proc_nr, uint64_t progress_hint,
uint64_t optargs_bitmask, xdrproc_t xdrp, char *args);
 extern int guestfs___recv (guestfs_h *g, const char *fn, struct
guestfs_message_header *hdr, struct guestfs_message_error *err, xdrproc_t xdrp,
char *ret);
 extern int guestfs___send_file (guestfs_h *g, const char *filename);
 extern int guestfs___recv_file (guestfs_h *g, const char *filename);
diff --git a/src/proto.c b/src/proto.c
index a5d9d2b..0d63af6 100644
--- a/src/proto.c
+++ b/src/proto.c
@@ -693,7 +693,8 @@ guestfs___accept_from_daemon (guestfs_h *g)
 }
 
 int
-guestfs___send (guestfs_h *g, int proc_nr, uint64_t progress_hint,
+guestfs___send (guestfs_h *g, int proc_nr,
+                uint64_t progress_hint, uint64_t optargs_bitmask,
                 xdrproc_t xdrp, char *args)
 {
   struct guestfs_message_header hdr;
@@ -726,7 +727,7 @@ guestfs___send (guestfs_h *g, int proc_nr, uint64_t
progress_hint,
   hdr.serial = serial;
   hdr.status = GUESTFS_STATUS_OK;
   hdr.progress_hint = progress_hint;
-  hdr.optargs_bitmask = 0;
+  hdr.optargs_bitmask = optargs_bitmask;
 
   if (!xdr_guestfs_message_header (&xdr, &hdr)) {
     error (g, _("xdr_guestfs_message_header failed"));
-- 
1.7.3.2

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into Xen guests.
http://et.redhat.com/~rjones/virt-p2v
-------------- next part --------------
>From 0710326ac5f1a06cf1dc3500617d04cf4cba5631 Mon Sep 17 00:00:00 2001
From: Richard W.M. Jones <rjones at redhat.com>
Date: Thu, 2 Dec 2010 13:35:14 +0000
Subject: [PATCH 2/2] New API: mkfs_opts, mkfs with optional arguments.

This is an extensible version of 'mkfs' which supports optional
arguments.  There is now no need for 'mkfs_b' since you should
use 'mkfs_opts' with the optional 'blocksize' argument instead.
---
 contrib/visualize-alignment/README  |    2 +-
 daemon/mkfs.c                       |   98 +++++++++++++++++-----------------
 generator/generator_actions.ml      |   29 ++++++++++-
 images/guest-aux/make-debian-img.sh |   10 ++--
 images/guest-aux/make-fedora-img.sh |   10 ++--
 images/guest-aux/make-ubuntu-img.sh |    4 +-
 src/MAX_PROC_NR                     |    2 +-
 7 files changed, 91 insertions(+), 64 deletions(-)

diff --git a/contrib/visualize-alignment/README
b/contrib/visualize-alignment/README
index 6c30f94..e68a902 100644
--- a/contrib/visualize-alignment/README
+++ b/contrib/visualize-alignment/README
@@ -54,7 +54,7 @@ qemu patch in the current directory.
       pvcreate /dev/vda : \
       vgcreate VG /dev/vda : \
       lvcreate LV VG 32 : \
-      mkfs-b ext4 4096 /dev/VG/LV
+      mkfs-opts ext4 /dev/VG/LV blocksize:4096
 
   Some points to note:
     * an ext4 filesystem, so it has a journal and extents
diff --git a/daemon/mkfs.c b/daemon/mkfs.c
index ad3014c..911fad3 100644
--- a/daemon/mkfs.c
+++ b/daemon/mkfs.c
@@ -31,12 +31,13 @@
 
 #define MAX_ARGS 16
 
-static int
-mkfs (const char *fstype, const char *device,
-      const char **extra, size_t nr_extra)
+/* Takes optional arguments, consult optargs_bitmask. */
+int
+do_mkfs_opts (const char *fstype, const char *device, int blocksize)
 {
   const char *argv[MAX_ARGS];
   size_t i = 0, j;
+  char blocksize_str[32];
   int r;
   char *err;
 
@@ -72,8 +73,47 @@ mkfs (const char *fstype, const char *device,
     argv[i++] = "-O";
   }
 
-  for (j = 0; j < nr_extra; ++j)
-    argv[i++] = extra[j];
+  /* Process blocksize parameter if set. */
+  if (optargs_bitmask & GUESTFS_MKFS_OPTS_BLOCKSIZE_BITMASK) {
+    if (blocksize <= 0 || !is_power_of_2 (blocksize)) {
+      reply_with_error ("block size must be > 0 and a power of
2");
+      return -1;
+    }
+
+    if (STREQ (fstype, "vfat") ||
+        STREQ (fstype, "msdos")) {
+      /* For VFAT map the blocksize into a cluster size.  However we
+       * have to determine the block device sector size in order to do
+       * this.
+       */
+      int sectorsize = do_blockdev_getss (device);
+      if (sectorsize == -1)
+        return -1;
+
+      int sectors_per_cluster = blocksize / sectorsize;
+      if (sectors_per_cluster < 1 || sectors_per_cluster > 128) {
+        reply_with_error ("unsupported cluster size for %s filesystem
(requested cluster size = %d, sector size = %d, trying sectors per cluster =
%d)",
+                          fstype, blocksize, sectorsize, sectors_per_cluster);
+        return -1;
+      }
+
+      snprintf (blocksize_str, sizeof blocksize_str, "%d",
sectors_per_cluster);
+      argv[i++] = "-s";
+      argv[i++] = blocksize_str;
+    }
+    else if (STREQ (fstype, "ntfs")) {
+      /* For NTFS map the blocksize into a cluster size. */
+      snprintf (blocksize_str, sizeof blocksize_str, "%d",
blocksize);
+      argv[i++] = "-c";
+      argv[i++] = blocksize_str;
+    }
+    else {
+      /* For all other filesystem types, try the -b option. */
+      snprintf (blocksize_str, sizeof blocksize_str, "%d",
blocksize);
+      argv[i++] = "-b";
+      argv[i++] = blocksize_str;
+    }
+  }
 
   argv[i++] = device;
   argv[i++] = NULL;
@@ -95,53 +135,13 @@ mkfs (const char *fstype, const char *device,
 int
 do_mkfs (const char *fstype, const char *device)
 {
-  return mkfs (fstype, device, NULL, 0);
+  optargs_bitmask = 0;
+  return do_mkfs_opts (fstype, device, 0);
 }
 
 int
 do_mkfs_b (const char *fstype, int blocksize, const char *device)
 {
-  const char *extra[2];
-  char n[32];
-
-  if (blocksize <= 0 || !is_power_of_2 (blocksize)) {
-    reply_with_error ("block size must be > 0 and a power of 2");
-    return -1;
-  }
-
-  if (STREQ (fstype, "vfat") ||
-      STREQ (fstype, "msdos")) {
-    /* For VFAT map the blocksize into a cluster size.  However we
-     * have to determine the block device sector size in order to do
-     * this.
-     */
-    int sectorsize = do_blockdev_getss (device);
-    if (sectorsize == -1)
-      return -1;
-
-    int sectors_per_cluster = blocksize / sectorsize;
-    if (sectors_per_cluster < 1 || sectors_per_cluster > 128) {
-      reply_with_error ("unsupported cluster size for %s filesystem
(requested cluster size = %d, sector size = %d, trying sectors per cluster =
%d)",
-                        fstype, blocksize, sectorsize, sectors_per_cluster);
-      return -1;
-    }
-
-    snprintf (n, sizeof n, "%d", sectors_per_cluster);
-    extra[0] = "-s";
-    extra[1] = n;
-  }
-  else if (STREQ (fstype, "ntfs")) {
-    /* For NTFS map the blocksize into a cluster size. */
-    snprintf (n, sizeof n, "%d", blocksize);
-    extra[0] = "-c";
-    extra[1] = n;
-  }
-  else {
-    /* For all other filesystem types, try the -b option. */
-    snprintf (n, sizeof n, "%d", blocksize);
-    extra[0] = "-b";
-    extra[1] = n;
-  }
-
-  return mkfs (fstype, device, extra, 2);
+  optargs_bitmask = GUESTFS_MKFS_OPTS_BLOCKSIZE_BITMASK;
+  return do_mkfs_opts (fstype, device, blocksize);
 }
diff --git a/generator/generator_actions.ml b/generator/generator_actions.ml
index a405fd4..5624dec 100644
--- a/generator/generator_actions.ml
+++ b/generator/generator_actions.ml
@@ -4186,7 +4186,7 @@ This gets the SELinux security context of the daemon.
 See the documentation about SELINUX in L<guestfs(3)>,
 and C<guestfs_setcon>");
 
-  ("mkfs_b", (RErr, [String "fstype"; Int
"blocksize"; Device "device"], []), 187, [],
+  ("mkfs_b", (RErr, [String "fstype"; Int
"blocksize"; Device "device"], []), 187, [DeprecatedBy
"mkfs_opts"],
    [InitEmpty, Always, TestOutput (
       [["part_disk"; "/dev/sda"; "mbr"];
        ["mkfs_b"; "ext2"; "4096";
"/dev/sda1"];
@@ -5606,6 +5606,33 @@ not refer to a logical volume.
 
 See also C<guestfs_is_lv>.");
 
+  ("mkfs_opts", (RErr, [String "fstype"; Device
"device"], [Int "blocksize"]), 278, [],
+   [InitEmpty, Always, TestOutput (
+      [["part_disk"; "/dev/sda"; "mbr"];
+       ["mkfs_opts"; "ext2"; "/dev/sda1";
"4096"];
+       ["mount_options"; ""; "/dev/sda1";
"/"];
+       ["write"; "/new"; "new file contents"];
+       ["cat"; "/new"]], "new file contents")],
+   "make a filesystem",
+   "\
+This function creates a filesystem on C<device>.  The filesystem
+type is C<fstype>, for example C<ext3>.
+
+The optional arguments are:
+
+=over 4
+
+=item C<blocksize>
+
+The filesystem block size.  Supported block sizes depend on the
+filesystem type, but typically they are C<1024>, C<2048> or
C<4096>
+for Linux ext2/3 filesystems.
+
+For VFAT and NTFS the C<blocksize> parameter is treated as
+the requested cluster size.
+
+=back");
+
 ]
 
 let all_functions = non_daemon_functions @ daemon_functions
diff --git a/images/guest-aux/make-debian-img.sh
b/images/guest-aux/make-debian-img.sh
index bb34672..2170cec 100755
--- a/images/guest-aux/make-debian-img.sh
+++ b/images/guest-aux/make-debian-img.sh
@@ -48,18 +48,18 @@ lvcreate var debian 32
 lvcreate home debian 32
 
 # Phony /boot filesystem.
-mkfs-b ext2 4096 /dev/sda1
+mkfs-opts ext2 /dev/sda1 blocksize:4096
 set-e2label /dev/sda1 BOOT
 set-e2uuid /dev/sda1 01234567-0123-0123-0123-012345678901
 
 # Phony root and other filesystems.
-mkfs-b ext2 4096 /dev/debian/root
+mkfs-opts ext2 /dev/debian/root blocksize:4096
 set-e2uuid /dev/debian/root 01234567-0123-0123-0123-012345678902
-mkfs-b ext2 4096 /dev/debian/usr
+mkfs-opts ext2 /dev/debian/usr blocksize:4096
 set-e2uuid /dev/debian/usr 01234567-0123-0123-0123-012345678903
-mkfs-b ext2 4096 /dev/debian/var
+mkfs-opts ext2 /dev/debian/var blocksize:4096
 set-e2uuid /dev/debian/var 01234567-0123-0123-0123-012345678904
-mkfs-b ext2 4096 /dev/debian/home
+mkfs-opts ext2 /dev/debian/home blocksize:4096
 set-e2uuid /dev/debian/home 01234567-0123-0123-0123-012345678905
 
 # Enough to fool inspection API.
diff --git a/images/guest-aux/make-fedora-img.sh
b/images/guest-aux/make-fedora-img.sh
index 3198930..8c38e51 100755
--- a/images/guest-aux/make-fedora-img.sh
+++ b/images/guest-aux/make-fedora-img.sh
@@ -48,12 +48,12 @@ lvcreate LV2 VG 32
 lvcreate LV3 VG 64
 
 # Phony /boot filesystem.
-mkfs-b ext2 4096 /dev/sda1
+mkfs-opts ext2 /dev/sda1 blocksize:4096
 set-e2label /dev/sda1 BOOT
 set-e2uuid /dev/sda1 01234567-0123-0123-0123-012345678901
 
 # Phony root filesystem.
-mkfs-b ext2 4096 /dev/VG/Root
+mkfs-opts ext2 /dev/VG/Root blocksize:4096
 set-e2label /dev/VG/Root ROOT
 set-e2uuid /dev/VG/Root 01234567-0123-0123-0123-012345678902
 
@@ -92,9 +92,9 @@ mknod 0777 10 10 /bin/test7
 
 # Other filesystems.
 # Note that these should be empty, for testing virt-df.
-mkfs-b ext2 4096 /dev/VG/LV1
-mkfs-b ext2 1024 /dev/VG/LV2
-mkfs-b ext2 2048 /dev/VG/LV3
+mkfs-opts ext2 /dev/VG/LV1 blocksize:4096
+mkfs-opts ext2 /dev/VG/LV2 blocksize:1024
+mkfs-opts ext2 /dev/VG/LV3 blocksize:2048
 EOF
 
 rm fstab.tmp
diff --git a/images/guest-aux/make-ubuntu-img.sh
b/images/guest-aux/make-ubuntu-img.sh
index 8d62e5d..f008c76 100755
--- a/images/guest-aux/make-ubuntu-img.sh
+++ b/images/guest-aux/make-ubuntu-img.sh
@@ -46,12 +46,12 @@ part-add /dev/sda p 64     524287
 part-add /dev/sda p 524288    -64
 
 # Phony /boot filesystem.
-mkfs-b ext2 4096 /dev/sda1
+mkfs-opts ext2 /dev/sda1 blocksize:4096
 set-e2label /dev/sda1 BOOT
 set-e2uuid /dev/sda1 01234567-0123-0123-0123-012345678901
 
 # Phony root filesystem (Ubuntu doesn't use LVM by default).
-mkfs-b ext2 4096 /dev/sda2
+mkfs-opts ext2 /dev/sda2 blocksize:4096
 set-e2uuid /dev/sda2 01234567-0123-0123-0123-012345678902
 
 # Enough to fool inspection API.
diff --git a/src/MAX_PROC_NR b/src/MAX_PROC_NR
index 2681747..3d242f5 100644
--- a/src/MAX_PROC_NR
+++ b/src/MAX_PROC_NR
@@ -1 +1 @@
-277
+278
-- 
1.7.3.2