10:05 a.m.
Factor out the connection and pool loading out of v2v_pool_dumpxml, so
it can be used in later implementations requiring a pool.
Should be just code motion.
---
v2v/domainxml-c.c | 84 +++++++++++++++++++++++++++++++++----------------------
1 file changed, 51 insertions(+), 33 deletions(-)
diff --git a/v2v/domainxml-c.c b/v2v/domainxml-c.c
index 4224d72..077c153 100644
--- a/v2v/domainxml-c.c
+++ b/v2v/domainxml-c.c
@@ -106,6 +106,55 @@ libvirt_auth_default_wrapper (virConnectCredentialPtr cred,
}
}
+virStoragePoolPtr
+connect_and_load_pool (const char *conn_uri, const char *pool_name)
+{
+ /* We have to assemble the error on the stack because a dynamic
+ * string couldn't be freed.
+ */
+ char errmsg[256];
+ virErrorPtr err;
+ virConnectPtr conn;
+ virStoragePoolPtr pool;
+
+ /* We have to call the default authentication handler, not least
+ * since it handles all the PolicyKit crap. However it also makes
+ * coding this simpler.
+ */
+ conn = virConnectOpenAuth (conn_uri, virConnectAuthPtrDefault,
+ VIR_CONNECT_RO);
+ if (conn == NULL) {
+ if (conn_uri)
+ snprintf (errmsg, sizeof errmsg,
+ _("cannot open libvirt connection '%s'"), conn_uri);
+ else
+ snprintf (errmsg, sizeof errmsg, _("cannot open libvirt connection"));
+ caml_invalid_argument (errmsg);
+ }
+
+ /* Suppress default behaviour of printing errors to stderr. Note
+ * you can't set this to NULL to ignore errors; setting it to NULL
+ * restores the default error handler ...
+ */
+ virConnSetErrorFunc (conn, NULL, ignore_errors);
+
+ /* Look up the pool. */
+ pool = virStoragePoolLookupByUUIDString (conn, pool_name);
+
+ if (!pool)
+ pool = virStoragePoolLookupByName (conn, pool_name);
+
+ if (!pool) {
+ err = virGetLastError ();
+ snprintf (errmsg, sizeof errmsg,
+ _("cannot find libvirt pool '%s': %s"), pool_name,
err->message);
+ virConnectClose (conn);
+ caml_invalid_argument (errmsg);
+ }
+
+ return pool;
+}
+
value
v2v_dumpxml (value passwordv, value connv, value domnamev)
{
@@ -230,42 +279,11 @@ v2v_pool_dumpxml (value connv, value poolnamev)
if (connv != Val_int (0))
conn_uri = String_val (Field (connv, 0)); /* Some conn */
- /* We have to call the default authentication handler, not least
- * since it handles all the PolicyKit crap. However it also makes
- * coding this simpler.
- */
- conn = virConnectOpenAuth (conn_uri, virConnectAuthPtrDefault,
- VIR_CONNECT_RO);
- if (conn == NULL) {
- if (conn_uri)
- snprintf (errmsg, sizeof errmsg,
- _("cannot open libvirt connection '%s'"), conn_uri);
- else
- snprintf (errmsg, sizeof errmsg, _("cannot open libvirt connection"));
- caml_invalid_argument (errmsg);
- }
-
- /* Suppress default behaviour of printing errors to stderr. Note
- * you can't set this to NULL to ignore errors; setting it to NULL
- * restores the default error handler ...
- */
- virConnSetErrorFunc (conn, NULL, ignore_errors);
-
/* Look up the pool. */
poolname = String_val (poolnamev);
- pool = virStoragePoolLookupByUUIDString (conn, poolname);
-
- if (!pool)
- pool = virStoragePoolLookupByName (conn, poolname);
-
- if (!pool) {
- err = virGetLastError ();
- snprintf (errmsg, sizeof errmsg,
- _("cannot find libvirt pool '%s': %s"), poolname,
err->message);
- virConnectClose (conn);
- caml_invalid_argument (errmsg);
- }
+ pool = connect_and_load_pool (conn_uri, poolname);
+ conn = virStoragePoolGetConnect (pool);
xml = virStoragePoolGetXMLDesc (pool, 0);
if (xml == NULL) {
--
2.1.0
10:05 a.m.
New subject: [PATCH 2/4] v2v: domainxml: add vol_dumpxml
Add a new vol_dumpxml to get the XML dump of a pool's volume.
---
v2v/domainxml-c.c | 68 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
v2v/domainxml.ml | 1 +
v2v/domainxml.mli | 6 +++++
3 files changed, 75 insertions(+)
diff --git a/v2v/domainxml-c.c b/v2v/domainxml-c.c
index 077c153..88fb6d5 100644
--- a/v2v/domainxml-c.c
+++ b/v2v/domainxml-c.c
@@ -304,6 +304,68 @@ v2v_pool_dumpxml (value connv, value poolnamev)
CAMLreturn (retv);
}
+value
+v2v_vol_dumpxml (value connv, value poolnamev, value volnamev)
+{
+ CAMLparam3 (connv, poolnamev, volnamev);
+ CAMLlocal1 (retv);
+ const char *conn_uri = NULL;
+ const char *poolname;
+ const char *volname;
+ /* We have to assemble the error on the stack because a dynamic
+ * string couldn't be freed.
+ */
+ char errmsg[256];
+ virErrorPtr err;
+ virConnectPtr conn;
+ virStoragePoolPtr pool;
+ virStorageVolPtr vol;
+ char *xml;
+
+ if (connv != Val_int (0))
+ conn_uri = String_val (Field (connv, 0)); /* Some conn */
+
+ /* Look up the pool. */
+ poolname = String_val (poolnamev);
+
+ pool = connect_and_load_pool (conn_uri, poolname);
+ conn = virStoragePoolGetConnect (pool);
+
+ /* Look up the volume. */
+ volname = String_val (volnamev);
+
+ vol = virStorageVolLookupByName (pool, volname);
+
+ if (!vol) {
+ err = virGetLastError ();
+ snprintf (errmsg, sizeof errmsg,
+ _("cannot find libvirt volume '%s': %s"), volname,
err->message);
+ virStoragePoolFree (pool);
+ virConnectClose (conn);
+ caml_invalid_argument (errmsg);
+ }
+
+ xml = virStorageVolGetXMLDesc (vol, 0);
+ if (xml == NULL) {
+ err = virGetLastError ();
+ snprintf (errmsg, sizeof errmsg,
+ _("cannot fetch XML description of volume '%s': %s"),
+ volname, err->message);
+ virStorageVolFree (vol);
+ virStoragePoolFree (pool);
+ virConnectClose (conn);
+ caml_invalid_argument (errmsg);
+ }
+ virStorageVolFree (vol);
+ virStoragePoolFree (pool);
+ virConnectClose (conn);
+
+ retv = caml_copy_string (xml);
+ free (xml);
+
+ CAMLreturn (retv);
+}
+
#else /* !HAVE_LIBVIRT */
value
@@ -318,4 +380,10 @@ v2v_pool_dumpxml (value connv, value poolv)
caml_invalid_argument ("virt-v2v was compiled without libvirt support");
}
+value
+v2v_vol_dumpxml (value connv, value poolnamev, value volnamev)
+{
+ caml_invalid_argument ("virt-v2v was compiled without libvirt support");
+}
+
#endif /* !HAVE_LIBVIRT */
diff --git a/v2v/domainxml.ml b/v2v/domainxml.ml
index 8782ea1..7dbfcbd 100644
--- a/v2v/domainxml.ml
+++ b/v2v/domainxml.ml
@@ -20,3 +20,4 @@
external dumpxml : ?password:string -> ?conn:string -> string -> string =
"v2v_dumpxml"
external pool_dumpxml : ?conn:string -> string -> string =
"v2v_pool_dumpxml"
+external vol_dumpxml : ?conn:string -> string -> string -> string =
"v2v_vol_dumpxml"
diff --git a/v2v/domainxml.mli b/v2v/domainxml.mli
index 47a91cf..7008769 100644
--- a/v2v/domainxml.mli
+++ b/v2v/domainxml.mli
@@ -32,3 +32,9 @@ val pool_dumpxml : ?conn:string -> string -> string
(** [pool_dumpxml ?conn pool] returns the libvirt XML of pool [pool].
The optional [?conn] parameter is the libvirt connection URI.
[pool] may be a pool name or UUID. *)
+
+val vol_dumpxml : ?conn:string -> string -> string -> string
+(** [vol_dumpxml ?conn pool vol] returns the libvirt XML of volume [vol],
+ which is part of the pool [pool].
+ The optional [?conn] parameter is the libvirt connection URI.
+ [pool] may be a pool name or UUID. *)
--
2.1.0
11:15 a.m.
New subject: [PATCH 2/4] v2v: domainxml: add vol_dumpxml
On Tue, Apr 14, 2015 at 05:05:40PM +0200, Pino Toscano wrote:
Add a new vol_dumpxml to get the XML dump of a pool's volume.
---
v2v/domainxml-c.c | 68 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
v2v/domainxml.ml | 1 +
v2v/domainxml.mli | 6 +++++
3 files changed, 75 insertions(+)
diff --git a/v2v/domainxml-c.c b/v2v/domainxml-c.c
index 077c153..88fb6d5 100644
--- a/v2v/domainxml-c.c
+++ b/v2v/domainxml-c.c
@@ -304,6 +304,68 @@ v2v_pool_dumpxml (value connv, value poolnamev)
CAMLreturn (retv);
}
+value
+v2v_vol_dumpxml (value connv, value poolnamev, value volnamev)
+{
+ CAMLparam3 (connv, poolnamev, volnamev);
+ CAMLlocal1 (retv);
+ const char *conn_uri = NULL;
+ const char *poolname;
+ const char *volname;
+ /* We have to assemble the error on the stack because a dynamic
+ * string couldn't be freed.
+ */
+ char errmsg[256];
+ virErrorPtr err;
+ virConnectPtr conn;
+ virStoragePoolPtr pool;
+ virStorageVolPtr vol;
+ char *xml;
+
+ if (connv != Val_int (0))
+ conn_uri = String_val (Field (connv, 0)); /* Some conn */
+
+ /* Look up the pool. */
+ poolname = String_val (poolnamev);
+
+ pool = connect_and_load_pool (conn_uri, poolname);
+ conn = virStoragePoolGetConnect (pool);
+
+ /* Look up the volume. */
+ volname = String_val (volnamev);
+
+ vol = virStorageVolLookupByName (pool, volname);
+
+ if (!vol) {
+ err = virGetLastError ();
+ snprintf (errmsg, sizeof errmsg,
+ _("cannot find libvirt volume '%s': %s"), volname,
err->message);
+ virStoragePoolFree (pool);
+ virConnectClose (conn);
+ caml_invalid_argument (errmsg);
+ }
+
+ xml = virStorageVolGetXMLDesc (vol, 0);
+ if (xml == NULL) {
+ err = virGetLastError ();
+ snprintf (errmsg, sizeof errmsg,
+ _("cannot fetch XML description of volume '%s': %s"),
+ volname, err->message);
+ virStorageVolFree (vol);
+ virStoragePoolFree (pool);
+ virConnectClose (conn);
+ caml_invalid_argument (errmsg);
+ }
+ virStorageVolFree (vol);
+ virStoragePoolFree (pool);
+ virConnectClose (conn);
+
+ retv = caml_copy_string (xml);
+ free (xml);
+
+ CAMLreturn (retv);
+}
+
#else /* !HAVE_LIBVIRT */
value
@@ -318,4 +380,10 @@ v2v_pool_dumpxml (value connv, value poolv)
caml_invalid_argument ("virt-v2v was compiled without libvirt support");
}
+value
+v2v_vol_dumpxml (value connv, value poolnamev, value volnamev)
+{
+ caml_invalid_argument ("virt-v2v was compiled without libvirt support");
+}
+
#endif /* !HAVE_LIBVIRT */
diff --git a/v2v/domainxml.ml b/v2v/domainxml.ml
index 8782ea1..7dbfcbd 100644
--- a/v2v/domainxml.ml
+++ b/v2v/domainxml.ml
@@ -20,3 +20,4 @@
external dumpxml : ?password:string -> ?conn:string -> string -> string =
"v2v_dumpxml"
external pool_dumpxml : ?conn:string -> string -> string =
"v2v_pool_dumpxml"
+external vol_dumpxml : ?conn:string -> string -> string -> string =
"v2v_vol_dumpxml"
diff --git a/v2v/domainxml.mli b/v2v/domainxml.mli
index 47a91cf..7008769 100644
--- a/v2v/domainxml.mli
+++ b/v2v/domainxml.mli
@@ -32,3 +32,9 @@ val pool_dumpxml : ?conn:string -> string -> string
(** [pool_dumpxml ?conn pool] returns the libvirt XML of pool [pool].
The optional [?conn] parameter is the libvirt connection URI.
[pool] may be a pool name or UUID. *)
+
+val vol_dumpxml : ?conn:string -> string -> string -> string
+(** [vol_dumpxml ?conn pool vol] returns the libvirt XML of volume [vol],
+ which is part of the pool [pool].
+ The optional [?conn] parameter is the libvirt connection URI.
+ [pool] may be a pool name or UUID. *)
--
ACK.
Same comment applies w.r.t. String_val however.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines. Supports shell scripting,
bindings from many languages. http://libguestfs.org
12:10 p.m.
New subject: [PATCH 2/4] v2v: domainxml: add vol_dumpxml
Add a new vol_dumpxml to get the XML dump of a pool's volume.
---
v2v/domainxml-c.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
v2v/domainxml.ml | 1 +
v2v/domainxml.mli | 6 ++++++
3 files changed, 68 insertions(+)
diff --git a/v2v/domainxml-c.c b/v2v/domainxml-c.c
index b5d2f81..fe9f6d2 100644
--- a/v2v/domainxml-c.c
+++ b/v2v/domainxml-c.c
@@ -305,6 +305,61 @@ v2v_pool_dumpxml (value connv, value poolnamev)
CAMLreturn (retv);
}
+value
+v2v_vol_dumpxml (value connv, value poolnamev, value volnamev)
+{
+ CAMLparam3 (connv, poolnamev, volnamev);
+ CAMLlocal1 (retv);
+ const char *volname;
+ /* We have to assemble the error on the stack because a dynamic
+ * string couldn't be freed.
+ */
+ char errmsg[256];
+ virErrorPtr err;
+ virConnectPtr conn;
+ virStoragePoolPtr pool;
+ virStorageVolPtr vol;
+ char *xml;
+
+ /* Look up the pool. */
+ pool = connect_and_load_pool (connv, poolnamev);
+ conn = virStoragePoolGetConnect (pool);
+
+ /* Look up the volume. */
+ volname = String_val (volnamev);
+
+ vol = virStorageVolLookupByName (pool, volname);
+
+ if (!vol) {
+ err = virGetLastError ();
+ snprintf (errmsg, sizeof errmsg,
+ _("cannot find libvirt volume '%s': %s"), volname,
err->message);
+ virStoragePoolFree (pool);
+ virConnectClose (conn);
+ caml_invalid_argument (errmsg);
+ }
+
+ xml = virStorageVolGetXMLDesc (vol, 0);
+ if (xml == NULL) {
+ err = virGetLastError ();
+ snprintf (errmsg, sizeof errmsg,
+ _("cannot fetch XML description of volume '%s': %s"),
+ volname, err->message);
+ virStorageVolFree (vol);
+ virStoragePoolFree (pool);
+ virConnectClose (conn);
+ caml_invalid_argument (errmsg);
+ }
+ virStorageVolFree (vol);
+ virStoragePoolFree (pool);
+ virConnectClose (conn);
+
+ retv = caml_copy_string (xml);
+ free (xml);
+
+ CAMLreturn (retv);
+}
+
#else /* !HAVE_LIBVIRT */
value
@@ -319,4 +374,10 @@ v2v_pool_dumpxml (value connv, value poolv)
caml_invalid_argument ("virt-v2v was compiled without libvirt support");
}
+value
+v2v_vol_dumpxml (value connv, value poolnamev, value volnamev)
+{
+ caml_invalid_argument ("virt-v2v was compiled without libvirt support");
+}
+
#endif /* !HAVE_LIBVIRT */
diff --git a/v2v/domainxml.ml b/v2v/domainxml.ml
index 8782ea1..7dbfcbd 100644
--- a/v2v/domainxml.ml
+++ b/v2v/domainxml.ml
@@ -20,3 +20,4 @@
external dumpxml : ?password:string -> ?conn:string -> string -> string =
"v2v_dumpxml"
external pool_dumpxml : ?conn:string -> string -> string =
"v2v_pool_dumpxml"
+external vol_dumpxml : ?conn:string -> string -> string -> string =
"v2v_vol_dumpxml"
diff --git a/v2v/domainxml.mli b/v2v/domainxml.mli
index 47a91cf..7008769 100644
--- a/v2v/domainxml.mli
+++ b/v2v/domainxml.mli
@@ -32,3 +32,9 @@ val pool_dumpxml : ?conn:string -> string -> string
(** [pool_dumpxml ?conn pool] returns the libvirt XML of pool [pool].
The optional [?conn] parameter is the libvirt connection URI.
[pool] may be a pool name or UUID. *)
+
+val vol_dumpxml : ?conn:string -> string -> string -> string
+(** [vol_dumpxml ?conn pool vol] returns the libvirt XML of volume [vol],
+ which is part of the pool [pool].
+ The optional [?conn] parameter is the libvirt connection URI.
+ [pool] may be a pool name or UUID. *)
--
2.1.0
10:05 a.m.
New subject: [PATCH 3/4] v2v: pass libvirt connection URI to parse_libvirt_xml
This makes it possible to connect to the right libvirt.
---
v2v/input_libvirt_other.ml | 2 +-
v2v/input_libvirt_vcenter_https.ml | 2 +-
v2v/input_libvirt_xen_ssh.ml | 2 +-
v2v/input_libvirtxml.ml | 2 +-
v2v/input_libvirtxml.mli | 2 +-
5 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/v2v/input_libvirt_other.ml b/v2v/input_libvirt_other.ml
index 14a209c..48c6092 100644
--- a/v2v/input_libvirt_other.ml
+++ b/v2v/input_libvirt_other.ml
@@ -70,7 +70,7 @@ object
*)
let xml = Domainxml.dumpxml ?password ?conn:libvirt_uri guest in
- let source, disks = Input_libvirtxml.parse_libvirt_xml ~verbose xml in
+ let source, disks = Input_libvirtxml.parse_libvirt_xml ?conn:libvirt_uri ~verbose xml
in
let disks =
List.map (fun { Input_libvirtxml.p_source_disk = disk } -> disk) disks in
{ source with s_disks = disks }
diff --git a/v2v/input_libvirt_vcenter_https.ml b/v2v/input_libvirt_vcenter_https.ml
index 630894d..d45d602 100644
--- a/v2v/input_libvirt_vcenter_https.ml
+++ b/v2v/input_libvirt_vcenter_https.ml
@@ -298,7 +298,7 @@ object
* that the domain is not running. (RHBZ#1138586)
*)
let xml = Domainxml.dumpxml ?password ?conn:libvirt_uri guest in
- let source, disks = parse_libvirt_xml ~verbose xml in
+ let source, disks = parse_libvirt_xml ?conn:libvirt_uri ~verbose xml in
(* Save the original source paths, so that we can remap them again
* in [#adjust_overlay_parameters].
diff --git a/v2v/input_libvirt_xen_ssh.ml b/v2v/input_libvirt_xen_ssh.ml
index 4a844c9..f8b0c7a 100644
--- a/v2v/input_libvirt_xen_ssh.ml
+++ b/v2v/input_libvirt_xen_ssh.ml
@@ -46,7 +46,7 @@ object
* that the domain is not running. (RHBZ#1138586)
*)
let xml = Domainxml.dumpxml ?password ?conn:libvirt_uri guest in
- let source, disks = parse_libvirt_xml ~verbose xml in
+ let source, disks = parse_libvirt_xml ?conn:libvirt_uri ~verbose xml in
(* Map the <source/> filename (which is relative to the remote
* Xen server) to an ssh URI. This is a JSON URI looking something
diff --git a/v2v/input_libvirtxml.ml b/v2v/input_libvirtxml.ml
index 2e8fa1f..ed850cd 100644
--- a/v2v/input_libvirtxml.ml
+++ b/v2v/input_libvirtxml.ml
@@ -33,7 +33,7 @@ and parsed_source =
| P_source_file of string
| P_dont_rewrite
-let parse_libvirt_xml ~verbose xml =
+let parse_libvirt_xml ?conn ~verbose xml =
if verbose then
printf "libvirt xml is:\n%s\n" xml;
diff --git a/v2v/input_libvirtxml.mli b/v2v/input_libvirtxml.mli
index 5674bb1..abe0c43 100644
--- a/v2v/input_libvirtxml.mli
+++ b/v2v/input_libvirtxml.mli
@@ -27,7 +27,7 @@ and parsed_source =
| P_source_file of string (** <source file> *)
| P_dont_rewrite (** s_qemu_uri is already set. *)
-val parse_libvirt_xml : verbose:bool -> string -> Types.source * parsed_disk list
+val parse_libvirt_xml : ?conn:string -> verbose:bool -> string -> Types.source *
parsed_disk list
(** Take libvirt XML and parse it into a {!Types.source} structure and a
list of source disks.
--
2.1.0
10:05 a.m.
New subject: [PATCH 4/4] v2v: start importing "volume" disk types (RHBZ#1146832)
Import disks stored as "file"-type volumes.
Side effect: a guest converted to libvirt using virt-v2v can be
converted again using virt-v2v.
---
v2v/input_libvirtxml.ml | 34 ++++++++++++++++++++++++++++++++--
1 file changed, 32 insertions(+), 2 deletions(-)
diff --git a/v2v/input_libvirtxml.ml b/v2v/input_libvirtxml.ml
index ed850cd..5eea015 100644
--- a/v2v/input_libvirtxml.ml
+++ b/v2v/input_libvirtxml.ml
@@ -182,8 +182,8 @@ let parse_libvirt_xml ?conn ~verbose xml =
| "" -> None
| format -> Some format in
- (* The <disk type='...'> attribute may be 'block',
'file' or
- * 'network'. We ignore any other types.
+ (* The <disk type='...'> attribute may be 'block',
'file',
+ * 'network' or 'volume'. We ignore any other types.
*)
match xpath_to_string "@type" "" with
| "block" ->
@@ -211,6 +211,36 @@ let parse_libvirt_xml ?conn ~verbose xml =
warning (f_"<disk type='network'> with <source
protocol='%s'> was ignored")
protocol
)
+ | "volume" ->
+ let pool = xpath_to_string "source/@pool" "" in
+ let vol = xpath_to_string "source/@volume" "" in
+ if pool <> "" && vol <> "" then (
+ let xml = Domainxml.vol_dumpxml ?conn pool vol in
+ let doc = Xml.parse_memory xml in
+ let xpathctx = Xml.xpath_new_context doc in
+
+ let xpath_to_string expr default =
+ let obj = Xml.xpath_eval_expression xpathctx expr in
+ if Xml.xpathobj_nr_nodes obj < 1 then default
+ else (
+ let node = Xml.xpathobj_node doc obj 0 in
+ Xml.node_as_string node
+ ) in
+
+ (* Use the format specified in the volume itself. *)
+ let format =
+ match xpath_to_string "/volume/target/format/@type" ""
with
+ | "" -> None
+ | format -> Some format in
+
+ match xpath_to_string "/volume/@type" "" with
+ | "" | "file" ->
+ let path = xpath_to_string "/volume/target/path/text()"
"" in
+ if path <> "" then
+ add_disk path format controller (P_source_file path)
+ | vol_type ->
+ warning (f_"<disk type='volume'> with <volume
type='%s'> was ignored") vol_type
+ )
| disk_type ->
warning (f_"<disk type='%s'> was ignored") disk_type
done;
--
2.1.0
11:18 a.m.
New subject: [PATCH 4/4] v2v: start importing "volume" disk types (RHBZ#1146832)
On Tue, Apr 14, 2015 at 05:05:42PM +0200, Pino Toscano wrote:
Import disks stored as "file"-type volumes.
Side effect: a guest converted to libvirt using virt-v2v can be
converted again using virt-v2v.
---
v2v/input_libvirtxml.ml | 34 ++++++++++++++++++++++++++++++++--
1 file changed, 32 insertions(+), 2 deletions(-)
diff --git a/v2v/input_libvirtxml.ml b/v2v/input_libvirtxml.ml
index ed850cd..5eea015 100644
--- a/v2v/input_libvirtxml.ml
+++ b/v2v/input_libvirtxml.ml
@@ -182,8 +182,8 @@ let parse_libvirt_xml ?conn ~verbose xml =
| "" -> None
| format -> Some format in
- (* The <disk type='...'> attribute may be 'block',
'file' or
- * 'network'. We ignore any other types.
+ (* The <disk type='...'> attribute may be 'block',
'file',
+ * 'network' or 'volume'. We ignore any other types.
*)
match xpath_to_string "@type" "" with
| "block" ->
@@ -211,6 +211,36 @@ let parse_libvirt_xml ?conn ~verbose xml =
warning (f_"<disk type='network'> with <source
protocol='%s'> was ignored")
protocol
)
+ | "volume" ->
+ let pool = xpath_to_string "source/@pool" "" in
+ let vol = xpath_to_string "source/@volume" "" in
+ if pool <> "" && vol <> "" then (
+ let xml = Domainxml.vol_dumpxml ?conn pool vol in
+ let doc = Xml.parse_memory xml in
+ let xpathctx = Xml.xpath_new_context doc in
+
+ let xpath_to_string expr default =
+ let obj = Xml.xpath_eval_expression xpathctx expr in
+ if Xml.xpathobj_nr_nodes obj < 1 then default
+ else (
+ let node = Xml.xpathobj_node doc obj 0 in
+ Xml.node_as_string node
+ ) in
+
+ (* Use the format specified in the volume itself. *)
+ let format =
+ match xpath_to_string "/volume/target/format/@type" ""
with
+ | "" -> None
+ | format -> Some format in
+
+ match xpath_to_string "/volume/@type" "" with
+ | "" | "file" ->
+ let path = xpath_to_string "/volume/target/path/text()"
"" in
+ if path <> "" then
+ add_disk path format controller (P_source_file path)
+ | vol_type ->
+ warning (f_"<disk type='volume'> with <volume
type='%s'> was ignored") vol_type
+ )
| disk_type ->
warning (f_"<disk type='%s'> was ignored") disk_type
done;
--
2.1.0
ACK 3/4 & 4/4.
Tests perhaps? Not sure how easy it is to set up & test a libvirt
volume though.
Thanks,
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines. Supports shell scripting,
bindings from many languages. http://libguestfs.org
11:14 a.m.
On Tue, Apr 14, 2015 at 05:05:39PM +0200, Pino Toscano wrote:
Factor out the connection and pool loading out of v2v_pool_dumpxml,
so
it can be used in later implementations requiring a pool.
Should be just code motion.
---
v2v/domainxml-c.c | 84 +++++++++++++++++++++++++++++++++----------------------
1 file changed, 51 insertions(+), 33 deletions(-)
diff --git a/v2v/domainxml-c.c b/v2v/domainxml-c.c
index 4224d72..077c153 100644
--- a/v2v/domainxml-c.c
+++ b/v2v/domainxml-c.c
@@ -106,6 +106,55 @@ libvirt_auth_default_wrapper (virConnectCredentialPtr cred,
}
}
+virStoragePoolPtr
+connect_and_load_pool (const char *conn_uri, const char *pool_name)
+{
+ /* We have to assemble the error on the stack because a dynamic
+ * string couldn't be freed.
+ */
+ char errmsg[256];
+ virErrorPtr err;
+ virConnectPtr conn;
+ virStoragePoolPtr pool;
+
+ /* We have to call the default authentication handler, not least
+ * since it handles all the PolicyKit crap. However it also makes
+ * coding this simpler.
+ */
+ conn = virConnectOpenAuth (conn_uri, virConnectAuthPtrDefault,
+ VIR_CONNECT_RO);
+ if (conn == NULL) {
+ if (conn_uri)
+ snprintf (errmsg, sizeof errmsg,
+ _("cannot open libvirt connection '%s'"), conn_uri);
+ else
+ snprintf (errmsg, sizeof errmsg, _("cannot open libvirt connection"));
+ caml_invalid_argument (errmsg);
+ }
+
+ /* Suppress default behaviour of printing errors to stderr. Note
+ * you can't set this to NULL to ignore errors; setting it to NULL
+ * restores the default error handler ...
+ */
+ virConnSetErrorFunc (conn, NULL, ignore_errors);
+
+ /* Look up the pool. */
+ pool = virStoragePoolLookupByUUIDString (conn, pool_name);
+
+ if (!pool)
+ pool = virStoragePoolLookupByName (conn, pool_name);
+
+ if (!pool) {
+ err = virGetLastError ();
+ snprintf (errmsg, sizeof errmsg,
+ _("cannot find libvirt pool '%s': %s"), pool_name,
err->message);
+ virConnectClose (conn);
+ caml_invalid_argument (errmsg);
+ }
+
+ return pool;
+}
+
value
v2v_dumpxml (value passwordv, value connv, value domnamev)
{
@@ -230,42 +279,11 @@ v2v_pool_dumpxml (value connv, value poolnamev)
if (connv != Val_int (0))
conn_uri = String_val (Field (connv, 0)); /* Some conn */
- /* We have to call the default authentication handler, not least
- * since it handles all the PolicyKit crap. However it also makes
- * coding this simpler.
- */
- conn = virConnectOpenAuth (conn_uri, virConnectAuthPtrDefault,
- VIR_CONNECT_RO);
- if (conn == NULL) {
- if (conn_uri)
- snprintf (errmsg, sizeof errmsg,
- _("cannot open libvirt connection '%s'"), conn_uri);
- else
- snprintf (errmsg, sizeof errmsg, _("cannot open libvirt connection"));
- caml_invalid_argument (errmsg);
- }
-
- /* Suppress default behaviour of printing errors to stderr. Note
- * you can't set this to NULL to ignore errors; setting it to NULL
- * restores the default error handler ...
- */
- virConnSetErrorFunc (conn, NULL, ignore_errors);
-
/* Look up the pool. */
poolname = String_val (poolnamev);
- pool = virStoragePoolLookupByUUIDString (conn, poolname);
-
- if (!pool)
- pool = virStoragePoolLookupByName (conn, poolname);
-
- if (!pool) {
- err = virGetLastError ();
- snprintf (errmsg, sizeof errmsg,
- _("cannot find libvirt pool '%s': %s"), poolname,
err->message);
- virConnectClose (conn);
- caml_invalid_argument (errmsg);
- }
+ pool = connect_and_load_pool (conn_uri, poolname);
+ conn = virStoragePoolGetConnect (pool);
xml = virStoragePoolGetXMLDesc (pool, 0);
if (xml == NULL) {
ACK.
Although this code doesn't suffer from it [I don't think], be aware
that the char * returned by String_val() may become suddenly invalid
if any OCaml allocation function is called (eg. caml_copy_string,
caml_alloc*, or any indirect function that could call those). The
reason is that a call into an allocation function might trigger the
garbage collector, causing especially a string on the minor heap to
move to the major heap (and other possibilities too).
As a result of this I'm always suspicious of code that calls
String_val, stashes the result in a variable, and then uses the result
over long stretches of code. It's safer to call String_val() multiple
times whenever it is needed (String_val is a no-op because
value == char * for OCaml strings).
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines. Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v
11:26 a.m.
On Tue, Apr 14, 2015 at 05:14:50PM +0100, Richard W.M. Jones wrote:
As a result of this I'm always suspicious of code that calls
String_val, stashes the result in a variable, and then uses the result
over long stretches of code. It's safer to call String_val() multiple
times whenever it is needed (String_val is a no-op because
value == char * for OCaml strings).
And another thing to say is if you want to pass a 'value' into other
private functions, in order to avoid having to call String_val() until
the last moment, then you usually need to surround those private
functions with:
static value internal_fn (value a)
{
CAMLparam1 (a);
// maybe also CAMLlocal (..);
...
CAMLreturn (Val_unit);
// or CAMLreturnT if returning a non-value
// or CAMLreturn0 if returning void
}
These are only needed for functions that may allocate on the OCaml
stack, eg. calling caml_alloc*, caml_copy*, etc (perhaps indirectly).
What these macros actually do is to set up a struct on the stack which
is linked to a global list of stack frames. If the GC moves an object
around, then it follows this list of stack frames, finding if a value
exists anywhere on the C stack pointing to that object, and adjusting
the value pointer to point to wherever the object moved to.
(It's helpful to examine the macro definition in the header files to
really see what's going on.)
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines. Supports shell scripting,
bindings from many languages. http://libguestfs.org
12:04 p.m.
On Tuesday 14 April 2015 17:26:19 Richard W.M. Jones wrote:
On Tue, Apr 14, 2015 at 05:14:50PM +0100, Richard W.M. Jones wrote:
> As a result of this I'm always suspicious of code that calls
> String_val, stashes the result in a variable, and then uses the result
> over long stretches of code. It's safer to call String_val() multiple
> times whenever it is needed (String_val is a no-op because
> value == char * for OCaml strings).
This is something I checked, although of course avoid being on the
"borderline" is better indeed.
And another thing to say is if you want to pass a 'value'
into other
private functions, in order to avoid having to call String_val() until
the last moment, then you usually need to surround those private
functions with:
static value internal_fn (value a)
{
CAMLparam1 (a);
// maybe also CAMLlocal (..);
...
CAMLreturn (Val_unit);
// or CAMLreturnT if returning a non-value
// or CAMLreturn0 if returning void
}
These are only needed for functions that may allocate on the OCaml
stack, eg. calling caml_alloc*, caml_copy*, etc (perhaps indirectly).
What these macros actually do is to set up a struct on the stack which
is linked to a global list of stack frames. If the GC moves an object
around, then it follows this list of stack frames, finding if a value
exists anywhere on the C stack pointing to that object, and adjusting
the value pointer to point to wherever the object moved to.
(It's helpful to examine the macro definition in the header files to
really see what's going on.)
This is something useful to know, thanks for the pointers. I actually
checked the basics of these OCaml C API, although not that in-depth.
I changed patches 1 & 2 according to what you've pointed out, sending
them soon for review.
Thanks,
--
Pino Toscano
12:10 p.m.
Factor out the connection and pool loading out of v2v_pool_dumpxml, so
it can be used in later implementations requiring a pool.
Should be just code motion.
---
v2v/domainxml-c.c | 101 ++++++++++++++++++++++++++++++++----------------------
1 file changed, 60 insertions(+), 41 deletions(-)
diff --git a/v2v/domainxml-c.c b/v2v/domainxml-c.c
index 4224d72..b5d2f81 100644
--- a/v2v/domainxml-c.c
+++ b/v2v/domainxml-c.c
@@ -106,6 +106,63 @@ libvirt_auth_default_wrapper (virConnectCredentialPtr cred,
}
}
+virStoragePoolPtr
+connect_and_load_pool (value connv, value poolnamev)
+{
+ CAMLparam2 (connv, poolnamev);
+ const char *conn_uri = NULL;
+ const char *poolname;
+ /* We have to assemble the error on the stack because a dynamic
+ * string couldn't be freed.
+ */
+ char errmsg[256];
+ virErrorPtr err;
+ virConnectPtr conn;
+ virStoragePoolPtr pool;
+
+ if (connv != Val_int (0))
+ conn_uri = String_val (Field (connv, 0)); /* Some conn */
+
+ /* We have to call the default authentication handler, not least
+ * since it handles all the PolicyKit crap. However it also makes
+ * coding this simpler.
+ */
+ conn = virConnectOpenAuth (conn_uri, virConnectAuthPtrDefault,
+ VIR_CONNECT_RO);
+ if (conn == NULL) {
+ if (conn_uri)
+ snprintf (errmsg, sizeof errmsg,
+ _("cannot open libvirt connection '%s'"), conn_uri);
+ else
+ snprintf (errmsg, sizeof errmsg, _("cannot open libvirt connection"));
+ caml_invalid_argument (errmsg);
+ }
+
+ /* Suppress default behaviour of printing errors to stderr. Note
+ * you can't set this to NULL to ignore errors; setting it to NULL
+ * restores the default error handler ...
+ */
+ virConnSetErrorFunc (conn, NULL, ignore_errors);
+
+ /* Look up the pool. */
+ poolname = String_val (poolnamev);
+
+ pool = virStoragePoolLookupByUUIDString (conn, poolname);
+
+ if (!pool)
+ pool = virStoragePoolLookupByName (conn, poolname);
+
+ if (!pool) {
+ err = virGetLastError ();
+ snprintf (errmsg, sizeof errmsg,
+ _("cannot find libvirt pool '%s': %s"), poolname,
err->message);
+ virConnectClose (conn);
+ caml_invalid_argument (errmsg);
+ }
+
+ CAMLreturnT (virStoragePoolPtr, pool);
+}
+
value
v2v_dumpxml (value passwordv, value connv, value domnamev)
{
@@ -216,8 +273,6 @@ v2v_pool_dumpxml (value connv, value poolnamev)
{
CAMLparam2 (connv, poolnamev);
CAMLlocal1 (retv);
- const char *conn_uri = NULL;
- const char *poolname;
/* We have to assemble the error on the stack because a dynamic
* string couldn't be freed.
*/
@@ -227,52 +282,16 @@ v2v_pool_dumpxml (value connv, value poolnamev)
virStoragePoolPtr pool;
char *xml;
- if (connv != Val_int (0))
- conn_uri = String_val (Field (connv, 0)); /* Some conn */
-
- /* We have to call the default authentication handler, not least
- * since it handles all the PolicyKit crap. However it also makes
- * coding this simpler.
- */
- conn = virConnectOpenAuth (conn_uri, virConnectAuthPtrDefault,
- VIR_CONNECT_RO);
- if (conn == NULL) {
- if (conn_uri)
- snprintf (errmsg, sizeof errmsg,
- _("cannot open libvirt connection '%s'"), conn_uri);
- else
- snprintf (errmsg, sizeof errmsg, _("cannot open libvirt connection"));
- caml_invalid_argument (errmsg);
- }
-
- /* Suppress default behaviour of printing errors to stderr. Note
- * you can't set this to NULL to ignore errors; setting it to NULL
- * restores the default error handler ...
- */
- virConnSetErrorFunc (conn, NULL, ignore_errors);
-
/* Look up the pool. */
- poolname = String_val (poolnamev);
-
- pool = virStoragePoolLookupByUUIDString (conn, poolname);
-
- if (!pool)
- pool = virStoragePoolLookupByName (conn, poolname);
-
- if (!pool) {
- err = virGetLastError ();
- snprintf (errmsg, sizeof errmsg,
- _("cannot find libvirt pool '%s': %s"), poolname,
err->message);
- virConnectClose (conn);
- caml_invalid_argument (errmsg);
- }
+ pool = connect_and_load_pool (connv, poolnamev);
+ conn = virStoragePoolGetConnect (pool);
xml = virStoragePoolGetXMLDesc (pool, 0);
if (xml == NULL) {
err = virGetLastError ();
snprintf (errmsg, sizeof errmsg,
_("cannot fetch XML description of pool '%s': %s"),
- poolname, err->message);
+ String_val (poolnamev), err->message);
virStoragePoolFree (pool);
virConnectClose (conn);
caml_invalid_argument (errmsg);
--
2.1.0
1:45 p.m.
v2 of both patches does look safer to me. ACK.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines. Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top
3510
days inactive
3510
days old
11 comments
2 participants
participants (2)
-
Pino Toscano -
Richard W.M. Jones