thr3ads.net - Libguestfs - [Libguestfs] [PATCH v4 0/6] Implement guestfs_add_libvirt

If this information is useful, please help other people find it:
Share via:

Richard W.M. Jones

2014-Dec-11 14:19 UTC

[Libguestfs] [PATCH v4 0/6] Implement guestfs_add_libvirt_dom.

Since v3:

 - Fix labelling over overlays (see 6/6)

 - Tested it with a test program which simulates what virt-manager
   will do.  See the attachment here:
   https://bugzilla.redhat.com/show_bug.cgi?id=1075164#c7

Rich.

Richard W.M. Jones

2014-Dec-11 14:19 UTC

head link

[Libguestfs] [PATCH v4 1/6] generator: Implement Pointer arguments.

This implements Pointer arguments properly, at least for certain
limited definitions of "implements" and "properly".

'Pointer' as an argument type is meant to indicate a pointer passed to
an API.  The canonical example is the following proposed API:

  int guestfs_add_libvirt_dom (guestfs_h *g, virDomainPtr dom, ...);

where 'dom' is described in the generator as:

  Pointer ("virDomainPtr", "dom")

Pointer existed already in the generator, but the implementation was
broken.  It is not used by any existing API.

There are two basic difficulties of implementing Pointer:

(1) In language bindings there is no portable way to turn (eg.) a Perl
Sys::Virt 'dom' object into a C virDomainPtr.

(2) We can't rely on <libvirt/libvirt.h> being included (since
it's an
optional dependency).

In this commit, we solve (2) by using a 'void *'.

We don't solve (1), really.  Instead we have a macro
POINTER_NOT_IMPLEMENTED which is used by currently all the non-C
language bindings.  It complains loudly and passes a NULL to the
underlying function.  The underlying function detects the NULL and
safely returns an error.  It is to be hoped that people will
contribute patches to make each language binding work, although in
some bindings it will always remain impossible to implement.
---
 generator/actions.ml            | 11 +++++++++--
 generator/c.ml                  |  2 +-
 generator/erlang.ml             |  2 +-
 generator/gobject.ml            | 10 +++++-----
 generator/golang.ml             |  4 ++--
 generator/java.ml               |  4 ++--
 generator/lua.ml                |  5 +++--
 generator/ocaml.ml              |  2 +-
 generator/perl.ml               |  2 +-
 generator/php.ml                | 15 +++++++++++----
 generator/python.ml             |  4 ++--
 generator/ruby.ml               |  3 ++-
 src/guestfs-internal-frontend.h | 11 +++++++++++
 13 files changed, 51 insertions(+), 24 deletions(-)

diff --git a/generator/actions.ml b/generator/actions.ml
index ba97eb3..93e3d0d 100644
--- a/generator/actions.ml
+++ b/generator/actions.ml
@@ -12400,9 +12400,16 @@ let is_documented { visibility = v } = match v with
   | VPublic | VStateTest -> true
   | VBindTest | VDebug | VInternal -> false
 
-let is_fish { visibility = v } = match v with
-  | VPublic | VDebug -> true
+let is_fish { visibility = v; style = (_, args, _) } +  (* Internal functions
are not exported to guestfish. *)
+  match v with
   | VStateTest | VBindTest | VInternal -> false
+  | VPublic | VDebug ->
+    (* Functions that take Pointer parameters cannot be used in
+     * guestfish, since there is no way the user could safely
+     * generate a pointer.
+     *)
+    not (List.exists (function Pointer _ -> true | _ -> false) args)
 
 let external_functions    List.filter is_external all_functions
diff --git a/generator/c.ml b/generator/c.ml
index 9e17d32..541c843 100644
--- a/generator/c.ml
+++ b/generator/c.ml
@@ -148,7 +148,7 @@ let rec generate_prototype ?(extern = true) ?(static =
false)
         pr "size_t %s_size" n
     | Pointer (t, n) ->
         next ();
-        pr "%s %s" t n
+        pr "void * /* really %s */ %s" t n
   ) args;
   if is_RBufferOut then (next (); pr "size_t *size_r");
   if optargs <> [] then (
diff --git a/generator/erlang.ml b/generator/erlang.ml
index f0c9c6a..35ddbe6 100644
--- a/generator/erlang.ml
+++ b/generator/erlang.ml
@@ -319,7 +319,7 @@ extern int64_t get_int64 (ETERM *term);
           | Int64 n ->
             pr "  int64_t %s = get_int64 (ARG (%d));\n" n i
           | Pointer (t, n) ->
-            assert false
+            pr "  void * /* %s */ %s = POINTER_NOT_IMPLEMENTED
(\"%s\");\n" t n t
       ) args;
 
       (* Optional arguments. *)
diff --git a/generator/gobject.ml b/generator/gobject.ml
index 499837a..3217b9c 100644
--- a/generator/gobject.ml
+++ b/generator/gobject.ml
@@ -89,8 +89,8 @@ let generate_gobject_proto name ?(single_line = true)
         pr "gchar *const *%s" n
       | BufferIn n ->
         pr "const guint8 *%s, gsize %s_size" n n
-      | Pointer _ ->
-        failwith "gobject bindings do not support Pointer arguments"
+      | Pointer (t, n) ->
+        pr "void * /* %s */ %s" t n
   ) args;
   if optargs <> [] then (
     pr ", %s *optargs" (camel_of_name f)
@@ -1056,7 +1056,7 @@ guestfs_session_close (GuestfsSession *session, GError
**err)
             pr " (transfer none) (array length=%s_size) (element-type
guint8): an array of binary data\n" n;
             pr " * @%s_size: The size of %s, in bytes" n n;
           | Pointer _ ->
-            failwith "gobject bindings do not support Pointer
arguments"
+            pr "pointer (not implemented in gobject bindings)"
           );
           pr "\n";
       ) args;
@@ -1202,8 +1202,8 @@ guestfs_session_close (GuestfsSession *session, GError
**err)
           | DeviceList n | Key n | FileIn n | FileOut n
           | GUID n ->
             pr "%s" n
-          | Pointer _ ->
-            failwith "gobject bindings do not support Pointer
arguments"
+          | Pointer (_, n) ->
+            pr "%s" n
       ) args;
       if is_RBufferOut then pr ", size_r";
       if optargs <> [] then pr ", argvp";
diff --git a/generator/golang.ml b/generator/golang.ml
index e67ca1b..b4b2482 100644
--- a/generator/golang.ml
+++ b/generator/golang.ml
@@ -317,7 +317,7 @@ func return_hashtable (argv **C.char) map[string]string {
           | StringList n
           | DeviceList n -> pr "%s []string" n
           | BufferIn n -> pr "%s []byte" n
-          | Pointer _ -> assert false
+          | Pointer (_, n) -> pr "%s int64" n
       ) args;
       if optargs <> [] then (
         if !comma then pr ", ";
@@ -457,7 +457,7 @@ func return_hashtable (argv **C.char) map[string]string {
           | StringList n
           | DeviceList n -> pr "c_%s" n
           | BufferIn n -> pr "c_%s, C.size_t (len (%s))" n n
-          | Pointer _ -> assert false
+          | Pointer _ -> pr "nil"
       ) args;
       (match ret with
       | RBufferOut _ -> pr ", &size"
diff --git a/generator/java.ml b/generator/java.ml
index e9c5949..f763611 100644
--- a/generator/java.ml
+++ b/generator/java.ml
@@ -898,7 +898,7 @@ get_all_event_callbacks (guestfs_h *g, size_t *len_rtn)
         | Int64 n ->
             pr "  int64_t %s;\n" n
         | Pointer (t, n) ->
-            pr "  %s %s;\n" t n
+            pr "  void * /* %s */ %s;\n" t n
       ) args;
 
       if optargs <> [] then (
@@ -965,7 +965,7 @@ get_all_event_callbacks (guestfs_h *g, size_t *len_rtn)
         | Int64 n ->
             pr "  %s = j%s;\n" n n
         | Pointer (t, n) ->
-            pr "  %s = (%s) j%s;\n" n t n
+            pr "  %s = POINTER_NOT_IMPLEMENTED (\"%s\");\n"
n t
       ) args;
 
       if optargs <> [] then (
diff --git a/generator/lua.ml b/generator/lua.ml
index 5d5619c..c1fa6f0 100644
--- a/generator/lua.ml
+++ b/generator/lua.ml
@@ -473,7 +473,7 @@ guestfs_lua_delete_event_callback (lua_State *L)
         | Bool n -> pr "  int %s;\n" n
         | Int n -> pr "  int %s;\n" n
         | Int64 n -> pr "  int64_t %s;\n" n
-        | Pointer (t, n) -> pr "  %s %s;\n" t n
+        | Pointer (t, n) -> pr "  void * /* %s */ %s;\n" t n
       ) args;
       if optargs <> [] then (
         pr "  struct %s optargs_s = { .bitmask = 0 };\n" c_function;
@@ -506,7 +506,8 @@ guestfs_lua_delete_event_callback (lua_State *L)
             pr "  %s = luaL_checkint (L, %d);\n" n i
           | Int64 n ->
             pr "  %s = get_int64 (L, %d);\n" n i
-          | Pointer (t, n) -> assert false
+          | Pointer (t, n) ->
+            pr "  %s = POINTER_NOT_IMPLEMENTED (\"%s\");\n"
n t
       ) args;
 
       if optargs <> [] then (
diff --git a/generator/ocaml.ml b/generator/ocaml.ml
index bd27f7a..70237b9 100644
--- a/generator/ocaml.ml
+++ b/generator/ocaml.ml
@@ -539,7 +539,7 @@ copy_table (char * const * argv)
         | Int64 n ->
             pr "  int64_t %s = Int64_val (%sv);\n" n n
         | Pointer (t, n) ->
-            pr "  %s %s = (%s) (intptr_t) Int64_val (%sv);\n" t n t n
+            pr "  void * /* %s */ %s = POINTER_NOT_IMPLEMENTED
(\"%s\");\n" t n t
       ) args;
 
       (* Optional arguments. *)
diff --git a/generator/perl.ml b/generator/perl.ml
index 3da45fd..dcf746d 100644
--- a/generator/perl.ml
+++ b/generator/perl.ml
@@ -369,7 +369,7 @@ PREINIT:
           | Bool n -> pr "      int %s;\n" n
           | Int n -> pr "      int %s;\n" n
           | Int64 n -> pr "      int64_t %s;\n" n
-          | Pointer (t, n) -> pr "      %s %s;\n" t n
+          | Pointer (t, n) -> pr "      void * /* %s */ %s;\n" t n
       ) args;
 
       (* PREINIT section (local variable declarations). *)
diff --git a/generator/php.ml b/generator/php.ml
index c7e0a27..2f30485 100644
--- a/generator/php.ml
+++ b/generator/php.ml
@@ -87,6 +87,7 @@ and generate_php_c ()  #include <php_guestfs_php.h>
 
 #include \"guestfs.h\"
+#include \"guestfs-internal-frontend.h\"
 
 static int res_guestfs_h;
 
@@ -202,8 +203,10 @@ PHP_FUNCTION (guestfs_last_error)
             pr "  char **%s;\n" n;
         | Bool n ->
             pr "  zend_bool %s;\n" n
-        | Int n | Int64 n | Pointer (_, n) ->
+        | Int n | Int64 n ->
             pr "  long %s;\n" n
+        | Pointer (t, n) ->
+            pr "  void * /* %s */ %s;\n" t n
         ) args;
 
       if optargs <> [] then (
@@ -241,7 +244,8 @@ PHP_FUNCTION (guestfs_last_error)
           | OptString n -> "s!"
           | StringList n | DeviceList n -> "a"
           | Bool n -> "b"
-          | Int n | Int64 n | Pointer (_, n) -> "l"
+          | Int n | Int64 n -> "l"
+          | Pointer _ -> ""
         ) args
       ) in
 
@@ -277,8 +281,9 @@ PHP_FUNCTION (guestfs_last_error)
             pr ", &z_%s" n
         | Bool n ->
             pr ", &%s" n
-        | Int n | Int64 n | Pointer (_, n) ->
+        | Int n | Int64 n ->
             pr ", &%s" n
+        | Pointer (_, n) -> ()
       ) args;
       List.iter (
         function
@@ -342,7 +347,9 @@ PHP_FUNCTION (guestfs_last_error)
             pr "    %s[c] = NULL;\n" n;
             pr "  }\n";
             pr "\n"
-        | Bool _ | Int _ | Int64 _ | Pointer _ -> ()
+        | Bool _ | Int _ | Int64 _ -> ()
+        | Pointer (t, n) ->
+            pr "  %s = POINTER_NOT_IMPLEMENTED (\"%s\");\n"
n t
         ) args;
 
       (* Optional arguments. *)
diff --git a/generator/python.ml b/generator/python.ml
index 07e87d2..82afb2e 100644
--- a/generator/python.ml
+++ b/generator/python.ml
@@ -295,7 +295,7 @@ put_table (char * const * const argv)
         | Int64 n -> pr "  long long %s;\n" n
         | Pointer (t, n) ->
             pr "  long long %s_int64;\n" n;
-            pr "  %s %s;\n" t n
+            pr "  void * /* %s */ %s;\n" t n
       ) args;
 
       (* Fetch the optional arguments as objects, so we can detect
@@ -370,7 +370,7 @@ put_table (char * const * const argv)
             pr "  %s = get_string_list (py_%s);\n" n n;
             pr "  if (!%s) goto out;\n" n
         | Pointer (t, n) ->
-            pr "  %s = (%s) (intptr_t) %s_int64;\n" n t n
+            pr "  %s = POINTER_NOT_IMPLEMENTED (\"%s\");\n"
n t
       ) args;
 
       pr "\n";
diff --git a/generator/ruby.ml b/generator/ruby.ml
index 88762ca..a031681 100644
--- a/generator/ruby.ml
+++ b/generator/ruby.ml
@@ -612,7 +612,8 @@ get_all_event_callbacks (guestfs_h *g, size_t *len_rtn)
         | Int64 n ->
           pr "  long long %s = NUM2LL (%sv);\n" n n
         | Pointer (t, n) ->
-          pr "  %s %s = (%s) (intptr_t) NUM2LL (%sv);\n" t n t n
+          pr "  (void) %sv;\n" n;
+          pr "  void * /* %s */ %s = POINTER_NOT_IMPLEMENTED
(\"%s\");\n" t n t
       ) args;
       pr "\n";
 
diff --git a/src/guestfs-internal-frontend.h b/src/guestfs-internal-frontend.h
index 5c5d957..acb7014 100644
--- a/src/guestfs-internal-frontend.h
+++ b/src/guestfs-internal-frontend.h
@@ -183,4 +183,15 @@ extern GUESTFS_DLL_PUBLIC int guestfs___add_libvirt_dom
(guestfs_h *g, virDomain
     }                                                                   \
   } while (0)
 
+/* Not all language bindings know how to deal with Pointer arguments.
+ * Those that don't will use this macro which complains noisily and
+ * returns NULL.
+ */
+#define POINTER_NOT_IMPLEMENTED(type)                                   \
+  (                                                                     \
+   fprintf (stderr, "*** WARNING: this language binding does not support
conversion of Pointer(%s), so the current function will always fail.  Patches to
fix this should be sent to the libguestfs upstream mailing list.\n", \
+            type),                                                      \
+   NULL                                                                 \
+)
+
 #endif /* GUESTFS_INTERNAL_FRONTEND_H_ */
-- 
2.1.0

Richard W.M. Jones

2014-Dec-11 14:19 UTC

head link

[Libguestfs] [PATCH v4 2/6] New(ish) API: guestfs_add_libvirt_dom.

This API already existed (as guestfs___add_libvirt_dom), and was used
by a few tools.

This commit changes it to a public API.

Note that for reasons outlined in the previous commit message, it is
impossible to call this from guestfish or from non-C language
bindings.
---
 align/scan.c                       |  8 ++++----
 df/df.c                            | 10 ++++++----
 df/domains.h                       |  2 ++
 generator/actions.ml               |  7 ++-----
 generator/c.ml                     |  3 ---
 gobject/Makefile.inc               |  2 ++
 perl/typemap                       |  4 ++++
 po/POTFILES                        |  1 +
 src/guestfs-internal-frontend.h    | 33 ---------------------------------
 src/libvirt-domain.c               | 37 +++++++++++++++++++------------------
 tests/c-api/Makefile.am            | 22 +++++++++++-----------
 tests/c-api/test-add-libvirt-dom.c |  2 +-
 12 files changed, 52 insertions(+), 79 deletions(-)

diff --git a/align/scan.c b/align/scan.c
index 7da468f..651d805 100644
--- a/align/scan.c
+++ b/align/scan.c
@@ -353,15 +353,15 @@ scan (guestfs_h *g, const char *prefix, FILE *fp)
 static int
 scan_work (guestfs_h *g, size_t i, FILE *fp)
 {
-  struct guestfs___add_libvirt_dom_argv optargs;
+  struct guestfs_add_libvirt_dom_argv optargs;
 
   optargs.bitmask -    GUESTFS___ADD_LIBVIRT_DOM_READONLY_BITMASK |
-    GUESTFS___ADD_LIBVIRT_DOM_READONLYDISK_BITMASK;
+    GUESTFS_ADD_LIBVIRT_DOM_READONLY_BITMASK |
+    GUESTFS_ADD_LIBVIRT_DOM_READONLYDISK_BITMASK;
   optargs.readonly = 1;
   optargs.readonlydisk = "read";
 
-  if (guestfs___add_libvirt_dom (g, domains[i].dom, &optargs) == -1)
+  if (guestfs_add_libvirt_dom_argv (g, domains[i].dom, &optargs) == -1)
     return -1;
 
   if (guestfs_launch (g) == -1)
diff --git a/df/df.c b/df/df.c
index 0dcf1c9..a13cc59 100644
--- a/df/df.c
+++ b/df/df.c
@@ -85,6 +85,8 @@ df_on_handle (guestfs_h *g, const char *name, const char
*uuid, FILE *fp)
 
 #if defined(HAVE_LIBVIRT)
 
+#include <libvirt/libvirt.h>
+
 /* The multi-threaded version.  This callback is called from the code
  * in "parallel.c".
  */
@@ -92,16 +94,16 @@ df_on_handle (guestfs_h *g, const char *name, const char
*uuid, FILE *fp)
 int
 df_work (guestfs_h *g, size_t i, FILE *fp)
 {
-  struct guestfs___add_libvirt_dom_argv optargs;
+  struct guestfs_add_libvirt_dom_argv optargs;
 
   optargs.bitmask -    GUESTFS___ADD_LIBVIRT_DOM_READONLY_BITMASK |
-    GUESTFS___ADD_LIBVIRT_DOM_READONLYDISK_BITMASK;
+    GUESTFS_ADD_LIBVIRT_DOM_READONLY_BITMASK |
+    GUESTFS_ADD_LIBVIRT_DOM_READONLYDISK_BITMASK;
   optargs.readonly = 1;
   optargs.readonlydisk = "read";
 
   /* Traditionally we have ignored errors from adding disks in virt-df. */
-  if (guestfs___add_libvirt_dom (g, domains[i].dom, &optargs) == -1)
+  if (guestfs_add_libvirt_dom_argv (g, domains[i].dom, &optargs) == -1)
     return 0;
 
   if (guestfs_launch (g) == -1)
diff --git a/df/domains.h b/df/domains.h
index 50e8762..ecf791b 100644
--- a/df/domains.h
+++ b/df/domains.h
@@ -21,6 +21,8 @@
 
 #if defined(HAVE_LIBVIRT)
 
+#include <libvirt/libvirt.h>
+
 /* The list of domains that we build up in get_all_libvirt_guests. */
 struct domain {
   virDomainPtr dom;
diff --git a/generator/actions.ml b/generator/actions.ml
index 93e3d0d..4bd0788 100644
--- a/generator/actions.ml
+++ b/generator/actions.ml
@@ -1705,12 +1705,10 @@ Disks with the E<lt>readonly/E<gt> flag are
skipped.
 The other optional parameters are passed directly through to
 C<guestfs_add_drive_opts>." };
 
-(*
-This interface is not quite baked yet. -- RWMJ 2010-11-11
   { defaults with
     name = "add_libvirt_dom";
-    style = RInt "nrdisks", [Pointer ("virDomainPtr",
"dom")], [Bool "readonly"; String "iface"; Bool
"live"; String "readonlydisk"; OString
"cachemode"; OString "discard"; OBool
"copyonread"];
-    in_fish = false;
+    style = RInt "nrdisks", [Pointer ("virDomainPtr",
"dom")], [OBool "readonly"; OString "iface"; OBool
"live"; OString "readonlydisk"; OString
"cachemode"; OString "discard"; OBool
"copyonread"];
+    config_only = true;
     shortdesc = "add the disk(s) from a libvirt domain";
     longdesc = "\
 This function adds the disk(s) attached to the libvirt domain C<dom>.
@@ -1745,7 +1743,6 @@ See C<guestfs_add_domain> for possible values.
 
 The other optional parameters are passed directly through to
 C<guestfs_add_drive_opts>." };
-*)
 
   { defaults with
     name = "inspect_get_package_format";
diff --git a/generator/c.ml b/generator/c.ml
index 541c843..10dea84 100644
--- a/generator/c.ml
+++ b/generator/c.ml
@@ -2182,9 +2182,6 @@ and generate_linker_script ()     
"guestfs___safe_malloc";
     "guestfs___safe_strdup";
     "guestfs___safe_memdup";
-
-    (* Used only by virt-df and virt-alignment-scan. *)
-    "guestfs___add_libvirt_dom";
   ] in
   let functions      List.flatten (
diff --git a/gobject/Makefile.inc b/gobject/Makefile.inc
index 56a2fc3..218eef7 100644
--- a/gobject/Makefile.inc
+++ b/gobject/Makefile.inc
@@ -47,6 +47,7 @@ guestfs_gobject_headers= \
   include/guestfs-gobject/optargs-add_domain.h \
   include/guestfs-gobject/optargs-add_drive.h \
   include/guestfs-gobject/optargs-add_drive_scratch.h \
+  include/guestfs-gobject/optargs-add_libvirt_dom.h \
   include/guestfs-gobject/optargs-btrfs_filesystem_resize.h \
   include/guestfs-gobject/optargs-btrfs_fsck.h \
   include/guestfs-gobject/optargs-btrfs_subvolume_create.h \
@@ -127,6 +128,7 @@ guestfs_gobject_sources= \
   src/optargs-add_domain.c \
   src/optargs-add_drive.c \
   src/optargs-add_drive_scratch.c \
+  src/optargs-add_libvirt_dom.c \
   src/optargs-btrfs_filesystem_resize.c \
   src/optargs-btrfs_fsck.c \
   src/optargs-btrfs_subvolume_create.c \
diff --git a/perl/typemap b/perl/typemap
index 43e7948..e6f95d2 100644
--- a/perl/typemap
+++ b/perl/typemap
@@ -3,6 +3,7 @@ char *		T_PV
 const char *	T_PV
 guestfs_h *	O_OBJECT_guestfs_h
 int64_t	  	O_OBJECT_int64
+void * /* virDomainPtr */ O_OBJECT_virDomainPtr
 
 INPUT
 O_OBJECT_guestfs_h
@@ -21,6 +22,9 @@ O_OBJECT_guestfs_h
 O_OBJECT_int64
     $var = my_SvIV64 ($arg);
 
+O_OBJECT_virDomainPtr
+    $var = POINTER_NOT_IMPLEMENTED (\"virDomainPtr\");
+
 OUTPUT
 O_OBJECT_guestfs_h
     sv_setiv ($arg, PTR2IV ($var));
diff --git a/po/POTFILES b/po/POTFILES
index 36f61b2..a8e211f 100644
--- a/po/POTFILES
+++ b/po/POTFILES
@@ -173,6 +173,7 @@ fuse/test-guestunmount-fd.c
 gobject/src/optargs-add_domain.c
 gobject/src/optargs-add_drive.c
 gobject/src/optargs-add_drive_scratch.c
+gobject/src/optargs-add_libvirt_dom.c
 gobject/src/optargs-btrfs_filesystem_resize.c
 gobject/src/optargs-btrfs_fsck.c
 gobject/src/optargs-btrfs_subvolume_create.c
diff --git a/src/guestfs-internal-frontend.h b/src/guestfs-internal-frontend.h
index acb7014..cddf719 100644
--- a/src/guestfs-internal-frontend.h
+++ b/src/guestfs-internal-frontend.h
@@ -127,39 +127,6 @@ extern void guestfs___cleanup_pclose (void *ptr);
  */
 #include "guestfs-internal-frontend-cleanups.h"
 
-#if defined(HAVE_LIBVIRT)
-
-#include <libvirt/libvirt.h>
-
-/* This was proposed as an external API, but there's a problem: the
- * generator is unable to bind a virDomainPtr in any language other
- * than C.  For now this API is only used by virt-df and
- * virt-alignment-scan (both C tools) and it's only exported
- * internally within the libguestfs code, not to external users.
- */
-
-struct guestfs___add_libvirt_dom_argv {
-  uint64_t bitmask;
-#define GUESTFS___ADD_LIBVIRT_DOM_READONLY_BITMASK (UINT64_C(1)<<0)
-  int readonly;
-#define GUESTFS___ADD_LIBVIRT_DOM_IFACE_BITMASK (UINT64_C(1)<<1)
-  const char *iface;
-#define GUESTFS___ADD_LIBVIRT_DOM_LIVE_BITMASK (UINT64_C(1)<<2)
-  int live;
-#define GUESTFS___ADD_LIBVIRT_DOM_READONLYDISK_BITMASK (UINT64_C(1)<<3)
-  const char *readonlydisk;
-#define GUESTFS___ADD_LIBVIRT_DOM_CACHEMODE_BITMASK (UINT64_C(1)<<4)
-  const char *cachemode;
-#define GUESTFS___ADD_LIBVIRT_DOM_DISCARD_BITMASK (UINT64_C(1)<<5)
-  const char *discard;
-#define GUESTFS___ADD_LIBVIRT_DOM_COPYONREAD_BITMASK (UINT64_C(1)<<6)
-  int copyonread;
-};
-
-extern GUESTFS_DLL_PUBLIC int guestfs___add_libvirt_dom (guestfs_h *g,
virDomainPtr dom, const struct guestfs___add_libvirt_dom_argv *optargs);
-
-#endif /* HAVE_LIBVIRT */
-
 /* Current program name.  Note <errno.h> must be included in all files
  * that want to use 'program_name'.
  */
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 59bb4bc..78503e5 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -67,7 +67,7 @@ guestfs__add_domain (guestfs_h *g, const char *domain_name,
   const char *cachemode;
   const char *discard;
   bool copyonread;
-  struct guestfs___add_libvirt_dom_argv optargs2 = { .bitmask = 0 };
+  struct guestfs_add_libvirt_dom_argv optargs2 = { .bitmask = 0 };
 
   libvirturi = optargs->bitmask & GUESTFS_ADD_DOMAIN_LIBVIRTURI_BITMASK
                ? optargs->libvirturi : NULL;
@@ -124,35 +124,35 @@ guestfs__add_domain (guestfs_h *g, const char
*domain_name,
   }
 
   if (readonly) {
-    optargs2.bitmask |= GUESTFS___ADD_LIBVIRT_DOM_READONLY_BITMASK;
+    optargs2.bitmask |= GUESTFS_ADD_LIBVIRT_DOM_READONLY_BITMASK;
     optargs2.readonly = readonly;
   }
   if (iface) {
-    optargs2.bitmask |= GUESTFS___ADD_LIBVIRT_DOM_IFACE_BITMASK;
+    optargs2.bitmask |= GUESTFS_ADD_LIBVIRT_DOM_IFACE_BITMASK;
     optargs2.iface = iface;
   }
   if (live) {
-    optargs2.bitmask |= GUESTFS___ADD_LIBVIRT_DOM_LIVE_BITMASK;
+    optargs2.bitmask |= GUESTFS_ADD_LIBVIRT_DOM_LIVE_BITMASK;
     optargs2.live = live;
   }
   if (readonlydisk) {
-    optargs2.bitmask |= GUESTFS___ADD_LIBVIRT_DOM_READONLYDISK_BITMASK;
+    optargs2.bitmask |= GUESTFS_ADD_LIBVIRT_DOM_READONLYDISK_BITMASK;
     optargs2.readonlydisk = readonlydisk;
   }
   if (cachemode) {
-    optargs2.bitmask |= GUESTFS___ADD_LIBVIRT_DOM_CACHEMODE_BITMASK;
+    optargs2.bitmask |= GUESTFS_ADD_LIBVIRT_DOM_CACHEMODE_BITMASK;
     optargs2.cachemode = cachemode;
   }
   if (discard) {
-    optargs2.bitmask |= GUESTFS___ADD_LIBVIRT_DOM_DISCARD_BITMASK;
+    optargs2.bitmask |= GUESTFS_ADD_LIBVIRT_DOM_DISCARD_BITMASK;
     optargs2.discard = discard;
   }
   if (copyonread) {
-    optargs2.bitmask |= GUESTFS___ADD_LIBVIRT_DOM_COPYONREAD_BITMASK;
+    optargs2.bitmask |= GUESTFS_ADD_LIBVIRT_DOM_COPYONREAD_BITMASK;
     optargs2.copyonread = copyonread;
   }
 
-  r = guestfs___add_libvirt_dom (g, dom, &optargs2);
+  r = guestfs_add_libvirt_dom_argv (g, dom, &optargs2);
 
  cleanup:
   if (dom) virDomainFree (dom);
@@ -179,9 +179,10 @@ struct add_disk_data {
 };
 
 GUESTFS_DLL_PUBLIC int
-guestfs___add_libvirt_dom (guestfs_h *g, virDomainPtr dom,
-                           const struct guestfs___add_libvirt_dom_argv
*optargs)
+guestfs__add_libvirt_dom (guestfs_h *g, void *domvp,
+                          const struct guestfs_add_libvirt_dom_argv *optargs)
 {
+  virDomainPtr dom = domvp;
   ssize_t r;
   int readonly;
   const char *iface;
@@ -197,16 +198,16 @@ guestfs___add_libvirt_dom (guestfs_h *g, virDomainPtr dom,
   CLEANUP_FREE char *label = NULL, *imagelabel = NULL;
 
   readonly -    optargs->bitmask &
GUESTFS___ADD_LIBVIRT_DOM_READONLY_BITMASK
+    optargs->bitmask & GUESTFS_ADD_LIBVIRT_DOM_READONLY_BITMASK
     ? optargs->readonly : 0;
   iface -    optargs->bitmask & GUESTFS___ADD_LIBVIRT_DOM_IFACE_BITMASK
+    optargs->bitmask & GUESTFS_ADD_LIBVIRT_DOM_IFACE_BITMASK
     ? optargs->iface : NULL;
   live -    optargs->bitmask & GUESTFS___ADD_LIBVIRT_DOM_LIVE_BITMASK
+    optargs->bitmask & GUESTFS_ADD_LIBVIRT_DOM_LIVE_BITMASK
     ? optargs->live : 0;
 
-  if ((optargs->bitmask &
GUESTFS___ADD_LIBVIRT_DOM_READONLYDISK_BITMASK)) {
+  if ((optargs->bitmask & GUESTFS_ADD_LIBVIRT_DOM_READONLYDISK_BITMASK))
{
     if (STREQ (optargs->readonlydisk, "error"))
       readonlydisk = readonlydisk_error;
     else if (STREQ (optargs->readonlydisk, "read"))
@@ -222,15 +223,15 @@ guestfs___add_libvirt_dom (guestfs_h *g, virDomainPtr dom,
   }
 
   cachemode -    optargs->bitmask &
GUESTFS___ADD_LIBVIRT_DOM_CACHEMODE_BITMASK
+    optargs->bitmask & GUESTFS_ADD_LIBVIRT_DOM_CACHEMODE_BITMASK
     ? optargs->cachemode : NULL;
 
   discard -    optargs->bitmask &
GUESTFS___ADD_LIBVIRT_DOM_DISCARD_BITMASK
+    optargs->bitmask & GUESTFS_ADD_LIBVIRT_DOM_DISCARD_BITMASK
     ? optargs->discard : NULL;
 
   copyonread -    optargs->bitmask &
GUESTFS___ADD_LIBVIRT_DOM_COPYONREAD_BITMASK
+    optargs->bitmask & GUESTFS_ADD_LIBVIRT_DOM_COPYONREAD_BITMASK
     ? optargs->copyonread : false;
 
   if (live && readonly) {
diff --git a/tests/c-api/Makefile.am b/tests/c-api/Makefile.am
index 94cdd0a..57f75fe 100644
--- a/tests/c-api/Makefile.am
+++ b/tests/c-api/Makefile.am
@@ -240,17 +240,17 @@ test_event_string_LDADD = \
 	$(LTLIBINTL) \
 	$(top_builddir)/gnulib/lib/libgnu.la
 
-#if HAVE_LIBVIRT
-#test_add_libvirt_dom_SOURCES = test-add-libvirt-dom.c
-#test_add_libvirt_dom_CPPFLAGS = \
-#	-I$(top_srcdir)/src -I$(top_builddir)/src -I$(top_srcdir)/gnulib/lib
-#test_add_libvirt_dom_CFLAGS = \
-#	$(LIBVIRT_CFLAGS) \
-#	$(WARN_CFLAGS) $(WERROR_CFLAGS)
-#test_add_libvirt_dom_LDADD = \
-#	$(top_builddir)/src/libguestfs.la $(LIBVIRT_LIBS) \
-#	$(LTLIBTHREAD) $(top_builddir)/gnulib/lib/libgnu.la
-#endif
+if HAVE_LIBVIRT
+test_add_libvirt_dom_SOURCES = test-add-libvirt-dom.c
+test_add_libvirt_dom_CPPFLAGS = \
+	-I$(top_srcdir)/src -I$(top_builddir)/src -I$(top_srcdir)/gnulib/lib
+test_add_libvirt_dom_CFLAGS = \
+	$(LIBVIRT_CFLAGS) \
+	$(WARN_CFLAGS) $(WERROR_CFLAGS)
+test_add_libvirt_dom_LDADD = \
+	$(top_builddir)/src/libguestfs.la $(LIBVIRT_LIBS) \
+	$(LTLIBTHREAD) $(top_builddir)/gnulib/lib/libgnu.la
+endif
 
 check-valgrind:
 	$(MAKE) VG="$(top_builddir)/run @VG@" check
diff --git a/tests/c-api/test-add-libvirt-dom.c
b/tests/c-api/test-add-libvirt-dom.c
index 4768f8e..952b9a8 100644
--- a/tests/c-api/test-add-libvirt-dom.c
+++ b/tests/c-api/test-add-libvirt-dom.c
@@ -1,5 +1,5 @@
 /* libguestfs
- * Copyright (C) 2010 Red Hat Inc.
+ * Copyright (C) 2010-2014 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
-- 
2.1.0

Richard W.M. Jones

2014-Dec-11 14:19 UTC

head link

[Libguestfs] [PATCH v4 3/6] python: Improve harness for running Python tests.

It now understands exit code 77 == skip, amongst other improvements.
---
 python/run-python-tests | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/python/run-python-tests b/python/run-python-tests
index bcc40d8..af849c7 100755
--- a/python/run-python-tests
+++ b/python/run-python-tests
@@ -16,8 +16,23 @@
 # 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
+errors=0
 
 for f in $srcdir/t/*.py; do
   $PYTHON $f
+  r=$?
+  case $r in
+      0) ;;
+      77)
+          echo "$f: test skipped"
+          ;;
+      *)
+          echo "FAIL: $f"
+          ((errors++))
+          ;;
+  esac
 done
+
+if [ $errors -gt 0 ]; then
+    exit 1
+fi
-- 
2.1.0

Richard W.M. Jones

2014-Dec-11 14:19 UTC

head link

[Libguestfs] [PATCH v4 4/6] python: Implement Pointer ("virDomainPtr", _) (RHBZ#1075164).

This allows the Python binding of guestfs_add_libvirt_dom to work.

There is a regression test to ensure this keeps working.

Note this requires a patched libvirt-python, supporting the
c_pointer() method.
---
 README                  |  3 +++
 generator/python.ml     | 19 +++++++++++--------
 python/run-python-tests |  3 +++
 python/t/910-libvirt.py | 42 ++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 59 insertions(+), 8 deletions(-)
 create mode 100644 python/t/910-libvirt.py

diff --git a/README b/README
index fc88eb9..9962ae8 100644
--- a/README
+++ b/README
@@ -223,6 +223,9 @@ The full requirements are described below.
 +--------------+-------------+---+-----------------------------------------+
 | Sys::Virt    |             | O | Perl bindings for libvirt.              |
 +--------------+-------------+---+-----------------------------------------+
+| libvirt-python             | O | For testing Python libvirt/libguestfs   |
+|              |             |   | interactions.                           |
++--------------+-------------+---+-----------------------------------------+
 | Win::Hivex   |             | O | Perl bindings for hivex.                |
 +--------------+-------------+---+-----------------------------------------+
 | Pod::Usage   |             | O | Perl module used by tests.              |
diff --git a/generator/python.ml b/generator/python.ml
index 82afb2e..e47195f 100644
--- a/generator/python.ml
+++ b/generator/python.ml
@@ -294,8 +294,8 @@ put_table (char * const * const argv)
         | Int n -> pr "  int %s;\n" n
         | Int64 n -> pr "  long long %s;\n" n
         | Pointer (t, n) ->
-            pr "  long long %s_int64;\n" n;
-            pr "  void * /* %s */ %s;\n" t n
+            pr "  void * /* %s */ %s;\n" t n;
+            pr "  PyObject *%s_long;\n" n
       ) args;
 
       (* Fetch the optional arguments as objects, so we can detect
@@ -324,11 +324,12 @@ put_table (char * const * const argv)
         | StringList _ | DeviceList _ -> pr "O"
         | Bool _ -> pr "i" (* XXX Python has booleans? *)
         | Int _ -> pr "i"
-        | Int64 _ | Pointer _ ->
+        | Int64 _ ->
             (* XXX Whoever thought it was a good idea to
              * emulate C's int/long/long long in Python?
              *)
             pr "L"
+        | Pointer _ -> pr "O"
         | BufferIn _ -> pr "s#"
       ) args;
 
@@ -347,7 +348,7 @@ put_table (char * const * const argv)
         | Bool n -> pr ", &%s" n
         | Int n -> pr ", &%s" n
         | Int64 n -> pr ", &%s" n
-        | Pointer (_, n) -> pr ", &%s_int64" n
+        | Pointer (_, n) -> pr ", &%s_long" n
         | BufferIn n -> pr ", &%s, &%s_size" n n
       ) args;
 
@@ -369,8 +370,8 @@ put_table (char * const * const argv)
         | StringList n | DeviceList n ->
             pr "  %s = get_string_list (py_%s);\n" n n;
             pr "  if (!%s) goto out;\n" n
-        | Pointer (t, n) ->
-            pr "  %s = POINTER_NOT_IMPLEMENTED (\"%s\");\n"
n t
+        | Pointer (_, n) ->
+            pr "  %s = PyLong_AsVoidPtr (%s_long);\n" n n
       ) args;
 
       pr "\n";
@@ -798,9 +799,11 @@ class GuestFS(object):
         | Pathname _ | Device _ | Mountable _
         | Dev_or_Path _ | Mountable_or_Path _ | String _ | Key _
         | FileIn _ | FileOut _ | OptString _ | Bool _ | Int _ | Int64 _
-        | BufferIn _ | Pointer _ | GUID _ -> ()
+        | BufferIn _ | GUID _ -> ()
         | StringList n | DeviceList n ->
-            pr "        %s = list (%s)\n" n n
+          pr "        %s = list (%s)\n" n n
+        | Pointer (_, n) ->
+          pr "        %s = %s.c_pointer()\n" n n
       ) args;
       pr "        self._check_not_closed ()\n";
       pr "        r = libguestfsmod.%s (self._o" f.name;
diff --git a/python/run-python-tests b/python/run-python-tests
index af849c7..285bb18 100755
--- a/python/run-python-tests
+++ b/python/run-python-tests
@@ -18,6 +18,9 @@
 
 errors=0
 
+guestsdir="$(cd ../tests/guests && pwd)"
+export guestsdir
+
 for f in $srcdir/t/*.py; do
   $PYTHON $f
   r=$?
diff --git a/python/t/910-libvirt.py b/python/t/910-libvirt.py
new file mode 100644
index 0000000..3479737
--- /dev/null
+++ b/python/t/910-libvirt.py
@@ -0,0 +1,42 @@
+# libguestfs Python bindings
+# Copyright (C) 2014 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.
+
+# The Python bindings for add_libvirt_dom require the libvirt-python
+# library to support a new method (.c_pointer()).  Ensure this keeps
+# working by testing it.  See:
+# https://bugzilla.redhat.com/show_bug.cgi?id=1075164
+
+import os
+import guestfs
+
+guestsdir = os.environ['guestsdir']
+
+try:
+    import libvirt
+except:
+    print "could not import python-libvirt"
+    exit (77)
+
+conn = libvirt.open ("test:///%s/guests.xml" % guestsdir)
+dom = conn.lookupByName ("blank-disk")
+
+g = guestfs.GuestFS ()
+
+r = g.add_libvirt_dom (dom, readonly=1)
+
+if r != 1:
+    raise "unexpected return value from add_libvirt_dom (%d)" % r
-- 
2.1.0

Richard W.M. Jones

2014-Dec-11 14:19 UTC

head link

[Libguestfs] [PATCH v4 5/6] debug: add-domain: Dump XML of original domain.

Useful for debugging labelling issues.
---
 src/libvirt-domain.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 78503e5..b7a9ad7 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -767,6 +767,8 @@ get_domain_xml (guestfs_h *g, virDomainPtr dom)
     return NULL;
   }
 
+  debug (g, "original domain XML:\n%s", xml);
+
   /* Parse the domain XML into an XML document. */
   doc = xmlReadMemory (xml, strlen (xml),
                        NULL, NULL, XML_PARSE_NONET);
-- 
2.1.0

Richard W.M. Jones

2014-Dec-11 14:19 UTC

head link

[Libguestfs] [PATCH v4 6/6] launch: libvirt: Fix labelling of overlay files.

We had code (added for RHBZ#912499) which labels overlay files
correctly so that libvirt can read them.  Unfortunately this code was
broken by subsequent commits: the new backend setting for the
imagelabel is only copied to the 'data' struct during launch, but the
create_cow_overlay callback is called before launch (when adding
drives).

The fix is easy: ensure create_cow_overlay_libvirt checks for the
backend setting and initializes the 'data' struct.

This change also means we need to free (data->selinux_imagelabel)
before setting it in launch (and we do the same for
data->selinux_label, although that's not strictly necessary).

For background on this, see:
https://bugzilla.redhat.com/show_bug.cgi?id=912499#c10
---
 src/launch-libvirt.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/src/launch-libvirt.c b/src/launch-libvirt.c
index 1f3c451..ea0502f 100644
--- a/src/launch-libvirt.c
+++ b/src/launch-libvirt.c
@@ -217,6 +217,15 @@ create_cow_overlay_libvirt (guestfs_h *g, void *datav,
struct drive *drv)
     return NULL;
 
 #if HAVE_LIBSELINUX
+  /* Since this function is called before launch, the field won't be
+   * initialized correctly, so we have to initialize it here.
+   */
+  guestfs_push_error_handler (g, NULL, NULL);
+  free (data->selinux_imagelabel);
+  data->selinux_imagelabel +    guestfs_get_backend_setting (g,
"internal_libvirt_imagelabel");
+  guestfs_pop_error_handler (g);
+
   if (data->selinux_imagelabel) {
     debug (g, "setting SELinux label on %s to %s",
            overlay, data->selinux_imagelabel);
@@ -350,8 +359,10 @@ launch_libvirt (guestfs_h *g, void *datav, const char
*libvirt_uri)
 
   /* Misc backend settings. */
   guestfs_push_error_handler (g, NULL, NULL);
+  free (data->selinux_label);
   data->selinux_label      guestfs_get_backend_setting (g,
"internal_libvirt_label");
+  free (data->selinux_imagelabel);
   data->selinux_imagelabel      guestfs_get_backend_setting (g,
"internal_libvirt_imagelabel");
   data->selinux_norelabel_disks -- 
2.1.0

Reasonably Related Threads

Search for more possibly parallel threads

Libguestfs - Dec 2014 - [PATCH v4 0/6] Implement guestfs_add_libvirt_dom.

[Libguestfs] [PATCH v4 0/6] Implement guestfs_add_libvirt_dom.

[Libguestfs] [PATCH v4 1/6] generator: Implement Pointer arguments.

[Libguestfs] [PATCH v4 2/6] New(ish) API: guestfs_add_libvirt_dom.

[Libguestfs] [PATCH v4 3/6] python: Improve harness for running Python tests.

[Libguestfs] [PATCH v4 4/6] python: Implement Pointer ("virDomainPtr", _) (RHBZ#1075164).

[Libguestfs] [PATCH v4 5/6] debug: add-domain: Dump XML of original domain.

[Libguestfs] [PATCH v4 6/6] launch: libvirt: Fix labelling of overlay files.

Reasonably Related Threads