[Libguestfs] [v2v PATCH] windows: use libosinfo only for in-disk virtio-win

2020-11-25 Thread Pino Toscano
Try to install the Windows drivers from virtio-win only in case it is
requested from a local directory (and not an ISO, or a block file).
---
 v2v/windows_virtio.ml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/v2v/windows_virtio.ml b/v2v/windows_virtio.ml
index 4e00cd61..207fff2e 100644
--- a/v2v/windows_virtio.ml
+++ b/v2v/windows_virtio.ml
@@ -298,7 +298,7 @@ and ddb_regedits inspect drv_name drv_pciid =
  * been copied.
  *)
 and copy_drivers g inspect driverdir =
-  [] <> copy_from_libosinfo g inspect driverdir ||
+  (is_directory virtio_win && [] <> copy_from_libosinfo g inspect driverdir) ||
 [] <> copy_from_virtio_win g inspect "/" driverdir
   virtio_iso_path_matches_guest_os
   (fun () ->
-- 
2.28.0

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs



Re: [Libguestfs] [common PATCH 0/3] SELinux_relabel: relabel only if enforcing (RHBZ#1828952)

2020-11-11 Thread Pino Toscano
On Wednesday, 23 September 2020 17:57:47 CET Pino Toscano wrote:
> Continuation/rework of:
> https://www.redhat.com/archives/libguestfs/2020-May/msg00020.html
> 
> This is my approach, as I explained here:
> https://bugzilla.redhat.com/show_bug.cgi?id=1828952#c4
> https://www.redhat.com/archives/libguestfs/2020-May/msg00035.html
> IOW: do not attempt to relabel if the guest is not enforcing, as it is
> either useless or may fail; few words more are in the comments of patch
> #3.

Are there any non-ideological actual technical objection to this patch
series?

If not, I'll push it in a week.

Thanks,
-- 
Pino Toscano

signature.asc
Description: This is a digitally signed message part.
___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [common PATCH 3/3] mlcustomize: do not relabel if not enforcing (RHBZ#1828952)

2020-09-24 Thread Pino Toscano
On Thursday, 24 September 2020 14:53:25 CEST Richard W.M. Jones wrote:
> On Thu, Sep 24, 2020 at 02:16:24PM +0200, Pino Toscano wrote:
> > On Thursday, 24 September 2020 13:53:57 CEST Richard W.M. Jones wrote:
> > > > Considering that /tmp is a general location for temporary files, it's
> > > > common that files may end with a tmp_t-alike label when moved back to
> > > > the destination place (e.g. after a rename()). That is not the only
> > > > situation like this that I saw in the past.
> > > > 
> > > > In permissive mode, all these situation are logged in the audit log,
> > > > yes, but they cause no blocks nor errors.
> > > > 
> > > > > It's also fine for an administrator to
> > > > > switch a system to permissive and then back to enforcing without
> > > > > relabelling or rebooting.
> > > > 
> > > > A mislabelled /etc/passwd is still read and used fine in permissive
> > > > mode. Switch back from permissive to enforcing without a relabelling
> > > > is generally not a good idea, especially after the system ran for a
> > > > lot of time after the switch to permissive.
> > > 
> > > It's seems true from what you wrote above that someone could copy
> > > /tmp/passwd -> /etc/passwd and it would have a wrong label.  But
> > > virt-v2v could fix that label, which even in permissive mode sounds
> > > like a win.
> > 
> > The question is: why? If the system had wrong labels even for system
> > files, and the administrator did not bother/want to fix them (because
> > of permissive), why should virt-v2v? Even if virt-v2v relabels a
> > permissive guest, the labels will get out of sync once the guest runs
> > again and does its own stuff, so there is no gain here.
> 
> I don't think this part is true (except in rather contrived situations).

I asked few questions, so "don't think it is true" does not seem
correct to me.

> It seems as if when setting the mode to Permissive when the guest is
> running normally, it *is* attempting to maintain the correct labels.

There is a difference between "attempting" and "succeeding". Permissive
does some effort, but there is *zero* guarantee that the labels of the
system are correct. And again: setting a Fedora/RHEL/CentOS system as
anything different than enforcing requires an explicit action (either
in /etc/selinux/config, or at runtime with `setenforce`, or when
installing via kickstart, etc), so this is a situation that was
explicitly requested.

> So virt-v2v should do the same thing.

Right: respect the administrator choice to not enforce the SELinux
policy on filesystems in any way.

-- 
Pino Toscano

signature.asc
Description: This is a digitally signed message part.
___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [common PATCH 3/3] mlcustomize: do not relabel if not enforcing (RHBZ#1828952)

2020-09-24 Thread Pino Toscano
On Thursday, 24 September 2020 13:53:57 CEST Richard W.M. Jones wrote:
> > Considering that /tmp is a general location for temporary files, it's
> > common that files may end with a tmp_t-alike label when moved back to
> > the destination place (e.g. after a rename()). That is not the only
> > situation like this that I saw in the past.
> > 
> > In permissive mode, all these situation are logged in the audit log,
> > yes, but they cause no blocks nor errors.
> > 
> > > It's also fine for an administrator to
> > > switch a system to permissive and then back to enforcing without
> > > relabelling or rebooting.
> > 
> > A mislabelled /etc/passwd is still read and used fine in permissive
> > mode. Switch back from permissive to enforcing without a relabelling
> > is generally not a good idea, especially after the system ran for a
> > lot of time after the switch to permissive.
> 
> It's seems true from what you wrote above that someone could copy
> /tmp/passwd -> /etc/passwd and it would have a wrong label.  But
> virt-v2v could fix that label, which even in permissive mode sounds
> like a win.

The question is: why? If the system had wrong labels even for system
files, and the administrator did not bother/want to fix them (because
of permissive), why should virt-v2v? Even if virt-v2v relabels a
permissive guest, the labels will get out of sync once the guest runs
again and does its own stuff, so there is no gain here.

> My question is what's the down-side to relabelling in permissive mode?

Time spend doing something that is not useful/used for the guest.

-- 
Pino Toscano

signature.asc
Description: This is a digitally signed message part.
___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [common PATCH 3/3] mlcustomize: do not relabel if not enforcing (RHBZ#1828952)

2020-09-24 Thread Pino Toscano
On Thursday, 24 September 2020 12:15:29 CEST Richard W.M. Jones wrote:
> On Wed, Sep 23, 2020 at 05:57:50PM +0200, Pino Toscano wrote:
> > Do not attempt to relabel a guest in case its SELinux enforcing mode is
> > not "enforcing", as it is either pointless, or it may fail because of an
> > invalid policy configured.
> > ---
> >  mlcustomize/SELinux_relabel.ml | 26 +-
> >  1 file changed, 25 insertions(+), 1 deletion(-)
> > 
> > diff --git a/mlcustomize/SELinux_relabel.ml b/mlcustomize/SELinux_relabel.ml
> > index 647aeda..db00e59 100644
> > --- a/mlcustomize/SELinux_relabel.ml
> > +++ b/mlcustomize/SELinux_relabel.ml
> > @@ -24,6 +24,9 @@ open Printf
> >  
> >  module G = Guestfs
> >  
> > +exception SELinux_not_enforcing
> > +(* Interal exception to signal a non-enforcing SELinux. *)
> > +
> >  (* Simple reimplementation of Array.mem, available only with OCaml >= 
> > 4.03. *)
> >  let array_find a l =
> >List.mem a (Array.to_list l)
> > @@ -35,12 +38,18 @@ let rec relabel (g : G.guestfs) =
> >use_setfiles g;
> >(* That worked, so we don't need to autorelabel. *)
> >g#rm_f "/.autorelabel"
> > -with Failure _ ->
> > +with
> > +| Failure _ ->
> >(* This is the fallback in case something in the setfiles
> > * method didn't work.  That includes the case where a non-SELinux
> > * host is processing an SELinux guest, and other things.
> > *)
> >g#touch "/.autorelabel"
> > +| SELinux_not_enforcing ->
> > +  (* This means that SELinux was not configured to be in enforcing 
> > mode,
> > +   * so silently accept this.
> > +   *)
> > +  ()
> >)
> >  
> >  and is_selinux_guest g =
> > @@ -59,6 +68,21 @@ and use_setfiles g =
> >g#aug_load ();
> >debug_augeas_errors g;
> >  
> > +  (* Get the SELinux enforcing mode, eg "enforcing", "permissive",
> > +   * "disabled".
> > +   * Use "disabled" if not specified, just like libselinux seems to do.
> > +   *)
> > +  let typ = read_selinux_config_key g "SELINUX" "disabled" in
> > +  (* Do not attempt any relabelling if the SELinux is not "enforcing":
> > +   * - in "permissive" mode SELinux is still running, however nothing is
> > +   *   enforced: this means labels can be wrong, and "it is fine"
> 
> I don't think it's fine.  As I showed here:
> 
> https://www.redhat.com/archives/libguestfs/2020-June/msg00115.html
> 
> in permissive mode labels are still being updated on disk.

This is true for default labels, yes.

> TBH I don't understand what you said here:
> 
> https://www.redhat.com/archives/libguestfs/2020-June/msg00117.html
> 
> about "both the labels and the policy may be all wrong".  If the
> administrator set the policy to permissive then labels ought still to
> be updated when the guest is running, and we ought to try to keep them
> updated if we can in v2v.

There are various cases when, even of an enforcing system, labels are
not kept up-to-date:

$ getenforce 
Enforcing
$ touch /tmp/test
$ ls -lZ /tmp/test 
-rw-rw-r--. 1 ptoscano ptoscano unconfined_u:object_r:user_tmp_t:s0 0 Sep 24 
12:26 /tmp/test
$ mv /tmp/test ~/var/
$ ls -lZ ~/var/test 
-rw-rw-r--. 1 ptoscano ptoscano unconfined_u:object_r:user_tmp_t:s0 0 Sep 24 
12:26 /home/ptoscano/var/test
$ restorecon -v ~/var/test 
Relabeled /home/ptoscano/var/test from unconfined_u:object_r:user_tmp_t:s0 to 
unconfined_u:object_r:user_home_t:s0
$ ls -lZ ~/var/test 
-rw-rw-r--. 1 ptoscano ptoscano unconfined_u:object_r:user_home_t:s0 0 Sep 24 
12:26 /home/ptoscano/var/test

Considering that /tmp is a general location for temporary files, it's
common that files may end with a tmp_t-alike label when moved back to
the destination place (e.g. after a rename()). That is not the only
situation like this that I saw in the past.

In permissive mode, all these situation are logged in the audit log,
yes, but they cause no blocks nor errors.

> It's also fine for an administrator to
> switch a system to permissive and then back to enforcing without
> relabelling or rebooting.

A mislabelled /etc/passwd is still read and used fine in permissive
mode. Switch back from permissive to enforcing without a relabelling
is generally not a good idea, especially after the system ran for a
lot of time after the switch to permissive.

-- 
Pino Toscano

signature.asc
Description: This is a digitally signed message part.
___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs

[Libguestfs] [common PATCH 3/3] mlcustomize: do not relabel if not enforcing (RHBZ#1828952)

2020-09-23 Thread Pino Toscano
Do not attempt to relabel a guest in case its SELinux enforcing mode is
not "enforcing", as it is either pointless, or it may fail because of an
invalid policy configured.
---
 mlcustomize/SELinux_relabel.ml | 26 +-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/mlcustomize/SELinux_relabel.ml b/mlcustomize/SELinux_relabel.ml
index 647aeda..db00e59 100644
--- a/mlcustomize/SELinux_relabel.ml
+++ b/mlcustomize/SELinux_relabel.ml
@@ -24,6 +24,9 @@ open Printf
 
 module G = Guestfs
 
+exception SELinux_not_enforcing
+(* Interal exception to signal a non-enforcing SELinux. *)
+
 (* Simple reimplementation of Array.mem, available only with OCaml >= 4.03. *)
 let array_find a l =
   List.mem a (Array.to_list l)
@@ -35,12 +38,18 @@ let rec relabel (g : G.guestfs) =
   use_setfiles g;
   (* That worked, so we don't need to autorelabel. *)
   g#rm_f "/.autorelabel"
-with Failure _ ->
+with
+| Failure _ ->
   (* This is the fallback in case something in the setfiles
* method didn't work.  That includes the case where a non-SELinux
* host is processing an SELinux guest, and other things.
*)
   g#touch "/.autorelabel"
+| SELinux_not_enforcing ->
+  (* This means that SELinux was not configured to be in enforcing mode,
+   * so silently accept this.
+   *)
+  ()
   )
 
 and is_selinux_guest g =
@@ -59,6 +68,21 @@ and use_setfiles g =
   g#aug_load ();
   debug_augeas_errors g;
 
+  (* Get the SELinux enforcing mode, eg "enforcing", "permissive",
+   * "disabled".
+   * Use "disabled" if not specified, just like libselinux seems to do.
+   *)
+  let typ = read_selinux_config_key g "SELINUX" "disabled" in
+  (* Do not attempt any relabelling if the SELinux is not "enforcing":
+   * - in "permissive" mode SELinux is still running, however nothing is
+   *   enforced: this means labels can be wrong, and "it is fine"
+   * - when "disabled" means SELinux is not running, so any relabelling
+   *   is pointless (other than potentially fail due to an invalid
+   *   SELINUXTYPE configuration)
+   *)
+  if typ <> "enforcing" then
+raise SELinux_not_enforcing;
+
   (* Get the SELinux policy name, eg. "targeted", "minimum".
* Use "targeted" if not specified, just like libselinux does.
*)
-- 
2.26.2

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs



[Libguestfs] [common PATCH 2/3] mlcustomize: refactor reading from /etc/selinux/config

2020-09-23 Thread Pino Toscano
Move the reading of values from /etc/selinux/config to an own function,
so it can be easily reused.

This is a simple refactoring.
---
 mlcustomize/SELinux_relabel.ml | 18 ++
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/mlcustomize/SELinux_relabel.ml b/mlcustomize/SELinux_relabel.ml
index 5df1f08..647aeda 100644
--- a/mlcustomize/SELinux_relabel.ml
+++ b/mlcustomize/SELinux_relabel.ml
@@ -62,14 +62,7 @@ and use_setfiles g =
   (* Get the SELinux policy name, eg. "targeted", "minimum".
* Use "targeted" if not specified, just like libselinux does.
*)
-  let policy =
-let config_path = "/files/etc/selinux/config" in
-let selinuxtype_path = config_path ^ "/SELINUXTYPE" in
-let keys = g#aug_ls config_path in
-if array_find selinuxtype_path keys then
-  g#aug_get selinuxtype_path
-else
-  "targeted" in
+  let policy = read_selinux_config_key g "SELINUXTYPE" "targeted" in
 
   g#aug_close ();
 
@@ -99,3 +92,12 @@ and use_setfiles g =
 
   (* Relabel everything. *)
   g#selinux_relabel ~force:true specfile "/"
+
+and read_selinux_config_key g key defval =
+  let config_path = "/files/etc/selinux/config" in
+  let key_path = config_path ^ "/" ^ key in
+  let keys = g#aug_ls config_path in
+  if array_find key_path keys then
+g#aug_get key_path
+  else
+defval
-- 
2.26.2

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs



[Libguestfs] [common PATCH 1/3] mlcustomize: Refactor SELinux_relabel code.

2020-09-23 Thread Pino Toscano
From: "Richard W.M. Jones" 

This shouldn't change the effect of this code.
---
 mlcustomize/SELinux_relabel.ml | 121 ++---
 1 file changed, 65 insertions(+), 56 deletions(-)

diff --git a/mlcustomize/SELinux_relabel.ml b/mlcustomize/SELinux_relabel.ml
index 44995df..5df1f08 100644
--- a/mlcustomize/SELinux_relabel.ml
+++ b/mlcustomize/SELinux_relabel.ml
@@ -28,65 +28,74 @@ module G = Guestfs
 let array_find a l =
   List.mem a (Array.to_list l)
 
-let relabel (g : G.guestfs) =
-  (* Is the guest using SELinux? *)
-  if g#is_file ~followsymlinks:true "/usr/sbin/load_policy" &&
- g#is_file ~followsymlinks:true "/etc/selinux/config" then (
-(* Is setfiles / SELinux relabelling functionality available? *)
-if g#feature_available [| "selinuxrelabel" |] then (
-  (* Use Augeas to parse /etc/selinux/config. *)
-  g#aug_init "/" (16+32) (* AUG_SAVE_NOOP | AUG_NO_LOAD *);
-  (* See: https://bugzilla.redhat.com/show_bug.cgi?id=975412#c0 *)
-  ignore (g#aug_rm "/augeas/load/*[\"/etc/selinux/config/\" !~ regexp('^') 
+ glob(incl) + regexp('/.*')]");
-  g#aug_load ();
-  debug_augeas_errors g;
-
-  (* Get the SELinux policy name, eg. "targeted", "minimum".
-   * Use "targeted" if not specified, just like libselinux does.
+let rec relabel (g : G.guestfs) =
+  (* Is the guest using SELinux?  (Otherwise this is a no-op). *)
+  if is_selinux_guest g then (
+try
+  use_setfiles g;
+  (* That worked, so we don't need to autorelabel. *)
+  g#rm_f "/.autorelabel"
+with Failure _ ->
+  (* This is the fallback in case something in the setfiles
+   * method didn't work.  That includes the case where a non-SELinux
+   * host is processing an SELinux guest, and other things.
*)
-  let policy =
-let config_path = "/files/etc/selinux/config" in
-let selinuxtype_path = config_path ^ "/SELINUXTYPE" in
-let keys = g#aug_ls config_path in
-if array_find selinuxtype_path keys then
-  g#aug_get selinuxtype_path
-else
-  "targeted" in
+  g#touch "/.autorelabel"
+  )
 
-  g#aug_close ();
+and is_selinux_guest g =
+  g#is_file ~followsymlinks:true "/usr/sbin/load_policy" &&
+  g#is_file ~followsymlinks:true "/etc/selinux/config"
 
-  (* Get the spec file name. *)
-  let specfile =
-sprintf "/etc/selinux/%s/contexts/files/file_contexts" policy in
+and use_setfiles g =
+  (* Is setfiles / SELinux relabelling functionality available? *)
+  if not (g#feature_available [| "selinuxrelabel" |]) then
+failwith "no selinux relabel feature";
 
-  (* RHEL 6.2 - 6.5 had a malformed specfile that contained the
-   * invalid regular expression "/var/run/spice-vdagentd.\pid"
-   * (instead of "\.p").  This stops setfiles from working on
-   * the guest.
-   *
-   * Because an SELinux relabel writes all over the filesystem,
-   * it seems reasonable to fix this problem in the specfile
-   * at the same time.  (RHBZ#1374232)
-   *)
-  if g#grep ~fixed:true "vdagentd.\\pid" specfile <> [||] then (
-debug "fixing invalid regular expression in %s" specfile;
-let old_specfile = specfile ^ "~" in
-g#mv specfile old_specfile;
-let content = g#read_file old_specfile in
-let content =
-  String.replace content "vdagentd.\\pid" "vdagentd\\.pid" in
-g#write specfile content;
-g#copy_attributes ~all:true old_specfile specfile
-  );
+  (* Use Augeas to parse /etc/selinux/config. *)
+  g#aug_init "/" (16+32) (* AUG_SAVE_NOOP | AUG_NO_LOAD *);
+  (* See: https://bugzilla.redhat.com/show_bug.cgi?id=975412#c0 *)
+  ignore (g#aug_rm "/augeas/load/*[\"/etc/selinux/config/\" !~ regexp('^') + 
glob(incl) + regexp('/.*')]");
+  g#aug_load ();
+  debug_augeas_errors g;
 
-  (* Relabel everything. *)
-  g#selinux_relabel ~force:true specfile "/";
+  (* Get the SELinux policy name, eg. "targeted", "minimum".
+   * Use "targeted" if not specified, just like libselinux does.
+   *)
+  let policy =
+let config_path = "/files/etc/selinux/config" in
+let selinuxtype_path = config_path ^ "/SELINUXTYPE" in
+let keys = g#aug_ls config_path in
+if array_find selinuxtype_path keys then
+  g#aug_get selinuxtype_path
+else
+  "targeted" in
 
-  (* If that worked, we don't need to autorelabel. *)
-  g#rm_f "/.autorelabel"
-)
-else (
-  (* SELinux guest, but not SELinux host.  Fallback to this. *)
-  g#touch "/.autorelabel"
-)
-  )
+  g#aug_close ();
+
+  (* Get the spec file name. *)
+  let specfile =
+sprintf "/etc/selinux/%s/contexts/files/file_contexts" policy in
+
+  (* RHEL 6.2 - 6.5 had a malformed specfile that contained the
+   * invalid regular expression "/var/run/spice-vdagentd.\pid"
+   * (instead of "\.p").  This stops setfiles from working on
+   * the guest.
+   *
+   * Because an SELinux relabel writes all 

[Libguestfs] [common PATCH 0/3] SELinux_relabel: relabel only if enforcing (RHBZ#1828952)

2020-09-23 Thread Pino Toscano
Continuation/rework of:
https://www.redhat.com/archives/libguestfs/2020-May/msg00020.html

This is my approach, as I explained here:
https://bugzilla.redhat.com/show_bug.cgi?id=1828952#c4
https://www.redhat.com/archives/libguestfs/2020-May/msg00035.html
IOW: do not attempt to relabel if the guest is not enforcing, as it is
either useless or may fail; few words more are in the comments of patch
#3.

Pino Toscano (2):
  mlcustomize: refactor reading from /etc/selinux/config
  mlcustomize: do not relabel if not enforcing (RHBZ#1828952)

Richard W.M. Jones (1):
  mlcustomize: Refactor SELinux_relabel code.

 mlcustomize/SELinux_relabel.ml | 153 -
 1 file changed, 94 insertions(+), 59 deletions(-)

-- 
2.26.2

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs



[Libguestfs] [v2v PATCH 1/3] linux: remove warning for packages with no files

2020-09-23 Thread Pino Toscano
Metapackages are valid packages with no files, used to easily install
something without manually installing bits.

This is the case of the "kernel" package in Fedora/RHEL/etc in the last
couple of years.
---
 v2v/linux_kernels.ml | 1 -
 1 file changed, 1 deletion(-)

diff --git a/v2v/linux_kernels.ml b/v2v/linux_kernels.ml
index 9a41225a..78c1ee59 100644
--- a/v2v/linux_kernels.ml
+++ b/v2v/linux_kernels.ml
@@ -107,7 +107,6 @@ let detect_kernels (g : G.guestfs) inspect family 
bootloader =
let files = Linux.file_list_of_package g inspect app in
 
if files = [] then (
- warning (f_"package ‘%s’ contains no files") name;
  None
)
else (
-- 
2.26.2

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs

[Libguestfs] [v2v PATCH 3/3] linux: remove special handling of packages with no files

2020-09-23 Thread Pino Toscano
We check for /boot/vmlinuz-* files in packages with files, and List.find
will raise Not_found if there are none. Iterating on an empty list gives
the same result, so there is no need to handle empty packages in a
special way.
---
 v2v/linux_kernels.ml | 321 +--
 1 file changed, 158 insertions(+), 163 deletions(-)

diff --git a/v2v/linux_kernels.ml b/v2v/linux_kernels.ml
index 78c1ee59..7e171eae 100644
--- a/v2v/linux_kernels.ml
+++ b/v2v/linux_kernels.ml
@@ -106,181 +106,176 @@ let detect_kernels (g : G.guestfs) inspect family 
bootloader =
(* For each kernel, list the files directly owned by the kernel. *)
let files = Linux.file_list_of_package g inspect app in
 
-   if files = [] then (
- None
-   )
-   else (
- (* Which of these is the kernel itself?  Also, make sure to check
-  * it exists by stat'ing it.
-  *)
- let vmlinuz = List.find (
-   fun filename -> String.is_prefix filename "/boot/vmlinuz-"
- ) files in
- let vmlinuz_stat =
-   try g#statns vmlinuz with G.Error _ -> raise Not_found in
+   (* Which of these is the kernel itself?  Also, make sure to check
+* it exists by stat'ing it.
+*)
+   let vmlinuz = List.find (
+ fun filename -> String.is_prefix filename "/boot/vmlinuz-"
+   ) files in
+   let vmlinuz_stat =
+ try g#statns vmlinuz with G.Error _ -> raise Not_found in
 
- (* Determine the modpath from the package, falling back to the
-  * version in the vmlinuz file name.
-  *)
- let modpath, version =
-   let prefix = "/lib/modules/" in
-   try
- let prefix_len = String.length prefix in
- List.find_map (
-   fun filename ->
- let filename_len = String.length filename in
- if filename_len > prefix_len &&
-String.is_prefix filename prefix then (
-   let version = String.sub filename prefix_len
-(filename_len - prefix_len) in
-   Some (filename, version)
- ) else
-   None
- ) files
-   with Not_found ->
- let version =
-   String.sub vmlinuz 14 (String.length vmlinuz - 14) in
- let modpath = prefix ^ version in
- modpath, version in
+   (* Determine the modpath from the package, falling back to the
+* version in the vmlinuz file name.
+*)
+   let modpath, version =
+ let prefix = "/lib/modules/" in
+ try
+   let prefix_len = String.length prefix in
+   List.find_map (
+ fun filename ->
+   let filename_len = String.length filename in
+   if filename_len > prefix_len &&
+  String.is_prefix filename prefix then (
+ let version = String.sub filename prefix_len
+  (filename_len - prefix_len) in
+ Some (filename, version)
+   ) else
+ None
+   ) files
+ with Not_found ->
+   let version =
+ String.sub vmlinuz 14 (String.length vmlinuz - 14) in
+   let modpath = prefix ^ version in
+   modpath, version in
 
- (* Check that the modpath exists. *)
- if not (g#is_dir ~followsymlinks:true modpath) then
-   raise Not_found;
+   (* Check that the modpath exists. *)
+   if not (g#is_dir ~followsymlinks:true modpath) then
+ raise Not_found;
 
- (* Find the initramfs which corresponds to the kernel.
-  * Since the initramfs is built at runtime, and doesn't have
-  * to be covered by the RPM file list, this is basically
-  * guesswork.
+   (* Find the initramfs which corresponds to the kernel.
+* Since the initramfs is built at runtime, and doesn't have
+* to be covered by the RPM file list, this is basically
+* guesswork.
+*)
+   let initrd =
+ let files = g#ls "/boot" in
+ let files = Array.to_list files in
+ let files =
+   List.filter (fun n -> PCRE.matches rex_initrd n) files in
+ let files =
+   List.filter (
+ fun n ->
+   String.find n version >= 0
+   ) files in
+ (* Don't consider kdump initramfs images (RHBZ#1138184). *)
+ let files =
+   List.filter (fun n -> 

[Libguestfs] [v2v PATCH 2/3] linux: avoid bogus "file" in empty RPM packages

2020-09-23 Thread Pino Toscano
RPM writes "(contains no files)" to stdout for packages with no files,
and thus we consider that string as single file in such packages.
Workaround this locally, so we do not get bogus files in the file
listing of RPMs with no files.
---
 v2v/linux.ml | 12 ++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/v2v/linux.ml b/v2v/linux.ml
index 6fd0eb50..5496d056 100644
--- a/v2v/linux.ml
+++ b/v2v/linux.ml
@@ -123,8 +123,16 @@ let file_list_of_package (g : Guestfs.guestfs) inspect app 
=
 let cmd = [| "rpm"; "-ql"; pkg_name |] in
 debug "%s" (String.concat " " (Array.to_list cmd));
 let files = g#command_lines cmd in
-let files = Array.to_list files in
-List.sort compare files
+(* RPM prints "(contains no files)" on stdout when a package
+ * has no files in it:
+ * https://github.com/rpm-software-management/rpm/issues/962
+ *)
+if files = [| "(contains no files)" |] then
+  []
+else (
+  let files = Array.to_list files in
+  List.sort compare files
+)
 
   | format ->
 error (f_"don’t know how to get list of files from package using %s")
-- 
2.26.2

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs

[Libguestfs] [v2v PATCH 1/2] linux: split kernel packages filtering from processing

2020-09-22 Thread Pino Toscano
Split the processing of the kernel packages in two phases:
- filtering only (by name)
- actual processing

This makes the filtering part easier to review/modify, and it is now
much easier to see (in the debug log) which packages are processed as
kernel packages.

There are no behaviour changes (other than an additional debug message).
---
 v2v/linux_kernels.ml | 20 +---
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/v2v/linux_kernels.ml b/v2v/linux_kernels.ml
index 1bc10948..dc0c285d 100644
--- a/v2v/linux_kernels.ml
+++ b/v2v/linux_kernels.ml
@@ -89,11 +89,19 @@ let detect_kernels (g : G.guestfs) inspect family 
bootloader =
 PCRE.compile "^initrd.img-.*$"
   else
 PCRE.compile "^initr(?:d|amfs)-.*(?:\\.img)?$" in
+let kernel_pkgs = List.filter (
+  fun { G.app2_name = name } ->
+name = "kernel"
+  || String.is_prefix name "kernel-"
+  || String.is_prefix name "linux-image-"
+) inspect.i_apps in
+if verbose () then (
+  let names = List.map (fun { G.app2_name = name } -> name) kernel_pkgs in
+  eprintf "candidate kernel packages in this guest: %s%!\n"
+(String.concat " " names)
+);
 List.filter_map (
-  function
-  | { G.app2_name = name } as app
-  when name = "kernel" || String.is_prefix name "kernel-"
-   || String.is_prefix name "linux-image-" ->
+  fun ({ G.app2_name = name } as app) ->
 (try
(* For each kernel, list the files directly owned by the kernel. *)
let files = Linux.file_list_of_package g inspect app in
@@ -277,9 +285,7 @@ let detect_kernels (g : G.guestfs) inspect family 
bootloader =
 
  with Not_found -> None
 )
-
-  | _ -> None
-) inspect.i_apps in
+) kernel_pkgs in
 
   if verbose () then (
 eprintf "installed kernel packages in this guest:\n";
-- 
2.26.2

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs



[Libguestfs] [v2v PATCH 2/2] linux: ignore -devel kernel packages

2020-09-22 Thread Pino Toscano
They usually contain only the sources of the kernel, useful to build
kernel modules.
---
 v2v/linux_kernels.ml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/v2v/linux_kernels.ml b/v2v/linux_kernels.ml
index dc0c285d..9a41225a 100644
--- a/v2v/linux_kernels.ml
+++ b/v2v/linux_kernels.ml
@@ -92,7 +92,7 @@ let detect_kernels (g : G.guestfs) inspect family bootloader =
 let kernel_pkgs = List.filter (
   fun { G.app2_name = name } ->
 name = "kernel"
-  || String.is_prefix name "kernel-"
+  || (String.is_prefix name "kernel-" && not (String.is_suffix name 
"-devel"))
   || String.is_prefix name "linux-image-"
 ) inspect.i_apps in
 if verbose () then (
-- 
2.26.2

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs



[Libguestfs] [PATCH] build: fix includedir in uninstalled libguestfs.pc

2020-09-22 Thread Pino Toscano
Update includedir with the new directory that contains guestfs.h.

Updates commit 75abec1f706e555cd6c9915be03c732b56a94596.
---
 lib/local/libguestfs.pc.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/local/libguestfs.pc.in b/lib/local/libguestfs.pc.in
index 129682be8..46cef1e16 100644
--- a/lib/local/libguestfs.pc.in
+++ b/lib/local/libguestfs.pc.in
@@ -26,7 +26,7 @@
 prefix=@abs_top_builddir@
 exec_prefix=@abs_top_builddir@
 libdir=@abs_top_builddir@/lib/.libs
-includedir=@abs_top_srcdir@/lib
+includedir=@abs_top_srcdir@/include
 
 Name: libguestfs
 Version: @VERSION@
-- 
2.26.2

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs



Re: [Libguestfs] [PATCH v2v] v2v: Set the number of vCPUs to same as host number of pCPUs.

2020-09-18 Thread Pino Toscano
On Friday, 18 September 2020 11:44:04 CEST Richard W.M. Jones wrote:
>let g = open_guestfs ~identifier:"v2v" () in
>g#set_memsize (g#get_memsize () * 14 / 5);
> +  (* Setting the number of vCPUs allows parallel mkinitrd. *)
> +  g#set_smp (Sysconf.nr_processors_online ());

IMHO this is not a good idea, for few reasons:

a) it unconditionally uses all the available CPUs with no way to change
it to any other value

b) virt-v2v is run also on systems that are not specifically dedicated
as conversion servers, for example the oVirt hosts (that run the oVirt
VMs); in this context, suddently using more CPUs than requested may
saturate the system resources

IMHO the right approach is to add a --smp N option like available in
other libguestfs tools; this way, the user can decide how many
resources virt-v2v can use.

-- 
Pino Toscano

signature.asc
Description: This is a digitally signed message part.
___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [PATCH v2 7/7] lib/canonical-name.c: Hide errors.

2020-09-17 Thread Pino Toscano
On Monday, 7 September 2020 11:44:00 CEST Richard W.M. Jones wrote:
> Fixes “XXX” comment.  This turns out to be necessary in order to
> suppress a warning when inspecting Windows BitLocker-encrypted guests.
> 
> The warning (which still appears in debugging output even with this
> patch) is:
> 
>   libguestfs: error: lvm_canonical_lv_name: /dev/mapper/cryptsda2: not a 
> logical volume
> ---
>  lib/canonical-name.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/canonical-name.c b/lib/canonical-name.c
> index 052bbf12c..11cf6fed6 100644
> --- a/lib/canonical-name.c
> +++ b/lib/canonical-name.c
> @@ -46,8 +46,9 @@ guestfs_impl_canonical_device_name (guestfs_h *g, const 
> char *device)
>}
>else if (STRPREFIX (device, "/dev/mapper/") ||
> STRPREFIX (device, "/dev/dm-")) {
> -/* XXX hide errors */
> +guestfs_push_error_handler (g, NULL, NULL);
>  ret = guestfs_lvm_canonical_lv_name (g, device);
> +guestfs_pop_error_handler (g);

Instead of ignoring all the errors from lvm_canonical_lv_name, isn't
there a way to avoid getting into this situation in the first place?
Right now it is not ignored, so if anything fails we can immediately
notice it, which won't happen anymore with the proposed change.

-- 
Pino Toscano

signature.asc
Description: This is a digitally signed message part.
___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [PATCH v2 1/7] New APIs: cryptsetup-open and cryptsetup-close.

2020-09-17 Thread Pino Toscano
On Monday, 7 September 2020 11:43:54 CEST Richard W.M. Jones wrote:
> This commit deprecates luks-open/luks-open-ro/luks-close for the more
> generic sounding names cryptsetup-open/cryptsetup-close, which also
> correspond directly to the cryptsetup commands.
> 
> The optional cryptsetup-open readonly flag is used to replace the
> functionality of luks-open-ro.
> 
> The optional cryptsetup-open crypttype parameter can be used to select
> the type (corresponding to cryptsetup open --type), which allows us to
> open BitLocker-encrypted disks with no extra effort.  As a convenience
> the crypttype parameter may be omitted, and libguestfs will use a
> heuristic (based on vfs-type output) to try to determine the correct
> type to use.

At least in my (non extensive) tests with cryptsetup, it seems it can
detect the right format even without --type=format or the luksOpen/etc
aliases.

What I'd do is:
- drop the autodetection: it is a mild duplication of what cryptsetup
  already does, and adding it in the API now means that it will be
  stuck there...
- maybe (need to think about it) make the crypttype parameter mandatory,
  so the user has to select the right format

> The deprecated functions and the new functions are all (re-)written in
> OCaml.

Regarding the deprecated functions: wouldn't it be better to have them
in the library, rather than in the daemon? The machinery needed in the
library is more or less than same anyway, and this way we can remove
two RPCs in the daemon.

Also, please bump the versions of the new APIs.

> +let cryptsetup_open ?(readonly = false) ?crypttype device key mapname =
> +  (* /dev/mapper/mapname must not exist already. *)

  /* Sanity check: /dev/mapper/mapname must not exist already.  Note
   * that the device-mapper control device (/dev/mapper/control) is
   * always there, so you can't ever have mapname == "control".
   */

This whole comment seems useful, it would be a pity to lose it in the
conversion. Also, unrelated to this patch: IMHO it would be a good idea
to explicitly note this limitation in the API docs.

> +  (* Write the key to a temporary file. *)
> +  let keyfile, chan = Filename.open_temp_file "crypt" ".key" in
> +  fchmod (descr_of_out_channel chan) 0o600;

This fchmod is not needed, as temporary files are already 600 by
default.

> +  (* Make sure we always remove the temporary file. *)
> +  protect ~f:(
> +fun () ->
> +  let args = ref [] in
> +  List.push_back_list args ["-d"; keyfile];
> +  if readonly then List.push_back args "--readonly";
> +  List.push_back_list args ["open"; device; mapname; "--type"; 
> crypttype];

Minor nit: even if this way makes sense more from a scope POV, I'd
instead build the args list out of the protect. Mostly for ease of
reading.

> +(* Deprecated APIs for backwards compatibility.
> + *
> + * For convenient for existing callers we do not specify the
> + * crypttype parameter.  This means that callers of these APIs
> + * will be able to open BitLocker drives transparently.  (If
> + * we wanted to be strict we would pass ~crypttype:"luks"
> + * here, but that seems to have only downsides).

I disagree here: if an user uses the luks_open API, IMHO they expect it
to be LUKS; if they want to use other types, then they want to switch
to the new cryptsetup_open. So I'd pass ~crypttype:"luks" in luks_open.

> +  { defaults with
> +name = "cryptsetup_open"; added = (1, 43, 1);
> +style = RErr, [String (Device, "device"); String (Key, "key"); String 
> (PlainString, "mapname")], [OBool "readonly"; OString "crypttype"];
> +impl = OCaml "Cryptsetup.cryptsetup_open";
> +optional = Some "luks";
> +test_excuse = "no way to format BitLocker, and smallest device is huge";
> +shortdesc = "open an encrypted block device";
> +longdesc = "\
> +This command opens a block device which has been encrypted
> +according to the Linux Unified Key Setup (LUKS) standard,
> +Windows BitLocker, or some other types.
> +
> +C is the encrypted block device or partition.
> +
> +The caller must supply one of the keys associated with the
> +encrypted block device, in the C parameter.
> +
> +This creates a new block device called F.
> +Reads and writes to this block device are decrypted from and
> +encrypted to the underlying C respectively.
> +
> +If the optional C parameter is not present then
> +libguestfs tries to guess the correct type (for example
> +LUKS or BitLocker).  However you can override this by
> +specifying one of the following types:
> +
> +=over 4
> +
> +=item C
> +
> +A Linux LUKS device.
> +
> +=item C
> +
> +A Windows BitLocker device.
> +
> +=back

Maybe add:

  Please refer to the L documentation for all the
  supported types.

-- 
Pino Toscano

signature.asc
Description: This is a digitally signed message part.
___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [supermin PATCH] rpm: check for SQLite-based RPM DB

2020-08-26 Thread Pino Toscano
On Wednesday, 26 August 2020 21:15:36 CEST Richard W.M. Jones wrote:
> On Wed, Aug 26, 2020 at 06:57:43PM +0200, Pino Toscano wrote:
> > Fedora 33 switched the DB of RPM to SQLite, so no more Packages/Name/etc
> > files. Because any missing file exception is ignored when checking for
> > --if-newer, the lack of the Package files was not properly noticed, so
> > the appliance was always considered out out date.
> > 
> > Check for the SQLite-based DB first, falling back to the old format.
> > 
> > Signed-off-by: Pino Toscano 
> > ---
> >  src/ph_rpm.ml | 5 -
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> > 
> > diff --git a/src/ph_rpm.ml b/src/ph_rpm.ml
> > index dbe3bda..0821126 100644
> > --- a/src/ph_rpm.ml
> > +++ b/src/ph_rpm.ml
> > @@ -234,7 +234,10 @@ let rpm_package_name pkg =
> >rpm.name
> >  
> >  let rpm_get_package_database_mtime () =
> > -  (lstat "/var/lib/rpm/Packages").st_mtime
> > +  try
> > +(lstat "/var/lib/rpm/rpmdb.sqlite").st_mtime
> > +  with Unix_error (ENOENT, _, _) ->
> > +(lstat "/var/lib/rpm/Packages").st_mtime
> >  
> >  (* Return the best provider of a particular RPM requirement.
> 
> Yes this looks fine, ACK.  If you really wanted you could factor out
> the (...).st_mtime, something like:
> 
>   (try
> lstat "/var/lib/rpm/rpmdb.sqlite"
>with Unix_error (ENOENT, _, _) ->
> lstat "/var/lib/rpm/Packages"
>).st_mtime

ACK. Sadly List.find_map is OCaml 4.10.0+ only.

Should I backport it to stable-5.2?

-- 
Pino Toscano

signature.asc
Description: This is a digitally signed message part.
___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs

[Libguestfs] [supermin PATCH] rpm: check for SQLite-based RPM DB

2020-08-26 Thread Pino Toscano
Fedora 33 switched the DB of RPM to SQLite, so no more Packages/Name/etc
files. Because any missing file exception is ignored when checking for
--if-newer, the lack of the Package files was not properly noticed, so
the appliance was always considered out out date.

Check for the SQLite-based DB first, falling back to the old format.

Signed-off-by: Pino Toscano 
---
 src/ph_rpm.ml | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/ph_rpm.ml b/src/ph_rpm.ml
index dbe3bda..0821126 100644
--- a/src/ph_rpm.ml
+++ b/src/ph_rpm.ml
@@ -234,7 +234,10 @@ let rpm_package_name pkg =
   rpm.name
 
 let rpm_get_package_database_mtime () =
-  (lstat "/var/lib/rpm/Packages").st_mtime
+  try
+(lstat "/var/lib/rpm/rpmdb.sqlite").st_mtime
+  with Unix_error (ENOENT, _, _) ->
+(lstat "/var/lib/rpm/Packages").st_mtime
 
 (* Return the best provider of a particular RPM requirement.
  *
-- 
2.26.2

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs



Re: [Libguestfs] [PATCH v2v] Add ALT support

2020-08-25 Thread Pino Toscano
On Monday, 24 August 2020 19:48:07 CEST Richard W.M. Jones wrote:
> From: Mikhail Gordeev 
> 

It would be nicer to have few more details in the commit message
and/or comments in the code, as there are things that definitely
deserve explanations.

Also, IMHO, it would be nice to have it split in different parts:
1) a simple patch that adds the ALT_family, in case it is needed (see
   below why IMHO it might not be)
2) a simple patch adding the distro in the register_convert_module
   matching function, and mentioning it in the documentation
3) the patch for the initrd work needed
4) the patch (if needed, see below) for the vmware tools libraries

Also, in patch (2) could go also the addition of ALT Linux to
docs/virt-v2v-support.pod.

> @@ -372,6 +373,9 @@ let convert (g : G.guestfs) inspect source_disks output 
> rcaps _ =
>  );
>  ) libraries
>)
> +  else if family = `ALT_family then (
> +remove := libraries
> +  )

The "if" of this else is:

  (* We only support removal of libraries on systems which use yum. *)
  if inspect.i_package_management = "yum" then (

This is done to resolve the packages that provide the installed VMware
libraries. The library packages are found earlier:

if String.is_prefix name "vmware-tools-libraries-" then
  List.push_front name libraries

So any package that starts with "vmware-tools-libraries-". From what I
remember, this pattern was used by very old packages provided by VMware
in their repository, and most probably even newer packages are not
using them anymore (in favour of vmware-tools).

The question is: do VMware tools package exist on ALT Linux? If so,
what are their package names? I don't think that VMware provide
packages for ALT, leaving only the open-vm-tools ones as possible.
Hence, unless strictly needed, I'd rather not include the above part.

> @@ -657,6 +661,50 @@ let convert (g : G.guestfs) inspect source_disks output 
> rcaps _ =
>  
>  run_update_initramfs_command ()
>)
> +  else if family = `ALT_family then (
> +(* ALT utilities to work with initrd does not support adding modules
> + * to an existing initrd
> + *)

So how are users supposed to include modules they want? Recreating the
initrd manually every time?

What about https://en.altlinux.org/Make-initrd ?

> +let kver = kernel.ki_version in
> +let kmodpath = kernel.ki_modpath in
> +
> +(* find module files *)
> +let files = g#find kmodpath |> Array.to_list in
> +let test f =
> +  let r m = sprintf ".*/%s\\.ko$" m |> Str.regexp in
> +  let rmodules = List.map r modules in
> +  let fold_f acc mr = acc || Str.string_match mr f 0 in
> +  List.fold_left fold_f false rmodules in
> +let files = List.filter test files in

kernel.ki_modules contains already a list of modules for that kernel,
so why are you searching for them again?

> +(* create new initrd with contents of old initrd *)
> +let tmpdir = g#mkdtemp "/tmp/new_initrd-XX" in
> +let old_initrd = initrd ^ ".pre-v2v" in
> +let cmd = sprintf "gzip -cd %s | cpio -imd --quiet -D %s"
> +  old_initrd tmpdir in
> +ignore (g#sh cmd);
> +
> +(* copy module files to new initrd *)
> +List.iter (fun file ->
> +  let dir_name = Filename.dirname file in
> +  let dir_name = sprintf "%s%s%s" tmpdir kmodpath dir_name in
> +  g#mkdir_p dir_name;
> +      g#cp (sprintf "%s/%s" kmodpath file) dir_name
> +) files;

So we are creating an initrd with all the modules of the kernel?
Is there really no way to just add the required modules for booting?

Thanks,
-- 
Pino Toscano

signature.asc
Description: This is a digitally signed message part.
___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] Running libguestfs-test-tool problem

2020-08-24 Thread Pino Toscano
On Monday, 24 August 2020 20:53:09 CEST Zachary Taylor wrote:
> Here is the output from running libguestfs-test-tool:
> 
> PATH=/home/zmt1002/.local/bin:/usr/local/bin:/usr/bin:/bin:/usr/local/sbin:/usr/lib/jvm/default/bin:/usr/bin/site_perl:/usr/bin/vendor_perl:/usr/bin/core_perl:/var/lib/snapd/snap/bin
> SELinux: sh: getenforce: command not found
> guestfs_get_append: (null)
> guestfs_get_autosync: 1
> guestfs_get_backend: direct
> guestfs_get_backend_settings: []
> guestfs_get_cachedir: /var/tmp
> guestfs_get_hv: /usr/bin/qemu-system-x86_64
> guestfs_get_memsize: 768
> guestfs_get_network: 0
> guestfs_get_path: /usr/lib/guestfs
> guestfs_get_pgroup: 0
> guestfs_get_program: libguestfs-test-tool
> guestfs_get_recovery_proc: 1
> guestfs_get_smp: 1
> guestfs_get_sockdir: /tmp
> guestfs_get_tmpdir: /tmp
> guestfs_get_trace: 0
> guestfs_get_verbose: 1
> host_cpu: x86_64
> Launching appliance, timeout set to 600 seconds.
> libguestfs: launch: program=libguestfs-test-tool
> libguestfs: launch: version=1.42.0
> libguestfs: launch: backend registered: unix
> libguestfs: launch: backend registered: uml
> libguestfs: launch: backend registered: libvirt
> libguestfs: launch: backend registered: direct
> libguestfs: launch: backend=direct
> libguestfs: launch: tmpdir=/tmp/libguestfsR3UAGS
> libguestfs: launch: umask=0022
> libguestfs: launch: euid=0
> libguestfs: begin building supermin appliance
> libguestfs: run supermin
> libguestfs: command: run: /usr/bin/supermin
> libguestfs: command: run: \ --build
> libguestfs: command: run: \ --verbose
> libguestfs: command: run: \ --if-newer
> libguestfs: command: run: \ --lock /var/tmp/.guestfs-0/lock
> libguestfs: command: run: \ --copy-kernel
> libguestfs: command: run: \ -f ext2
> libguestfs: command: run: \ --host-cpu x86_64
> libguestfs: command: run: \ /usr/lib/guestfs/supermin.d
> libguestfs: command: run: \ -o /var/tmp/.guestfs-0/appliance.d
> supermin: version: 5.2.0
> supermin: package handler: arch/pacman
> supermin: acquiring lock on /var/tmp/.guestfs-0/lock
> supermin: build: /usr/lib/guestfs/supermin.d
> supermin: reading the supermin appliance
> supermin: build: visiting /usr/lib/guestfs/supermin.d/base.tar.gz type 
> gzip base image (tar)
> supermin: build: visiting /usr/lib/guestfs/supermin.d/daemon.tar.gz type 
> gzip base image (tar)
> supermin: build: visiting /usr/lib/guestfs/supermin.d/excludefiles type 
> uncompressed excludefiles
> supermin: build: visiting /usr/lib/guestfs/supermin.d/hostfiles type 
> uncompressed hostfiles
> supermin: build: visiting /usr/lib/guestfs/supermin.d/init.tar.gz type 
> gzip base image (tar)
> supermin: build: visiting /usr/lib/guestfs/supermin.d/packages type 
> uncompressed packages
> supermin: build: visiting /usr/lib/guestfs/supermin.d/udev-rules.tar.gz 
> type gzip base image (tar)
> supermin: mapping package names to installed packages
> supermin: failure: failed to parse epoch:version-release field 2.04-11.1
> libguestfs: error: /usr/bin/supermin exited with error status 1, see 
> debug messages above

This was a supermin bug that was fixed recently:
https://bugzilla.redhat.com/show_bug.cgi?id=1858625
https://github.com/libguestfs/supermin/commit/1b6802ba85898c947f2276c365bf8be7eb5f1642

-- 
Pino Toscano

signature.asc
Description: This is a digitally signed message part.
___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs

[Libguestfs] Translations using Weblate

2020-08-21 Thread Pino Toscano
Hi,

with the changes done last week in libguestfs and virt-v2v, we are now
using Weblate as our translation infrastructure; in particular, we are
using the Fedora instance:
  https://translate.fedoraproject.org/projects/libguestfs/

I set up all the various components required, i.e. messages and docs
for libguestfs and virt-v2v. The docs components are "linked" to the
components of the messages, as they are in the same repository. This
means that
- updating the repository of the component with the messages will
  trigger the update of the docs component automatically
- there is only one pull request for the repository with all the changes
  in the different components

The recent changes I did also imply some workflow changes:
1) the translation catalogs (i.e. the .pot files) are no more generated
   automatically during the build. This proved challenging, as updating
   the catalog required complete dependencies for all the sources,
   which may not be built depending on the available dependencies
2) we will need to regenerate the translation catalogs manually, at
   least for now; considering we are on github, a possible way to
   automate this is to use a GitHub Action [1], similar to the one we
   set up in virt-manager [2]
3) similar to (1), .po files are no more updated from the template
   during the build; Weblate handles them now, and I enabled the Weblate
   addon to run msgmerge every time the template changes

As I mentioned above, now Weblate "owns" changes to the translations:
this means we don't have to change them manually anymore, and issues
must be fixed directly in Weblate (which will be then send upstream).
Weblate has a lot of checks for problems in messages and even style
nits, so hopefully this should greatly reduce the issues in
translations.

An important configuration bit needed is to notify Weblate every time
the are commits pushed in the repositories; this is done by using GitHub
webhooks, which must be configured per repository as described in the
Weblate documentation [3].

The preferred way to get translation updates on GitHub/Gitlab is via
PRs; I know that we generally don't use them, however I believe we
can accept them from Weblate, as it would simplify things a bit.
There are not often changes in translations, so IMHO there will not
be a big traffic of PRs. Also, I configured our components to push
translations changes to upstream after 24h of no activity, and to batch
commits by author, so there will be less commits sent as PR.
The default configuration in GitHub for PR merging is with git merges,
which can be changed: in the "Settings" tab of a repository, select
"Options", then uncheck "Allow merge commits". Weblate takes care of
rebasing each PR according to the upstream changes, so a "fast forward"
merge will usually work.

I think I wrote down all the important bits I wanted to mention.
Of course there will be more things to mention in the future that I
cannot think about at the moment, so please do ask for anything
related.

[1] https://github.com/features/actions
[2] 
https://github.com/virt-manager/virt-manager/blob/master/.github/workflows/translations.yml
[3] 
https://docs.weblate.org/en/latest/admin/continuous.html#automatically-receiving-changes-from-github

-- 
Pino Toscano

signature.asc
Description: This is a digitally signed message part.
___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs

[Libguestfs] [v2v PATCH] libvirt: read password file outside libvirt auth callback

2020-08-17 Thread Pino Toscano
This way errors that occur while reading the password file are properly
propagated, instead of being reported as errors of the libvirt
authentication callback.

Reported by: Ming Xie.
---
 v2v/libvirt_utils.ml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/v2v/libvirt_utils.ml b/v2v/libvirt_utils.ml
index 4d0b8639..1a24b049 100644
--- a/v2v/libvirt_utils.ml
+++ b/v2v/libvirt_utils.ml
@@ -24,8 +24,8 @@ open Common_gettext.Gettext
 module. *)
 
 let auth_for_password_file ?password_file () =
+  let password = Option.map read_first_line_from_file password_file in
   let auth_fn creds =
-let password = Option.map read_first_line_from_file password_file in
 List.map (
   function
   | { Libvirt.Connect.typ = Libvirt.Connect.CredentialPassphrase } -> 
password
-- 
2.26.2

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs



[Libguestfs] [v2v PATCH 10/14] po: extract again messages from C sources

2020-08-13 Thread Pino Toscano
There are C sources (mostly in libguestfs-common) with user messages,
and at least some of them may actually be shown by virt-v2v.

Hence, extract them again, so they can be translated.
---
 Makefile.am| 14 +-
 po/Makefile.am |  8 ++--
 po/POTFILES| 24 
 3 files changed, 43 insertions(+), 3 deletions(-)
 create mode 100644 po/POTFILES

diff --git a/Makefile.am b/Makefile.am
index 06e12a3c..e316f88c 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -98,9 +98,10 @@ EXTRA_DIST = \
 # When doing 'make dist' update a few files automatically.
 #
 #  ChangeLog  - changelog (created from git)
+#  po/POTFILES- files with ordinary extensions, but not OCaml files
 #  po/POTFILES-ml - OCaml files, which need a special tool to translate
 
-dist-hook: ChangeLog po/POTFILES-ml
+dist-hook: ChangeLog po/POTFILES po/POTFILES-ml
cp ChangeLog $(distdir)/ChangeLog
 
 ChangeLog: configure.ac
@@ -108,6 +109,17 @@ ChangeLog: configure.ac
git log --decorate=false > $@-t
mv $@-t $@
 
+# For more information about translations, see po/Makefile.am.
+po/POTFILES: configure.ac
+   rm -f $@ $@-t
+   cd $(srcdir); \
+   find $(DIST_SUBDIRS) -name '*.c' | \
+   grep -v -E '^(gnulib|po-docs|tests|test-data|bundled)/' | \
+   grep -v -E '/(dummy\.c)$$' | \
+   grep -v -E '.*-(tests)\.c$$' | \
+   LC_ALL=C sort -u > $@-t
+   mv $@-t $@
+
 po/POTFILES-ml: configure.ac
rm -f $@ $@-t
cd $(srcdir); \
diff --git a/po/Makefile.am b/po/Makefile.am
index 8d981d91..f1509d60 100644
--- a/po/Makefile.am
+++ b/po/Makefile.am
@@ -25,13 +25,14 @@ MSGID_BUGS_ADDRESS = 
https://bugzilla.redhat.com/enter_bug.cgi?component=libgues
 # Don't use LINGUAS (uppercase) as Gentoo defines it (RHBZ#804464).
 linguas := $(shell cat $(srcdir)/LINGUAS)
 
+POTFILES:= $(shell $(SED) 's,^,$(top_srcdir)/,' $(srcdir)/POTFILES)
 POTFILES_ML := $(shell $(SED) 's,^,$(top_srcdir)/,' $(srcdir)/POTFILES-ml)
 POFILES := $(linguas:%=%.po)
 GMOFILES:= $(linguas:%=%.gmo)
 
 EXTRA_DIST = \
LINGUAS \
-   POTFILES-ml \
+   POTFILES POTFILES-ml \
$(DOMAIN).pot \
$(POFILES) \
$(GMOFILES)
@@ -59,7 +60,7 @@ XGETTEXT_ARGS = \
 FIX_CHARSET = \
$(SED) -i 's|text/plain; charset=CHARSET|text/plain; charset=utf-8|g'
 
-$(DOMAIN).pot: Makefile POTFILES-ml $(POTFILES_ML)
+$(DOMAIN).pot: Makefile POTFILES $(POTFILES) POTFILES-ml $(POTFILES_ML)
rm -f $@-t
touch $@-t
 if HAVE_OCAML_GETTEXT
@@ -67,6 +68,9 @@ if HAVE_OCAML_GETTEXT
 \
$(FIX_CHARSET) $@-t
 endif
+   $(XGETTEXT) -j -o $@-t $(XGETTEXT_ARGS) \
+ --files-from=$(abs_srcdir)/POTFILES
+   $(FIX_CHARSET) $@-t
mv $@-t $@
 
 .po.gmo:
diff --git a/po/POTFILES b/po/POTFILES
new file mode 100644
index ..02a410d7
--- /dev/null
+++ b/po/POTFILES
@@ -0,0 +1,24 @@
+common/mlpcre/pcre-c.c
+common/mltools/JSON_parser-c.c
+common/mltools/getopt-c.c
+common/mltools/tools_utils-c.c
+common/mltools/uri-c.c
+common/mlutils/c_utils-c.c
+common/mlutils/unix_utils-c.c
+common/mlxml/xml-c.c
+common/options/config.c
+common/options/decrypt.c
+common/options/display-options.c
+common/options/domain.c
+common/options/inspect.c
+common/options/keys.c
+common/options/options.c
+common/options/uri.c
+common/qemuopts/qemuopts.c
+common/utils/cleanups.c
+common/utils/gnulib-cleanups.c
+common/utils/libxml2-cleanups.c
+common/utils/stringlists-utils.c
+common/utils/utils.c
+v2v/libosinfo-c.c
+v2v/qemuopts-c.c
-- 
2.26.2

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs



[Libguestfs] [v2v PATCH 14/14] po-docs: update translations from catalog templates

2020-08-13 Thread Pino Toscano
Run a msgmerge of the translations with the up-to-date template.
Do it manually before adding virt-v2v to Weblate, so we can edit them
(e.g. removing the obsolete messages, which are largely from libguestfs
sources).
---
 
This patch was truncated for mailing list posting.
Originally it was ~13M.
 
 po-docs/cs.po|   3960 +-
 po-docs/de.po|   9299 +---
 po-docs/en_GB.po |  62274 +---
 po-docs/es.po|   5760 +--
 po-docs/eu.po|   3993 +-
 po-docs/fr.po|  30007 +---
 po-docs/ja.po|  37459 +--
 po-docs/nl.po|   3815 +-
 po-docs/pt_BR.po |   3790 +-
 po-docs/tg.po|   4009 +-
 po-docs/uk.po| 111660 +---
 po-docs/zh_CN.po |   3939 +-
 12 files changed, 16947 insertions(+), 263018 deletions(-)

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs



[Libguestfs] [v2v PATCH 06/14] po-docs: split pot and po handling

2020-08-13 Thread Pino Toscano
With the Weblate adoption, we let it update the po files from the
catalog template. The po4a behaviour of extracting the template,
merging the existing translations, and creating the translated PODs at
once is problematic. Hence, split the extraction and the translated POD
generation in two.

Use po4a-gettextize to extract the catalog template only, not doing it
anymore automatically at each build. There is no more need for a
po4a.conf file.

Use po4a-translate to create translated PODs from the po files, keeping
the fixup of the generated files (to avoid spurious =encoding, etc).
Add a silent rule to hide the po4a-translate command lines by default.

These changes also allow us to get rid of the POD existance checks with
associated error message pointing to the update-po rule. Now each
translated POD file is generated because of make dependency, and it
depends only on its po file.
---
 .gitignore  |  1 -
 m4/guestfs-progs.m4 |  5 +++--
 po-docs/Makefile.am | 37 ++---
 po-docs/language.mk | 29 +++--
 subdir-rules.mk |  3 +++
 5 files changed, 27 insertions(+), 48 deletions(-)

diff --git a/.gitignore b/.gitignore
index 54423b1a..28ddd182 100644
--- a/.gitignore
+++ b/.gitignore
@@ -83,7 +83,6 @@ Makefile.in
 /po-docs/*/*.3
 /po-docs/*/*.5
 /po-docs/*/*.8
-/po-docs/po4a.conf
 /po-docs/*/*.pod
 /podwrapper.1
 /podwrapper.pl
diff --git a/m4/guestfs-progs.m4 b/m4/guestfs-progs.m4
index 1847e241..edb45cf9 100644
--- a/m4/guestfs-progs.m4
+++ b/m4/guestfs-progs.m4
@@ -45,8 +45,9 @@ AC_PATH_PROGS([GENISOIMAGE],[genisoimage mkisofs],[no],
 test "x$GENISOIMAGE" = "xno" && AC_MSG_ERROR([genisoimage must be installed])
 
 dnl po4a for translating man pages and POD files (optional).
-AC_CHECK_PROG([PO4A],[po4a],[po4a],[no])
-AM_CONDITIONAL([HAVE_PO4A], [test "x$PO4A" != "xno"])
+AC_CHECK_PROG([PO4A_GETTEXTIZE],[po4a-gettextize],[po4a-gettextize],[no])
+AC_CHECK_PROG([PO4A_TRANSLATE],[po4a-translate],[po4a-translate],[no])
+AM_CONDITIONAL([HAVE_PO4A], [test "x$PO4A_GETTEXTIZE" != "xno" && test 
"x$PO4A_TRANSLATE" != "xno"])
 
 dnl Check for db_load (optional).
 GUESTFS_FIND_DB_TOOL([DB_LOAD], [load])
diff --git a/po-docs/Makefile.am b/po-docs/Makefile.am
index 0660eba0..0667b784 100644
--- a/po-docs/Makefile.am
+++ b/po-docs/Makefile.am
@@ -34,47 +34,22 @@ EXTRA_DIST = \
$(linguas:%=%.po) \
podfiles
 
-CLEANFILES += po4a.conf
-
 # Build the final man pages from the translated POD files.  Each
 # language directory contains a Makefile.am that we need to keep up to
 # date (note each $lang/Makefile.am should be identical).
 # XXX Is there a better way?
 SUBDIRS = $(linguas_translated)
 
-update-po: virt-v2v-docs.pot
-
-# Note: po4a puts the following junk at the top of every POD file it
-# generates:
-#  - a warning
-#  - a probably bogus =encoding line
-# Remove both.
-# XXX Fix po4a so it doesn't do this.
-virt-v2v-docs.pot: po4a.conf
-   $(PO4A) \
- -M utf-8 -L utf-8 -A utf-8 \
- -v \
- -k 0 \
+virt-v2v-docs.pot:
+   $(PO4A_GETTEXTIZE) \
+ -f pod \
+ -M utf-8 -L utf-8 \
  --package-name $(PACKAGE_NAME) \
  --package-version $(PACKAGE_VERSION) \
  --msgid-bugs-address libguestfs@redhat.com \
  --copyright-holder "Red Hat Inc." \
- po4a.conf
-   for f in `cd $(srcdir); find $(linguas_translated) -name '*.pod'`; do \
- $(SED) '0,/^=encoding/d' < $$f > $$f.new; \
- mv $$f.new $$f; \
-   done
-
-po4a.conf: podfiles
-   rm -f $@-t
-   echo "[po_directory] $(srcdir)" >> $@-t
-   echo >> $@-t
-   for f in `cat podfiles`; do \
- out=`basename -- $$f .pod`.pod; \
- echo "[type: pod] $$f \$$lang:\$$lang/$$out" >> $@-t; \
- echo >> $@-t; \
-   done;
-   mv $@-t $@
+ -p $@ \
+ $(patsubst %,-m %,$(shell cat $(srcdir)/podfiles))
 
 podfiles: Makefile
rm -f $@ $@-t
diff --git a/po-docs/language.mk b/po-docs/language.mk
index ab402c32..54f621b5 100644
--- a/po-docs/language.mk
+++ b/po-docs/language.mk
@@ -22,7 +22,7 @@ include $(top_srcdir)/subdir-rules.mk
 LINGUA = $(shell basename -- `pwd`)
 
 # Before 1.23.23, the old Perl tools were called *.pl.
-CLEANFILES += *.pl
+CLEANFILES += *.pl *.pod
 
 MANPAGES = \
virt-v2v.1 \
@@ -54,19 +54,20 @@ virt-v2v.1: key-option.pod keys-from-stdin-option.pod
  --man $@ \
  $<
 
-# If a POD file is missing, the user needs to run make update-po.
-# This cannot be done automatically by make because it would be unsafe
-# to run po4a or update podfiles potentially in parallel.  Therefore
-# tell the user what to do and stop.
-$(podfiles):
-   @if ! test -f $@; then \
- echo "***"; \
- echo "*** You need to run the following commands:"; \
- echo "*** rm po-docs/podfiles; make -C po-docs update-po"; \
- echo "*** After that, rerun make."; \
- echo "***"; \
- 

[Libguestfs] [v2v PATCH 12/14] po: update translations from catalog templates

2020-08-13 Thread Pino Toscano
Run a msgmerge of the translations with the up-to-date template.
Do it manually before adding virt-v2v to Weblate, so we can edit them
(e.g. removing the obsolete messages, which are largely from libguestfs
sources).
---

This patch was truncated for mailing list posting.
Originally it was ~2.8M.

 po/cs.po| 1294 ++-
 po/de.po| 2003 ++---
 po/en_GB.po | 1343 ++--
 po/es.po| 2911 ++---
 po/eu.po| 1232 ++-
 po/fr.po| 3562 ++
 po/gu.po| 2096 ++
 po/hi.po| 1944 ++---
 po/ja.po| 3746 ++--
 po/kn.po| 1928 ++---
 po/ml.po| 1910 ++--
 po/mr.po| 2602 ++
 po/nl.po| 3279 ++--
 po/or.po| 1911 ++--
 po/pa.po| 2610 ++
 po/pl.po| 3101 ++
 po/ru.po| 1231 ++-
 po/ta.po| 1903 ++--
 po/te.po| 1899 ++--
 po/tg.po| 1227 ++-
 po/uk.po| 6020 ++-
 po/zh_CN.po | 1246 ++-
 22 files changed, 14749 insertions(+), 36249 deletions(-)

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs



[Libguestfs] [v2v PATCH 00/14] Adaptations to Weblate

2020-08-13 Thread Pino Toscano
We are migrating to Weblate (the Fedora instance, in particular) for
translations instead of Zanata. Adapt our tooling a bit to the different
workflow:
- Weblate takes care of updating the po files whenever a new translation
  catalog is available, so stop doing that on our own: this meant also
  tweaking the po4a usage for POD documentations, resulting in simpler
  rules (IMHO)
- ensure that the references to sources files in translation catalogs
  are relative to the top-level directory, so Weblate (and users too,
  actually) can properly locate the sources
- extract also C sources
- regenerate the catalogs
- drop references to Zanata
- update the translations to do edits before handing them over to
  Weblate
- remove empty translations (v2v is translated less than libguestfs)

Pino Toscano (14):
  po: turn language list into LINGUAS file
  po-docs: turn language list into LINGUAS file
  po: remove rules for pot/po update
  podfiles: add missing documentation
  po-docs: add missing dependencies for virt-v2v.1
  po-docs: split pot and po handling
  po: fix references to OCaml sources
  po-docs: fix references to sources
  Remove references to Zanata
  po: extract again messages from C sources
  po/po-docs: update catalog templates
  po: update translations from catalog templates
  po: remove empty translations
  po-docs: update translations from catalog templates

 .gitignore|  1 -
 Makefile.am   | 18 +-
 m4/guestfs-progs.m4   |  5 +-
 po-docs/LINGUAS   | 12 +
 po-docs/Makefile.am   | 62 +-
 po-docs/cs.po |   3960 +-
 po-docs/de.po |   9299 +--
 po-docs/en_GB.po  |  62274 +--
 po-docs/es.po |   5760 +-
 po-docs/eu.po |   3993 +-
 po-docs/fr.po |  30007 +-
 po-docs/ja.po |  37459 +---
 po-docs/language.mk   | 31 +-
 po-docs/nl.po |   3815 +-
 po-docs/podfiles  | 31 +-
 po-docs/pt_BR.po  |   3790 +-
 po-docs/tg.po |   4009 +-
 po-docs/uk.po | 111660 +--
 po-docs/virt-v2v-docs.pot |   3783 +-
 po-docs/zh_CN.po  |   3939 +-
 po/LINGUAS| 15 +
 po/Makefile.am| 32 +-
 po/POTFILES   | 24 +
 po/cs.po  |   1294 +-
 po/de.po  |   2003 +-
 po/en_GB.po   |   2684 -
 po/es.po  |   2911 +-
 po/eu.po  |   2541 -
 po/fr.po  |   3562 +-
 po/gu.po  |   2096 +-
 po/hi.po  |   1944 +-
 po/ja.po  |   3746 +-
 po/kn.po  |   1928 +-
 po/ml.po  |   1910 +-
 po/mr.po  |   2602 +-
 po/nl.po  |   3279 +-
 po/or.po  |   1911 +-
 po/pa.po  |   2610 +-
 po/pl.po  |   3101 +-
 po/ru.po  |   2541 -
 po/ta.po  |   3292 --
 po/te.po  |   3288 --
 po/tg.po  |   2529 -
 po/uk.po  |   6020 +-
 po/virt-v2v.pot   |   2014 +-
 po/zh_CN.po   |   2581 -
 subdir-rules.mk   |  3 +
 zanata-pull.sh| 30 -
 zanata.xml|  7 -
 49 files changed, 29826 insertions(+), 316580 deletions(-)
 create mode 100644 po-docs/LINGUAS
 create mode 100644 po/LINGUAS
 create mode 100644 po/POTFILES
 delete mode 100644 po/en_GB.po
 delete mode 100644 po/eu.po
 delete mode 100644 po/ru.po
 delete mode 100644 po/ta.po
 delete mode 100644 po/te.po
 delete mode 100644 po/tg.po
 delete mode 100644 po/zh_CN.po
 delete mode 100755 zanata-pull.sh
 delete mode 100644 zanata.xml

-- 
2.26.2

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs



[Libguestfs] [v2v PATCH 11/14] po/po-docs: update catalog templates

2020-08-13 Thread Pino Toscano
Regenerate the catalog templates according to the updated extraction
rules (fixed paths, and added sources).
---

This patch was shortened for mailing list posting.
Originally it was ~114K.

 po-docs/virt-v2v-docs.pot | 3783 ++---
 po/virt-v2v.pot   | 2014 +---
 2 files changed, 2765 insertions(+), 3032 deletions(-)

diff --git a/po-docs/virt-v2v-docs.pot b/po-docs/virt-v2v-docs.pot
index c464daea..f747348f 100644
--- a/po-docs/virt-v2v-docs.pot
+++ b/po-docs/virt-v2v-docs.pot
@@ -8,30 +8,25 @@ msgid ""
 msgstr ""
 "Project-Id-Version: virt-v2v 1.43.1\n"
 "Report-Msgid-Bugs-To: libguestfs@redhat.com\n"
-"POT-Creation-Date: 2020-07-06 15:21+0100\n"
+"POT-Creation-Date: 2020-08-13 13:17+0200\n"
 "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n"
 "Last-Translator: FULL NAME \n"
 "Language-Team: LANGUAGE \n"
 "Language: \n"
 "MIME-Version: 1.0\n"
-"Content-Type: text/plain; charset=utf-8\n"
+"Content-Type: text/plain; charset=UTF-8\n"
 "Content-Transfer-Encoding: 8bit\n"
 
 #. type: =end
-#: ../common/mlcustomize/customize-options.pod:1
-#: ../common/mlcustomize/customize-options.pod:25
-#: ../docs/virt-v2v-release-notes-1.42.pod:141
-#: ../docs/virt-v2v-release-notes-1.42.pod:145
-#: ../virt-v2v-1.43.1/common/mlcustomize/customize-options.pod:1
-#: ../virt-v2v-1.43.1/common/mlcustomize/customize-options.pod:25
-#: ../virt-v2v-1.43.1/docs/virt-v2v-release-notes-1.42.pod:141
-#: ../virt-v2v-1.43.1/docs/virt-v2v-release-notes-1.42.pod:145
+#: common/mlcustomize/customize-options.pod:1
+#: common/mlcustomize/customize-options.pod:25
+#: docs/virt-v2v-release-notes-1.42.pod:141
+#: docs/virt-v2v-release-notes-1.42.pod:145
 msgid "comment"
 msgstr ""
 
 [10600+ lines follow...]
diff --git a/po/virt-v2v.pot b/po/virt-v2v.pot
index cd04fdd6..ab31f32f 100644
--- a/po/virt-v2v.pot
+++ b/po/virt-v2v.pot
@@ -1,2007 +1,2699 @@
 # SOME DESCRIPTIVE TITLE.
-# Copyright (C) YEAR THE PACKAGE'S COPYRIGHT HOLDER
-# This file is distributed under the same license as the PACKAGE package.
+# Copyright (C) YEAR Red Hat Inc.
+# This file is distributed under the same license as the virt-v2v package.
 # FIRST AUTHOR , YEAR.
 #
 #, fuzzy
 msgid ""
 msgstr ""
-"Project-Id-Version: PACKAGE VERSION\n"
-"Report-Msgid-Bugs-To: \n"
-"POT-Creation-Date: 2020-07-06 14:21+\n"
+"Project-Id-Version: virt-v2v 1.43.1\n"
+"Report-Msgid-Bugs-To: https://bugzilla.redhat.com/enter_bug.cgi?;
+"component=libguestfs=Virtualization+Tools\n"
+"POT-Creation-Date: 2020-08-13 13:16+0200\n"
 "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n"
 "Last-Translator: FULL NAME \n"
 "Language-Team: LANGUAGE \n"
+"Language: \n"
 "MIME-Version: 1.0\n"
-"Content-Type: text/plain; charset=utf-8\n"
+"Content-Type: text/plain; charset=UTF-8\n"
 "Content-Transfer-Encoding: 8bit\n"
 "Plural-Forms: nplurals=INTEGER; plural=EXPRESSION;\n"
 
-#: ../v2v/input_ova.ml:152 ../common/mltools/tools_utils.ml:187
+#: v2v/input_ova.ml:152 common/mltools/tools_utils.ml:187
 msgid "%s"
 msgstr ""

 [3300+ lines follow...] 

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs



[Libguestfs] [v2v PATCH 13/14] po: remove empty translations

2020-08-13 Thread Pino Toscano
---

This patch was greatly shortened for mailing list posting.
Originally it was ~495K.

 po/LINGUAS  |7 -
 po/en_GB.po | 2701 --
 po/eu.po| 2701 --
 po/ru.po| 2704 ---
 po/ta.po| 2701 --
 po/te.po| 2701 --
 po/tg.po| 2700 --
 po/zh_CN.po | 2699 --
 8 files changed, 18914 deletions(-)
 delete mode 100644 po/en_GB.po
 delete mode 100644 po/eu.po
 delete mode 100644 po/ru.po
 delete mode 100644 po/ta.po
 delete mode 100644 po/te.po
 delete mode 100644 po/tg.po
 delete mode 100644 po/zh_CN.po

diff --git a/po/LINGUAS b/po/LINGUAS
index 69604489..ed6cea94 100644
--- a/po/LINGUAS
+++ b/po/LINGUAS
@@ -1,8 +1,6 @@
 cs
 de
-en_GB
 es
-eu
 fr
 gu
 hi
@@ -14,9 +12,4 @@ nl
 or
 pa
 pl
-ru
-ta
-te
-tg
 uk
-zh_CN
diff --git a/po/en_GB.po b/po/en_GB.po
deleted file mode 100644
index 37b7817c..
--- a/po/en_GB.po
+++ /dev/null
diff --git a/po/eu.po b/po/eu.po
deleted file mode 100644
index 2556796c..
--- a/po/eu.po
+++ /dev/null
diff --git a/po/ru.po b/po/ru.po
deleted file mode 100644
index e088b77d..
--- a/po/ru.po
+++ /dev/null
diff --git a/po/ta.po b/po/ta.po
deleted file mode 100644
index 4b53d510..
--- a/po/ta.po
+++ /dev/null
diff --git a/po/te.po b/po/te.po
deleted file mode 100644
index 4d83db02..
--- a/po/te.po
+++ /dev/null
diff --git a/po/tg.po b/po/tg.po
deleted file mode 100644
index 89888480..
--- a/po/tg.po
+++ /dev/null
diff --git a/po/zh_CN.po b/po/zh_CN.po
deleted file mode 100644
index 0ed8422b..
--- a/po/zh_CN.po
+++ /dev/null

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs



[Libguestfs] [v2v PATCH 01/14] po: turn language list into LINGUAS file

2020-08-13 Thread Pino Toscano
Use a LINGUAS file with the list of available translations instead of
defining them in a make variable. This way Weblate will be able to
update the list using an available addon.
---
 po/LINGUAS | 22 ++
 po/Makefile.am |  3 ++-
 2 files changed, 24 insertions(+), 1 deletion(-)
 create mode 100644 po/LINGUAS

diff --git a/po/LINGUAS b/po/LINGUAS
new file mode 100644
index ..69604489
--- /dev/null
+++ b/po/LINGUAS
@@ -0,0 +1,22 @@
+cs
+de
+en_GB
+es
+eu
+fr
+gu
+hi
+ja
+kn
+ml
+mr
+nl
+or
+pa
+pl
+ru
+ta
+te
+tg
+uk
+zh_CN
diff --git a/po/Makefile.am b/po/Makefile.am
index bfef889c..b4c2d3ff 100644
--- a/po/Makefile.am
+++ b/po/Makefile.am
@@ -23,13 +23,14 @@ MSGID_BUGS_ADDRESS = 
https://bugzilla.redhat.com/enter_bug.cgi?component=libgues
 
 # Languages.
 # Don't use LINGUAS (uppercase) as Gentoo defines it (RHBZ#804464).
-linguas := cs de en_GB es eu fr gu hi ja kn ml mr nl or pa pl ru ta te tg 
uk zh_CN
+linguas := $(shell cat $(srcdir)/LINGUAS)
 
 POTFILES_ML := $(shell $(SED) 's,^,$(top_srcdir)/,' $(srcdir)/POTFILES-ml)
 POFILES := $(linguas:%=%.po)
 GMOFILES:= $(linguas:%=%.gmo)
 
 EXTRA_DIST = \
+   LINGUAS \
POTFILES-ml \
$(DOMAIN).pot \
$(POFILES) \
-- 
2.26.2

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs



[Libguestfs] [v2v PATCH 08/14] po-docs: fix references to sources

2020-08-13 Thread Pino Toscano
Start the message extraction from the toplevel source directory, so the
file references are relative to that, instead of relative to this
po-docs subdirectory.

In addition, remove the reference to POTFILES-pl, as there are no Perl
sources in virt-v2v.

Also update/regenerate podfiles accordingly.
---
 po-docs/Makefile.am | 16 
 po-docs/language.mk |  2 +-
 po-docs/podfiles| 32 
 3 files changed, 25 insertions(+), 25 deletions(-)

diff --git a/po-docs/Makefile.am b/po-docs/Makefile.am
index 0667b784..8c5c9b1e 100644
--- a/po-docs/Makefile.am
+++ b/po-docs/Makefile.am
@@ -41,24 +41,24 @@ EXTRA_DIST = \
 SUBDIRS = $(linguas_translated)
 
 virt-v2v-docs.pot:
-   $(PO4A_GETTEXTIZE) \
+   cd $(top_srcdir) && $(PO4A_GETTEXTIZE) \
  -f pod \
  -M utf-8 -L utf-8 \
  --package-name $(PACKAGE_NAME) \
  --package-version $(PACKAGE_VERSION) \
  --msgid-bugs-address libguestfs@redhat.com \
  --copyright-holder "Red Hat Inc." \
- -p $@ \
+ -p $(abs_srcdir)/$@ \
  $(patsubst %,-m %,$(shell cat $(srcdir)/podfiles))
 
 podfiles: Makefile
rm -f $@ $@-t
-   find $(top_srcdir) -name '*.pod' | \
- grep -v /debian/ | \
- grep -v /virt-v2v-1 | \
- grep -v /po-docs/ | \
+   cd $(top_srcdir) && find . -name '*.pod' -printf '%P\n'| \
+ grep -v ^debian/ | \
+ grep -v ^virt-v2v-1 | \
+ grep -v ^po-docs/ | \
+ grep -v ^stamp- | \
  grep -v /stamp- \
- > $@-t
-   for f in `cat $(top_srcdir)/po/POTFILES-pl`; do echo $(top_srcdir)/$$f; 
done >> $@-t
+ > $(abs_srcdir)/$@-t
LC_ALL=C sort -o $@-t $@-t
mv $@-t $@
diff --git a/po-docs/language.mk b/po-docs/language.mk
index 54f621b5..05c63d8c 100644
--- a/po-docs/language.mk
+++ b/po-docs/language.mk
@@ -65,7 +65,7 @@ virt-v2v.1: key-option.pod keys-from-stdin-option.pod
  -f pod \
  -M utf-8 -L utf-8 \
  -k 0 \
- -m $(srcdir)/../$(shell grep '/$(notdir $@)$$' 
$(top_srcdir)/po-docs/podfiles) \
+ -m $(top_srcdir)/$(shell grep '/$(notdir $@)$$' 
$(top_srcdir)/po-docs/podfiles) \
  -p $< \
  | $(SED) '0,/^=encoding/d' > $@
 
diff --git a/po-docs/podfiles b/po-docs/podfiles
index a1063091..e0a776a5 100644
--- a/po-docs/podfiles
+++ b/po-docs/podfiles
@@ -1,16 +1,16 @@
-../common/mlcustomize/customize-options.pod
-../common/mlcustomize/customize-synopsis.pod
-../common/options/blocksize-option.pod
-../common/options/key-option.pod
-../common/options/keys-from-stdin-option.pod
-../docs/virt-v2v-copy-to-local.pod
-../docs/virt-v2v-hacking.pod
-../docs/virt-v2v-input-vmware.pod
-../docs/virt-v2v-input-xen.pod
-../docs/virt-v2v-output-local.pod
-../docs/virt-v2v-output-openstack.pod
-../docs/virt-v2v-output-rhv.pod
-../docs/virt-v2v-release-notes-1.42.pod
-../docs/virt-v2v-support.pod
-../docs/virt-v2v.pod
-../test-harness/virt-v2v-test-harness.pod
+common/mlcustomize/customize-options.pod
+common/mlcustomize/customize-synopsis.pod
+common/options/blocksize-option.pod
+common/options/key-option.pod
+common/options/keys-from-stdin-option.pod
+docs/virt-v2v-copy-to-local.pod
+docs/virt-v2v-hacking.pod
+docs/virt-v2v-input-vmware.pod
+docs/virt-v2v-input-xen.pod
+docs/virt-v2v-output-local.pod
+docs/virt-v2v-output-openstack.pod
+docs/virt-v2v-output-rhv.pod
+docs/virt-v2v-release-notes-1.42.pod
+docs/virt-v2v-support.pod
+docs/virt-v2v.pod
+test-harness/virt-v2v-test-harness.pod
-- 
2.26.2

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs



[Libguestfs] [v2v PATCH 04/14] podfiles: add missing documentation

2020-08-13 Thread Pino Toscano
---
 po-docs/podfiles | 1 +
 1 file changed, 1 insertion(+)

diff --git a/po-docs/podfiles b/po-docs/podfiles
index 6a9ce650..a1063091 100644
--- a/po-docs/podfiles
+++ b/po-docs/podfiles
@@ -2,6 +2,7 @@
 ../common/mlcustomize/customize-synopsis.pod
 ../common/options/blocksize-option.pod
 ../common/options/key-option.pod
+../common/options/keys-from-stdin-option.pod
 ../docs/virt-v2v-copy-to-local.pod
 ../docs/virt-v2v-hacking.pod
 ../docs/virt-v2v-input-vmware.pod
-- 
2.26.2

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs



[Libguestfs] [v2v PATCH 02/14] po-docs: turn language list into LINGUAS file

2020-08-13 Thread Pino Toscano
Use a LINGUAS file with the list of available translations instead of
defining them in a make variable. This way Weblate will be able to
update the list using an available addon, and we do not need to list
those not built.

Accordingly, rename the variable with built languages to
'linguas_translated'.
---
 po-docs/LINGUAS | 12 
 po-docs/Makefile.am | 15 +++
 2 files changed, 19 insertions(+), 8 deletions(-)
 create mode 100644 po-docs/LINGUAS

diff --git a/po-docs/LINGUAS b/po-docs/LINGUAS
new file mode 100644
index ..85b7927a
--- /dev/null
+++ b/po-docs/LINGUAS
@@ -0,0 +1,12 @@
+cs
+de
+en_GB
+es
+eu
+fr
+ja
+nl
+pt_BR
+tg
+uk
+zh_CN
diff --git a/po-docs/Makefile.am b/po-docs/Makefile.am
index 6ef0e233..0660eba0 100644
--- a/po-docs/Makefile.am
+++ b/po-docs/Makefile.am
@@ -21,18 +21,17 @@ include $(top_srcdir)/subdir-rules.mk
 # into the po/ directory and the translations into the usual
 # libguestfs.pot file.
 
-# Languages that we translate.
+# Languages.
 # Don't use LINGUAS (uppercase) as Gentoo defines it (RHBZ#804464).
-linguas = ja uk
+linguas := $(shell cat $(srcdir)/LINGUAS)
 
-# Languages that we have PO files for, but we don't translate
-# because there is insufficient coverage.
-linguas_not_translated = cs de en_GB es eu fr nl pt_BR tg zh_CN
+# Languages that we translate, as they have enough coverage.
+linguas_translated = ja uk
 
 EXTRA_DIST = \
+   LINGUAS \
virt-v2v-docs.pot \
$(linguas:%=%.po) \
-   $(linguas_not_translated:%=%.po) \
podfiles
 
 CLEANFILES += po4a.conf
@@ -41,7 +40,7 @@ CLEANFILES += po4a.conf
 # language directory contains a Makefile.am that we need to keep up to
 # date (note each $lang/Makefile.am should be identical).
 # XXX Is there a better way?
-SUBDIRS = $(linguas)
+SUBDIRS = $(linguas_translated)
 
 update-po: virt-v2v-docs.pot
 
@@ -61,7 +60,7 @@ virt-v2v-docs.pot: po4a.conf
  --msgid-bugs-address libguestfs@redhat.com \
  --copyright-holder "Red Hat Inc." \
  po4a.conf
-   for f in `cd $(srcdir); find $(linguas) -name '*.pod'`; do \
+   for f in `cd $(srcdir); find $(linguas_translated) -name '*.pod'`; do \
  $(SED) '0,/^=encoding/d' < $$f > $$f.new; \
  mv $$f.new $$f; \
done
-- 
2.26.2

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs



[Libguestfs] [v2v PATCH 05/14] po-docs: add missing dependencies for virt-v2v.1

2020-08-13 Thread Pino Toscano
The virt-v2v man page uses also additional POD snippets, so list them
as dependencies to make sure they are up-to-date.

This does not change the behaviour at the moment, however it will matter
when each traslated POD file will be generated on its own.
---
 po-docs/language.mk | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/po-docs/language.mk b/po-docs/language.mk
index 26ba67ae..ab402c32 100644
--- a/po-docs/language.mk
+++ b/po-docs/language.mk
@@ -46,6 +46,8 @@ EXTRA_DIST = \
 
 all-local: $(MANPAGES)
 
+virt-v2v.1: key-option.pod keys-from-stdin-option.pod
+
 %.1: %.pod
$(PODWRAPPER) \
  --no-strict-checks \
-- 
2.26.2

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs



[Libguestfs] [v2v PATCH 07/14] po: fix references to OCaml sources

2020-08-13 Thread Pino Toscano
Start the message extraction from the toplevel source directory, so the
file references are relative to that, instead of relative to this po
subdirectory.
---
 po/Makefile.am | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/po/Makefile.am b/po/Makefile.am
index 790c5710..8d981d91 100644
--- a/po/Makefile.am
+++ b/po/Makefile.am
@@ -63,7 +63,7 @@ $(DOMAIN).pot: Makefile POTFILES-ml $(POTFILES_ML)
rm -f $@-t
touch $@-t
 if HAVE_OCAML_GETTEXT
-   $(OCAML_GETTEXT) --action extract --extract-pot $@-t $(POTFILES_ML)
+   cd $(top_srcdir) && $(OCAML_GETTEXT) --action extract --extract-pot 
$(abs_srcdir)/$@-t $(shell cat $(abs_srcdir)/POTFILES-ml)
 \
$(FIX_CHARSET) $@-t
 endif
-- 
2.26.2

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs



[Libguestfs] [v2v PATCH 09/14] Remove references to Zanata

2020-08-13 Thread Pino Toscano
We migrated to Weblate, and Zanata is being decommissioned.
---
 Makefile.am|  4 +---
 zanata-pull.sh | 30 --
 zanata.xml |  7 ---
 3 files changed, 1 insertion(+), 40 deletions(-)
 delete mode 100755 zanata-pull.sh
 delete mode 100644 zanata.xml

diff --git a/Makefile.am b/Makefile.am
index 6d0c270b..06e12a3c 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -93,9 +93,7 @@ EXTRA_DIST = \
website/index.css \
website/index.html.in \
website/pod.css \
-   website/standard.css \
-   zanata.xml \
-   zanata-pull.sh
+   website/standard.css
 
 # When doing 'make dist' update a few files automatically.
 #
diff --git a/zanata-pull.sh b/zanata-pull.sh
deleted file mode 100755
index ee5f8830..
--- a/zanata-pull.sh
+++ /dev/null
@@ -1,30 +0,0 @@
-#!/bin/bash -
-# Pull translations from Zanata.
-# Copyright (C) 2011-2020 Red Hat Inc.
-#
-# This program is free software; you can redistribute it and/or modify
-# it under the terms of the GNU General Public License as published by
-# the Free Software Foundation; either version 2 of the License, or
-# (at your option) any later version.
-#
-# This program is distributed in the hope that it will be useful,
-# but WITHOUT ANY WARRANTY; without even the implied warranty of
-# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-# GNU General Public License for more details.
-#
-# You should have received a copy of the GNU General Public License
-# along with this program; if not, write to the Free Software
-# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
-
-set -e
-
-echo zanata po pull
-zanata po pull
-
-# Remove PO files that have no translations in them.
-for f in po/*.po po-docs/*.po; do
-if ! grep -q '^msgstr "[^"]' $f; then
-echo rm $f
-rm $f
-fi
-done
diff --git a/zanata.xml b/zanata.xml
deleted file mode 100644
index 649c4254..
--- a/zanata.xml
+++ /dev/null
@@ -1,7 +0,0 @@
-
-http://zanata.org/namespace/config/;>
-  https://fedora.zanata.org/
-  virt-v2v
-  master
-  gettext
-
-- 
2.26.2

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs



[Libguestfs] [v2v PATCH 03/14] po: remove rules for pot/po update

2020-08-13 Thread Pino Toscano
Weblate will handle the update of the po files from the translation
catalog, so avoid stomping on its feet by doing the same.

The translation catalog will be regenerated manually periodically.
---
 po/Makefile.am | 19 ---
 1 file changed, 19 deletions(-)

diff --git a/po/Makefile.am b/po/Makefile.am
index b4c2d3ff..790c5710 100644
--- a/po/Makefile.am
+++ b/po/Makefile.am
@@ -38,17 +38,6 @@ EXTRA_DIST = \
 
 if HAVE_GNU_GETTEXT
 
-dist-hook:
-   $(MAKE) update-po
-   cp *.po *.gmo $(distdir)/
-
-update-po:
-   $(MAKE) $(DOMAIN).pot
-   $(MAKE) $(POFILES)
-   $(MAKE) update-gmo
-
-update-gmo: Makefile $(GMOFILES)
-
 XGETTEXT_ARGS = \
--default-domain=$(DOMAIN) \
--from-code=utf-8 \
@@ -80,9 +69,6 @@ if HAVE_OCAML_GETTEXT
 endif
mv $@-t $@
 
-%.po: $(DOMAIN).pot
-   cd $(srcdir) && $(MSGMERGE) -N --update $@ $(DOMAIN).pot
-
 .po.gmo:
rm -f $@ $@-t
$(MSGFMT) -c -o $@-t $(srcdir)/$<
@@ -102,9 +88,4 @@ install-data-hook: $(GMOFILES)
 
 .PRECIOUS: $(DOMAIN).pot $(POFILES)
 
-else
-
-# If no GNU gettext, 'make update-po' is a dummy rule.
-update-po:
-
 endif
-- 
2.26.2

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs



Re: [Libguestfs] [PATCH] po-docs: turn language list into LINGUAS file

2020-08-13 Thread Pino Toscano
On Thursday, 13 August 2020 12:20:01 CEST Pino Toscano wrote:
> Use a LINGUAS file with the list of available translations instead of
> defining them in a make variable. This way Weblate will be able to
> update the list using an available addon, and we do not need to list
> those not built.
> 
> Accordingly, rename the variable with built languages to
> 'linguas_translated'.
> 
> Signed-off-by: Pino Toscano 
> ---
>  po-docs/LINGUAS | 12 
>  po-docs/Makefile.am | 12 +---
>  2 files changed, 17 insertions(+), 7 deletions(-)
>  create mode 100644 po-docs/LINGUAS
> 
> diff --git a/po-docs/LINGUAS b/po-docs/LINGUAS
> new file mode 100644
> index 0..85b7927a3
> --- /dev/null
> +++ b/po-docs/LINGUAS
> @@ -0,0 +1,12 @@
> +cs
> +de
> +en_GB
> +es
> +eu
> +fr
> +ja
> +nl
> +pt_BR
> +tg
> +uk
> +zh_CN
> diff --git a/po-docs/Makefile.am b/po-docs/Makefile.am
> index 028fbbcea..ed21cb152 100644
> --- a/po-docs/Makefile.am
> +++ b/po-docs/Makefile.am
> @@ -21,25 +21,23 @@ include $(top_srcdir)/subdir-rules.mk
>  # into the po/ directory and the translations into the usual
>  # libguestfs.pot file.
>  
> -# Languages that we translate.
> +# Languages.
>  # Don't use LINGUAS (uppercase) as Gentoo defines it (RHBZ#804464).
> -linguas = ja uk
> +linguas := $(shell cat $(srcdir)/LINGUAS)
>  
> -# Languages that we have PO files for, but we don't translate
> -# because there is insufficient coverage.
> -linguas_not_translated = cs de en_GB es eu fr nl pt_BR tg zh_CN
> +# Languages that we translate, as they have enough coverage.
> +linguas_translated = ja uk
>  
>  EXTRA_DIST = \
>   libguestfs-docs.pot \
>   $(linguas:%=%.po) \
> - $(linguas_not_translated:%=%.po) \
>   podfiles

Obviously, with LINGUAS added to EXTRA_DIST.

>  
>  # Build the final man pages from the translated POD files.  Each
>  # language directory contains a Makefile.am that we need to keep up to
>  # date (note each $lang/Makefile.am should be identical).
>  # XXX Is there a better way?
> -SUBDIRS = $(linguas)
> +SUBDIRS = $(linguas_translated)
>  
>  libguestfs-docs.pot:
>   cd $(top_srcdir) && $(PO4A_GETTEXTIZE) \
> 


-- 
Pino Toscano

signature.asc
Description: This is a digitally signed message part.
___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs

[Libguestfs] [PATCH] po-docs: turn language list into LINGUAS file

2020-08-13 Thread Pino Toscano
Use a LINGUAS file with the list of available translations instead of
defining them in a make variable. This way Weblate will be able to
update the list using an available addon, and we do not need to list
those not built.

Accordingly, rename the variable with built languages to
'linguas_translated'.

Signed-off-by: Pino Toscano 
---
 po-docs/LINGUAS | 12 
 po-docs/Makefile.am | 12 +---
 2 files changed, 17 insertions(+), 7 deletions(-)
 create mode 100644 po-docs/LINGUAS

diff --git a/po-docs/LINGUAS b/po-docs/LINGUAS
new file mode 100644
index 0..85b7927a3
--- /dev/null
+++ b/po-docs/LINGUAS
@@ -0,0 +1,12 @@
+cs
+de
+en_GB
+es
+eu
+fr
+ja
+nl
+pt_BR
+tg
+uk
+zh_CN
diff --git a/po-docs/Makefile.am b/po-docs/Makefile.am
index 028fbbcea..ed21cb152 100644
--- a/po-docs/Makefile.am
+++ b/po-docs/Makefile.am
@@ -21,25 +21,23 @@ include $(top_srcdir)/subdir-rules.mk
 # into the po/ directory and the translations into the usual
 # libguestfs.pot file.
 
-# Languages that we translate.
+# Languages.
 # Don't use LINGUAS (uppercase) as Gentoo defines it (RHBZ#804464).
-linguas = ja uk
+linguas := $(shell cat $(srcdir)/LINGUAS)
 
-# Languages that we have PO files for, but we don't translate
-# because there is insufficient coverage.
-linguas_not_translated = cs de en_GB es eu fr nl pt_BR tg zh_CN
+# Languages that we translate, as they have enough coverage.
+linguas_translated = ja uk
 
 EXTRA_DIST = \
libguestfs-docs.pot \
$(linguas:%=%.po) \
-   $(linguas_not_translated:%=%.po) \
podfiles
 
 # Build the final man pages from the translated POD files.  Each
 # language directory contains a Makefile.am that we need to keep up to
 # date (note each $lang/Makefile.am should be identical).
 # XXX Is there a better way?
-SUBDIRS = $(linguas)
+SUBDIRS = $(linguas_translated)
 
 libguestfs-docs.pot:
cd $(top_srcdir) && $(PO4A_GETTEXTIZE) \
-- 
2.26.2

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs



Re: [Libguestfs] [PATCH v2] appliance: extract UUID from QCOW2 disk image

2020-08-12 Thread Pino Toscano
On Wednesday, 12 August 2020 17:36:10 CEST Andrey Shinkevich wrote:
> For the appliance of the QCOW2 format, get the UUID of the disk by
> reading the first 256k bytes with 'qemu-img dd' command. Then pass the
> read block to the 'file' command. In case of failure, run the 'file'
> command again directly.
> 
> Suggested-by: Denis V. Lunev 
> Signed-off-by: Andrey Shinkevich 
> ---
> v2:
>   01: The order of the function calls to direct  command on the 
> appliance
>   and the indirect one (through the temporary file) has been swapped.
> 
>  lib/appliance-kcmdline.c | 70 
> +++-
>  1 file changed, 69 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/appliance-kcmdline.c b/lib/appliance-kcmdline.c
> index fbeb4f4..c78524b 100644
> --- a/lib/appliance-kcmdline.c
> +++ b/lib/appliance-kcmdline.c
> @@ -71,7 +71,7 @@ read_uuid (guestfs_h *g, void *retv, const char *line, 
> size_t len)
>   * The L command does the hard work.
>   */
>  static char *
> -get_root_uuid (guestfs_h *g, const char *appliance)
> +do_get_root_uuid (guestfs_h *g, const char *appliance)

get_root_uuid_with_file seems a better name, and gives less confusion
than get_root_uuid & do_get_root_uuid.

>  {
>CLEANUP_CMD_CLOSE struct command *cmd = guestfs_int_new_command (g);
>char *ret = NULL;
> @@ -96,6 +96,74 @@ get_root_uuid (guestfs_h *g, const char *appliance)
>  }
>  
>  /**
> + * Read the first 256k bytes of the in_file with L command
> + * and write them into the out_file. That may be useful to get UUID of
> + * the QCOW2 disk image with further L command.
> + * The function returns zero if successful, otherwise -1.
> + */
> +static int
> +run_qemu_img_dd (guestfs_h *g, const char *in_file, char *out_file)
> +{
> +  CLEANUP_CMD_CLOSE struct command *cmd = guestfs_int_new_command (g);
> +  int r;
> +
> +  guestfs_int_cmd_add_arg (cmd, "qemu-img");
> +  guestfs_int_cmd_add_arg (cmd, "dd");
> +  guestfs_int_cmd_add_arg_format (cmd, "if=%s", in_file);
> +  guestfs_int_cmd_add_arg_format (cmd, "of=%s", out_file);
> +  guestfs_int_cmd_add_arg (cmd, "bs=256k");
> +  guestfs_int_cmd_add_arg (cmd, "count=1");
> +
> +  r = guestfs_int_cmd_run (cmd);
> +  if (r == -1) {
> +error (g, "Failed to run qemu-img");
> +return -1;
> +  }
> +  if (!WIFEXITED (r) || WEXITSTATUS (r) != 0) {
> +guestfs_int_external_command_failed (g, r, "qemu-img dd", NULL);
> +return -1;
> +  }
> +
> +  return 0;
> +}
> +
> +/**
> + * Get the UUID from the appliance disk image.
> + */
> +static char *
> +get_root_uuid (guestfs_h *g, const char *appliance)
> +{
> +  char *uuid = NULL;
> +  int ret;
> +  char tmp_file[] = "/tmp/libguestfsXX";

Please do not hardcode a location, and use the internal temporary
directory instead. The internal guestfs_int_make_temp_path will do
the right job.

Thanks,
-- 
Pino Toscano

signature.asc
Description: This is a digitally signed message part.
___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs

[Libguestfs] [PATCH 2/9] po: remove rules for pot/po update

2020-08-12 Thread Pino Toscano
Weblate will handle the update of the po files from the translation
catalog, so avoid stomping on its feet by doing the same.

The translation catalog will be regenerated manually periodically.
---
 po/Makefile.am | 19 ---
 1 file changed, 19 deletions(-)

diff --git a/po/Makefile.am b/po/Makefile.am
index 8c1d6dd60..2c7eeb5da 100644
--- a/po/Makefile.am
+++ b/po/Makefile.am
@@ -40,17 +40,6 @@ EXTRA_DIST = \
 
 if HAVE_GNU_GETTEXT
 
-dist-hook:
-   $(MAKE) update-po
-   cp *.po *.gmo $(distdir)/
-
-update-po:
-   $(MAKE) $(DOMAIN).pot
-   $(MAKE) $(POFILES)
-   $(MAKE) update-gmo
-
-update-gmo: Makefile $(GMOFILES)
-
 XGETTEXT_ARGS = \
--default-domain=$(DOMAIN) \
--from-code=utf-8 \
@@ -87,9 +76,6 @@ endif
  --files-from=$(abs_srcdir)/POTFILES-pl
mv $@-t $@
 
-%.po: $(DOMAIN).pot
-   cd $(srcdir) && $(MSGMERGE) -N --update $@ $(DOMAIN).pot
-
 .po.gmo:
rm -f $@ $@-t
$(MSGFMT) -c -o $@-t $(srcdir)/$<
@@ -109,9 +95,4 @@ install-data-hook: $(GMOFILES)
 
 .PRECIOUS: $(DOMAIN).pot $(POFILES)
 
-else
-
-# If no GNU gettext, 'make update-po' is a dummy rule.
-update-po:
-
 endif
-- 
2.26.2

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs



[Libguestfs] [PATCH 6/9] po: fix references to OCaml sources

2020-08-12 Thread Pino Toscano
Start the message extraction from the toplevel source directory, so the
file references are relative to that, instead of relative to this po
subdirectory.
---
 po/Makefile.am | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/po/Makefile.am b/po/Makefile.am
index 2c7eeb5da..08210c681 100644
--- a/po/Makefile.am
+++ b/po/Makefile.am
@@ -65,7 +65,7 @@ $(DOMAIN).pot: Makefile POTFILES $(POTFILES) POTFILES-pl 
$(POTFILES_PL) POTFILES
rm -f $@-t
touch $@-t
 if HAVE_OCAML_GETTEXT
-   $(OCAML_GETTEXT) --action extract --extract-pot $@-t $(POTFILES_ML)
+   cd $(top_srcdir) && $(OCAML_GETTEXT) --action extract --extract-pot 
$(abs_srcdir)/$@-t $(shell cat $(abs_srcdir)/POTFILES-ml)
 \
$(FIX_CHARSET) $@-t
 endif
-- 
2.26.2

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs



[Libguestfs] [PATCH 4/9] podfiles: add missing documentation

2020-08-12 Thread Pino Toscano
---
 po-docs/podfiles | 1 +
 1 file changed, 1 insertion(+)

diff --git a/po-docs/podfiles b/po-docs/podfiles
index 0e3cbf307..b788b57d7 100644
--- a/po-docs/podfiles
+++ b/po-docs/podfiles
@@ -12,6 +12,7 @@
 ../common/mlcustomize/customize-synopsis.pod
 ../common/options/blocksize-option.pod
 ../common/options/key-option.pod
+../common/options/keys-from-stdin-option.pod
 ../customize/virt-customize.pod
 ../daemon/guestfsd.pod
 ../df/virt-df.pod
-- 
2.26.2

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs



[Libguestfs] [PATCH 8/9] po/po-docs: update catalog templates

2020-08-12 Thread Pino Toscano
Regenerate the catalog templates according to the updated extraction
rules (mostly fixed paths).
---

This patch was greatly shortened for mailing list posting.
Originally it was ~3.5M.

 po-docs/libguestfs-docs.pot | 37211 +-
 po/libguestfs.pot   |  1597 +-
 2 files changed, 19304 insertions(+), 19504 deletions(-)

diff --git a/po-docs/libguestfs-docs.pot b/po-docs/libguestfs-docs.pot
index 5e693e5cf..8e33712fe 100644
--- a/po-docs/libguestfs-docs.pot
+++ b/po-docs/libguestfs-docs.pot
@@ -8,103 +8,88 @@ msgid ""
 msgstr ""
 "Project-Id-Version: libguestfs 1.43.1\n"
 "Report-Msgid-Bugs-To: libguestfs@redhat.com\n"
-"POT-Creation-Date: 2020-07-06 15:46+0100\n"
+"POT-Creation-Date: 2020-08-12 15:34+0200\n"
 "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n"
 "Last-Translator: FULL NAME \n"
 "Language-Team: LANGUAGE \n"
 "Language: \n"
 "MIME-Version: 1.0\n"
-"Content-Type: text/plain; charset=utf-8\n"
+"Content-Type: text/plain; charset=UTF-8\n"
 "Content-Transfer-Encoding: 8bit\n"
 
 #. type: =head1
-#: ../align/virt-alignment-scan.pod:1
-#: ../appliance/libguestfs-make-fixed-appliance.pod:1
-#: ../builder/virt-builder-repository.pod:8 ../builder/virt-builder.pod:8
-#: ../builder/virt-index-validate.pod:1 ../cat/virt-cat.pod:1
-#: ../cat/virt-filesystems.pod:1 ../cat/virt-log.pod:1 ../cat/virt-ls.pod:1
-#: ../cat/virt-tail.pod:1 ../customize/virt-customize.pod:1
-#: ../daemon/guestfsd.pod:1 ../df/virt-df.pod:1 ../dib/virt-dib.pod:1
-#: ../diff/virt-diff.pod:1 ../docs/guestfs-building.pod:1
-#: ../docs/guestfs-faq.pod:1 ../docs/guestfs-hacking.pod:1
-#: ../docs/guestfs-internals.pod:1 ../docs/guestfs-performance.pod:1
-#: ../docs/guestfs-recipes.pod:8 ../docs/guestfs-release-notes-1.10.pod:1
-#: ../docs/guestfs-release-notes-1.12.pod:1
-#: ../docs/guestfs-release-notes-1.14.pod:1
-#: ../docs/guestfs-release-notes-1.16.pod:1
-#: ../docs/guestfs-release-notes-1.18.pod:1
-#: ../docs/guestfs-release-notes-1.20.pod:1
-#: ../docs/guestfs-release-notes-1.22.pod:1
-#: ../docs/guestfs-release-notes-1.24.pod:1
-#: ../docs/guestfs-release-notes-1.26.pod:1
-#: ../docs/guestfs-release-notes-1.28.pod:1
-#: ../docs/guestfs-release-notes-1.30.pod:1
-#: ../docs/guestfs-release-notes-1.32.pod:1
-#: ../docs/guestfs-release-notes-1.34.pod:1
-#: ../docs/guestfs-release-notes-1.36.pod:1
-#: ../docs/guestfs-release-notes-1.38.pod:1
-#: ../docs/guestfs-release-notes-1.4.pod:1
-#: ../docs/guestfs-release-notes-1.40.pod:1
-#: ../docs/guestfs-release-notes-1.42.pod:1
-#: ../docs/guestfs-release-notes-1.6.pod:1
-#: ../docs/guestfs-release-notes-1.8.pod:1
-#: ../docs/guestfs-release-notes-historical.pod:1
-#: ../docs/guestfs-security.pod:1 ../docs/guestfs-testing.pod:1
-#: ../edit/virt-edit.pod:1 ../erlang/examples/guestfs-erlang.pod:1
-#: ../examples/guestfs-examples.pod:1 ../fish/guestfish.pod:1
-#: ../fish/libguestfs-tools.conf.pod:1 ../fish/virt-copy-in.pod:1
-#: ../fish/virt-copy-out.pod:1 ../fish/virt-tar-in.pod:1
-#: ../fish/virt-tar-out.pod:1 ../format/virt-format.pod:1
-#: ../fuse/guestmount.pod:1 ../fuse/guestunmount.pod:1
-#: ../get-kernel/virt-get-kernel.pod:1 ../gobject/guestfs-gobject.pod:1
-#: ../golang/examples/guestfs-golang.pod:1 ../inspector/virt-inspector.pod:1
-#: ../java/examples/guestfs-java.pod:1 ../lib/guestfs.pod:1
-#: ../lua/examples/guestfs-lua.pod:1 ../make-fs/virt-make-fs.pod:1
-#: ../ocaml/examples/guestfs-ocaml.pod:1 ../perl/examples/guestfs-perl.pod:1
-#: ../python/examples/guestfs-python.pod:1 ../rescue/virt-rescue.pod:1
-#: ../resize/virt-resize.pod:1 ../ruby/examples/guestfs-ruby.pod:1
-#: ../sparsify/virt-sparsify.pod:1 ../sysprep/virt-sysprep.pod:1
-#: ../test-tool/libguestfs-test-tool.pod:1 ../tools/virt-list-filesystems:27
-#: ../tools/virt-list-partitions:27 ../tools/virt-tar:28
-#: ../tools/virt-win-reg:32
+#: align/virt-alignment-scan.pod:1
+#: appliance/libguestfs-make-fixed-appliance.pod:1
+#: builder/virt-builder-repository.pod:8 builder/virt-builder.pod:8
+#: builder/virt-index-validate.pod:1 cat/virt-cat.pod:1
+#: cat/virt-filesystems.pod:1 cat/virt-log.pod:1 cat/virt-ls.pod:1
+#: cat/virt-tail.pod:1 customize/virt-customize.pod:1 daemon/guestfsd.pod:1
+#: df/virt-df.pod:1 dib/virt-dib.pod:1 diff/virt-diff.pod:1
+#: docs/guestfs-building.pod:1 docs/guestfs-faq.pod:1
+#: docs/guestfs-hacking.pod:1 docs/guestfs-internals.pod:1
+#: docs/guestfs-performance.pod:1 docs/guestfs-recipes.pod:8
+#: docs/guestfs-release-notes-1.10.pod:1 docs/guestfs-release-notes-1.12.pod:1
+#: docs/guestfs-release-notes-1.14.pod:1 docs/guestfs-release-notes-1.16.pod:1
+#: docs/guestfs-release-notes-1.18.pod:1 docs/guestfs-release-notes-1.20.pod:1
+#: docs/guestfs-release-notes-1.22.pod:1 docs/guestfs-release-notes-1.24.pod:1
+#: docs/guestfs-release-notes-1.26.pod:1 docs/guestfs-release-notes-1.28.pod:1
+#: docs/guestfs-release-notes-1.30.pod:1 docs/guestfs-release-notes-1.32.pod:1
+#: docs/guestfs-release-notes-1.34.pod:1 docs/guestfs-release-notes-1.36.pod:1
+#: 

[Libguestfs] [PATCH 0/9] Adaptations to Weblate

2020-08-12 Thread Pino Toscano
We are migrating to Weblate (the Fedora instance, in particular) for
translations instead of Zanata. Adapt our tooling a bit to the different
workflow:
- Weblate takes care of updating the po files whenever a new translation
  catalog is available, so stop doing that on our own: this meant also
  tweaking the po4a usage for POD documentations, resulting in simpler
  rules (IMHO)
- ensure that the references to sources files in translation catalogs
  are relative to the top-level directory, so Weblate (and users too,
  actually) can properly locate the sources
- regenerate the catalogs
- drop references to Zanata

Pino Toscano (9):
  po: turn language list into LINGUAS file
  po: remove rules for pot/po update
  po-docs: add missing dependencies for guestfish.1
  podfiles: add missing documentation
  po-docs: split pot and po handling
  po: fix references to OCaml sources
  po-docs: fix references to sources
  po/po-docs: update catalog templates
  Remove references to Zanata

 .gitignore  | 1 -
 Makefile.am | 4 +-
 docs/guestfs-hacking.pod|14 -
 m4/guestfs-progs.m4 | 5 +-
 po-docs/Makefile.am |50 +-
 po-docs/language.mk |31 +-
 po-docs/libguestfs-docs.pot | 37211 +-
 po-docs/podfiles|   181 +-
 po/LINGUAS  |22 +
 po/Makefile.am  |24 +-
 po/libguestfs.pot   |  1597 +-
 subdir-rules.mk | 3 +
 zanata-pull.sh  |30 -
 zanata.xml  | 8 -
 14 files changed, 19456 insertions(+), 19725 deletions(-)
 create mode 100644 po/LINGUAS
 delete mode 100755 zanata-pull.sh
 delete mode 100644 zanata.xml

-- 
2.26.2

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs



[Libguestfs] [PATCH 5/9] po-docs: split pot and po handling

2020-08-12 Thread Pino Toscano
With the Weblate adoption, we let it update the po files from the
catalog template. The po4a behaviour of extracting the template,
merging the existing translations, and creating the translated PODs at
once is problematic. Hence, split the extraction and the translated POD
generation in two.

Use po4a-gettextize to extract the catalog template only, not doing it
anymore automatically at each build. There is no more need for a
po4a.conf file.

Use po4a-translate to create translated PODs from the po files, keeping
the fixup of the generated files (to avoid spurious =encoding, etc).
Add a silent rule to hide the po4a-translate command lines by default.

These changes also allow us to get rid of the POD existance checks with
associated error message pointing to the update-po rule. Now each
translated POD file is generated because of make dependency, and it
depends only on its po file.

Signed-off-by: Pino Toscano 
---
 .gitignore  |  1 -
 m4/guestfs-progs.m4 |  5 +++--
 po-docs/Makefile.am | 37 ++---
 po-docs/language.mk | 29 +++--
 subdir-rules.mk |  3 +++
 5 files changed, 27 insertions(+), 48 deletions(-)

diff --git a/.gitignore b/.gitignore
index cdc90453c..ab4cd0667 100644
--- a/.gitignore
+++ b/.gitignore
@@ -444,7 +444,6 @@ Makefile.in
 /po-docs/*/*.3
 /po-docs/*/*.5
 /po-docs/*/*.8
-/po-docs/po4a.conf
 /po-docs/*/*.pod
 /podwrapper.1
 /podwrapper.pl
diff --git a/m4/guestfs-progs.m4 b/m4/guestfs-progs.m4
index c04ab8813..bf1f83c9d 100644
--- a/m4/guestfs-progs.m4
+++ b/m4/guestfs-progs.m4
@@ -59,8 +59,9 @@ AC_CHECK_PROG([XMLLINT],[xmllint],[xmllint],[no])
 AM_CONDITIONAL([HAVE_XMLLINT],[test "x$XMLLINT" != "xno"])
 
 dnl po4a for translating man pages and POD files (optional).
-AC_CHECK_PROG([PO4A],[po4a],[po4a],[no])
-AM_CONDITIONAL([HAVE_PO4A], [test "x$PO4A" != "xno"])
+AC_CHECK_PROG([PO4A_GETTEXTIZE],[po4a-gettextize],[po4a-gettextize],[no])
+AC_CHECK_PROG([PO4A_TRANSLATE],[po4a-translate],[po4a-translate],[no])
+AM_CONDITIONAL([HAVE_PO4A], [test "x$PO4A_GETTEXTIZE" != "xno" && test 
"x$PO4A_TRANSLATE" != "xno"])
 
 dnl Check for db_dump, db_load (optional).
 GUESTFS_FIND_DB_TOOL([DB_DUMP], [dump])
diff --git a/po-docs/Makefile.am b/po-docs/Makefile.am
index 24d1a0338..aaffa0520 100644
--- a/po-docs/Makefile.am
+++ b/po-docs/Makefile.am
@@ -35,47 +35,22 @@ EXTRA_DIST = \
$(linguas_not_translated:%=%.po) \
podfiles
 
-CLEANFILES += po4a.conf
-
 # Build the final man pages from the translated POD files.  Each
 # language directory contains a Makefile.am that we need to keep up to
 # date (note each $lang/Makefile.am should be identical).
 # XXX Is there a better way?
 SUBDIRS = $(linguas)
 
-update-po: libguestfs-docs.pot
-
-# Note: po4a puts the following junk at the top of every POD file it
-# generates:
-#  - a warning
-#  - a probably bogus =encoding line
-# Remove both.
-# XXX Fix po4a so it doesn't do this.
-libguestfs-docs.pot: po4a.conf
-   $(PO4A) \
- -M utf-8 -L utf-8 -A utf-8 \
- -v \
- -k 0 \
+libguestfs-docs.pot:
+   $(PO4A_GETTEXTIZE) \
+ -f pod \
+ -M utf-8 -L utf-8 \
  --package-name $(PACKAGE_NAME) \
  --package-version $(PACKAGE_VERSION) \
  --msgid-bugs-address libguestfs@redhat.com \
  --copyright-holder "Red Hat Inc." \
- po4a.conf
-   for f in `cd $(srcdir); find $(linguas) -name '*.pod'`; do \
- $(SED) '0,/^=encoding/d' < $$f > $$f.new; \
- mv $$f.new $$f; \
-   done
-
-po4a.conf: podfiles
-   rm -f $@-t
-   echo "[po_directory] $(srcdir)" >> $@-t
-   echo >> $@-t
-   for f in `cat podfiles`; do \
- out=`basename -- $$f .pod`.pod; \
- echo "[type: pod] $$f \$$lang:\$$lang/$$out" >> $@-t; \
- echo >> $@-t; \
-   done;
-   mv $@-t $@
+ -p $@ \
+ $(patsubst %,-m %,$(shell cat $(srcdir)/podfiles))
 
 podfiles: Makefile
rm -f $@ $@-t
diff --git a/po-docs/language.mk b/po-docs/language.mk
index ff25a0719..354facafe 100644
--- a/po-docs/language.mk
+++ b/po-docs/language.mk
@@ -22,7 +22,7 @@ include $(top_srcdir)/subdir-rules.mk
 LINGUA = $(shell basename -- `pwd`)
 
 # Before 1.23.23, the old Perl tools were called *.pl.
-CLEANFILES += *.pl
+CLEANFILES += *.pl *.pod
 
 MANPAGES = \
guestfish.1 \
@@ -195,19 +195,20 @@ virt-p2v.1: virt-p2v.pod virt-p2v-kernel-config.pod
  --section 8 \
  $<
 
-# If a POD file is missing, the user needs to run make update-po.
-# This cannot be done automatically by make because it would be unsafe
-# to run po4a or update podfiles potentially in parallel.  Therefore
-# tell the user what to do and stop.
-$(podfiles):
-   @if ! test -f $@; then \
- echo "***"; \
- echo "

[Libguestfs] [PATCH 9/9] Remove references to Zanata

2020-08-12 Thread Pino Toscano
We migrated to Weblate, and Zanata is being decommissioned.
---
 Makefile.am  |  4 +---
 docs/guestfs-hacking.pod | 14 --
 zanata-pull.sh   | 30 --
 zanata.xml   |  8 
 4 files changed, 1 insertion(+), 55 deletions(-)
 delete mode 100755 zanata-pull.sh
 delete mode 100644 zanata.xml

diff --git a/Makefile.am b/Makefile.am
index f9ca98602..0a8f139a3 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -261,9 +261,7 @@ EXTRA_DIST = \
website/index.css \
website/index.html.in \
website/pod.css \
-   website/standard.css \
-   zanata.xml \
-   zanata-pull.sh
+   website/standard.css
 
 WEBSITESDIR = $(HOME)/d/websites
 
diff --git a/docs/guestfs-hacking.pod b/docs/guestfs-hacking.pod
index 68040fd4c..90923f450 100644
--- a/docs/guestfs-hacking.pod
+++ b/docs/guestfs-hacking.pod
@@ -1029,20 +1029,6 @@ Finalize F
 
 =item *
 
-Push and pull from Zanata.
-
-Run:
-
- zanata push
-
-to push the latest POT files to Zanata.  Then run:
-
- ./zanata-pull.sh
-
-which is a wrapper to pull the latest translated F<*.po> files.
-
-=item *
-
 Consider updating gnulib to latest upstream version.
 
 =item *
diff --git a/zanata-pull.sh b/zanata-pull.sh
deleted file mode 100755
index ee5f88303..0
--- a/zanata-pull.sh
+++ /dev/null
@@ -1,30 +0,0 @@
-#!/bin/bash -
-# Pull translations from Zanata.
-# Copyright (C) 2011-2020 Red Hat Inc.
-#
-# This program is free software; you can redistribute it and/or modify
-# it under the terms of the GNU General Public License as published by
-# the Free Software Foundation; either version 2 of the License, or
-# (at your option) any later version.
-#
-# This program is distributed in the hope that it will be useful,
-# but WITHOUT ANY WARRANTY; without even the implied warranty of
-# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-# GNU General Public License for more details.
-#
-# You should have received a copy of the GNU General Public License
-# along with this program; if not, write to the Free Software
-# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
-
-set -e
-
-echo zanata po pull
-zanata po pull
-
-# Remove PO files that have no translations in them.
-for f in po/*.po po-docs/*.po; do
-if ! grep -q '^msgstr "[^"]' $f; then
-echo rm $f
-rm $f
-fi
-done
diff --git a/zanata.xml b/zanata.xml
deleted file mode 100644
index ee4c20c62..0
--- a/zanata.xml
+++ /dev/null
@@ -1,8 +0,0 @@
-
-http://zanata.org/namespace/config/;>
-  https://fedora.zanata.org/
-  libguestfs
-  master
-  gettext
-
-
-- 
2.26.2

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs



[Libguestfs] [PATCH 7/9] po-docs: fix references to sources

2020-08-12 Thread Pino Toscano
Start the message extraction from the toplevel source directory, so the
file references are relative to that, instead of relative to this
po-docs subdirectory.

Also update/regenerate podfiles accordingly.
---
 po-docs/Makefile.am |  17 +++--
 po-docs/language.mk |   2 +-
 po-docs/podfiles| 182 ++--
 3 files changed, 101 insertions(+), 100 deletions(-)

diff --git a/po-docs/Makefile.am b/po-docs/Makefile.am
index aaffa0520..028fbbcea 100644
--- a/po-docs/Makefile.am
+++ b/po-docs/Makefile.am
@@ -42,24 +42,25 @@ EXTRA_DIST = \
 SUBDIRS = $(linguas)
 
 libguestfs-docs.pot:
-   $(PO4A_GETTEXTIZE) \
+   cd $(top_srcdir) && $(PO4A_GETTEXTIZE) \
  -f pod \
  -M utf-8 -L utf-8 \
  --package-name $(PACKAGE_NAME) \
  --package-version $(PACKAGE_VERSION) \
  --msgid-bugs-address libguestfs@redhat.com \
  --copyright-holder "Red Hat Inc." \
- -p $@ \
+ -p $(abs_srcdir)/$@ \
  $(patsubst %,-m %,$(shell cat $(srcdir)/podfiles))
 
 podfiles: Makefile
rm -f $@ $@-t
-   find $(top_srcdir) -name '*.pod' | \
- grep -v /debian/ | \
- grep -v /libguestfs-1 | \
- grep -v /po-docs/ | \
+   cd $(top_srcdir) && find . -name '*.pod' -printf '%P\n'| \
+ grep -v ^debian/ | \
+ grep -v ^libguestfs-1 | \
+ grep -v ^po-docs/ | \
+ grep -v ^stamp- | \
  grep -v /stamp- \
- > $@-t
-   for f in `cat $(top_srcdir)/po/POTFILES-pl`; do echo $(top_srcdir)/$$f; 
done >> $@-t
+ > $(abs_srcdir)/$@-t
+   cat $(top_srcdir)/po/POTFILES-pl >> $@-t
LC_ALL=C sort -o $@-t $@-t
mv $@-t $@
diff --git a/po-docs/language.mk b/po-docs/language.mk
index 354facafe..88d7d3431 100644
--- a/po-docs/language.mk
+++ b/po-docs/language.mk
@@ -206,7 +206,7 @@ virt-p2v.1: virt-p2v.pod virt-p2v-kernel-config.pod
  -f pod \
  -M utf-8 -L utf-8 \
  -k 0 \
- -m $(srcdir)/../$(shell grep '/$(notdir $@)$$' 
$(top_srcdir)/po-docs/podfiles) \
+ -m $(top_srcdir)/$(shell grep '/$(notdir $@)$$' 
$(top_srcdir)/po-docs/podfiles) \
  -p $< \
  | $(SED) '0,/^=encoding/d' > $@
 
diff --git a/po-docs/podfiles b/po-docs/podfiles
index b788b57d7..9bde01aab 100644
--- a/po-docs/podfiles
+++ b/po-docs/podfiles
@@ -1,91 +1,91 @@
-../align/virt-alignment-scan.pod
-../appliance/libguestfs-make-fixed-appliance.pod
-../builder/virt-builder-repository.pod
-../builder/virt-builder.pod
-../builder/virt-index-validate.pod
-../cat/virt-cat.pod
-../cat/virt-filesystems.pod
-../cat/virt-log.pod
-../cat/virt-ls.pod
-../cat/virt-tail.pod
-../common/mlcustomize/customize-options.pod
-../common/mlcustomize/customize-synopsis.pod
-../common/options/blocksize-option.pod
-../common/options/key-option.pod
-../common/options/keys-from-stdin-option.pod
-../customize/virt-customize.pod
-../daemon/guestfsd.pod
-../df/virt-df.pod
-../dib/virt-dib.pod
-../diff/virt-diff.pod
-../docs/guestfs-building.pod
-../docs/guestfs-faq.pod
-../docs/guestfs-hacking.pod
-../docs/guestfs-internals.pod
-../docs/guestfs-performance.pod
-../docs/guestfs-recipes.pod
-../docs/guestfs-release-notes-1.10.pod
-../docs/guestfs-release-notes-1.12.pod
-../docs/guestfs-release-notes-1.14.pod
-../docs/guestfs-release-notes-1.16.pod
-../docs/guestfs-release-notes-1.18.pod
-../docs/guestfs-release-notes-1.20.pod
-../docs/guestfs-release-notes-1.22.pod
-../docs/guestfs-release-notes-1.24.pod
-../docs/guestfs-release-notes-1.26.pod
-../docs/guestfs-release-notes-1.28.pod
-../docs/guestfs-release-notes-1.30.pod
-../docs/guestfs-release-notes-1.32.pod
-../docs/guestfs-release-notes-1.34.pod
-../docs/guestfs-release-notes-1.36.pod
-../docs/guestfs-release-notes-1.38.pod
-../docs/guestfs-release-notes-1.4.pod
-../docs/guestfs-release-notes-1.40.pod
-../docs/guestfs-release-notes-1.42.pod
-../docs/guestfs-release-notes-1.6.pod
-../docs/guestfs-release-notes-1.8.pod
-../docs/guestfs-release-notes-historical.pod
-../docs/guestfs-security.pod
-../docs/guestfs-testing.pod
-../docs/internal-documentation.pod
-../edit/virt-edit.pod
-../erlang/examples/guestfs-erlang.pod
-../examples/guestfs-examples.pod
-../fish/guestfish-actions.pod
-../fish/guestfish-commands.pod
-../fish/guestfish-prepopts.pod
-../fish/guestfish.pod
-../fish/libguestfs-tools.conf.pod
-../fish/virt-copy-in.pod
-../fish/virt-copy-out.pod
-../fish/virt-tar-in.pod
-../fish/virt-tar-out.pod
-../format/virt-format.pod
-../fuse/guestmount.pod
-../fuse/guestunmount.pod
-../get-kernel/virt-get-kernel.pod
-../gobject/guestfs-gobject.pod
-../golang/examples/guestfs-golang.pod
-../inspector/virt-inspector.pod
-../java/examples/guestfs-java.pod
-../lib/guestfs-actions.pod
-../lib/guestfs-availability.pod
-../lib/guestfs-structs.pod
-../lib/guestfs.pod
-../lua/examples/guestfs-lua.pod
-../make-fs/virt-make-fs.pod
-../ocaml/examples/guestfs-ocaml.pod
-../perl/examples/guestfs-perl.pod

[Libguestfs] [PATCH 1/9] po: turn language list into LINGUAS file

2020-08-12 Thread Pino Toscano
Use a LINGUAS file with the list of available translations instead of
defining them in a make variable. This way Weblate will be able to
update the list using an available addon.

Signed-off-by: Pino Toscano 
---
 po/LINGUAS | 22 ++
 po/Makefile.am |  3 ++-
 2 files changed, 24 insertions(+), 1 deletion(-)
 create mode 100644 po/LINGUAS

diff --git a/po/LINGUAS b/po/LINGUAS
new file mode 100644
index 0..696044892
--- /dev/null
+++ b/po/LINGUAS
@@ -0,0 +1,22 @@
+cs
+de
+en_GB
+es
+eu
+fr
+gu
+hi
+ja
+kn
+ml
+mr
+nl
+or
+pa
+pl
+ru
+ta
+te
+tg
+uk
+zh_CN
diff --git a/po/Makefile.am b/po/Makefile.am
index ea525dc6c..8c1d6dd60 100644
--- a/po/Makefile.am
+++ b/po/Makefile.am
@@ -23,7 +23,7 @@ MSGID_BUGS_ADDRESS = 
https://bugzilla.redhat.com/enter_bug.cgi?component=libgues
 
 # Languages.
 # Don't use LINGUAS (uppercase) as Gentoo defines it (RHBZ#804464).
-linguas := cs de en_GB es eu fr gu hi ja kn ml mr nl or pa pl ru ta te tg 
uk zh_CN
+linguas := $(shell cat $(srcdir)/LINGUAS)
 
 POTFILES:= $(shell $(SED) 's,^,$(top_srcdir)/,' $(srcdir)/POTFILES)
 POTFILES_PL := $(shell $(SED) 's,^,$(top_srcdir)/,' $(srcdir)/POTFILES-pl)
@@ -32,6 +32,7 @@ POFILES := $(linguas:%=%.po)
 GMOFILES:= $(linguas:%=%.gmo)
 
 EXTRA_DIST = \
+   LINGUAS \
POTFILES POTFILES-pl POTFILES-ml \
$(DOMAIN).pot \
$(POFILES) \
-- 
2.26.2

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs



[Libguestfs] [PATCH 3/9] po-docs: add missing dependencies for guestfish.1

2020-08-12 Thread Pino Toscano
The guestfish man page uses also additional POD snippets, so list them
as dependencies to make sure they are up-to-date.

This does not change the behaviour at the moment, however it will matter
when each traslated POD file will be generated on its own.
---
 po-docs/language.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/po-docs/language.mk b/po-docs/language.mk
index 108cf8c4c..ff25a0719 100644
--- a/po-docs/language.mk
+++ b/po-docs/language.mk
@@ -126,7 +126,7 @@ guestfs.3: guestfs.pod guestfs-actions.pod 
guestfs-availability.pod guestfs-stru
 # generated in any translated manual.  To fix this we need to expand
 # out all the %.1 pattern rules below.
 
-guestfish.1: guestfish.pod guestfish-actions.pod guestfish-commands.pod 
guestfish-prepopts.pod
+guestfish.1: guestfish.pod guestfish-actions.pod guestfish-commands.pod 
guestfish-prepopts.pod blocksize-option.pod key-option.pod 
keys-from-stdin-option.pod
$(PODWRAPPER) \
  --no-strict-checks \
  --man $@ \
-- 
2.26.2

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs



[Libguestfs] [v2v PATCH] libosinfo: remove auto-cleanup for OsinfoList

2020-08-05 Thread Pino Toscano
Avoid using an auto-cleanup for OsinfoList, duplicating the cleanup
everywhere needed.
---
 v2v/libosinfo-c.c | 18 +-
 1 file changed, 5 insertions(+), 13 deletions(-)

diff --git a/v2v/libosinfo-c.c b/v2v/libosinfo-c.c
index 322e7d3d..75c2fae4 100644
--- a/v2v/libosinfo-c.c
+++ b/v2v/libosinfo-c.c
@@ -49,17 +49,6 @@
 #if !IS_LIBOSINFO_VERSION(1, 8, 0)
 G_DEFINE_AUTOPTR_CLEANUP_FUNC(OsinfoFilter, g_object_unref)
 G_DEFINE_AUTOPTR_CLEANUP_FUNC(OsinfoLoader, g_object_unref)
-/*
- * Because of a bug in OsinfoList in libosinfo 1.7.0 (fixed in 1.8.0),
- * and a glib auto-cleanup addition for Module classes in 2.63.3,
- * avoid declaring this when:
- * - libosinfo is >= 1.7.0 and < 1.8.0
- * - glib is >= 2.63.3
- * (the 1.8.0 check is not done, as already covered by the check above)
- */
-#if !IS_LIBOSINFO_VERSION(1, 7, 0) || !GLIB_CHECK_VERSION(2, 63, 3)
-G_DEFINE_AUTOPTR_CLEANUP_FUNC(OsinfoList, g_object_unref)
-#endif
 G_DEFINE_AUTOPTR_CLEANUP_FUNC(OsinfoOsList, g_object_unref)
 #endif
 
@@ -157,7 +146,7 @@ v2v_osinfo_os_find_os_by_short_id (value dbv, value osv)
   CAMLlocal1 (rv);
   g_autoptr(OsinfoFilter) filter = NULL;
   g_autoptr(OsinfoOsList) os_list = NULL;
-  g_autoptr(OsinfoList) list = NULL;
+  OsinfoList *list;
   OsinfoOs *os;
 
   os_list = osinfo_db_get_os_list (OsinfoDb_t_val (dbv));
@@ -165,11 +154,14 @@ v2v_osinfo_os_find_os_by_short_id (value dbv, value osv)
   osinfo_filter_add_constraint (filter, OSINFO_PRODUCT_PROP_SHORT_ID, 
String_val (osv));
   list = osinfo_list_new_filtered (OSINFO_LIST(os_list), filter);
 
-  if (osinfo_list_get_length (list) == 0)
+  if (osinfo_list_get_length (list) == 0) {
+g_object_unref (list);
 caml_raise_not_found ();
+  }
 
   os = OSINFO_OS(osinfo_list_get_nth (list, 0));
   rv = Val_OsinfoOs_t (dbv, os);
+  g_object_unref (list);
 
   CAMLreturn (rv);
 }
-- 
2.26.2

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs



Re: [Libguestfs] [PATCH 7/7] lib/canonical-name.c: Hide errors.

2020-07-27 Thread Pino Toscano
On Monday, 30 March 2020 17:03:42 CEST Richard W.M. Jones wrote:
> Fixes “XXX” comment.  This turns out to be necessary in order to
> suppress a warning when inspecting Windows BitLocker-encrypted guests.
> ---
>  lib/canonical-name.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/canonical-name.c b/lib/canonical-name.c
> index 052bbf12c..11cf6fed6 100644
> --- a/lib/canonical-name.c
> +++ b/lib/canonical-name.c
> @@ -46,8 +46,9 @@ guestfs_impl_canonical_device_name (guestfs_h *g, const 
> char *device)
>}
>else if (STRPREFIX (device, "/dev/mapper/") ||
> STRPREFIX (device, "/dev/dm-")) {
> -/* XXX hide errors */
> +guestfs_push_error_handler (g, NULL, NULL);
>  ret = guestfs_lvm_canonical_lv_name (g, device);
> +guestfs_pop_error_handler (g);

Which warning is it? This also ignores all the errors from the API.

-- 
Pino Toscano

signature.asc
Description: This is a digitally signed message part.
___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [PATCH 1/7] New APIs: cryptsetup-open and cryptsetup-close.

2020-07-27 Thread Pino Toscano
On Monday, 30 March 2020 17:03:36 CEST Richard W.M. Jones wrote:
> This commit deprecates luks-open/luks-open-ro/luks-close for the more
> generic sounding names cryptsetup-open/cryptsetup-close, which also
> correspond directly to the cryptsetup commands.
> 
> The optional cryptsetup-open readonly flag is used to replace the
> functionality of luks-open-ro.
> 
> The optional cryptsetup-open crypttype parameter can be used to select
> the type (corresponding to cryptsetup open --type), which allows us to
> open BitLocker-encrypted disks with no extra effort.  As a convenience
> the crypttype parameter may be omitted, and libguestfs will use a
> heuristic (based on vfs-type output) to try to determine the correct
> type to use.

At least in my (non extensive) tests with cryptsetup, it seems it can
detect the right format even without --type=format or the luksOpen/etc
aliases.

What I'd do is:
- drop the autodetection: it is a mild duplication of what cryptsetup
  already does, and adding it in the API now means that it will be
  stuck there...
- maybe (need to think about it) make the crypttype parameter mandatory,
  so the user has to select the right format

> The deprecated functions and the new functions are all (re-)written in
> OCaml.

Regarding the deprecated functions: wouldn't it be better to have them
in the library, rather than in the daemon? The machinery needed in the
library is more or less than same anyway, and this way we can remove
two RPCs in the daemon.

Also, please bump the versions of the new APIs.

> +let cryptsetup_open ?(readonly = false) ?crypttype device key mapname =
> +  (* /dev/mapper/mapname must not exist already. *)

  /* Sanity check: /dev/mapper/mapname must not exist already.  Note
   * that the device-mapper control device (/dev/mapper/control) is
   * always there, so you can't ever have mapname == "control".
   */

This whole comment seems useful, it would be a pity to lose it in the
conversion. Also, unrelated to this patch: IMHO it would be a good idea
to explicitly note this limitation in the API docs.

> +  (* Write the key to a temporary file. *)
> +  let keyfile, chan = Filename.open_temp_file "crypt" ".key" in
> +  fchmod (descr_of_out_channel chan) 0o600;

This fchmod is not needed, as temporary files are already 600 by
default.

> +  output_string chan key;
> +  close_out chan;
> +
> +  (* Make sure we always remove the temporary file. *)
> +  protect ~f:(
> +fun () ->
> +  let args = ref [] in
> +  List.push_back_list args ["-d"; keyfile];
> +  if readonly then List.push_back args "--readonly";
> +  List.push_back_list args ["open"; device; mapname; "--type"; 
> crypttype];

Minor nit: even if this way makes sense more from a scope POV, I'd
instead build the args list out of the protect. Mostly for ease of
reading.

> +(* Deprecated APIs for backwards compatibility.
> + *
> + * For convenient for existing callers we do not specify the
> + * crypttype parameter.  This means that callers of these APIs
> + * will be able to open BitLocker drives transparently.  (If
> + * we wanted to be strict we would pass ~crypttype:"luks"
> + * here, but that seems to have only downsides).

I disagree here: if an user uses the luks_open API, IMHO they expect it
to be LUKS; if they want to use other types, then they want to switch
to the new cryptsetup_open. So I'd pass ~crypttype:"luks" in luks_open.

> +  { defaults with
> +name = "cryptsetup_open"; added = (1, 43, 1);
> +style = RErr, [String (Device, "device"); String (Key, "key"); String 
> (PlainString, "mapname")], [OBool "readonly"; OString "crypttype"];
> +impl = OCaml "Cryptsetup.cryptsetup_open";
> +optional = Some "luks";
> +test_excuse = "no way to format BitLocker, and smallest device is huge";
> +shortdesc = "open an encrypted block device";
> +longdesc = "\
> +This command opens a block device which has been encrypted
> +according to the Linux Unified Key Setup (LUKS) standard,
> +Windows BitLocker, or some other types.
> +
> +C is the encrypted block device or partition.
> +
> +The caller must supply one of the keys associated with the
> +encrypted block device, in the C parameter.
> +
> +This creates a new block device called F.
> +Reads and writes to this block device are decrypted from and
> +encrypted to the underlying C respectively.
> +
> +If the optional C parameter is not present then
> +libguestfs tries to guess the correct type (for example
> +LUKS or BitLocker).  However you can override this by
> +specifying one of the following types:
> +
> +=over 4
>

Re: [Libguestfs] [PATCH common 3/4] options: Ignore errors from guestfs_luks_uuid.

2020-07-27 Thread Pino Toscano
On Monday, 30 March 2020 16:58:48 CEST Richard W.M. Jones wrote:
> For BitLocker disks cryptsetup does not (yet? ever?) support reading
> UUIDs and this function will fail.  This does not matter here so just
> ignore the error.
> 
> Updates commit bb4a2dc17a78b53437896d4215ae82df8e11b788.
> ---
>  options/decrypt.c | 6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/options/decrypt.c b/options/decrypt.c
> index 58f8df9..069a83f 100644
> --- a/options/decrypt.c
> +++ b/options/decrypt.c
> @@ -83,7 +83,11 @@ inspect_do_decrypt (guestfs_h *g, struct key_store *ks)
>CLEANUP_FREE char *mapname = make_mapname (partitions[i]);
>  
>  #ifdef GUESTFS_HAVE_LUKS_UUID
> -  CLEANUP_FREE char *uuid = guestfs_luks_uuid (g, partitions[i]);
> +  /* This may fail for Windows BitLocker disks, so hide errors. */
> +  CLEANUP_FREE char *uuid;
> +  guestfs_push_error_handler (g, NULL, NULL);
> +  uuid = guestfs_luks_uuid (g, partitions[i]);
> +  guestfs_pop_error_handler (g);

The problem is that this suppressed all the errors, even valid ones in
Linux guests, which we want to know.

What is the actual error? Can we detect and selectively ignore it
instead?

-- 
Pino Toscano

signature.asc
Description: This is a digitally signed message part.
___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [PATCH common 2/4] options: Generate cryptsetup mapnames beginning with "crypt..." not "luks..."

2020-07-27 Thread Pino Toscano
On Monday, 30 March 2020 16:58:47 CEST Richard W.M. Jones wrote:
> -  for (; device[i] != '\0' && len >= 1; ++i) {
> -if (c_isalnum (device[i])) {
> -  *mapname++ = device[i];
> -  len--;
> -}
> +  if (asprintf (, "crypt%s", [i]) == -1)
> +error (EXIT_FAILURE, errno, "asprintf");
> +
> +  for (i = 5; i < strlen (ret); ++i) {
> +if (!c_isalnum (ret[i]))
> +  memmove ([i], [i+1], strlen ([i]));
>}

I'd say that this can be simplified even more: instead of first copying
the string (asprintf) and then remove all the unwanted characters by
shifting the characters after (with memmove), simply allocate a string
of the maximum length needed and then copy the wanted characters.

... that is, use the old approach.

-- 
Pino Toscano

signature.asc
Description: This is a digitally signed message part.
___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [PATCH] daemon: inspect_fs_windows: Handle parted errors

2020-06-30 Thread Pino Toscano
On Tuesday, 30 June 2020 10:33:40 CEST Sam Eiderman wrote:
> By creating an empty disk and using it as the first disk of the vm (i.e.
> /dev/sda, /dev/sdb{1,2} contains the windows fses) we change the
> iteration order of the disks.
> This causes inspect_os() to fail since Parted returns a Unix_error if
> the device does not contain any partitions - fix this by handling this
> Unix_error.
> 
> Signed-off-by: Sam Eiderman 
> ---
>  daemon/inspect_fs_windows.ml | 11 ---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/daemon/inspect_fs_windows.ml b/daemon/inspect_fs_windows.ml
> index c4a05bc38..bc6b98b60 100644
> --- a/daemon/inspect_fs_windows.ml
> +++ b/daemon/inspect_fs_windows.ml
> @@ -365,8 +365,10 @@ and map_registry_disk_blob_mbr devices blob =
>  let device =
>List.find (
>  fun dev ->
> -  Parted.part_get_parttype dev = "msdos" &&
> -pread dev 4 0x01b8 = diskid
> +  try
> +Parted.part_get_parttype dev = "msdos" &&
> +  pread dev 4 0x01b8 = diskid
> +  with Unix.Unix_error (Unix.EINVAL, _, _) -> false

The old C inspection code did not check for the partition type:
https://github.com/libguestfs/libguestfs/blob/stable-1.36/lib/inspect-fs-windows.c#L591
The partition type check was added recently:
https://github.com/libguestfs/libguestfs/commit/7b4d13626ff878b124d01ec452a4fd0052934133

TBH I'd rather prefer that we check for the situation explicitly
before, rather than catching a generic EINVAL error. Or, even better,
try to avoid altogether the situation that 7b4d13626ff878 checks,
handling this issue as well.

> @@ -402,7 +404,10 @@ and map_registry_disk_blob_gpt partitions blob =
>  fun part ->
>let partnum = Devsparts.part_to_partnum part in
>let device = Devsparts.part_to_dev part in
> -  let typ = Parted.part_get_parttype device in
> +  let typ =
> +try
> +  Parted.part_get_parttype device
> +with Unix.Unix_error (Unix.EINVAL, _, _) -> "unknown" in
>if typ <> "gpt" then false
>else (

Ditto.

-- 
Pino Toscano

signature.asc
Description: This is a digitally signed message part.
___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [PATCH v3] python: Fix UnicodeError in inspect_list_applications2() (RHBZ#1684004)

2020-06-30 Thread Pino Toscano
On Tuesday, 30 June 2020 10:53:54 CEST Sam Eiderman wrote:
> Hey Pino,
> 
> Can you search for the previous patches I submitted? I had some discussions
> regarding this with Daniel and Nir.

Sure, I did read those, and I took it into account. What I said does not
invalidate nor contradict that.

-- 
Pino Toscano

signature.asc
Description: This is a digitally signed message part.
___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [PATCH v3] python: Fix UnicodeError in inspect_list_applications2() (RHBZ#1684004)

2020-06-30 Thread Pino Toscano
On Sunday, 26 April 2020 20:14:03 CEST Sam Eiderman wrote:
> The python3 bindings create PyUnicode objects from application strings
> on the guest (i.e. installed rpm, deb packages).
> It is documented that rpm package fields such as description should be
> utf8 encoded - however in some cases they are not a valid unicode
> string, on SLES11 SP4 the encoding of the description of the following
> packages is latin1 and they fail to be converted to unicode using
> guestfs_int_py_fromstring() (which invokes PyUnicode_FromString()):

Sorry, I wanted to reach our resident Python maintainers to get their
feedback, and so far had no time for it. Will do it shortly.

BTW do you have a reproducer I can actually try freely?

> diff --git a/python/handle.c b/python/handle.c
> index 2fb8c18f0..fe89dc58a 100644
> --- a/python/handle.c
> +++ b/python/handle.c
> @@ -387,7 +387,7 @@ guestfs_int_py_fromstring (const char *str)
>  #if PY_MAJOR_VERSION < 3
>return PyString_FromString (str);
>  #else
> -  return PyUnicode_FromString (str);
> +  return guestfs_int_py_fromstringsize (str, strlen (str));
>  #endif
>  }
>  
> @@ -397,7 +397,12 @@ guestfs_int_py_fromstringsize (const char *str, size_t 
> size)
>  #if PY_MAJOR_VERSION < 3
>return PyString_FromStringAndSize (str, size);
>  #else
> -  return PyUnicode_FromStringAndSize (str, size);
> +  PyObject *s = PyUnicode_FromString (str);
> +  if (s == NULL) {
> +PyErr_Clear ();
> +s = PyUnicode_Decode (str, strlen(str), "latin1", "strict");

Minor nit: space between "strlen" and the opening bracket.

Also, isn't there any error we can check as a way to detect this
situation, rather than always attempting to decode it as latin1?

Thanks,
-- 
Pino Toscano

signature.asc
Description: This is a digitally signed message part.
___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [PATCH libguestfs-common 2/2] mlcustomize: Fall back to autorelabel if specfile does not exist (RHBZ#1828952).

2020-06-24 Thread Pino Toscano
On Wednesday, 24 June 2020 10:58:12 CEST Richard W.M. Jones wrote:
> On Mon, May 18, 2020 at 11:12:29AM +0200, Pino Toscano wrote:
> > On Tuesday, 5 May 2020 17:44:15 CEST Richard W.M. Jones wrote:
> > > https://bugzilla.redhat.com/show_bug.cgi?id=1828952#c2
> > 
> > I think we need to do a different approach than this patch.
> > 
> > The biggest thing is that currently we check only SELINUXTYPE for the
> > actual policy, however we do not check SELINUX in case SELinux is in
> > enforcing mode at all.
> > 
> > IMHO we rather need to read /etc/selinux/ first:
> > - if enforcing, go ahead with the current relabeling: check SELINUXTYPE,
> >   get the policy path, etc; if set like this, then most probably the
> >   SELINUXTYPE points to a valid policy, otherwise the guest would not
> >   even boot
> > - if permissive or disabled, do not perform any relabeling, including
> >   touching /.autorelabel; this is because SELinux was disabled, so
> >   attempting any relabeling might result in failures
> 
> Permissive and disabled seem to be different cases.  If it's
> permissive then SELinux is still "running" (but not enforcing
> decisions).

This is true from the POV of the running guest, not so much from the
POV of libguestfs that inspects/manipulate the offline guest.
In this case, both the labels and the policy may be all wrong, and
SELinux will not error out in permissive mode (only "complain").
And at least in case of permissive mode, this is "fine": the
administrator configured the guest that way, as a mild "do not bother
me", so it is expected that labels may be not set/updated. In case they
want to switch SELinux to enforcing, they will need to
a) ensure the policy is correct (including file contexts)
b) relabel all the filesystems
before the actual switch.

> Anyway I think we at least need to treat enforcing and permissive the
> same way.

Because of the above, I think instead that trying to relabel a
permissive guest is not a good idea: we may apply wrong
policies/labels.

-- 
Pino Toscano

signature.asc
Description: This is a digitally signed message part.
___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs

[Libguestfs] [v2v PATCH] libosinfo: declare autocleanup funcs with libosinfo < 1.8.0

2020-06-19 Thread Pino Toscano
libosinfo 1.8.0 declares them automatically for all of its classes, so
there is no need to declare ours. This requires fixing the definition of
the IS_LIBOSINFO_VERSION macro to wrap its body in brackets.

While in the process, simplify the workaround for a related bug by
removing a now-useless check.
---
 v2v/libosinfo-c.c | 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/v2v/libosinfo-c.c b/v2v/libosinfo-c.c
index e5327dff..322e7d3d 100644
--- a/v2v/libosinfo-c.c
+++ b/v2v/libosinfo-c.c
@@ -40,8 +40,13 @@
 #define V2V_LIBOSINFO_VERSION_HEX \
 MAKE_VERSION_HEX(OSINFO_MAJOR_VERSION, OSINFO_MINOR_VERSION, 
OSINFO_MICRO_VERSION)
 #define IS_LIBOSINFO_VERSION(maj, min, mic) \
-V2V_LIBOSINFO_VERSION_HEX >= MAKE_VERSION_HEX(maj, min, mic)
+(V2V_LIBOSINFO_VERSION_HEX >= MAKE_VERSION_HEX(maj, min, mic))
 
+/*
+ * libosinfo 1.8.0 provides auto-cleanup functions for all its classes,
+ * so avoid declaring our own.
+ */
+#if !IS_LIBOSINFO_VERSION(1, 8, 0)
 G_DEFINE_AUTOPTR_CLEANUP_FUNC(OsinfoFilter, g_object_unref)
 G_DEFINE_AUTOPTR_CLEANUP_FUNC(OsinfoLoader, g_object_unref)
 /*
@@ -50,11 +55,13 @@ G_DEFINE_AUTOPTR_CLEANUP_FUNC(OsinfoLoader, g_object_unref)
  * avoid declaring this when:
  * - libosinfo is >= 1.7.0 and < 1.8.0
  * - glib is >= 2.63.3
+ * (the 1.8.0 check is not done, as already covered by the check above)
  */
-#if !IS_LIBOSINFO_VERSION(1, 7, 0) || IS_LIBOSINFO_VERSION(1, 8, 0) || 
!GLIB_CHECK_VERSION(2, 63, 3)
+#if !IS_LIBOSINFO_VERSION(1, 7, 0) || !GLIB_CHECK_VERSION(2, 63, 3)
 G_DEFINE_AUTOPTR_CLEANUP_FUNC(OsinfoList, g_object_unref)
 #endif
 G_DEFINE_AUTOPTR_CLEANUP_FUNC(OsinfoOsList, g_object_unref)
+#endif
 
 typedef OsinfoDb *OsinfoDb_t;
 typedef OsinfoOs *OsinfoOs_t;
-- 
2.25.4

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs



Re: [Libguestfs] [PATCH virt-v2v] v2v: Allow temporary directory to be set on a global basis.

2020-06-16 Thread Pino Toscano
On Wednesday, 10 June 2020 12:31:33 CEST Richard W.M. Jones wrote:
> I finally got access to the container.  This is how it's configured:
> 
> * / => an overlay fs.
> 
> There is sufficient space here, and there are no "funny" restrictions,
> to be able to create the libguestfs appliance.  I proved this by
> setting TMPDIR to a temporary directory under / and running
> libguestfs-test-tool.
> 
> There appears to be quite a lot of free space here, so in fact the
> v2vovl files could easily be stored here too.  (They only store the
> conversion delta, not the full guest images.)
> 
> * /var/tmp => an NFS mount from a PVC
> 
> This is a large (2T) external NFS mount.  I actually started two pods
> to see if they got the same NFS mount point, and they do.  Also I
> wrote files to /var/tmp in one pod and they were visible in the other.
> So this seems shared.  Also it uses root squash (so root:root is
> mapped to 99:99).  For both reasons this cannot be used for the
> appliance.  If it was mounted at another location it might be used for
> the v2vovl files.
> 
> I've attached the exact mount details at the end of this email.
> 
> My conclusion is that we could do one of two things:
> 
> Either:
> 
> (1) Easiest solution is simply not mount anything under /var/tmp, and
> let it be local storage.  Assuming all these containers are getting ~40G
> of local storage, that's more than enough for virt-v2v to run and
> store the appliance and overlays.  Everything should just work once
> you remove that /var/tmp mountpoint and leave it as local storage.
> 
> ie these lines are removed:
> - mountPath: /var/tmp
>   name: v2v-conversion-temp
> 
> Or:
> 
> (2) We could implement more fine-grained temporary directory control,
> allowing the appliance and v2vovl* files to be placed separately.
> However it would still be wrong to mount the place where libguestfs
> creates the appliance (by default /var/tmp) on NFS.
> 
> If you do this then you'd want to mount the large NFS storage
> somewhere else, and there would be a new environment variable
> (V2V_TMPDIR was my proposal IIRC) which you would point to the NFS
> mount.  /var/tmp would be local storage, and used for the appliance.
> (There are other ways to do this if for some reason /var/tmp must be NFS.)

Or:

(3) set LIBGUESTFS_CACHEDIR away from /var/tmp or NFS-mounted places,
so we avoid any root_squash issue, and avoid any sharing of temporary
files that linger after the container execution.

-- 
Pino Toscano

signature.asc
Description: This is a digitally signed message part.
___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [PATCH v2v 0/2] v2v: nbdkit: Don't use password=- parameter.

2020-06-15 Thread Pino Toscano
On Monday, 1 June 2020 18:43:36 CEST Richard W.M. Jones wrote:
> Part 2 fix for:
> https://bugzilla.redhat.com/show_bug.cgi?id=1842440
> 
> Actually this fix on its own should be sufficient, but probably we
> want the nbdkit fixes too.
> 
> Note this uses actual OCaml 4.05 features!  ("let open" and the
> Unix.tcgetattr functions).

BTW Unix.tcgetattr is very old, even old 3.11 had it (found in some old
tests of mine).

LGTM; IMHO printing user/host in the password prompt would make it more
clear as to for which host the password is asked.

-- 
Pino Toscano

signature.asc
Description: This is a digitally signed message part.
___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] virt-v2v: error: redefinition of 'glib_autoptr_clear_OsinfoList'

2020-06-01 Thread Pino Toscano
On Monday, 1 June 2020 04:17:00 CEST Kevin Locke wrote:
> Hi All,
> 
> I'm attempting to compile virt-v2v 1.42.0 on Debian testing.  With gcc
> 9.3.0, libosinfo 1.7.1, and glib 2.64.2, `./configure && make` fails
> on libosinfo-c.c with the following error message:
> 
> -8<
> In file included from 
> /usr/lib/x86_64-linux-gnu/glib-2.0/include/glibconfig.h:9,
>  from /usr/include/glib-2.0/glib/gtypes.h:32,
>  from /usr/include/glib-2.0/glib/galloca.h:32,
>  from /usr/include/glib-2.0/glib.h:30,
>  from /usr/include/glib-2.0/gobject/gbinding.h:28,
>  from /usr/include/glib-2.0/glib-object.h:22,
>  from /usr/include/libosinfo-1.0/osinfo/osinfo.h:28,
>  from libosinfo-c.c:25:
> /usr/include/glib-2.0/glib/gmacros.h:1028:49: error: redefinition of 
> 'glib_autoptr_clear_OsinfoList'
>  1028 | #define _GLIB_AUTOPTR_CLEAR_FUNC_NAME(TypeName) 
> glib_autoptr_clear_##TypeName
>   | ^~~
> /usr/include/glib-2.0/glib/gmacros.h:1044:36: note: in expansion of macro 
> '_GLIB_AUTOPTR_CLEAR_FUNC_NAME'
>  1044 |   static G_GNUC_UNUSED inline void 
> _GLIB_AUTOPTR_CLEAR_FUNC_NAME(TypeName) (TypeName *_ptr) \
>   |^
> /usr/include/glib-2.0/glib/gmacros.h:1061:3: note: in expansion of macro 
> '_GLIB_DEFINE_AUTOPTR_CLEANUP_FUNCS'
>  1061 |   _GLIB_DEFINE_AUTOPTR_CLEANUP_FUNCS(TypeName, TypeName, func)
>   |   ^~
> libosinfo-c.c:47:1: note: in expansion of macro 
> 'G_DEFINE_AUTOPTR_CLEANUP_FUNC'
>47 | G_DEFINE_AUTOPTR_CLEANUP_FUNC(OsinfoList, g_object_unref)
>   | ^
> /usr/include/glib-2.0/glib/gmacros.h:1028:49: note: previous definition of 
> 'glib_autoptr_clear_OsinfoList' was here
>  1028 | #define _GLIB_AUTOPTR_CLEAR_FUNC_NAME(TypeName) 
> glib_autoptr_clear_##TypeName
>   | ^~~
> /usr/include/glib-2.0/glib/gmacros.h:1044:36: note: in expansion of macro 
> '_GLIB_AUTOPTR_CLEAR_FUNC_NAME'
>  1044 |   static G_GNUC_UNUSED inline void 
> _GLIB_AUTOPTR_CLEAR_FUNC_NAME(TypeName) (TypeName *_ptr) \
>   |^
> /usr/include/glib-2.0/glib/gmacros.h:1056:3: note: in expansion of macro 
> '_GLIB_DEFINE_AUTOPTR_CLEANUP_FUNCS'
>  1056 |   _GLIB_DEFINE_AUTOPTR_CLEANUP_FUNCS(ModuleObjName, ParentName, 
> _GLIB_AUTOPTR_CLEAR_FUNC_NAME(ParentName))
>   |   ^~
> /usr/include/glib-2.0/gobject/gtype.h:1500:3: note: in expansion of macro 
> '_GLIB_DEFINE_AUTOPTR_CHAINUP'
>  1500 |   _GLIB_DEFINE_AUTOPTR_CHAINUP (ModuleObjName, ParentName)
>\
>   |   ^~~~
> /usr/include/libosinfo-1.0/osinfo/osinfo_list.h:33:1: note: in expansion of 
> macro 'G_DECLARE_DERIVABLE_TYPE'
>33 | G_DECLARE_DERIVABLE_TYPE(OsinfoList, osinfo_list, OSINFO, LIST, 
> GObject)
>   | ^~~
> -8<
> 
> The full build log is available at
> https://kevinlocke.name/misc/virt-v2v/virt-v2v-1.42.0-buildlog.txt
> 
> Is this a version incompatibility or something I've misconfigured?
> If someone could point me in the right direction, I'd appreciate it.

Uhm, that's indeed a problem, and apparently due to a bug in libosinfo
and a change in glib.

I just posted a change that should fix this, see its message for a
longer explanation of the issue:
https://www.redhat.com/archives/libguestfs/2020-June/msg3.html
tested on Fedora 31 and Debian testing.

I mentioned your name in the commit message to give you credit for
the report, is it OK for you? (If not, I will remove it before
pushing.)

Thanks,
-- 
Pino Toscano

signature.asc
Description: This is a digitally signed message part.
___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs

[Libguestfs] [v2v PATCH v2] libosinfo: do not declare OsinfoList auto-cleanup in certain situations

2020-06-01 Thread Pino Toscano
libosinfo changed the way OsinfoList is declared in 1.7.0, however that
was changed back to the old way in 1.8.0; the change was an ABI break,
and made OsinfoList a Module class.  Starting from 2.63.3, Module
classes have already auto-cleanup functions declared for them, leading
to double declarations in certain setups.

Hence, do some version check dance to declare the OsinfoList only when
using "safe" versions of libosinfo and glib.

Reported by: Kevin Locke.
---
 v2v/libosinfo-c.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/v2v/libosinfo-c.c b/v2v/libosinfo-c.c
index 1ab6bb4d..e5327dff 100644
--- a/v2v/libosinfo-c.c
+++ b/v2v/libosinfo-c.c
@@ -44,7 +44,16 @@
 
 G_DEFINE_AUTOPTR_CLEANUP_FUNC(OsinfoFilter, g_object_unref)
 G_DEFINE_AUTOPTR_CLEANUP_FUNC(OsinfoLoader, g_object_unref)
+/*
+ * Because of a bug in OsinfoList in libosinfo 1.7.0 (fixed in 1.8.0),
+ * and a glib auto-cleanup addition for Module classes in 2.63.3,
+ * avoid declaring this when:
+ * - libosinfo is >= 1.7.0 and < 1.8.0
+ * - glib is >= 2.63.3
+ */
+#if !IS_LIBOSINFO_VERSION(1, 7, 0) || IS_LIBOSINFO_VERSION(1, 8, 0) || 
!GLIB_CHECK_VERSION(2, 63, 3)
 G_DEFINE_AUTOPTR_CLEANUP_FUNC(OsinfoList, g_object_unref)
+#endif
 G_DEFINE_AUTOPTR_CLEANUP_FUNC(OsinfoOsList, g_object_unref)
 
 typedef OsinfoDb *OsinfoDb_t;
-- 
2.25.4

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs



Re: [Libguestfs] [v2v PATCH] libosinfo: declare OsinfoList auto-cleanup with glib < 2.63.3

2020-06-01 Thread Pino Toscano
On Monday, 1 June 2020 09:15:24 CEST Pino Toscano wrote:
> Starting from glib 2.63.3, Module classes have already auto-cleanup
> functions declared for them.
> 
> Reported by: Kevin Locke.
> ---

Actually this requires few more checks, I'll shortly send a v2.

-- 
Pino Toscano

signature.asc
Description: This is a digitally signed message part.
___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs

[Libguestfs] [v2v PATCH] libosinfo: declare OsinfoList auto-cleanup with glib < 2.63.3

2020-06-01 Thread Pino Toscano
Starting from glib 2.63.3, Module classes have already auto-cleanup
functions declared for them.

Reported by: Kevin Locke.
---
 v2v/libosinfo-c.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/v2v/libosinfo-c.c b/v2v/libosinfo-c.c
index 1ab6bb4d..e6827f76 100644
--- a/v2v/libosinfo-c.c
+++ b/v2v/libosinfo-c.c
@@ -44,7 +44,9 @@
 
 G_DEFINE_AUTOPTR_CLEANUP_FUNC(OsinfoFilter, g_object_unref)
 G_DEFINE_AUTOPTR_CLEANUP_FUNC(OsinfoLoader, g_object_unref)
+#if !GLIB_CHECK_VERSION(2, 63, 3)
 G_DEFINE_AUTOPTR_CLEANUP_FUNC(OsinfoList, g_object_unref)
+#endif
 G_DEFINE_AUTOPTR_CLEANUP_FUNC(OsinfoOsList, g_object_unref)
 
 typedef OsinfoDb *OsinfoDb_t;
-- 
2.25.4

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs



Re: [Libguestfs] [PATCH v2v] v2v: -it vddk: Don't use nbdkit readahead filter with VDDK (RHBZ#1832805).

2020-05-28 Thread Pino Toscano
On Thursday, 28 May 2020 13:22:53 CEST Richard W.M. Jones wrote:
> This filter deliberately tries to coalesce reads into larger requests.
> Unfortunately VMware has low limits on the size of requests it can
> serve to a VDDK client and the larger requests would break with errors
> like this:
> 
>   nbdkit: vddk[3]: error: [NFC ERROR] NfcFssrvrProcessErrorMsg: received NFC 
> error 5 from server: Failed to allocate the requested 33554456 bytes
> 
> We already increase the maximum request size by changing the
> configuration on the VMware server, but it's not sufficient for VDDK
> with the readahead filter.
> 
> As readahead is only an optimization, the simplest solution is to
> disable this filter when we're using nbdkit-vddk-plugin.
> 
> Thanks: Ming Xie
> ---

LGTM.

-- 
Pino Toscano

signature.asc
Description: This is a digitally signed message part.
___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [PATCH v2v] v2v: Remove extraneous '=' when setting --bandwidth/--bandwidth-file.

2020-05-28 Thread Pino Toscano
On Thursday, 28 May 2020 12:51:16 CEST Richard W.M. Jones wrote:
> Commit c3a54d6aed6dfc65f9ffa59976bb8d20044c03a8 ("v2v: Add standalone
> nbdkit module.") was supposed to be a simple refactoring but it broke
> the --bandwidth and --bandwidth-file options (amongst other things).
> 
> Because of an extra '=' character which was accidentally left over, it
> would add an extra character in the nbdkit-rate-filter command line.
> For example:
> 
>   virt-v2v .. --bandwidth 200M
> 
> would invoke:
> 
>   nbdkit .. --filter rate rate==200M
> 
> which causes a parse error.  The --bandwidth-file option does not
> invoke a parse error but does not work, for similar reasons.
> 
> Thanks: Ming Xie
> ---

Remember to mention RHBZ#1841096 in the commit message.

Otherwise, LGTM.

-- 
Pino Toscano

signature.asc
Description: This is a digitally signed message part.
___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [PATCH virt-v2v v2] v2v: -o libvirt: Remove cache=none.

2020-05-26 Thread Pino Toscano
On Tuesday, 19 May 2020 17:51:40 CEST Richard W.M. Jones wrote:
> Traditionally if you did live migration (KVM to KVM), you had to
> ensure that cache=none was set on all disks of the guest up front.
> This was because of quirks in how NFS works (I think the close-to-open
> consistency and the fact that during live migration both qemus have
> the file open), and we had to assume the worst case that a guest might
> be backed by NFS.
> 
> Because of this when virt-v2v converts a guest to run on KVM using
> libvirt it sets cache=none.
> 
> This is not necessary with modern qemu.  If qemu supports the
> drop-cache property of the file block driver, which libvirt will
> automatically detect for us, then libvirt live migration is able to
> tell qemu to drop cached data at the right time even if the backing is
> NFS.
> 
> It also had a significant performance impact.  In some synthetic
> benchmarks it could show 2 or 3 times slower performance.
> 
> Thanks: Ming Xie, Peter Krempa.
> ---

There is one additional occurrence of it in virt-v2v-output-local.pod.
With that fixed, LGTM.

Thanks,
-- 
Pino Toscano

signature.asc
Description: This is a digitally signed message part.
___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [PATCH 4/4] sysprep: add FreeIPA offline unenrollment (RHBZ#1789592)

2020-05-26 Thread Pino Toscano
On Monday, 4 May 2020 15:22:14 CEST Pino Toscano wrote:
> This new operation unenrolls the guest from a FreeIPA server offline, by
> removing the configuration files and certificates.
> 
> Thanks to Christian Heimes for the hints.
> ---
>  sysprep/Makefile.am   |  1 +
>  sysprep/sysprep_operation_unenroll_freeipa.ml | 65 +++
>  2 files changed, 66 insertions(+)
>  create mode 100644 sysprep/sysprep_operation_unenroll_freeipa.ml

Following the feedback of Christian and François, this operation was
renamed to "ipa-client", and pushed.

-- 
Pino Toscano

signature.asc
Description: This is a digitally signed message part.
___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs

[Libguestfs] [v2v PATCH] vCenter: require curl in #precheck

2020-05-26 Thread Pino Toscano
The curl binary is used in the VCenter module, so require it up-front.
This let us remove the need to point the user to check whether curl is
installed in an error message.
---
 v2v/input_libvirt_vcenter_https.ml | 13 -
 v2v/vCenter.ml |  2 +-
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/v2v/input_libvirt_vcenter_https.ml 
b/v2v/input_libvirt_vcenter_https.ml
index ed2e5eed..2265f76e 100644
--- a/v2v/input_libvirt_vcenter_https.ml
+++ b/v2v/input_libvirt_vcenter_https.ml
@@ -19,6 +19,7 @@
 (** [-i libvirt] when the source is VMware vCenter *)
 
 open Common_gettext.Gettext
+open Std_utils
 open Tools_utils
 open Unix_utils.Env
 
@@ -33,13 +34,23 @@ open Printf
 (* Subclass specialized for handling VMware vCenter over https. *)
 class input_libvirt_vcenter_https
 libvirt_conn input_conn input_password parsed_uri server guest =
+
+  let error_unless_curl_command_exists () =
+let curl_binary = "curl" in
+try ignore (which curl_binary)
+with Executable_not_found _ ->
+  error (f_"the ‘%s’ program is not available.  It is needed to 
communicate with vCenter.")
+curl_binary
+  in
+
 object (self)
   inherit input_libvirt libvirt_conn ~input_conn guest
 
   val mutable dcPath = ""
 
   method precheck () =
-error_if_libvirt_does_not_support_json_backingfile ()
+error_if_libvirt_does_not_support_json_backingfile ();
+error_unless_curl_command_exists ()
 
   method source ?bandwidth () =
 debug "input_libvirt_vcenter_https: source: server %s" server;
diff --git a/v2v/vCenter.ml b/v2v/vCenter.ml
index 586ea3e2..33120e88 100644
--- a/v2v/vCenter.ml
+++ b/v2v/vCenter.ml
@@ -189,7 +189,7 @@ and fetch_headers_from_url password_file uri sslverify 
https_url =
 match statuses with
 | [] ->
dump_response stderr;
-   error (f_"vcenter: no status code in output of ‘curl’ command.  Is 
‘curl’ installed?")
+   error (f_"vcenter: no status code in output of ‘curl’ command.")
 | ss ->
   let s = List.hd (List.rev ss) in
   String.sub s (String.index s ' ' + 1) 3 in
-- 
2.25.4

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] libguestfs-tools failure that I can't figure out

2020-05-26 Thread Pino Toscano
On Monday, 25 May 2020 04:15:23 CEST Ben Wellborn wrote:
> I'm not sure of the history of the server's libpcap install.  Too many
> chefs and only one pot.  I know, that doesn't help.  Your question did
> prompt me to check versions and discover that there's a 1.9.0 version in
> the BaseOS repo.  I removed the previous package and loaded the new one.
> 
> a quick ldd test
> ~]# ldd /usr/lib/systemd/systemd | grep libpcap
> libpcap.so.1 => /lib64/libpcap.so.1 (0x7fc395a38000)

This looks fine.

Also, looking at the libpcap-and-rebuild-testing.txt you provided
previously, it looked like there is a package, pfring, that installs
libpcap in /usr/local/lib:

supermin: rpm: multiple providers: requirement libpcap.so.1()(64bit): 
providers: libpcap pfring
supermin: rpm: multiple providers: picked pfring

At least according to a quick search, it seems that this is the case:
https://centos.pkgs.org/7/forensics-x86_64/pfring-7.6.0-2990.x86_64.rpm.html
This is... bad: a package that:
- installs a library in /usr/local
- may override a system-critical library
- even RPM-provides the system library (!)
This is a scholar example of things that can go wrong when packaging
local stuff :-/

Do you still have that package installed? Can you please try by
temporarly removing it?

> but when I ran virt-sysprep it complains about libpcap again:
> "systemd-tmpfiles: error while loading shared libraries: libpcap.so.1:
> cannot open shared object file: No such file or directory"

Try to clean the cached libguestfs appliance before, using
$ rm -rf /var/tmp/.guestfs-$(id -u)

-- 
Pino Toscano

signature.asc
Description: This is a digitally signed message part.
___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs

[Libguestfs] [v2v PATCH] -i libvirt: print URI without connecting

2020-05-25 Thread Pino Toscano
Pass (again) around the libvirt URI string in the various input_libvirt
subclasses so that input_libvirt#as_options does not need to connect to
print the connection URI.

As related change: pass input_conn as non-optional string parameter in
classes that require one (all but input_libvirt_other, basically). This
avoids the need for extra checks.
---
 v2v/input_libvirt.ml| 10 +-
 v2v/input_libvirt_other.ml  | 12 
 v2v/input_libvirt_other.mli |  4 ++--
 v2v/input_libvirt_vcenter_https.ml  |  4 ++--
 v2v/input_libvirt_vcenter_https.mli |  2 +-
 v2v/input_libvirt_vddk.ml   |  9 ++---
 v2v/input_libvirt_vddk.mli  |  4 ++--
 v2v/input_libvirt_xen_ssh.ml|  4 ++--
 v2v/input_libvirt_xen_ssh.mli   |  2 +-
 9 files changed, 25 insertions(+), 26 deletions(-)

diff --git a/v2v/input_libvirt.ml b/v2v/input_libvirt.ml
index cd5f351c..352fae94 100644
--- a/v2v/input_libvirt.ml
+++ b/v2v/input_libvirt.ml
@@ -53,22 +53,22 @@ let input_libvirt input_conn input_password input_transport 
guest =
 
 | Some _, None, _   (* No scheme? *)
 | Some _, Some "", _ ->
-  Input_libvirt_other.input_libvirt_other libvirt_conn guest
+  Input_libvirt_other.input_libvirt_other libvirt_conn ?input_conn guest
 
 (* vCenter over https. *)
 | Some server, Some ("esx"|"gsx"|"vpx"), None ->
Input_libvirt_vcenter_https.input_libvirt_vcenter_https
- libvirt_conn input_password parsed_uri server guest
+ libvirt_conn orig_uri input_password parsed_uri server guest
 
 (* vCenter or ESXi using nbdkit vddk plugin *)
 | Some server, Some ("esx"|"gsx"|"vpx"), Some (`VDDK vddk_options) ->
Input_libvirt_vddk.input_libvirt_vddk
- libvirt_conn input_conn input_password vddk_options parsed_uri guest
+ libvirt_conn orig_uri input_password vddk_options parsed_uri guest
 
 (* Xen over SSH *)
 | Some server, Some "xen+ssh", _ ->
   Input_libvirt_xen_ssh.input_libvirt_xen_ssh
-libvirt_conn input_password parsed_uri server guest
+libvirt_conn orig_uri input_password parsed_uri server guest
 
 (* Old virt-v2v also supported qemu+ssh://.  However I am
  * deliberately not supporting this in new virt-v2v.  Don't
@@ -79,6 +79,6 @@ let input_libvirt input_conn input_password input_transport 
guest =
 | Some _, Some _, _ ->
   warning (f_"no support for remote libvirt connections to '-ic %s'.  The 
conversion may fail when it tries to read the source disks.")
 orig_uri;
-  Input_libvirt_other.input_libvirt_other libvirt_conn guest
+  Input_libvirt_other.input_libvirt_other libvirt_conn ?input_conn guest
 
 let () = Modules_list.register_input_module "libvirt"
diff --git a/v2v/input_libvirt_other.ml b/v2v/input_libvirt_other.ml
index e00944db..6a19ae52 100644
--- a/v2v/input_libvirt_other.ml
+++ b/v2v/input_libvirt_other.ml
@@ -40,12 +40,16 @@ let error_if_libvirt_does_not_support_json_backingfile () =
 error (f_"because of libvirt bug https://bugzilla.redhat.com/1134878 you 
must EITHER upgrade to libvirt >= 2.1.0 OR set this environment 
variable:\n\nexport LIBGUESTFS_BACKEND=direct\n\nand then rerun the virt-v2v 
command.")
 
 (* Superclass. *)
-class virtual input_libvirt libvirt_conn guest =
+class virtual input_libvirt libvirt_conn ?input_conn guest =
 object (self)
   inherit input
 
   method as_options =
-sprintf "-i libvirt -ic %s %s" (Libvirt.Connect.get_uri self#conn) guest
+sprintf "-i libvirt%s %s"
+  (match input_conn with
+  | None -> ""
+  | Some uri -> " -ic " ^ uri)
+  guest
 
   method private conn : Libvirt.rw Libvirt.Connect.t =
 Lazy.force libvirt_conn
@@ -54,9 +58,9 @@ end
 (* Subclass specialized for handling anything that's *not* VMware vCenter
  * or Xen.
  *)
-class input_libvirt_other libvirt_conn guest =
+class input_libvirt_other libvirt_conn ?input_conn guest =
 object (self)
-  inherit input_libvirt libvirt_conn guest
+  inherit input_libvirt libvirt_conn ?input_conn guest
 
   method source ?bandwidth () =
 debug "input_libvirt_other: source ()";
diff --git a/v2v/input_libvirt_other.mli b/v2v/input_libvirt_other.mli
index c528c3ee..ae2c0c6d 100644
--- a/v2v/input_libvirt_other.mli
+++ b/v2v/input_libvirt_other.mli
@@ -20,11 +20,11 @@
 
 val error_if_libvirt_does_not_support_json_backingfile : unit -> unit
 
-class virtual input_libvirt : Libvirt.rw Libvirt.Connect.t Lazy.t -> string -> 
object
+class virtual input_libvirt : Libvirt.rw Libvirt.Connect.t Lazy.t -> 
?input_conn:string -> string -> object
   method precheck : unit -> unit
   method as_options : string
   method virtual source : ?bandwidth:Types.bandwidth -> unit -> Types.source * 
Types.source_disk list
   method private conn : Libvirt.rw Libvirt.Connect.t
 end
 
-val input_libvirt_other : Libvirt.rw Libvirt.Connect.t Lazy.t -> string -> 
Types.input
+val input_libvirt_other : Libvirt.rw Libvirt.Connect.t 

[Libguestfs] [v2v PATCH] libvirt: make use of libvirt's default auth handler (RHBZ#1838425)

2020-05-21 Thread Pino Toscano
Use the default libvirt authentication handler as base for ours,
overriding it with our callback only in case we have a password to
supply.
---
 v2v/libvirt_utils.ml | 12 
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/v2v/libvirt_utils.ml b/v2v/libvirt_utils.ml
index 7df17b29..4d0b8639 100644
--- a/v2v/libvirt_utils.ml
+++ b/v2v/libvirt_utils.ml
@@ -33,10 +33,14 @@ let auth_for_password_file ?password_file () =
 ) creds
   in
 
-  {
-Libvirt.Connect.credtype = [ Libvirt.Connect.CredentialPassphrase ];
-cb = auth_fn;
-  }
+  let base_auth = Libvirt.Connect.get_auth_default () in
+
+  if password_file = None then
+base_auth
+  else
+{ base_auth with
+  cb = auth_fn;
+}
 
 let get_domain conn name =
   let dom =
-- 
2.25.4

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs



[Libguestfs] [v2v PATCH] vCenter: fix parsing of HTTP status string (RHBZ#1837328)

2020-05-19 Thread Pino Toscano
vCenter 7 answers with an HTTP/2 status string, so we cannot extract
the status code from it by using fixed positions in that string.
Hence, pick the status code by reading what's after the whitespace.

Tested with vCenter 6.5 and 7.
---
 v2v/vCenter.ml | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/v2v/vCenter.ml b/v2v/vCenter.ml
index c28a4ced..4c128b0c 100644
--- a/v2v/vCenter.ml
+++ b/v2v/vCenter.ml
@@ -190,7 +190,9 @@ and fetch_headers_from_url password_file uri sslverify 
https_url =
 | [] ->
dump_response stderr;
error (f_"vcenter: no status code in output of ‘curl’ command.  Is 
‘curl’ installed?")
-| ss -> String.sub (List.hd (List.rev ss)) 9 3 in
+| ss ->
+  let s = List.hd (List.rev ss) in
+  String.sub s (String.index s ' ' + 1) 3 in
 
   let headers =
 List.map (
-- 
2.25.4

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [PATCH libguestfs-common 2/2] mlcustomize: Fall back to autorelabel if specfile does not exist (RHBZ#1828952).

2020-05-18 Thread Pino Toscano
On Tuesday, 5 May 2020 17:44:15 CEST Richard W.M. Jones wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1828952#c2

I think we need to do a different approach than this patch.

The biggest thing is that currently we check only SELINUXTYPE for the
actual policy, however we do not check SELINUX in case SELinux is in
enforcing mode at all.

IMHO we rather need to read /etc/selinux/ first:
- if enforcing, go ahead with the current relabeling: check SELINUXTYPE,
  get the policy path, etc; if set like this, then most probably the
  SELINUXTYPE points to a valid policy, otherwise the guest would not
  even boot
- if permissive or disabled, do not perform any relabeling, including
  touching /.autorelabel; this is because SELinux was disabled, so
  attempting any relabeling might result in failures

-- 
Pino Toscano

signature.asc
Description: This is a digitally signed message part.
___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [PATCH libguestfs-common 1/2] mlcustomize: Refactor SELinux_relabel code.

2020-05-18 Thread Pino Toscano
On Tuesday, 5 May 2020 17:44:14 CEST Richard W.M. Jones wrote:
> This shouldn't change the effect of this code.
> ---

This is generally OK for me, however I'd create a custom exception to
signal the "cannot relabel using selinux_relabel" instead of reusing
Failure. This way we can catch this specific exception, and avoid
catching potential Failure issues in the future.

-- 
Pino Toscano

signature.asc
Description: This is a digitally signed message part.
___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs

[Libguestfs] [PATCH v2 0/2] add FreeIPA offline unenrollment (RHBZ#1789592)

2020-05-07 Thread Pino Toscano
This patch series adds a new virt-sysprep operation to offline unenroll
a guest from FreeIPA. It does so by removing some configuration files
and certificates.

Changes from v1:
- the other patches were pushed, as unrelated and approved
- created a new kerberos-hostkeytab operation

Pino Toscano (2):
  sysprep: add IPA offline unenrollment (RHBZ#1789592)
  sysprep: add Kerberos keytab file removal

 sysprep/Makefile.am   |  2 +
 .../sysprep_operation_kerberos_hostkeytab.ml  | 38 +++
 sysprep/sysprep_operation_unenroll_ipa.ml | 66 +++
 3 files changed, 106 insertions(+)
 create mode 100644 sysprep/sysprep_operation_kerberos_hostkeytab.ml
 create mode 100644 sysprep/sysprep_operation_unenroll_ipa.ml

-- 
2.25.4

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs



[Libguestfs] [PATCH v2 1/2] sysprep: add IPA offline unenrollment (RHBZ#1789592)

2020-05-07 Thread Pino Toscano
This new operation unenrolls the guest from a IPA server offline, by
removing the configuration files and certificates.

Thanks to Christian Heimes and François Cami for the hints.
---
 sysprep/Makefile.am   |  1 +
 sysprep/sysprep_operation_unenroll_ipa.ml | 66 +++
 2 files changed, 67 insertions(+)
 create mode 100644 sysprep/sysprep_operation_unenroll_ipa.ml

diff --git a/sysprep/Makefile.am b/sysprep/Makefile.am
index 451a3478f..43137ce65 100644
--- a/sysprep/Makefile.am
+++ b/sysprep/Makefile.am
@@ -66,6 +66,7 @@ operations = \
sssd_db_log \
tmp_files \
udev_persistent_net \
+   unenroll_ipa \
user_account \
utmp yum_uuid
 
diff --git a/sysprep/sysprep_operation_unenroll_ipa.ml 
b/sysprep/sysprep_operation_unenroll_ipa.ml
new file mode 100644
index 0..32d4947d8
--- /dev/null
+++ b/sysprep/sysprep_operation_unenroll_ipa.ml
@@ -0,0 +1,66 @@
+(* virt-sysprep
+ * Copyright (C) 2020 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.
+ *)
+
+open Sysprep_operation
+open Common_gettext.Gettext
+
+module G = Guestfs
+
+let unenroll_ipa_perform (g : Guestfs.guestfs) root side_effects =
+  let typ = g#inspect_get_type root in
+  if typ = "linux" then (
+(* Simple paths with no side effects. *)
+let paths = [ "/etc/ipa/ca.crt";
+  "/etc/ipa/default.conf";
+  "/var/lib/ipa-client/sysrestore/*";
+  "/var/lib/ipa-client/pki/*" ] in
+let paths = List.concat (List.map Array.to_list (List.map g#glob_expand 
paths)) in
+List.iter (
+  fun filename ->
+try g#rm filename with G.Error _ -> ()
+) paths;
+
+(* Certificates in the system CA store. *)
+let certs = [ "/etc/pki/ca-trust/source/anchors/ipa-ca.crt";
+  "/usr/local/share/ca-certificates/ipa-ca.crt";
+  "/etc/pki/ca-trust/source/ipa.p11-kit" ] in
+List.iter (
+  fun filename ->
+try
+  g#rm filename;
+  side_effects#update_system_ca_store ()
+with
+  G.Error _ -> ()
+) certs
+  )
+
+let op = {
+  defaults with
+name = "unenroll-ipa";
+enabled_by_default = true;
+heading = s_"Remove the IPA files";
+pod_description = Some (s_"\
+Remove all the files related to an IPA (Identity, Policy, Audit) system.
+This effectively unenrolls the guest from an IPA server without interacting
+with it.
+
+This operation does not run C.");
+perform_on_filesystems = Some unenroll_ipa_perform;
+}
+
+let () = register_operation op
-- 
2.25.4

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs

[Libguestfs] [PATCH v2 2/2] sysprep: add Kerberos keytab file removal

2020-05-07 Thread Pino Toscano
This new operation removes the Kerberos /etc/krb5.keytab file from the
guest.

Thanks to Christian Heimes and François Cami for the hints.

Related to RHBZ#1789592.
---
 sysprep/Makefile.am   |  1 +
 .../sysprep_operation_kerberos_hostkeytab.ml  | 38 +++
 2 files changed, 39 insertions(+)
 create mode 100644 sysprep/sysprep_operation_kerberos_hostkeytab.ml

diff --git a/sysprep/Makefile.am b/sysprep/Makefile.am
index 43137ce65..95cc7e358 100644
--- a/sysprep/Makefile.am
+++ b/sysprep/Makefile.am
@@ -44,6 +44,7 @@ operations = \
firewall_rules \
fs_uuids \
kerberos_data \
+   kerberos_hostkeytab \
lvm_uuids \
logfiles \
machine_id \
diff --git a/sysprep/sysprep_operation_kerberos_hostkeytab.ml 
b/sysprep/sysprep_operation_kerberos_hostkeytab.ml
new file mode 100644
index 0..cb3023353
--- /dev/null
+++ b/sysprep/sysprep_operation_kerberos_hostkeytab.ml
@@ -0,0 +1,38 @@
+(* virt-sysprep
+ * Copyright (C) 2020 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.
+ *)
+
+open Sysprep_operation
+open Common_gettext.Gettext
+
+module G = Guestfs
+
+let kerberos_hostkeytab_perform (g : Guestfs.guestfs) root side_effects =
+  let typ = g#inspect_get_type root in
+  if typ <> "windows" then (
+(try g#rm "/etc/krb5.keytab" with G.Error _ -> ())
+  )
+
+let op = {
+  defaults with
+name = "kerberos-hostkeytab";
+enabled_by_default = true;
+heading = s_"Remove the Kerberos host keytab file in the guest";
+perform_on_filesystems = Some kerberos_hostkeytab_perform;
+}
+
+let () = register_operation op
-- 
2.25.4

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [PATCH 0/4] sysprep: add FreeIPA offline unenrollment (RHBZ#1789592)

2020-05-04 Thread Pino Toscano
On Monday, 4 May 2020 15:22:10 CEST Pino Toscano wrote:
> It requires a change in libguestfs-common before the series is applied.

https://www.redhat.com/archives/libguestfs/2020-May/msg00016.html
which I failed to group in the same thread...

-- 
Pino Toscano

signature.asc
Description: This is a digitally signed message part.
___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs

[Libguestfs] [PATCH 1/4] customize: port do_run to run_in_guest_command

2020-05-04 Thread Pino Toscano
Make use of the new helper function in Tools_utils to run commands in
the guest.
---
 customize/customize_run.ml | 48 ++
 1 file changed, 7 insertions(+), 41 deletions(-)

diff --git a/customize/customize_run.ml b/customize/customize_run.ml
index 3eacdaca0..f2ee20413 100644
--- a/customize/customize_run.ml
+++ b/customize/customize_run.ml
@@ -30,12 +30,6 @@ open Append_line
 module G = Guestfs
 
 let run (g : G.guestfs) root (ops : ops) =
-  (* Is the host_cpu compatible with the guest arch?  ie. Can we
-   * run commands in this guest?
-   *)
-  let guest_arch = g#inspect_get_arch root in
-  let guest_arch_compatible = guest_arch_compatible guest_arch in
-
   (* Based on the guest type, choose a log file location. *)
   let logfile =
 match g#inspect_get_type root with
@@ -54,42 +48,14 @@ let run (g : G.guestfs) root (ops : ops) =
 
   (* Useful wrapper for scripts. *)
   let do_run ~display ?(warn_failed_no_network = false) cmd =
-if not guest_arch_compatible then
+let incompatible_fn () =
+  let guest_arch = g#inspect_get_arch root in
   error (f_"host cpu (%s) and guest arch (%s) are not compatible, so you 
cannot use command line options that involve running commands in the guest.  
Use --firstboot scripts instead.")
-Guestfs_config.host_cpu guest_arch;
-
-(* Add a prologue to the scripts:
- * - Pass environment variables through from the host.
- * - Send stdout and stderr to a log file so we capture all output
- *   in error messages.
- * - Use setarch when running x86_64 host + i686 guest.
- * Also catch errors and dump the log file completely on error.
- *)
-let env_vars =
-  List.filter_map (
-fun name ->
-  try Some (sprintf "export %s=%s" name (quote (Sys.getenv name)))
-  with Not_found -> None
-  ) [ "http_proxy"; "https_proxy"; "ftp_proxy"; "no_proxy" ] in
-let env_vars = String.concat "\n" env_vars ^ "\n" in
-
-let cmd =
-  match Guestfs_config.host_cpu, guest_arch with
-  | "x86_64", ("i386"|"i486"|"i586"|"i686") ->
-sprintf "setarch i686 <<\"__EOCMD\"
-%s
-__EOCMD
-" cmd
-  | _ -> cmd in
-
-let cmd = sprintf "\
-exec >>%s 2>&1
-%s
-%s
-" (quote logfile) env_vars cmd in
-
-debug "running command:\n%s" cmd;
-try ignore (g#sh cmd)
+Guestfs_config.host_cpu guest_arch
+in
+
+try
+  run_in_guest_command g root ~logfile ~incompatible_fn cmd
 with
   G.Error msg ->
 debug_logfile ();
-- 
2.25.4

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs



[Libguestfs] [PATCH 4/4] sysprep: add FreeIPA offline unenrollment (RHBZ#1789592)

2020-05-04 Thread Pino Toscano
This new operation unenrolls the guest from a FreeIPA server offline, by
removing the configuration files and certificates.

Thanks to Christian Heimes for the hints.
---
 sysprep/Makefile.am   |  1 +
 sysprep/sysprep_operation_unenroll_freeipa.ml | 65 +++
 2 files changed, 66 insertions(+)
 create mode 100644 sysprep/sysprep_operation_unenroll_freeipa.ml

diff --git a/sysprep/Makefile.am b/sysprep/Makefile.am
index 451a3478f..30254c717 100644
--- a/sysprep/Makefile.am
+++ b/sysprep/Makefile.am
@@ -66,6 +66,7 @@ operations = \
sssd_db_log \
tmp_files \
udev_persistent_net \
+   unenroll_freeipa \
user_account \
utmp yum_uuid
 
diff --git a/sysprep/sysprep_operation_unenroll_freeipa.ml 
b/sysprep/sysprep_operation_unenroll_freeipa.ml
new file mode 100644
index 0..5dd2bcc61
--- /dev/null
+++ b/sysprep/sysprep_operation_unenroll_freeipa.ml
@@ -0,0 +1,65 @@
+(* virt-sysprep
+ * Copyright (C) 2020 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.
+ *)
+
+open Sysprep_operation
+open Common_gettext.Gettext
+
+module G = Guestfs
+
+let unenroll_freeipa_perform (g : Guestfs.guestfs) root side_effects =
+  let typ = g#inspect_get_type root in
+  if typ = "linux" then (
+(* Simple paths with no side effects. *)
+let paths = [ "/etc/ipa/ca.crt";
+  "/etc/ipa/default.conf";
+  "/var/lib/ipa-client/sysrestore/*";
+  "/var/lib/ipa-client/pki/*" ] in
+let paths = List.concat (List.map Array.to_list (List.map g#glob_expand 
paths)) in
+List.iter (
+  fun filename ->
+try g#rm filename with G.Error _ -> ()
+) paths;
+
+(* Certificates in the system CA store. *)
+let certs = [ "/etc/pki/ca-trust/source/anchors/ipa-ca.crt";
+  "/usr/local/share/ca-certificates/ipa-ca.crt";
+  "/etc/pki/ca-trust/source/ipa.p11-kit" ] in
+List.iter (
+  fun filename ->
+try
+  g#rm filename;
+  side_effects#update_system_ca_store ()
+with
+  G.Error _ -> ()
+) certs
+  )
+
+let op = {
+  defaults with
+name = "unenroll-freeipa";
+enabled_by_default = true;
+heading = s_"Offline unenroll the guest from FreeIPA";
+pod_description = Some (s_"\
+Unenroll the guest from FreeIPA, reverting and cleaning up the local
+server settings only, without interacting with the FreeIPA server.
+
+This operation does not run C.");
+perform_on_filesystems = Some unenroll_freeipa_perform;
+}
+
+let () = register_operation op
-- 
2.25.4

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs



[Libguestfs] [PATCH 0/4] sysprep: add FreeIPA offline unenrollment (RHBZ#1789592)

2020-05-04 Thread Pino Toscano
This patch series adds a new virt-sysprep operation to offline unenroll
a guest from FreeIPA. It does so by removing some configuration files
and certificates.

It requires a change in libguestfs-common before the series is applied.

Pino Toscano (4):
  customize: port do_run to run_in_guest_command
  sysprep: add a update_system_ca_store side effect
  sysprep: ca-certificates: request system CA store update
  sysprep: add FreeIPA offline unenrollment (RHBZ#1789592)

 customize/customize_run.ml| 48 ++
 sysprep/Makefile.am   |  1 +
 sysprep/main.ml   |  5 ++
 sysprep/sysprep_operation.ml  |  3 +
 sysprep/sysprep_operation.mli |  2 +
 sysprep/sysprep_operation_ca_certificates.ml  |  8 ++-
 sysprep/sysprep_operation_unenroll_freeipa.ml | 65 +++
 sysprep/utils.ml  | 32 +
 sysprep/utils.mli |  4 ++
 9 files changed, 126 insertions(+), 42 deletions(-)
 create mode 100644 sysprep/sysprep_operation_unenroll_freeipa.ml

-- 
2.25.4

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs



[Libguestfs] [PATCH 2/4] sysprep: add a update_system_ca_store side effect

2020-05-04 Thread Pino Toscano
Add a simple side effect to make operation flag that a regeneration of
the system CA store is needed. In case it is flagged, regenerate the
system CA store directly, or using a firstboot script in case of
incompatible architectures.

This change is almost a no-op, since no operation requires the
regeneration of the system CA store yet.
---
 sysprep/main.ml   |  5 +
 sysprep/sysprep_operation.ml  |  3 +++
 sysprep/sysprep_operation.mli |  2 ++
 sysprep/utils.ml  | 32 
 sysprep/utils.mli |  4 
 5 files changed, 46 insertions(+)

diff --git a/sysprep/main.ml b/sysprep/main.ml
index db2388cfa..087d1a17f 100644
--- a/sysprep/main.ml
+++ b/sysprep/main.ml
@@ -25,6 +25,7 @@ open Common_gettext.Gettext
 open Getopt.OptionName
 
 open Sysprep_operation
+open Utils
 
 module G = Guestfs
 
@@ -236,6 +237,10 @@ read the man page virt-sysprep(1).
 Sysprep_operation.perform_operations_on_filesystems
   ?operations g root side_effects;
 
+(* Do we need to update the system CA store? *)
+if side_effects#get_update_system_ca_store then
+  update_system_ca_store g root;
+
 (* Unmount everything in this guest. *)
 g#umount_all ();
 
diff --git a/sysprep/sysprep_operation.ml b/sysprep/sysprep_operation.ml
index 0013ff504..53f042402 100644
--- a/sysprep/sysprep_operation.ml
+++ b/sysprep/sysprep_operation.ml
@@ -27,10 +27,13 @@ class filesystem_side_effects =
 object
   val mutable m_created_file = false
   val mutable m_changed_file = false
+  val mutable m_update_system_ca_store = false
   method created_file () = m_created_file <- true
   method get_created_file = m_created_file
   method changed_file () = m_changed_file <- true
   method get_changed_file = m_changed_file
+  method update_system_ca_store () = m_update_system_ca_store <- true
+  method get_update_system_ca_store = m_update_system_ca_store
 end
 
 class device_side_effects = object end
diff --git a/sysprep/sysprep_operation.mli b/sysprep/sysprep_operation.mli
index 3d9f12550..2a02d5e79 100644
--- a/sysprep/sysprep_operation.mli
+++ b/sysprep/sysprep_operation.mli
@@ -23,6 +23,8 @@ class filesystem_side_effects : object
   method get_created_file : bool
   method changed_file : unit -> unit
   method get_changed_file : bool
+  method update_system_ca_store : unit -> unit
+  method get_update_system_ca_store : bool
 end
 (** The callback should indicate if it has side effects by calling
 methods in this class. *)
diff --git a/sysprep/utils.ml b/sysprep/utils.ml
index 4c45d42de..29460b3c0 100644
--- a/sysprep/utils.ml
+++ b/sysprep/utils.ml
@@ -20,6 +20,9 @@
 
 open Printf
 
+open Tools_utils
+open Common_gettext.Gettext
+
 let rec pod_of_list ?(style = `Dot) xs =
   match style with
   | `Verbatim -> String.concat "\n" (List.map ((^) " ") xs)
@@ -31,3 +34,32 @@ and _pod_of_list delim xs =
   "=over 4\n\n" ^
   String.concat "" (List.map (sprintf "=item %s\n\n%s\n\n" delim) xs) ^
   "=back"
+
+let rec update_system_ca_store g root =
+  let cmd = update_system_ca_store_command g root in
+  match cmd with
+  | None -> ()
+  | Some cmd ->
+(* Try to run the command directly if possible, adding it as
+ * firstboot script in case of incompatible architectures.
+ *)
+let cmd = String.concat " " cmd in
+let incompatible_fn () =
+  Firstboot.add_firstboot_script g root cmd cmd
+in
+
+run_in_guest_command g root ~incompatible_fn cmd
+
+and update_system_ca_store_command g root =
+  let typ = g#inspect_get_type root in
+  let distro = g#inspect_get_distro root in
+  match typ, distro with
+  | "linux", 
("fedora"|"rhel"|"centos"|"scientificlinux"|"oraclelinux"|"redhat-based") ->
+Some [ "update-ca-trust"; "extract" ]
+
+  | "linux", ("debian"|"ubuntu"|"kalilinux") ->
+Some [ "update-ca-certificates" ]
+
+  | _, _ ->
+warning (f_"updating the system CA store on this guest %s/%s is not 
supported") typ distro;
+None
diff --git a/sysprep/utils.mli b/sysprep/utils.mli
index a57a0d876..82779620e 100644
--- a/sysprep/utils.mli
+++ b/sysprep/utils.mli
@@ -26,3 +26,7 @@ val pod_of_list : ?style:[`Verbatim|`Star|`Dash|`Dot] -> 
string list -> string
 use a space-indented (verbatim) block.  [`Star], [`Dash] or [`Dot]
 meaning use a real list with [*], [-] or [·].  The default
 style is [·] ([`Dot]). *)
+
+val update_system_ca_store : Guestfs.guestfs -> string -> unit
+(** Update the system CA store on the guest for the specified root
+(which is fully mounted). *)
-- 
2.25.4

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs

[Libguestfs] [common PATCH] mltools: add run_in_guest_command helper

2020-05-04 Thread Pino Toscano
Add an helper function to run a command in the guest, checking for the
host/guest compatibility.  This is mostly extracted from the internal
do_run helper currently in the Customize_run module of virt-customize.
---
 mltools/tools_utils.ml  | 50 +
 mltools/tools_utils.mli | 10 +
 2 files changed, 60 insertions(+)

diff --git a/mltools/tools_utils.ml b/mltools/tools_utils.ml
index 1271802..d54ec58 100644
--- a/mltools/tools_utils.ml
+++ b/mltools/tools_utils.ml
@@ -679,3 +679,53 @@ let with_timeout op timeout ?(sleep = 2) fn =
loop ()
   in
   loop ()
+
+let run_in_guest_command g root ?logfile ?incompatible_fn cmd =
+  (* Is the host_cpu compatible with the guest arch?  ie. Can we
+   * run commands in this guest?
+   *)
+  let guest_arch = g#inspect_get_arch root in
+  let guest_arch_compatible = guest_arch_compatible guest_arch in
+  if not guest_arch_compatible then (
+match incompatible_fn with
+| None -> ()
+| Some fn -> fn ()
+  )
+  else (
+(* Add a prologue to the scripts:
+ * - Pass environment variables through from the host.
+ * - Optionally send stdout and stderr to a log file so we capture
+ *   all output in error messages.
+ * - Use setarch when running x86_64 host + i686 guest.
+ *)
+let env_vars =
+  List.filter_map (
+fun name ->
+  try Some (sprintf "export %s=%s" name (quote (Sys.getenv name)))
+  with Not_found -> None
+  ) [ "http_proxy"; "https_proxy"; "ftp_proxy"; "no_proxy" ] in
+let env_vars = String.concat "\n" env_vars ^ "\n" in
+
+let cmd =
+  match Guestfs_config.host_cpu, guest_arch with
+  | "x86_64", ("i386"|"i486"|"i586"|"i686") ->
+sprintf "setarch i686 <<\"__EOCMD\"
+%s
+__EOCMD
+" cmd
+  | _ -> cmd in
+
+let logfile_redirect =
+  match logfile with
+  | None -> ""
+  | Some logfile -> sprintf "exec >>%s 2>&1" (quote logfile) in
+
+let cmd = sprintf "\
+%s
+%s
+%s
+" (logfile_redirect) env_vars cmd in
+
+debug "running command:\n%s" cmd;
+ignore (g#sh cmd)
+  )
diff --git a/mltools/tools_utils.mli b/mltools/tools_utils.mli
index ab70f58..102abff 100644
--- a/mltools/tools_utils.mli
+++ b/mltools/tools_utils.mli
@@ -212,3 +212,13 @@ val with_timeout : string -> int -> ?sleep:int -> (unit -> 
'a option) -> 'a
 calls {!error} and the program exits.  The error message will
 contain the diagnostic string [op] to identify the operation
 which timed out. *)
+
+val run_in_guest_command : Guestfs.guestfs -> string -> ?logfile:string -> 
?incompatible_fn:(unit -> unit) -> string -> unit
+(** [run_in_guest_command g root ?incompatible_archs_fn cmd]
+runs a command in the guest, which is already mounted for the
+specified [root].  The command is run directly in case the
+architecture of the host and the guest are compatible, optionally
+calling [?incompatible_fn] in case they are not.
+
+[?logfile] is an optional file in the guest to where redirect
+stdout and stderr of the command. *)
-- 
2.25.4

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs



[Libguestfs] [PATCH 3/4] sysprep: ca-certificates: request system CA store update

2020-05-04 Thread Pino Toscano
In case any certificate is removed from the guest, regenerate the system
CA store.
---
 sysprep/sysprep_operation_ca_certificates.ml | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/sysprep/sysprep_operation_ca_certificates.ml 
b/sysprep/sysprep_operation_ca_certificates.ml
index e481cebf8..a2b7986c1 100644
--- a/sysprep/sysprep_operation_ca_certificates.ml
+++ b/sysprep/sysprep_operation_ca_certificates.ml
@@ -39,7 +39,11 @@ let ca_certificates_perform (g : Guestfs.guestfs) root 
side_effects =
 let set = StringSet.diff set excepts in
 StringSet.iter (
   fun filename ->
-try g#rm filename with G.Error _ -> ()
+try
+  g#rm filename;
+  side_effects#update_system_ca_store ()
+with
+  G.Error _ -> ()
 ) set
   )
 
@@ -48,6 +52,8 @@ let op = {
 name = "ca-certificates";
 enabled_by_default = false;
 heading = s_"Remove CA certificates in the guest";
+pod_description = Some (s_"\
+In case any certificate is removed, the system CA store is updated.");
 perform_on_filesystems = Some ca_certificates_perform;
 }
 
-- 
2.25.4

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs



Re: [Libguestfs] virt-v2v: error: no href in ovf:File (id=)

2020-04-24 Thread Pino Toscano
On Friday, 24 April 2020 19:12:10 CEST Andrew Thurber (anthurbe) wrote:
> Thank you Pino,
> Please explain a bit more, I'm not sure I have the same situation as 
> https://bugzilla.redhat.com/1709722
> In my case it complains there is no href, but there is.

Not all the disks have a non-empty ovf:fileRef, see in particular:

> >  ovf:format="http://www.vmware.com/interfaces/specifications/vmdk.html#streamOptimized;
>  ovf:populatedSize="0"/>
> >  ovf:format="http://www.vmware.com/interfaces/specifications/vmdk.html#streamOptimized;
>  ovf:populatedSize="0"/>
> >  ovf:format="http://www.vmware.com/interfaces/specifications/vmdk.html#streamOptimized;
>  ovf:populatedSize="0"/>

-- 
Pino Toscano

signature.asc
Description: This is a digitally signed message part.
___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] virt-v2v: error: no href in ovf:File (id=)

2020-04-24 Thread Pino Toscano
Hi,

On Friday, 24 April 2020 14:57:38 CEST Andrew Thurber (anthurbe) wrote:
> This multi-disk ovf generates “no href in ovf:File (id=)”
> Other single-disk ovfs on the same system work. I don’t have another 
> multi-disk ova to try.
> I’ve compared the syntax with the test file on github and it appears to be 
> essentially the same:
> virt-v2v/tests/test-v2v-i-ova-two-disks.ovf
> Any suggestions?

> [ with text changed for privacy ]
> 
> Head of the .ovf file:
> 
> 
>  xmlns="http://schemas.dmtf.org/ovf/envelope/1; 
> xmlns:cim="http://schemas.dmtf.org/wbem/wscim/1/common; 
> xmlns:ovf="http://schemas.dmtf.org/ovf/envelope/1; 
> xmlns:rasd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData;
>  xmlns:vmw="http://www.vmware.com/schema/ovf; 
> xmlns:vssd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_VirtualSystemSettingData;
>  xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance;>
>   
>  ovf:href="aa-bb--1.2.3-45-release-200401.ova_v1.2_signed-disk1.vmdk" 
> ovf:id="file1" ovf:size="1530541056"/>
>  ovf:href="aa-bb--1.2.3-45-release-200401.ova_v1.2_signed-disk2.vmdk" 
> ovf:id="file2" ovf:size="6476920320"/>
>  ovf:href="aa-bb--1.2.3-45-release-200401.ova_v1.2_signed-disk3.vmdk" 
> ovf:id="file3" ovf:size="1696059392"/>
>   
>   
> IP Network Stack in use by the product
> 
>   IPv4 Network
>   Use IPv4 network stack for management and data 
> traffic.
> 
> 
>   IPv6 Network
>   Use IPv6 network stack for management and data 
> traffic.
> 
> 
>   IPv4 Network on a Single Interface (demo mode)
>   Use a single interface for IPv4 management and data 
> traffic.
> 
>   
>   
> Virtual disk information
>  ovf:diskId="vmdisk1" ovf:fileRef="file1" 
> ovf:format="http://www.vmware.com/interfaces/specifications/vmdk.html#streamOptimized;
>  ovf:populatedSize="3584360448"/>
>  ovf:diskId="vmdisk2" ovf:fileRef="file2" 
> ovf:format="http://www.vmware.com/interfaces/specifications/vmdk.html#streamOptimized;
>  ovf:populatedSize="16995385344"/>
>  ovf:diskId="vmdisk6" ovf:fileRef="file3" 
> ovf:format="http://www.vmware.com/interfaces/specifications/vmdk.html#streamOptimized;
>  ovf:populatedSize="1704001536"/>
>  ovf:diskId="vmdisk3" ovf:fileRef="" 
> ovf:format="http://www.vmware.com/interfaces/specifications/vmdk.html#streamOptimized;
>  ovf:populatedSize="0"/>
>  ovf:diskId="vmdisk4" ovf:fileRef="" 
> ovf:format="http://www.vmware.com/interfaces/specifications/vmdk.html#streamOptimized;
>  ovf:populatedSize="0"/>
>  ovf:diskId="vmdisk5" ovf:fileRef="" 
> ovf:format="http://www.vmware.com/interfaces/specifications/vmdk.html#streamOptimized;
>  ovf:populatedSize="0"/>
>   
> …
> 
> And the virt-v2v error:
> 
> [root@localhost ~]# virt-v2v -v -x -i ova 
> aa-bb--1.2.3-45-release-200401.ova -of qcow2
> virt-v2v: libguestfs 
> 1.38.4rhel=8,release=14.module_el8.1.0+248+298dec18,libvirt (x86_64)
> libvirt version: 4.5.0
> [   0.0] Opening the source -i ova aa-bb--1.2.3-45-release-200401.ova
> libguestfs: trace: set_verbose true
> libguestfs: trace: set_verbose = 0
> libguestfs: trace: get_cachedir
> libguestfs: trace: get_cachedir = "/var/tmp"
> qemu-img info json:'{ "file": { "driver": "raw", "offset": 512, "size": 512, 
> "file": { "filename": "/tmp/v2vqemuimgtst8f2fb6.img" } } }' >/dev/null
> qemu-img supports "offset" and "size" in json URLs: true
> libguestfs: trace: set_verbose true
> libguestfs: trace: set_verbose = 0
> libguestfs: trace: get_backend
> libguestfs: trace: get_backend = "libvirt"
> libvirt supports  "raw" driver in json URL: true
> tar -tf 'aa-bb--1.2.3-45-release-200401.ova'
> tar -xf 'aa-bb--1.2.3-45-release-200401.ova' -C '/var/tmp/ova.1MI0Ml' 
> 'aa-bb--1.2.3-45-release-200401.ova_v6.5_signed.ovf' 
> 'aa-bb--1.2.3-45-release-200401.ova_v1.2_signed.mf'
> 
> virt-v2v: error: no href in ovf:File (id=)

This is the same case of https://bugzilla.redhat.com/1709722

In short, disks with no references are meant to be allocated
dynamically, and this is something virt-v2v does not support yet.

> Side note: I tried building libguestfs from source but failed so
> haven’t tried with latest (separate issue, but I think I resolved
> dependencies, then hit a podwrapper.pl issue).

As mentioned, not even virt-v2v from git master supports this.
If you can provide more details on the issues you got when building from
sources I can provide hints.

-- 
Pino Toscano

signature.asc
Description: This is a digitally signed message part.
___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] virt-v2v valgrind errors in libosinfo

2020-04-14 Thread Pino Toscano
On Tuesday, 14 April 2020 15:16:49 CEST Richard W.M. Jones wrote:
> On Tue, Apr 14, 2020 at 12:37:07PM +0200, Pino Toscano wrote:
> > > Unfortunately we never free the database.
> > 
> > Hm it is never freed? Wouldn't that result in actual leaks, since
> > OsinfoDb_t_finalize (g_object_unref'ing the OsinfoDb) wouldn't be
> > called?
> 
> I was thinking because of this:
> 
> https://github.com/libguestfs/virt-v2v/blob/cc294b7735dda467179b93a061d3631ac3547f26/v2v/libosinfo_utils.ml#L24
> 
> which IIUC will allocate a DB (on first access) but it is never
> released.

Oh this is interesting. I read in the documentation that custom blocks
have tag Custom_tag, which is higher than No_scan_tag, and thus they
aren't scanned by the GC. Indeed, even trying to Gc.finalize on the
object inside the lazy seems to have no effect.

I guess this is becase db is a top-level variable in Libosinfo_utils;
is there a way to change that behaviour somehow?

> > > It could be that to express
> > > this properly we'd need to expose (db, os) tuples to the OCaml garbage
> > > collector.
> > 
> > I thought about this, and according to knowledge this would be needed
> > only if we want osinfo_os objects alive even when the osinfo_db gets
> > "out of scope". Considering that the osinfo_db is kept basically
> > statically this should be fine.
> 
> Right.
> 
> I don't believe the current code is wrong, my concern is more about
> clearing up valgrind errors before the release.

Yup, it makes sense to do that.

-- 
Pino Toscano

signature.asc
Description: This is a digitally signed message part.
___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] virt-v2v valgrind errors in libosinfo

2020-04-14 Thread Pino Toscano
On Tuesday, 14 April 2020 11:53:30 CEST Richard W.M. Jones wrote:
> I've suppressed some OCaml and libosinfo valgrind errors in virt-v2v.
> 
> The remaining valgrind errors are here:
> 
>   http://oirase.annexia.org/tmp/v2vvg/
> 
> They all seem to be basically the same.  But I couldn't work out if
> these are expected leaks in the libosinfo code (in which case we
> should suppress them),

I think this might have to do with the glib allocator (libosinfo is
based on glib, so it allocate using it), that allocates in chunks by
default to avoid fragmentation, IIRC. Do you still get the same issues
if you export G_SLICE=always-malloc when running valgrind?

> or if they are actual bugs because we are
> missing a true destructor here:
> 
>   
> https://github.com/libguestfs/virt-v2v/blob/8e870da79b5a61513f568b0b81c773084b8d7997/v2v/libosinfo-c.c#L91
> 
> Perhaps there's a reason why we cannot have a destructor, for example
> that the C database is supposed to hold references to the OS objects?

This is correct according to the way we use OsinfoOs so far, i.e. only
by using what OsinfoDb has, because OsinfoDb holds references on the
OsinfoOs objects. I talked with Fabiano (main libosinfo developer)
about this, and sadly OsinfoDb has also references to other objects
(like the devices) inside each OsinfoOs, so it is not possible to get
the ownership of all the OsinfoOs (g_object_ref) and then get rid of
the OsinfoDb (g_object_unref).

> Unfortunately we never free the database.

Hm it is never freed? Wouldn't that result in actual leaks, since
OsinfoDb_t_finalize (g_object_unref'ing the OsinfoDb) wouldn't be
called?

> It could be that to express
> this properly we'd need to expose (db, os) tuples to the OCaml garbage
> collector.

I thought about this, and according to knowledge this would be needed
only if we want osinfo_os objects alive even when the osinfo_db gets
"out of scope". Considering that the osinfo_db is kept basically
statically this should be fine.

> BTW the informational string given here seems to be wrong - copy and
> paste error?
> 
>   
> https://github.com/libguestfs/virt-v2v/blob/8e870da79b5a61513f568b0b81c773084b8d7997/v2v/libosinfo-c.c#L90

Oops yes. "Db" and "Os" sadly look too similar...

-- 
Pino Toscano

signature.asc
Description: This is a digitally signed message part.
___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [PATCH virt-v2v] v2v: Allow temporary directory to be set on a global basis.

2020-04-07 Thread Pino Toscano
On Tuesday, 7 April 2020 14:59:11 CEST Tomáš Golembiovský wrote:
> On Tue, Apr 07, 2020 at 02:33:20PM +0200, Pino Toscano wrote:
> > On Tuesday, 7 April 2020 14:18:47 CEST Richard W.M. Jones wrote:
> > > On Tue, Apr 07, 2020 at 01:25:02PM +0200, Pino Toscano wrote:
> > > > The important thing is still that that you need to have space for the
> > > > temporary files somewhere: be it /var/tmp, /mnt/scratch, whatever.
> > > > Because of this, and the fact that usually containers are created
> > > > fresh, the cache of the supermin appliance starts to make little sense,
> > > > and then a very simple solution is to point libguestfs to that extra
> > > > space:
> > > > 
> > > >   $ dir=$(mktemp --tmpdir -d 
> > > > /path/to/big/temporary/space/libguestfs.XX)
> > > >   $ export LIBGUESTGS_CACHEDIR=$dir
> > > >   $ export TMPDIR=$dir  # optionally
> > > >   $ libguestfs-test-tool
> > > >   [etc]
> > > >   $ rm -rf $dir
> > > > 
> > > > Easy to use, already doable, solves all the issues.
> > > 
> > > So AIUI there are a few problems with this (although I'm still
> > > investigating and trying to make a local reproducer):
> > > 
> > >  - The external large space may be on NFS, with the usual problems
> > >there like root_squash, no SELinux labels, slowness.  This means
> > >it's not suitable for the appliance, but might nevertheless be
> > >suitable for overlay files.
> > 
> > If that is the only big storage space attached to a container, I do
> > not see any alternative than use it, with all the caveats associated.
> 
> I have to aggree with this. You cannot avoid situations where the
> appliance is on a network drive. If there are any inherent risks the
> best you can do is let user know about those (documentation?).
> 
> > 
> > Also, if we take as possible scenario the situation where /var/tmp is
> > not that big, then we need to consider that may not be big enough to
> > even store the cached supermin appliance (which is more than
> > 300/350MB).
> > 
> > >  - The external large space may be shared with other containers, and
> > >I'm not convinced that our locking in supermin will be safe if
> > >multiple parallel instances start up at the same time.  We
> > >certainly never tested it, and don't currently advise it.
> > 
> > That's why my suggestion above creates a specific temporary directory
> > for each container: even with a shared /var/tmp, there will not be any
> > cache stepping up on each other toes. This is something that this
> > separate cachedir for virt-v2v does not solve at all.
> 
> Currently we don't share the temporary volume between instances in
> Kubevirt, but the assumption that this can happen is valid and should be
> considered.
> 
> > 
> > > > This whole problem started from a QE report on leftover files after
> > > > failed migrations: bz#1820282.
> > > 
> > > (I should note also there are two bugs which I personally think we can
> > > solve with the same fix, but they are completely different bugs.)
> > 
> > I still do not understand how these changes have anything to do with
> > bug 1814611, which in an offline discussion we found out that has
> > mostly two causes:
> > - the way the NFS storage is mounted over the /var/tmp in the
> >   container, so what you create as root is not really with the UID/GID
> >   expected
> > - the fixed appliance in the container was not actually used, and thus
> >   a rebuilt of the the supermin appliance was attempted, failing due
> >   to the first issue
> 
> I am still not convinced this is the case. Based on the logs I shared in
> private email I still believe that the fixed appliance was used
> properly. You assumed that the appliance is not used because the cache
> directory is being created, but as I also pointed out the cache
> directory created in all situations because qemu temporary files are
> stored there [1][2].

This may be the case, i.e. something else before even the supermin
appliance check that triggers the creation of the cachedir.

In the end though, libguestfs prefers a supermin appliance before a
fixed appliance; the whole logic is here:
https://github.com/libguestfs/libguestfs/blob/c2c11382bbeb4500f3388a31ffd08cfc18b0de40/lib/appliance.c
In particular, see the locate_or_build_appliance function and the
comment before it, and contains_fixed_appliance &
contains_supermin_appliance helper functions.

If you have a setup like:

/usr/lib64/gue

Re: [Libguestfs] [PATCH virt-v2v] v2v: Allow temporary directory to be set on a global basis.

2020-04-07 Thread Pino Toscano
On Tuesday, 7 April 2020 14:18:47 CEST Richard W.M. Jones wrote:
> On Tue, Apr 07, 2020 at 01:25:02PM +0200, Pino Toscano wrote:
> > The important thing is still that that you need to have space for the
> > temporary files somewhere: be it /var/tmp, /mnt/scratch, whatever.
> > Because of this, and the fact that usually containers are created
> > fresh, the cache of the supermin appliance starts to make little sense,
> > and then a very simple solution is to point libguestfs to that extra
> > space:
> > 
> >   $ dir=$(mktemp --tmpdir -d /path/to/big/temporary/space/libguestfs.XX)
> >   $ export LIBGUESTGS_CACHEDIR=$dir
> >   $ export TMPDIR=$dir  # optionally
> >   $ libguestfs-test-tool
> >   [etc]
> >   $ rm -rf $dir
> > 
> > Easy to use, already doable, solves all the issues.
> 
> So AIUI there are a few problems with this (although I'm still
> investigating and trying to make a local reproducer):
> 
>  - The external large space may be on NFS, with the usual problems
>there like root_squash, no SELinux labels, slowness.  This means
>it's not suitable for the appliance, but might nevertheless be
>suitable for overlay files.

If that is the only big storage space attached to a container, I do
not see any alternative than use it, with all the caveats associated.

Also, if we take as possible scenario the situation where /var/tmp is
not that big, then we need to consider that may not be big enough to
even store the cached supermin appliance (which is more than
300/350MB).

>  - The external large space may be shared with other containers, and
>I'm not convinced that our locking in supermin will be safe if
>multiple parallel instances start up at the same time.  We
>certainly never tested it, and don't currently advise it.

That's why my suggestion above creates a specific temporary directory
for each container: even with a shared /var/tmp, there will not be any
cache stepping up on each other toes. This is something that this
separate cachedir for virt-v2v does not solve at all.

> > This whole problem started from a QE report on leftover files after
> > failed migrations: bz#1820282.
> 
> (I should note also there are two bugs which I personally think we can
> solve with the same fix, but they are completely different bugs.)

I still do not understand how these changes have anything to do with
bug 1814611, which in an offline discussion we found out that has
mostly two causes:
- the way the NFS storage is mounted over the /var/tmp in the
  container, so what you create as root is not really with the UID/GID
  expected
- the fixed appliance in the container was not actually used, and thus
  a rebuilt of the the supermin appliance was attempted, failing due
  to the first issue

Can you please explain me exactly how switching the location of
temporary files (that were not mentioned in the bug at all) will fix
this situation?

> > What this report doesn't say, however,
> > is that beside the mentioned files that virt-v2v creates, there are
> > also leftover files that libguestfs itself creates. These files are
> > usually downloaded from the guest for the inspection, and generally not
> > that big compared to e.g. the overlays that virt-v2v creates.
> > Nonetheless, an abrupt exit of virt-v2v will leave those in place, and
> > they will still slowly fill up the space on /var/tmp (or whatever is
> > the location of $LIBGUESTFS_CACHEDIR).
> 
> I guess that small files being left around aren't really a problem.
> The problem they have is large files being left around, and I can
> understand why that would be an issue and not the small files.

Nobody is saying that the leftover files are not a problem. I'm saying
that also the small files are a sort of problem -- sure, less critical,
however still there and ready to show up any time, especially if the
concern is the space of /var/tmp.

-- 
Pino Toscano

signature.asc
Description: This is a digitally signed message part.
___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [PATCH virt-v2v] v2v: Allow temporary directory to be set on a global basis.

2020-04-07 Thread Pino Toscano
On Monday, 6 April 2020 19:45:44 CEST Daniel P. Berrangé wrote:
> On Thu, Apr 02, 2020 at 05:51:32PM +0200, Pino Toscano wrote:
> > On Thursday, 2 April 2020 17:30:39 CEST Richard W.M. Jones wrote:
> > > On Thu, Apr 02, 2020 at 03:21:14PM +0200, Pino Toscano wrote:
> > > > On Thursday, 2 April 2020 14:49:18 CEST Richard W.M. Jones wrote:
> > > > > Previously we placed large files in g#get_cachedir () (usually
> > > > > /var/tmp).  However the problem is this ties the libguestfs appliance
> > > > > and the virt-v2v overlay files to the same location.
> > > > > 
> > > > > When virt-v2v is run in a container, or any other situation where
> > > > > local storage is limited, it's helpful to be able to put the overlay
> > > > > files on an externally mounted PVC, which might be using NFS and
> > > > > shared between containers.  But putting the libguestfs appliance on
> > > > > NFS in a shared location is certainly not recommended.
> > > > > 
> > > > > This allows the two locations to be set separately:
> > > > > 
> > > > >   VIRT_V2V_TMPDIR - location of large temporary files, can use NFS
> > > > > and may be shared
> > > > > 
> > > > >   LIBGUESTFS_CACHEDIR - location of libguestfs appliance
> > > > > 
> > > > > Another motivation for this patch is to allow more reliable cleanup of
> > > > > temporary files by an external process, as described in the updated
> > > > > documentation.
> > > > > ---
> > > > 
> > > > I do not understand the motivation behind this, which adds yet another
> > > > location with temporary files in addition to:
> > > > - LIBGUESTFS_TMPDIR - $TMPDIR by default (which itself is /tmp by
> > > >   default)
> > > > - LIBGUESTFS_CACHEDIR - /var/tmp by default (with a .guestfs-$UID
> > > >   subdirectory for the appliance)
> > > > 
> > > > Before this patch, almost all the temporary files are stored directly
> > > > or in subdirectories of $TMPDIR, except big files such as overlays and
> > > > OVA extracted content that are in CACHEDIR. With the proposed changes,
> > > > _all_ the temporary files will be in CACHEDIR, so there are the
> > > > following problems:
> > > > - this directory will be cluttered with a lot more files than before
> > > > - if it is shared, then other places where it is mounted will see the
> > > >   same files
> > > > - if it is shared, then creating temporary files will possibly mean
> > > >   doing network I/O
> > > > - if virt-v2v exits uncleantly, there will be a lot more files to
> > > >   cleanup than now
> > > > - even without being shared, /var/tmp is persistent unlike /tmp (which
> > > >   can be tmpfs-backed on some distros/setups), meaning old temporary
> > > >   files will linger way more
> > > 
> > > How about if we confine the change to just large files (ie. ones
> > > which are currently placed in cachedir)?
> > 
> > This is already the case, isn't it?
> > 
> > > However the way that virt-v2v works at the moment means you cannot put
> > > the large files (especially v2vovl*.qcow2) in a different place from
> > > the libguestfs appliance.  This means that if you have only "some"
> > > space in /var/tmp -- enough for the appliance, but not enough for the
> > > potentially much larger space required by v2vovl*.qcow2 with multiple
> > > copies of virt-v2v running -- then you cannot separate the overlays to
> > > another directory.  This isn't just a problem for containers.
> > 
> > /var/tmp is a temporary directory, hence it ought to have enough space
> > for big temporary files. This is nothing special for libguestfs or
> > virt-v2v.
> 
> It is not valid to assume that /var/tmp is arbitrarily big enough
> for any files v2v might want to create. AFAIK, there's no real
> rule about how large it is expected to be. Linux distros typically
> don't do fine grained partitioning by default, so /var/tmp is often
> the same filesystem as /, but it is not unusual for admins to have
> / relatively small, and have a separate data partition somewhere
> which has the vast majority of storage space available to the host.
> 
> In the container world the / is indeed generally of a fairly
> limited size, as that's an unpacked container image filesystem
> usually on 

Re: [Libguestfs] [v2v PATCH 2/2] Consolidate handling of temporary files/dirs

2020-04-06 Thread Pino Toscano
On Monday, 6 April 2020 17:48:35 CEST Eric Blake wrote:
> On 4/6/20 10:40 AM, Pino Toscano wrote:
> > Create two temporary directories for all the files created during the
> > virt-v2v run:
> > 1) tmpdir, created as $TMPDIR/virt-v2v.XX, for all the small files
> > 2) cachedir, created as $LIBGUESTFS_CACHEDIR/virt-v2v.XX, for the
> > big files (e.g. disks)
> > This way there is no need to manually schedule all the temporary files
> > and directories for removal when the application quits.
> > ---
> 
> Side note:
> 
> > +++ b/v2v/output_glance.ml
> > @@ -27,16 +27,6 @@ open Types
> >   open Utils
> >   
> >   class output_glance () =
> > -  (* Although glance can slurp in a stream from stdin, unfortunately
> > -   * 'qemu-img convert' cannot write to a stream (although I guess
> > -   * it could be implemented at least for raw).  Therefore we have
> > -   * to write to a temporary file.  XXX
> > -   *)
> > -  let tmpdir =
> > -let base_dir = (open_guestfs ())#get_cachedir () in
> > -let t = Mkdtemp.temp_dir ~base_dir "glance." in
> > -rmdir_on_exit t;
> > -t in
> >   object
> > inherit output
> >   
> > @@ -60,8 +50,12 @@ object
> > method supported_firmware = [ TargetBIOS; TargetUEFI ]
> >   
> > method prepare_targets _ overlays _ =
> > -(* Write targets to a temporary local file - see above for reason. *)
> > -List.map (fun (_, ov) -> TargetFile (tmpdir // ov.ov_sd)) overlays
> > +(* Although glance can slurp in a stream from stdin, unfortunately
> > + * 'qemu-img convert' cannot write to a stream (although I guess
> > + * it could be implemented at least for raw).  Therefore we have
> > + * to write to a temporary file.  XXX
> > + *)
> > +List.map (fun (_, ov) -> TargetFile (cachedir // ov.ov_sd)) overlays
> 
> I know this was just code motion, but does the nbdkit streaming plugin 
> help here?

I honestly don't know, I don't have a lot of nbdkit-foo :-)

This is the glance output mode, which creates new templates in the
Glance OpenStack service from each disk of the source VM. In recent
times it was sort of obsoleted by the openstack output mode, which
DTRT by creating a VM in Cinder.

It can be a good improvement, although certainly nothing more than
"nice to have" because of the explanation above.

-- 
Pino Toscano

signature.asc
Description: This is a digitally signed message part.
___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs

[Libguestfs] [v2v PATCH 1/2] v2v: nbdkit: change base dir for nbdkit sockets/pidfiles

2020-04-06 Thread Pino Toscano
Since this new temporary directory will contain UNIX sockets for
communicating with nbdkit, then its path must not be too long.

Use the existing directory that libguestfs exposes for this, i.e.
sockdir.
---
 v2v/nbdkit.ml | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/v2v/nbdkit.ml b/v2v/nbdkit.ml
index 65317f9b..46b20c9d 100644
--- a/v2v/nbdkit.ml
+++ b/v2v/nbdkit.ml
@@ -103,9 +103,12 @@ let add_filter_if_available cmd filter =
   if probe_filter filter then add_filter cmd filter else cmd
 
 let run_unix cmd =
-  (* Create a temporary directory where we place the socket and PID file. *)
+  (* Create a temporary directory where we place the socket and PID file.
+   * Use the libguestfs socket directory, so it is more likely the full path
+   * of the UNIX sockets will fit in the (limited) socket pathname.
+   *)
   let tmpdir =
-let base_dir = (open_guestfs ())#get_cachedir () in
+let base_dir = (open_guestfs ())#get_sockdir () in
 let t = Mkdtemp.temp_dir ~base_dir "v2vnbdkit." in
 (* tmpdir must be readable (but not writable) by "other" so that
  * qemu can open the sockets.
-- 
2.25.1

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs



[Libguestfs] [v2v PATCH] tests: fix location to generated images

2020-04-06 Thread Pino Toscano
Some of the libvirt XMLs for tests refer to a generated phony disk
image, and so far the relative path used worked almost by chance.
(For each disk, the path to it was ../test-data/etc, and since the
overlay pointing to it is stored directly in $LIBGUESTFS_CACHEDIR,
which is $buildir/tmp, then the relative path was resolved.)

Instead, have configure place the right top-level directory in those
XMLs, so the full path points to the generated disks. This fixes the
tests for at least two cases:
- we change the place where the temporary overlays are stored
- in srcdir!=builddir builds

There is no functional change to the tests themselves.
---
 .gitignore   | 7 +++
 configure.ac | 7 +++
 tests/Makefile.am| 9 +
 tests/rhbz1232192.sh | 2 +-
 tests/{rhbz1232192.xml => rhbz1232192.xml.in}| 4 ++--
 tests/test-v2v-cdrom.sh  | 2 +-
 tests/{test-v2v-cdrom.xml => test-v2v-cdrom.xml.in}  | 4 ++--
 tests/test-v2v-floppy.sh | 2 +-
 tests/{test-v2v-floppy.xml => test-v2v-floppy.xml.in}| 4 ++--
 tests/test-v2v-mac.sh| 2 +-
 tests/{test-v2v-mac.xml => test-v2v-mac.xml.in}  | 2 +-
 tests/test-v2v-networks-and-bridges.sh   | 2 +-
 ...-bridges.xml => test-v2v-networks-and-bridges.xml.in} | 2 +-
 tests/test-v2v-print-source.sh   | 2 +-
 ...v2v-print-source.xml => test-v2v-print-source.xml.in} | 2 +-
 tests/test-v2v-sound.sh  | 2 +-
 tests/{test-v2v-sound.xml => test-v2v-sound.xml.in}  | 2 +-
 17 files changed, 32 insertions(+), 25 deletions(-)
 rename tests/{rhbz1232192.xml => rhbz1232192.xml.in} (75%)
 rename tests/{test-v2v-cdrom.xml => test-v2v-cdrom.xml.in} (89%)
 rename tests/{test-v2v-floppy.xml => test-v2v-floppy.xml.in} (89%)
 rename tests/{test-v2v-mac.xml => test-v2v-mac.xml.in} (96%)
 rename tests/{test-v2v-networks-and-bridges.xml => 
test-v2v-networks-and-bridges.xml.in} (96%)
 rename tests/{test-v2v-print-source.xml => test-v2v-print-source.xml.in} (90%)
 rename tests/{test-v2v-sound.xml => test-v2v-sound.xml.in} (93%)

diff --git a/.gitignore b/.gitignore
index 0914ea9b..b504a4c2 100644
--- a/.gitignore
+++ b/.gitignore
@@ -128,7 +128,14 @@ Makefile.in
 /test-harness/stamp-virt-v2v-test-harness.pod
 /test-harness/virt-v2v-test-harness.1
 /tests/libvirt-is-version
+/tests/rhbz1232192.xml
+/tests/test-v2v-cdrom.xml
 /tests/test-v2v-conversion-of-*.sh
+/tests/test-v2v-floppy.xml
+/tests/test-v2v-mac.xml
+/tests/test-v2v-networks-and-bridges.xml
+/tests/test-v2v-print-source.xml
+/tests/test-v2v-sound.xml
 /tests/windows.vmdk
 /v2v/.depend
 /v2v/config.ml
diff --git a/configure.ac b/configure.ac
index 73cf2081..e8a3133a 100644
--- a/configure.ac
+++ b/configure.ac
@@ -153,6 +153,13 @@ AC_CONFIG_FILES([Makefile
  test-harness/Makefile
  test-harness/META
  tests/Makefile
+ tests/rhbz1232192.xml
+ tests/test-v2v-cdrom.xml
+ tests/test-v2v-floppy.xml
+ tests/test-v2v-mac.xml
+ tests/test-v2v-networks-and-bridges.xml
+ tests/test-v2v-print-source.xml
+ tests/test-v2v-sound.xml
  v2v/Makefile
  v2v/config.ml])
 
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 267e4454..4456dc4c 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -177,11 +177,9 @@ EXTRA_DIST += \
test-v2v-bad-networks-and-bridges.sh \
test-v2v-cdrom.expected \
test-v2v-cdrom.sh \
-   test-v2v-cdrom.xml \
test-v2v-copy-to-local.sh \
test-v2v-floppy.expected \
test-v2v-floppy.sh \
-   test-v2v-floppy.xml \
test-v2v-i-disk.sh \
test-v2v-i-ova-as-root.ovf \
test-v2v-i-ova-as-root.sh \
@@ -232,8 +230,6 @@ EXTRA_DIST += \
test-v2v-machine-readable.sh \
test-v2v-mac-expected.xml \
test-v2v-mac.sh \
-   test-v2v-mac.xml \
-   test-v2v-networks-and-bridges-expected.xml \
test-v2v-networks-and-bridges.sh \
test-v2v-networks-and-bridges.xml \
test-v2v-no-copy.sh \
@@ -258,12 +254,9 @@ EXTRA_DIST += \
test-v2v-print-estimate.sh \
test-v2v-print-source.expected \
test-v2v-print-source.sh \
-   test-v2v-print-source.xml \
test-v2v-conversion-of.sh \
test-v2v-sound.sh \
-   test-v2v-sound.xml \
test-v2v-trim.sh \
test-v2v-virtio-win-iso.sh \
test-v2v-windows-conversion.sh \
-   rhbz1232192.sh \
-   rhbz1232192.xml
+   rhbz1232192.sh
diff --git a/tests/rhbz1232192.sh b/tests/rhbz1232192.sh
index 80f86003..2d4b342b 100755
--- a/tests/rhbz1232192.sh
+++ b/tests/rhbz1232192.sh
@@ 

Re: [Libguestfs] [PATCH virt-v2v v2 2/2] v2v: Allow large temporary directory to be set on a global basis.

2020-04-06 Thread Pino Toscano
On Monday, 6 April 2020 11:19:12 CEST Richard W.M. Jones wrote:
> Previously we placed large files in g#get_cachedir () (usually
> /var/tmp).  However the problem is this ties the libguestfs appliance
> and the virt-v2v overlay files to the same location.
> 
> When virt-v2v is run in a container, or any other situation where
> local storage is limited, it's helpful to be able to put the overlay
> files on an externally mounted PVC, which might be using NFS and
> shared between containers.  But putting the libguestfs appliance on
> NFS in a shared location is certainly not recommended.
> 
> This allows the two locations to be set separately:
> 
>   VIRT_V2V_TMPDIR - location of large temporary files, can use NFS
> and may be shared
> 
>   LIBGUESTFS_CACHEDIR - location of libguestfs appliance
> 
> Another motivation for this patch is to allow more reliable cleanup of
> large temporary files by an external process, as described in the
> updated documentation.
> 
> Small temporary files are placed in $TMPDIR (usually /tmp).  I cleaned
> up some existing code which used /var/tmp for small temporaries.

(^ this last change reverts 0bc1411fc8693dd981efef0088b3d335a11332cf )

> ---

This is mostly a repost of v1, with even few things that got worse
(I'll mention it later on). Since my requests [1][2] for actual use
cases and motivations behind this went unanswered other than with
- "because containers"
- "because why not"
then, I'll keep NACK-ing this patch.

[1] https://www.redhat.com/archives/libguestfs/2020-April/msg8.html
[2] https://www.redhat.com/archives/libguestfs/2020-April/msg00010.html

Since the problem is "let's find out what to clean on failure", I
propose a different approach, as also looking at this patch pointed out
to me:
- create a single virt-v2v.XX temporary directory in
  LIBGUESTFS_TMPDIR for small files, like the vmx file, the rhv-upload
  Python scripts, etc
- create a single virt-v2v.XX temporary directory in
  LIBGUESTFS_CACHEDIR for big files
Advantages:
- no more need for the various Mkdtemp.temp_dir all around
- only two well-defined names for temporary stuff that virt-v2v saves
- similar to what done in other tools (eg virt-builder, 75fbe4511e05df)

I'll send a patch for this shortly.

-- 
Pino Toscano

signature.asc
Description: This is a digitally signed message part.
___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs

  1   2   3   4   5   6   7   8   9   10   >