from:"Pino Toscano"

[Libguestfs] [PATCH 2/3] inspect: ignore special CD devices on FreeBSD install discs

2013-11-28 Thread Pino Toscano

/etc/fstab in installation discs of FreeBSD can have an entry pointing
to the mounted CD itself; skip it as it is done with other CD devices in
check_fstab.
---
 src/inspect-fs-unix.c | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/src/inspect-fs-unix.c b/src/inspect-fs-unix.c
index 8e0f135..a9e5cad 100644
--- a/src/inspect-fs-unix.c
+++ b/src/inspect-fs-unix.c
@@ -916,10 +916,15 @@ check_fstab (guestfs_h *g, struct inspect_fs *fs)
 if (spec == NULL)
   return -1;
 
-/* Ignore /dev/fd (floppy disks) (RHBZ#642929) and CD-ROM drives. */
+/* Ignore /dev/fd (floppy disks) (RHBZ#642929) and CD-ROM drives.
+ *
+ * /dev/iso9660/FREEBSD_INSTALL can be found in FreeBSDs installation
+ * discs.
+ */
 if ((STRPREFIX (spec, /dev/fd)  c_isdigit (spec[7])) ||
 STREQ (spec, /dev/floppy) ||
-STREQ (spec, /dev/cdrom))
+STREQ (spec, /dev/cdrom) ||
+STRPREFIX (spec, /dev/iso9660/))
   continue;
 
 snprintf (augpath, sizeof augpath, %s/file, *entry);
-- 
1.8.3.1

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

[Libguestfs] [PATCH 3/3] inspect: improve detection of FreeBSD install discs

2013-11-28 Thread Pino Toscano

Check for /boot/loader.rc as install disc detection, using it to mark
FreeBSD install discs.
Also, check for /mfsroot.gz to see whether such disc is also a live one.

See also RHBZ#1033207.
---
 src/inspect-fs-cd.c | 19 ++-
 src/inspect-fs.c|  3 ++-
 2 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/src/inspect-fs-cd.c b/src/inspect-fs-cd.c
index eaeaa6f..fff0629 100644
--- a/src/inspect-fs-cd.c
+++ b/src/inspect-fs-cd.c
@@ -327,6 +327,16 @@ check_isolinux_installer_root (guestfs_h *g, struct 
inspect_fs *fs)
   return 0;
 }
 
+/* FreeBSD with /boot/loader.rc.
+ */
+static int
+check_freebsd_installer_root (guestfs_h *g, struct inspect_fs *fs)
+{
+  fs-type = OS_TYPE_FREEBSD;
+
+  return 0;
+}
+
 /* Windows 2003 and similar versions.
  *
  * NB: txtsetup file contains Windows \r\n line endings, which guestfs_grep
@@ -430,7 +440,8 @@ guestfs___check_installer_root (guestfs_h *g, struct 
inspect_fs *fs)
* need to unpack this and look inside to tell the difference.
*/
   if (guestfs_is_file (g, /casper/filesystem.squashfs)  0 ||
-  guestfs_is_file (g, /live/filesystem.squashfs)  0)
+  guestfs_is_file (g, /live/filesystem.squashfs)  0 ||
+  guestfs_is_file (g, /mfsroot.gz)  0)
 fs-is_live_disk = 1;
 
   /* Debian/Ubuntu. */
@@ -461,6 +472,12 @@ guestfs___check_installer_root (guestfs_h *g, struct 
inspect_fs *fs)
   return -1;
   }
 
+  /* FreeBSD. */
+  else if (guestfs_is_file (g, /boot/loader.rc)  0) {
+if (check_freebsd_installer_root (g, fs) == -1)
+  return -1;
+  }
+
   /* Windows 2003 64 bit */
   else if (guestfs_is_file (g, /amd64/txtsetup.sif)  0) {
 fs-arch = safe_strdup (g, x86_64);
diff --git a/src/inspect-fs.c b/src/inspect-fs.c
index 0473e92..89c9335 100644
--- a/src/inspect-fs.c
+++ b/src/inspect-fs.c
@@ -320,7 +320,8 @@ check_filesystem (guestfs_h *g, const char *mountable,
 guestfs_is_file (g, /.discinfo)  0 ||
 guestfs_is_file (g, /i386/txtsetup.sif)  0 ||
 guestfs_is_file (g, /amd64/txtsetup.sif)  0 ||
-guestfs_is_file (g, /freedos/freedos.ico)  0)) {
+guestfs_is_file (g, /freedos/freedos.ico)  0 ||
+guestfs_is_file (g, /boot/loader.rc)  0)) {
 fs-is_root = 1;
 fs-format = OS_FORMAT_INSTALLER;
 if (guestfs___check_installer_root (g, fs) == -1)
-- 
1.8.3.1

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

[Libguestfs] [PATCH 3/3, v2] inspect: improve detection of FreeBSD install discs

2013-11-28 Thread Pino Toscano

Check for /boot/loader.rc as install disc detection, using it to mark
FreeBSD install discs.
Also, check for /mfsroot.gz to see whether such disc is also a live one.

See also RHBZ#1033207.
---
 src/inspect-fs-cd.c | 8 +++-
 src/inspect-fs.c| 3 ++-
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/src/inspect-fs-cd.c b/src/inspect-fs-cd.c
index eaeaa6f..45d7bb5 100644
--- a/src/inspect-fs-cd.c
+++ b/src/inspect-fs-cd.c
@@ -430,7 +430,8 @@ guestfs___check_installer_root (guestfs_h *g, struct 
inspect_fs *fs)
* need to unpack this and look inside to tell the difference.
*/
   if (guestfs_is_file (g, /casper/filesystem.squashfs)  0 ||
-  guestfs_is_file (g, /live/filesystem.squashfs)  0)
+  guestfs_is_file (g, /live/filesystem.squashfs)  0 ||
+  guestfs_is_file (g, /mfsroot.gz)  0)
 fs-is_live_disk = 1;
 
   /* Debian/Ubuntu. */
@@ -461,6 +462,11 @@ guestfs___check_installer_root (guestfs_h *g, struct 
inspect_fs *fs)
   return -1;
   }
 
+  /* FreeBSD with /boot/loader.rc. */
+  else if (guestfs_is_file (g, /boot/loader.rc)  0) {
+fs-type = OS_TYPE_FREEBSD;
+  }
+
   /* Windows 2003 64 bit */
   else if (guestfs_is_file (g, /amd64/txtsetup.sif)  0) {
 fs-arch = safe_strdup (g, x86_64);
diff --git a/src/inspect-fs.c b/src/inspect-fs.c
index 0473e92..89c9335 100644
--- a/src/inspect-fs.c
+++ b/src/inspect-fs.c
@@ -320,7 +320,8 @@ check_filesystem (guestfs_h *g, const char *mountable,
 guestfs_is_file (g, /.discinfo)  0 ||
 guestfs_is_file (g, /i386/txtsetup.sif)  0 ||
 guestfs_is_file (g, /amd64/txtsetup.sif)  0 ||
-guestfs_is_file (g, /freedos/freedos.ico)  0)) {
+guestfs_is_file (g, /freedos/freedos.ico)  0 ||
+guestfs_is_file (g, /boot/loader.rc)  0)) {
 fs-is_root = 1;
 fs-format = OS_FORMAT_INSTALLER;
 if (guestfs___check_installer_root (g, fs) == -1)
-- 
1.8.3.1

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

Re: [Libguestfs] [PATCH 3/3] inspect: improve detection of FreeBSD install discs

2013-11-28 Thread Pino Toscano

On Thursday 28 November 2013 14:12:16 Richard W.M. Jones wrote:
 On Thu, Nov 28, 2013 at 02:48:38PM +0100, Pino Toscano wrote:
  Check for /boot/loader.rc as install disc detection, using it to
  mark FreeBSD install discs.
  Also, check for /mfsroot.gz to see whether such disc is also a live
  one.
  
  See also RHBZ#1033207.
  ---
  
   src/inspect-fs-cd.c | 19 ++-
   src/inspect-fs.c|  3 ++-
   2 files changed, 20 insertions(+), 2 deletions(-)
  
  diff --git a/src/inspect-fs-cd.c b/src/inspect-fs-cd.c
  index eaeaa6f..fff0629 100644
  --- a/src/inspect-fs-cd.c
  +++ b/src/inspect-fs-cd.c
  @@ -327,6 +327,16 @@ check_isolinux_installer_root (guestfs_h *g,
  struct inspect_fs *fs) 
 return 0;
   
   }
  
  +/* FreeBSD with /boot/loader.rc.
  + */
  +static int
  +check_freebsd_installer_root (guestfs_h *g, struct inspect_fs *fs)
  +{
  +  fs-type = OS_TYPE_FREEBSD;
  +
  +  return 0;
  +}
  +
  
   /* Windows 2003 and similar versions.
   
*
* NB: txtsetup file contains Windows \r\n line endings, which
guestfs_grep 
  @@ -430,7 +440,8 @@ guestfs___check_installer_root (guestfs_h *g,
  struct inspect_fs *fs) 
  * need to unpack this and look inside to tell the difference.
  */
 
 if (guestfs_is_file (g, /casper/filesystem.squashfs)  0 ||
  
  -  guestfs_is_file (g, /live/filesystem.squashfs)  0)
  +  guestfs_is_file (g, /live/filesystem.squashfs)  0 ||
  +  guestfs_is_file (g, /mfsroot.gz)  0)
  
   fs-is_live_disk = 1;
 
 /* Debian/Ubuntu. */
  
  @@ -461,6 +472,12 @@ guestfs___check_installer_root (guestfs_h *g,
  struct inspect_fs *fs) 
 return -1;
 
 }
  
  +  /* FreeBSD. */
  +  else if (guestfs_is_file (g, /boot/loader.rc)  0) {
  +if (check_freebsd_installer_root (g, fs) == -1)
  +  return -1;
  +  }
 
 I think you should just inline this function, makes the code simpler.

That is OK too. I added it to follow what has been done for the other 
OSes/cases, to have an own function in case there is more work to do.
It can be split again, if needed, I guess.

Sending a v2 of this.

-- 
Pino Toscano

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

[Libguestfs] [PATCH] builder: translate more user-visible strings

2013-12-04 Thread Pino Toscano

Translate also the OS version and Notes strings that appear in the
--long output.
---
 builder/list_entries.ml | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/builder/list_entries.ml b/builder/list_entries.ml
index 8c24fe2..a1d0696 100644
--- a/builder/list_entries.ml
+++ b/builder/list_entries.ml
@@ -47,7 +47,7 @@ let list_entries ?(list_long = false) ~sources index =
   printf \n
 )
 else (  (* Long *)
-  printf %-24s %s\n os-version: name;
+  printf %-24s %s\n (s_OS version:) name;
   (match printable_name with
   | None - ()
   | Some name - printf %-24s %s\n (s_Full name:) name;
@@ -62,7 +62,7 @@ let list_entries ?(list_long = false) ~sources index =
   | None - ()
   | Some notes -
 printf \n;
-printf Notes:\n\n%s\n notes
+printf (f_Notes:\n\n%s\n) notes
   );
   printf \n
 )
-- 
1.8.3.1

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

[Libguestfs] [PATCH 0/3] Small improvements to i18n extraction/handling

2013-12-05 Thread Pino Toscano

Hi,

here there are few patches to improve the extraction of translatable
messages, and the usage of messages with plural forms.

Pino Toscano (3):
  po: fix broken message extraction
  po: fix dependencies for libguestfs.pot extraction
  fish: improve the command error messages

 generator/fish.ml | 20 
 po/Makefile.am| 24 +++-
 2 files changed, 23 insertions(+), 21 deletions(-)

Thanks,
-- 
Pino Toscano

-- 
1.8.3.1

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

[Libguestfs] [PATCH 3/3] fish: improve the command error messages

2013-12-05 Thread Pino Toscano

- when a command needs no parameters, tell that explicitly instead of
  command should have 0 parameters
- use gettext's plural form when printing the number of required
  arguments
- improve the error message for a variable number of parameters limited
  only in the maximum number of them, using also a plural form
---
 generator/fish.ml | 20 
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/generator/fish.ml b/generator/fish.ml
index 65f1acb..da0584b 100644
--- a/generator/fish.ml
+++ b/generator/fish.ml
@@ -362,12 +362,24 @@ Guestfish will prompt for these separately.
 
   if argc_minimum = argc_maximum then (
 pr   if (argc != %d) {\n argc_minimum;
-pr fprintf (stderr, _(\%%s should have %%d parameter(s)\\n\), 
cmd, %d);\n
-  argc_minimum;
+if argc_minimum = 0 then (
+  pr fprintf (stderr, _(\%%s should have no parameters\\n\), 
cmd);\n;
+) else (
+  pr fprintf (stderr, ngettext(\%%s should have %%d 
parameter\\n\,\n;
+  pr   \%%s should have %%d 
parameters\\n\,\n;
+  pr   %d),\n
+argc_minimum;
+  pr  cmd, %d);\n
+argc_minimum;
+)
   ) else if argc_minimum = 0 then (
 pr   if (argc  %d) {\n argc_maximum;
-pr fprintf (stderr, _(\%%s should have %%d-%%d 
parameter(s)\\n\), cmd, %d, %d);\n
-  argc_minimum argc_maximum;
+pr fprintf (stderr, ngettext(\%%s should have at most %%d 
parameter\\n\,\n;
+pr   \%%s should have at most %%d 
parameters\\n\,\n;
+pr   %d),\n
+  argc_maximum;
+pr  cmd, %d);\n
+  argc_maximum;
   ) else (
 pr   if (argc  %d || argc  %d) {\n argc_minimum argc_maximum;
 pr fprintf (stderr, _(\%%s should have %%d-%%d 
parameter(s)\\n\), cmd, %d, %d);\n
-- 
1.8.3.1

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

[Libguestfs] [PATCH 1/3] po: fix broken message extraction

2013-12-05 Thread Pino Toscano

Extracting separately the pot for the various languages and then
creating manually the global pot (by manually joining the above ones
after having stripped their headers) is wrong, since other than being
an hack it can create an invalid pot when the same message appears in
sources written in different languages.

Instead, a cleaner and safer solution is to first let ocaml-gettext
(if available) extract the messages for the ml files, and then use
xgettext to extract the messages for the other languages, joining the
new messages to the existing (or not) pot file.
---
 po/Makefile.am | 22 ++
 1 file changed, 6 insertions(+), 16 deletions(-)

diff --git a/po/Makefile.am b/po/Makefile.am
index e2bfe45..b0a8038 100644
--- a/po/Makefile.am
+++ b/po/Makefile.am
@@ -66,24 +66,14 @@ XGETTEXT_ARGS = \
--directory=$(top_srcdir)
 
 $(DOMAIN).pot: Makefile $(POTFILES) $(POTFILES-pl) $(POTFILES-ml)
-   rm -f $@-t $@-pl $@-ml
-   $(XGETTEXT) -o $@-t $(XGETTEXT_ARGS) \
- --files-from=$(abs_srcdir)/POTFILES
-   $(XGETTEXT) -o $@-pl $(XGETTEXT_ARGS) --language=Perl \
- --files-from=$(abs_srcdir)/POTFILES-pl
-# Don't trust msgcat since it will definitely screw up.  Instead, chop
-# the head from the second file and append it to the first.
-   echo  $@-t
-   awk '/^#:/{i++}i{print}'  $@-pl  $@-t
-   rm $@-pl
+   rm -f $@-t
 if HAVE_OCAML_GETTEXT
-   $(OCAML_GETTEXT) --action extract --extract-pot $@-ml $(POTFILES_ML)
-# Don't trust msgcat since it will definitely screw up.  Instead, chop
-# the head from the second file and append it to the first.
-   echo  $@-t
-   awk '/^#:/{i++}i{print}'  $@-ml  $@-t
-   rm $@-ml
+   $(OCAML_GETTEXT) --action extract --extract-pot $@-t $(POTFILES_ML)
 endif
+   $(XGETTEXT) -j -o $@-t $(XGETTEXT_ARGS) \
+ --files-from=$(abs_srcdir)/POTFILES
+   $(XGETTEXT) -j -o $@-t $(XGETTEXT_ARGS) --language=Perl \
+ --files-from=$(abs_srcdir)/POTFILES-pl
mv $@-t $@
 
 %.po: $(DOMAIN).pot
-- 
1.8.3.1

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

Re: [Libguestfs] [PATCH 1/3] po: fix broken message extraction

2013-12-05 Thread Pino Toscano

On Thursday 05 December 2013 16:01:59 Richard W.M. Jones wrote:
 This appears to be OK, and the results look sane:
 
 $ ll po/libguestfs.pot*
 -rw-rw-r--. 1 rjones rjones 180001 Dec  5 15:52 po/libguestfs.pot
 -rw-rw-r--. 1 rjones rjones 178207 Dec  5 15:40 po/libguestfs.pot-backup
 
 and I also checked the pot file (which is basically impossible) but it
 doesn't look as if messages are being dropped.

You don't really need to manually check the .pot file (even if doing it
is certainly a good thing) to see whether its strings changed. :)
You can do something like the following:

# create an english translation of the current pot
$ msgen -o libguestfs_old.po libguestfs.pot
# check it is fully translated (the warning can be ignored, since it is
# about the header which we didn't fill, but we are not interested in
# it anyway)
libguestfs_old.po: warning: Charset CHARSET is not a portable encoding name.
Message conversion to user's charset might not work.
1353 translated messages.
# edit po/Makefile.am as willing
[...]
# create the new pot
$ rm po/libguestfs.pot  make -C po libguestfs.pot
# update the english translation using the new pot
$ msgmerge -o libguestfs_new.po libguestfs_old.po libguestfs.pot
# check the new translation is still fully translated
libguestfs_new.po: warning: Charset CHARSET is not a portable encoding name.
Message conversion to user's charset might not work.
1353 translated messages.

(the actual number of messages in the pot could be different, but it is
not relevant for the method, anyway)

-- 
Pino Toscano

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

Re: [Libguestfs] [PATCH 2/3] po: fix dependencies for libguestfs.pot extraction

2013-12-05 Thread Pino Toscano

On Thursday 05 December 2013 18:31:59 Richard W.M. Jones wrote:
 On Thu, Dec 05, 2013 at 07:02:28PM +0100, Pino Toscano wrote:
  On Thursday 05 December 2013 15:53:58 Richard W.M. Jones wrote:
   On Thu, Dec 05, 2013 at 04:30:05PM +0100, Pino Toscano wrote:
Fix the dependencies of the libguestfs.pot target: other than
using
the right make variables holding the contents of the POTFILES,
depend also on the POTFILES themselves.
---

 po/Makefile.am | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/po/Makefile.am b/po/Makefile.am
index b0a8038..a8343ec 100644
--- a/po/Makefile.am
+++ b/po/Makefile.am
@@ -65,7 +65,7 @@ XGETTEXT_ARGS = \

--msgid-bugs-address=$(MSGID_BUGS_ADDRESS) \
--directory=$(top_srcdir)

-$(DOMAIN).pot: Makefile $(POTFILES) $(POTFILES-pl)
$(POTFILES-ml)
+$(DOMAIN).pot: Makefile POTFILES $(POTFILES) POTFILES-pl
$(POTFILES_PL) POTFILES-ml $(POTFILES_ML)

rm -f $@-t
 
 if HAVE_OCAML_GETTEXT
 
$(OCAML_GETTEXT) --action extract --extract-pot $@-t
$(POTFILES_ML)
   
   So I agree that $(POTFILES-pl) is definitely wrong.  Not sure
   exactly
   what we were thinking about there ...
   
   But, won't the addition of the literal file names break separate
   compilation?  In particular, $(POTFILES_PL) is supposed to be the
   correct path to the file POTFILES-pl (and correspondingly for the
   other files), so it shouldn't be necessary to list both
   POTFILES-pl
   and $(POTFILES_PL).
 
 This ^^ is bogus.  I misunderstood the $(SED) in the macros.
 Obviously $(POTFILES) expands to the contents of the POTFILES file,
 ie. a list of filenames.
 
  The idea behind the addition is make the libguestfs.pot generation
  dependent on both the POTFILES files and the actual sources, so
  either adding a new source to any POTFILES or changing any of the
  listed sources in any POTFILES will trigger a new pot rebuild.
 
 Right, so I believe the first version (posted above) is correct,
 because $(srcdir)/.. is not necessary (because automake is supposed to
 add a VPATH for these builds).
 
 Which apparently are called VPATH builds, not separate compilation.

Ah, ok.

 If you agree, I will push the first version of the 2/3 patch.

Yes, any of the two versions should be good for me.

Thanks,
-- 
Pino Toscano

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

[Libguestfs] [PATCH] builder: adapt test-virt-builder-list.sh output expectation

2013-12-06 Thread Pino Toscano

ee0e56f43e55307fefa1d04505ed6477d604d220 slightly changes the output of
--list but test-virt-builder-list.sh has not been updated accordingly.

Adapting the expected output makes test-virt-builder-list.sh pass again.
---
 builder/test-virt-builder-list.sh | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/builder/test-virt-builder-list.sh 
b/builder/test-virt-builder-list.sh
index 11305a9..ea4f561 100755
--- a/builder/test-virt-builder-list.sh
+++ b/builder/test-virt-builder-list.sh
@@ -41,7 +41,7 @@ long_list=$(./virt-builder --no-check-signature --no-cache 
--list --long)
 if [ $long_list != Source URI: $VIRT_BUILDER_SOURCE
 Fingerprint: F777 4FB1 AD07 4A7E 8C87 67EA 9173 8F73 E1B7 68A0
 
-os-version:  phony-debian
+OS version:  phony-debian
 Full name:   Phony Debian
 Minimum/default size:512.0M
 
@@ -49,7 +49,7 @@ Notes:
 
 Phony Debian look-alike used for testing.
 
-os-version:  phony-fedora
+OS version:  phony-fedora
 Full name:   Phony Fedora
 Minimum/default size:1.0G
 
@@ -57,7 +57,7 @@ Notes:
 
 Phony Fedora look-alike used for testing.
 
-os-version:  phony-ubuntu
+OS version:  phony-ubuntu
 Full name:   Phony Ubuntu
 Minimum/default size:512.0M
 
@@ -65,7 +65,7 @@ Notes:
 
 Phony Ubuntu look-alike used for testing.
 
-os-version:  phony-windows
+OS version:  phony-windows
 Full name:   Phony Windows
 Minimum/default size:512.0M
 
-- 
1.8.3.1

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

Re: [Libguestfs] Use of qemu-kvm command line arguments in libguestfs (was: Re: Fwd: ...)

2013-12-06 Thread Pino Toscano

On Monday 02 December 2013 09:09:44 Richard W.M. Jones wrote:
 On Mon, Dec 02, 2013 at 01:17:09AM +0100, Pino Toscano wrote:
  On Friday 29 November 2013 18:53:38 you wrote:
 I'm marking options with NONE (not used at all), or M (used in
 master) and/or RH7 (used in rhel-7.0).
 
  * -nographic
 
 M + RH7
 I guess that could be changed into (roughly)
 
   if (qemu_supports (g, data, -display))
   { ADD_CMDLINE (-display); ADD_CMDLINE (none); }
   else
   
 ADD_CMDLINE (-nographic);

Yup, something like that should work.  Did you have a look at
the
qemu code to find out what other side-effects -nographic has? 
Are
they important side-effects?
  
  It seems most of the effects are related to the default devices
  (serial, parallel, vrtcon, etc) that are added; OTOH, we always use
  -nodefaults so those are not added, so at least there should be no
  difference from this p.o.v.
  
  The other changes related to the use of nographics are in few
  target-
  related places:
  - in lm32 hardware (milkymist-tmu2), no display device is created
  when 
the display type is nographics (but it is when the type is
none?
hmm..)
  
  - in nvram support, the FW_CFG_NOGRAPHIC bit is set to 1 if the
  display 
is nographics, while 0 otherwise
  
  - in sparc (not sparc64 though, it seems), the ESCC serial device is
  
created disabled if the display type is nographics, while
enabled
otherwise
  
  I'm not totally sure about these architecture-specific bits, but at
  least from a quick glance it could be relatively safe to make
  -display none preferred over -nographics, at it seems more like
  device graphics is enabled (as if it was SDL, curses, etc), just
  with no output at all).
 
 It sounds fine, as long as qemu 1.2 had -display none.

It seems like this feature was added in commit
4171d32e6eea47bf2cd160ace0ec3639e10b3aa9, which is available in
qemu = 0.15.0.

It is not easy to check for it in qemu's --help text, as the help bits 
for -display span over multiple lines, so I guess it could be just 
switched altogether.

I tried switching it locally, and the unit tests still passed. 
(Currently using qemu 1.4.2, as shipped in f19.)
Would it be acceptable to do this change in master, to gather more 
feedback about it?

-- 
Pino Toscano

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

Re: [Libguestfs] [PATCH] builder: adapt test-virt-builder-list.sh output expectation

2013-12-06 Thread Pino Toscano

On Friday 06 December 2013 14:51:41 Richard W.M. Jones wrote:
 On Fri, Dec 06, 2013 at 03:00:41PM +0100, Pino Toscano wrote:
  ee0e56f43e55307fefa1d04505ed6477d604d220 slightly changes the output
  of --list but test-virt-builder-list.sh has not been updated
  accordingly.
  
  Adapting the expected output makes test-virt-builder-list.sh pass
  again.
  
  -os-version:  phony-debian
  +OS version:  phony-debian
 
 Now I look at this, it's not OS version (operating system version).
 It's literally the string os-version.  See the man page:
 
 http://libguestfs.org/virt-builder.1.html#synopsis
 
 So I think the original change is wrong :-(

Oops, sorry again for the small issue, and thanks for having reverted 
that string change.

-- 
Pino Toscano

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

Re: [Libguestfs] [PATCH] builder: translate more user-visible strings

2013-12-06 Thread Pino Toscano

On Friday 06 December 2013 14:50:08 Richard W.M. Jones wrote:
 On Fri, Dec 06, 2013 at 02:58:27PM +0100, Pino Toscano wrote:
  OTOH, that makes me think whether virt-builder shouldn't have some
  kind of porcelain/XML/JSON/CSV/etc output for --list to ease its
  parsing, as it could be translated and changed anytime (like done
  above). Good idea? If so, which format?
 
 First of all cannot change the existing format, but we can certainly
 add a flag to choose an alternate format.  cf. 'qemu-img info' has a
 flag that lets you select between human and json.

Yes, that's one of the things I had in mind, adding a new output mode 
for --list aside the current one. We could just mimic qemu-img then, 
with --format=json|human (with human being the default one providing the 
current output).

 I have no strong opinions about the format to use.  One which doesn't
 require any external library, but since this is output rather than
 input we probably don't need an external library.

libguestfs has libyajl as JSON parsing library so theoretically it could 
be optionally used as well. But yeah, virt-builder could it do without.

/me adds to low-priority TODO

-- 
Pino Toscano

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

[Libguestfs] [PATCH] sysprep: allow to specify globbing for --delete

2013-12-09 Thread Pino Toscano

Adapt the globbing part from the old --remote-path work previously
proposed for sysprep [1], allowing --delete to perform globbing when
deleting paths.

[1] https://www.redhat.com/archives/libguestfs/2013-October/msg00045.html
---
 sysprep/sysprep_operation_delete.ml | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/sysprep/sysprep_operation_delete.ml 
b/sysprep/sysprep_operation_delete.ml
index de78a87..59d5485 100644
--- a/sysprep/sysprep_operation_delete.ml
+++ b/sysprep/sysprep_operation_delete.ml
@@ -27,7 +27,9 @@ let add_paths path = paths := path :: !paths
 
 let path_perform g root =
   let paths = List.rev !paths in
-  List.iter g#rm_rf paths;
+  if paths  [] then (
+List.iter (fun glob - Array.iter g#rm_rf (g#glob_expand glob)) paths
+  );
   []
 
 let op = {
-- 
1.8.3.1

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

[Libguestfs] [PATCH] launch: switch from -nographic to -display none

2013-12-09 Thread Pino Toscano

The latter is a better way to disable the qemu display output as we
need to, without enabling extra devices (which are disabled already,
anyway).

Also, related to the change above, ban the -display parameter from the
ones that can be supplied by the user.
---
 configure.ac|  8 
 src/launch-direct.c | 12 
 src/launch.c|  1 +
 3 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/configure.ac b/configure.ac
index ae676c4..eb0e493 100644
--- a/configure.ac
+++ b/configure.ac
@@ -797,16 +797,16 @@ working.
 AC_MSG_FAILURE([$QEMU version must be = 1.0.])
 fi
 
-AC_MSG_CHECKING([that $QEMU -nographic -machine accel=kvm:tcg -device ? 
works])
-if $QEMU -nographic -machine accel=kvm:tcg -device \? AS_MESSAGE_LOG_FD 
21; then
+AC_MSG_CHECKING([that $QEMU -display none -machine accel=kvm:tcg -device ? 
works])
+if $QEMU -display none -machine accel=kvm:tcg -device \? 
AS_MESSAGE_LOG_FD 21; then
 AC_MSG_RESULT([yes])
 else
 AC_MSG_RESULT([no])
-AC_MSG_FAILURE([$QEMU -nographic -machine accel=kvm:tcg -device ? 
doesn't work.])
+AC_MSG_FAILURE([$QEMU -display none -machine accel=kvm:tcg -device ? 
doesn't work.])
 fi
 
 AC_MSG_CHECKING([for virtio-serial support in $QEMU])
-if $QEMU $QEMU_OPTIONS -nographic -machine accel=kvm:tcg -device \? 21 | 
grep -sq virtio-serial; then
+if $QEMU $QEMU_OPTIONS -display none -machine accel=kvm:tcg -device \? 
21 | grep -sq virtio-serial; then
 AC_MSG_RESULT([yes])
 else
 AC_MSG_RESULT([no])
diff --git a/src/launch-direct.c b/src/launch-direct.c
index 8d0d8ce..f45f582 100644
--- a/src/launch-direct.c
+++ b/src/launch-direct.c
@@ -349,7 +349,8 @@ launch_direct (guestfs_h *g, void *datav, const char *arg)
   if (qemu_supports (g, data, -nodefaults))
 ADD_CMDLINE (-nodefaults);
 
-  ADD_CMDLINE (-nographic);
+  ADD_CMDLINE (-display);
+  ADD_CMDLINE (none);
 
 #ifdef MACHINE_TYPE
   ADD_CMDLINE (-M);
@@ -918,7 +919,8 @@ test_qemu (guestfs_h *g, struct backend_direct_data *data)
   data-qemu_devices = NULL;
 
   guestfs___cmd_add_arg (cmd1, g-hv);
-  guestfs___cmd_add_arg (cmd1, -nographic);
+  guestfs___cmd_add_arg (cmd1, -display);
+  guestfs___cmd_add_arg (cmd1, none);
   guestfs___cmd_add_arg (cmd1, -help);
   guestfs___cmd_set_stdout_callback (cmd1, read_all, data-qemu_help,
  CMD_STDOUT_FLAG_WHOLE_BUFFER);
@@ -927,7 +929,8 @@ test_qemu (guestfs_h *g, struct backend_direct_data *data)
 goto error;
 
   guestfs___cmd_add_arg (cmd2, g-hv);
-  guestfs___cmd_add_arg (cmd2, -nographic);
+  guestfs___cmd_add_arg (cmd2, -display);
+  guestfs___cmd_add_arg (cmd2, none);
   guestfs___cmd_add_arg (cmd2, -version);
   guestfs___cmd_set_stdout_callback (cmd2, read_all, data-qemu_version,
  CMD_STDOUT_FLAG_WHOLE_BUFFER);
@@ -938,7 +941,8 @@ test_qemu (guestfs_h *g, struct backend_direct_data *data)
   parse_qemu_version (g, data);
 
   guestfs___cmd_add_arg (cmd3, g-hv);
-  guestfs___cmd_add_arg (cmd3, -nographic);
+  guestfs___cmd_add_arg (cmd3, -display);
+  guestfs___cmd_add_arg (cmd3, none);
   guestfs___cmd_add_arg (cmd3, -machine);
   guestfs___cmd_add_arg (cmd3, accel=kvm:tcg);
   guestfs___cmd_add_arg (cmd3, -device);
diff --git a/src/launch.c b/src/launch.c
index 60f6811..9c1c33a 100644
--- a/src/launch.c
+++ b/src/launch.c
@@ -295,6 +295,7 @@ guestfs__config (guestfs_h *g,
   if (STREQ (hv_param, -kernel) ||
   STREQ (hv_param, -initrd) ||
   STREQ (hv_param, -nographic) ||
+  STREQ (hv_param, -display) ||
   STREQ (hv_param, -serial) ||
   STREQ (hv_param, -full-screen) ||
   STREQ (hv_param, -std-vga) ||
-- 
1.8.3.1

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

Re: [Libguestfs] [PATCH] sysprep: mention globbing in help for --delete

2013-12-09 Thread Pino Toscano

On Monday 09 December 2013 17:58:37 Richard W.M. Jones wrote:
 On Mon, Dec 09, 2013 at 06:17:00PM +0100, Pino Toscano wrote:
  Followup of ed4bcb119cb908e98ecc1107dcd8b740ee2f484f.
  ---
  
   sysprep/sysprep_operation_delete.ml | 5 -
   1 file changed, 4 insertions(+), 1 deletion(-)
  
  diff --git a/sysprep/sysprep_operation_delete.ml
  b/sysprep/sysprep_operation_delete.ml index 59d5485..a08d4d0 100644
  --- a/sysprep/sysprep_operation_delete.ml
  +++ b/sysprep/sysprep_operation_delete.ml
  @@ -40,7 +40,10 @@ let op = {
  
   pod_description = Some (s_\
   
   Delete specified files or directories.
  
  -Use the I--delete option to specify a path to remove.);
  +Use the I--delete option to specify a path to remove.
  +
  +You can use shell glob characters in the specified path, i.e.
  +C/some/dir/prefix*.);
 
 How about /var/log/*.log as the example?  Also I think we should tell
 people to quote the arg to defend it from the shell.  So it could look
 like this:
 
 -Use the I--delete option to specify a path to remove.);
 +Use the I--delete option to specify a path to remove.
 +
 +You can use shell glob characters in the specified path,
 +for example:
 +
 + virt-sysprep --delete '/var/log/*.log');
 
 (Putting a space in front makes verbatim text in POD)

Good starting point, I've spelt the escaping need explicitly; updated 
patch coming.

-- 
Pino Toscano

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

Re: [Libguestfs] [PATCH] lib: fix newline in error output

2013-12-10 Thread Pino Toscano

On Monday 09 December 2013 19:26:47 Richard W.M. Jones wrote:
 On Mon, Dec 09, 2013 at 08:13:19PM +0100, Pino Toscano wrote:
  See also RHBZ#923355.
  ---
  
   src/actions-support.c | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)
  
  diff --git a/src/actions-support.c b/src/actions-support.c
  index d0e3e67..00d8cdc 100644
  --- a/src/actions-support.c
  +++ b/src/actions-support.c
  @@ -68,7 +68,7 @@ int
  
   guestfs___check_appliance_up (guestfs_h *g, const char *caller)
   {
   
 if (g-state == CONFIG || g-state == LAUNCHING) {
  
  -error (g, %s: call launch before using this function\\n(in
  guestfish, don't forget to use the 'run' command), +error (g,
  %s: call launch before using this function\n(in guestfish, don't
  forget to use the 'run' command), 
  caller);
   
   return -1;
 
 }
 
 ACK.
 
 That's a stupid mistake :-(
 
 I believe this actually fixes RHBZ#923355?  So probably best to put
 that in the subject line of the bug before you push it, and also to
 close the bug.

The bug seemed a bit more generic, not just about this specific 
occurrence. That's why I left a comment in it, and just mentioned it in 
the commit message of this patch without the closing syntax.

I just changed the commit message and pushed it, and the bug closing 
will come shortly.

Thanks,
-- 
Pino Toscano

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

Re: [Libguestfs] [PATCH] sysprep: handle distro specific sysv scripts

2013-12-12 Thread Pino Toscano

On Thursday 12 December 2013 16:27:30 Olaf Hering wrote:
 On Thu, Dec 12, Pino Toscano wrote:
  Sure, but as a Should-Start means it is a weak dependency, and can
  be
  skipped, which is what I don't want.
 
 Are you saying in Debian the Should-Start is not handled properly?
 Even if Should-Start is weak it still has to be taken into account.

No, I'm saying a weak dependency is no guarantee the provided firstboot 
commands can run successfully, and running them after $all should 
provide a better safety.

-- 
Pino Toscano

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

[Libguestfs] [PATCH] drives: add CD-ROM disk images as read-only drives (RHBZ#563450).

2013-12-16 Thread Pino Toscano

The current add_cdrom way basically appends a new raw -cdrom /path
parameter to the qemu invocation (even when using libvirt as backend),
hence such images are seen as CD-ROM drives inside the appliance.
However, there is no need for such particular behaviour, as they need to
be handled as normal (read-only) drives.

Adding CD-ROM disk images as drives also changes the device names used
for them inside the appliance from /dev/srN to the usual e.g. /dev/sdX.

These changes fix different issues:
- it is possible to start guestfish without adding disks with -a, then
  just add-cdrom and run
- list-devices does not cause guestfsd to crash when sorting the list
  of devices (exposed by the test case in RHBZ#563450)
- the result of list-devices now reflects the order images were added
  (RHBZ#563450)

add_cdrom is still deprecated, but now in favour of add_drive_ro
(instead of add_drive), with its documentation reflecting that.

Add two small regression tests for the fixes described above.
---
 generator/actions.ml |  6 ++---
 src/drives.c | 13 +-
 tests/regressions/Makefile.am|  2 ++
 tests/regressions/rhbz563450.sh  | 54 
 tests/regressions/rhbz563450b.sh | 43 
 5 files changed, 103 insertions(+), 15 deletions(-)
 create mode 100755 tests/regressions/rhbz563450.sh
 create mode 100755 tests/regressions/rhbz563450b.sh

diff --git a/generator/actions.ml b/generator/actions.ml
index cb6d137..b884efb 100644
--- a/generator/actions.ml
+++ b/generator/actions.ml
@@ -241,14 +241,14 @@ Do not call this.  See: Cguestfs_shutdown instead. };
   { defaults with
 name = add_cdrom;
 style = RErr, [String filename], [];
-deprecated_by = Some add_drive; config_only = true;
+deprecated_by = Some add_drive_ro; config_only = true;
 blocking = false;
 shortdesc = add a CD-ROM disk image to examine;
 longdesc = \
 This function adds a virtual CD-ROM disk image to the guest.
 
-BDo not use this function!  ISO files are just ordinary
-read-only disk images.  Use Cguestfs_add_drive_ro instead. };
+The image is added as read-only drive, so this function is equivalent
+of Cguestfs_add_drive_ro. };
 
   { defaults with
 name = add_drive_ro;
diff --git a/src/drives.c b/src/drives.c
index 16798a3..cddde4f 100644
--- a/src/drives.c
+++ b/src/drives.c
@@ -1081,18 +1081,7 @@ guestfs__add_drive_scratch (guestfs_h *g, int64_t size,
 int
 guestfs__add_cdrom (guestfs_h *g, const char *filename)
 {
-  if (strchr (filename, ':') != NULL) {
-error (g, _(filename cannot contain ':' (colon) character. 
-This is a limitation of qemu.));
-return -1;
-  }
-
-  if (access (filename, F_OK) == -1) {
-perrorf (g, %s, filename);
-return -1;
-  }
-
-  return guestfs_config (g, -cdrom, filename);
+  return guestfs__add_drive_ro (g, filename);
 }
 
 /* Depending on whether we are hotplugging or not, this function
diff --git a/tests/regressions/Makefile.am b/tests/regressions/Makefile.am
index 5b0d1c3..741f45f 100644
--- a/tests/regressions/Makefile.am
+++ b/tests/regressions/Makefile.am
@@ -21,6 +21,8 @@ TESTS = \
rhbz501893 \
rhbz503169c13.sh \
rhbz557655.sh \
+   rhbz563450.sh \
+   rhbz563450b.sh \
rhbz576879.sh \
rhbz578407.sh \
rhbz580246.sh \
diff --git a/tests/regressions/rhbz563450.sh b/tests/regressions/rhbz563450.sh
new file mode 100755
index 000..6fa6f2b
--- /dev/null
+++ b/tests/regressions/rhbz563450.sh
@@ -0,0 +1,54 @@
+#!/bin/bash -
+# libguestfs
+# Copyright (C) 2013 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.
+
+# https://bugzilla.redhat.com/show_bug.cgi?id=563450
+# Test the order of added images
+
+set -e
+export LANG=C
+
+rm -f test.out
+
+../../fish/guestfish --ro  test.out EOF
+add-drive-ro ../guests/fedora.img
+add-cdrom ../data/test.iso
+add-drive-ro ../guests/debian.img
+
+run
+
+list-devices
+echo 
+list-partitions
+
+ping-daemon
+EOF
+
+if [ $(cat test.out) != /dev/sda
+/dev/sdb
+/dev/sdc
+
+/dev/sda1
+/dev/sda2
+/dev/sdc1
+/dev/sdc2 ]; then
+echo $0: unexpected output:
+cat test.out
+exit 1
+fi
+
+rm -f test.out
diff --git a/tests/regressions/rhbz563450b.sh b/tests/regressions/rhbz563450b.sh
new file

[Libguestfs] [PATCH] inspect: fix detection of newer CirrOS versions (RHBZ#1045450).

2013-12-20 Thread Pino Toscano

Add an own case for CirrOS, based on the /etc/cirros/version file
provided in newer version instead of the Buildroot-generated
/etc/br-version.
---
 src/inspect-fs-unix.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/src/inspect-fs-unix.c b/src/inspect-fs-unix.c
index a9e5cad..c70960a 100644
--- a/src/inspect-fs-unix.c
+++ b/src/inspect-fs-unix.c
@@ -574,6 +574,18 @@ guestfs___check_linux_root (guestfs_h *g, struct 
inspect_fs *fs)
   return -1;
 
   }
+  /* CirrOS versions providing a own version file.
+   */
+  else if (guestfs_is_file_opts (g, /etc/cirros/version,
+ GUESTFS_IS_FILE_OPTS_FOLLOWSYMLINKS, 1, -1)  
0) {
+fs-distro = OS_DISTRO_CIRROS;
+
+if (parse_release_file (g, fs, /etc/cirros/version) == -1)
+  return -1;
+
+if (guestfs___parse_major_minor (g, fs) == -1)
+  return -1;
+  }
   /* Buildroot (http://buildroot.net) is an embedded Linux distro
* toolkit.  It is used by specific distros such as Cirros.
*/
-- 
1.8.3.1

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

[Libguestfs] [PATCH] tests/mountable: skip if btrfs is not available

2013-12-23 Thread Pino Toscano

This test uses btrfs, so skip it if either the btrfs feature or the
btrfs filesystem is not available.
---
 tests/mountable/test-internal-parse-mountable.c | 13 +
 1 file changed, 13 insertions(+)

diff --git a/tests/mountable/test-internal-parse-mountable.c 
b/tests/mountable/test-internal-parse-mountable.c
index ed3264e..0638fc0 100644
--- a/tests/mountable/test-internal-parse-mountable.c
+++ b/tests/mountable/test-internal-parse-mountable.c
@@ -33,6 +33,7 @@ main (int argc, char *argv[])
   guestfs_h *g;
   struct guestfs_internal_mountable *mountable;
   const char *devices[] = { /dev/VG/LV, NULL };
+  const char *feature[] = { btrfs, NULL };
 
   g = guestfs_create ();
   if (g == NULL) {
@@ -48,6 +49,18 @@ main (int argc, char *argv[])
 
   if (guestfs_launch (g) == -1) goto error;
 
+  if (!guestfs_feature_available (g, (char **) feature)) {
+printf (skipping test because btrfs is not available\n);
+guestfs_close (g);
+exit (77);
+  }
+
+  if (!guestfs_filesystem_available (g, btrfs)) {
+printf (skipping test because btrfs filesystem is not available\n);
+guestfs_close (g);
+exit (77);
+  }
+
   if (guestfs_part_disk (g, /dev/sda, mbr) == -1) goto error;
 
   if (guestfs_pvcreate (g, /dev/sda1) == -1) goto error;
-- 
1.8.3.1

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

Re: [Libguestfs] [Bug 1046905] New: RFE: add argument to virt-sysprep to disable individual default operations

2014-01-07 Thread Pino Toscano

On Tuesday 07 January 2014 12:12:43 Richard W.M. Jones wrote:
 On Tue, Jan 07, 2014 at 11:16:08AM +0100, Pino Toscano wrote:
  On Friday 27 December 2013 10:58:15 you wrote:
   virt-sysprep either runs with all default operations or a selected
   list of operations with the --enable argument.  A few times I've
   found I'd like to use the default list, but minus one or two
   operations in particular, however there's no easy way to specify
   this.
   
   A --disable argument that took the default operation list and
   skipped
   selected operations would be useful.
  
  A rough idea I had about this is adding a new --operations
  parameter,
  which would take a a comma-separated list of operations (just like
  the current --enable), but with the following differences:
  - a leading minus would disable the specified operation
  - it would recognize the meta-keywords all (for all the available
  
operations) and defaults (for the ones enabled by default)
  
  Processing the list of operations would add/remove them from the
  current set of operations, bailing out whether the resulting set is
  empty (just like right now --enable rejects an empty string).
  
  This way, you could write e.g.:
  - --operations defaults,-hostname,user-account
  
runs the default ones but not hostname, and the non-default
user-account
  
  - --operations all
  
easy shortcut to run all the available operations
  
  and so on.
  
  --enable could just be an alias for --operations, or this new syntax
  could be added directly to --enable directly.
  
  Maybe I'm over-engineering, but IMHO seems a better way than
  potentially adding a --disable argument and deal with the order of
  --enable  --disable and their interactions.
 
 Seems reasonable.
 
 I guess only one --operations opt is allowed?
 
 Or would we allow multiple and concatenate them together?  So that
 
   --operations foo,bar --operations -baz
 
 would be equivalent to
 
   --operations foo,bar,-baz
 
 ?
 
 Multiple --operations could be useful for scripting, ie. it would let
 shell users write:
 
   opts=--operations all -a disk.img
   if [ $disable_foo ]; then opts=$opts --operations -foo; fi

I agree, allowing multiple --operations is a good thing. After all, 
given the proposed behaviour, every --operation would alter the existing 
set of operations (empty at the first --operation invocation, as with
--enable), instead of starting from scratch every time.

 Another thing to test is whether the OCaml argument parser[1] can
 handle '--operations -foo' without thinking that -foo is a separate
 arg.

Ah yes, I read about the unsupported --foo=arg syntax.
OTOH, it seems that --foo --foo, with --foo taking an Arg.String, 
gives --foo as value for it, so should be safe for us.

-- 
Pino Toscano

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

[Libguestfs] RFC: copy-attributes command

2014-01-07 Thread Pino Toscano

Hi,

attached there is a prototype of patch for adding a new copy-attributes 
command. Such command would allow copy the attributes of a file to 
another, so for example in guestfish:
  copy-attributes foo bar permissions:true xattributes:false
would only copy the permissions of foo to bar, not copying its extended 
attributes too.

Just few notes:
- my first daemon command, so possibly I could be missing something
- copy_xattrs is in xattr.c to avoid spreading the usage of xattr API in
  many places
- copy_xattrs does a bit of code repetition with other stuff in xattr.c,
  but I'm not sure how to avoid it without making the xattr listing code
  (getxattrs) a bit more complex that what it is already

Comments?

-- 
Pino Toscanodiff --git a/daemon/daemon.h b/daemon/daemon.h
index b77d764..6535658 100644
--- a/daemon/daemon.h
+++ b/daemon/daemon.h
@@ -231,6 +231,9 @@ extern void journal_finalize (void);
 /*-- in proto.c --*/
 extern void main_loop (int sock) __attribute__((noreturn));
 
+/*-- in xattr.c --*/
+extern int copy_xattrs (const char *src, const char *dest);
+
 /* ordinary daemon functions use these to indicate errors
  * NB: you don't need to prefix the string with the current command,
  * it is added automatically by the client-side RPC stubs.
diff --git a/daemon/file.c b/daemon/file.c
index f348f87..fd6da71 100644
--- a/daemon/file.c
+++ b/daemon/file.c
@@ -28,6 +28,7 @@
 #include guestfs_protocol.h
 #include daemon.h
 #include actions.h
+#include optgroups.h
 
 GUESTFSD_EXT_CMD(str_file, file);
 GUESTFSD_EXT_CMD(str_zcat, zcat);
@@ -584,3 +585,46 @@ do_filesize (const char *path)
 
   return buf.st_size;
 }
+
+int
+do_copy_attributes (const char *src, const char *dest, int permissions, int xattributes)
+{
+  int r;
+  struct stat srcstat, deststat;
+
+  CHROOT_IN;
+  r = stat (src, srcstat);
+  CHROOT_OUT;
+
+  if (r == -1) {
+reply_with_perror (stat: %s, src);
+return -1;
+  }
+
+  CHROOT_IN;
+  r = stat (dest, deststat);
+  CHROOT_OUT;
+
+  if (r == -1) {
+reply_with_perror (stat: %s, dest);
+return -1;
+  }
+
+  if (permissions  ((srcstat.st_mode  0777) != (deststat.st_mode  0777))) {
+CHROOT_IN;
+r = chmod (dest, (srcstat.st_mode  0777));
+CHROOT_OUT;
+
+if (r == -1) {
+  reply_with_perror (chmod: %s, dest);
+  return -1;
+}
+  }
+
+  if (xattributes  optgroup_linuxxattrs_available ()) {
+if (!copy_xattrs (src, dest))
+  return -1;
+  }
+
+  return 0;
+}
diff --git a/daemon/xattr.c b/daemon/xattr.c
index af8bfd4..97a94d5 100644
--- a/daemon/xattr.c
+++ b/daemon/xattr.c
@@ -545,8 +545,98 @@ do_lgetxattr (const char *path, const char *name, size_t *size_r)
   return buf; /* caller frees */
 }
 
+int
+copy_xattrs (const char *src, const char *dest)
+{
+#if defined(HAVE_LISTXATTR)  defined(HAVE_GETXATTR)  defined(HAVE_SETXATTR)
+  ssize_t len, vlen, ret;
+  CLEANUP_FREE char *buf = NULL, *attrval = NULL;
+  size_t i, attrval_len = 0;
+
+  CHROOT_IN;
+  len = listxattr (src, NULL, 0);
+  CHROOT_OUT;
+  if (len == -1) {
+reply_with_perror (listxattr: %s, src);
+goto error;
+  }
+
+  buf = malloc (len);
+  if (buf == NULL) {
+reply_with_perror (malloc);
+goto error;
+  }
+
+  CHROOT_IN;
+  len = listxattr (src, buf, len);
+  CHROOT_OUT;
+  if (len == -1) {
+reply_with_perror (listxattr: %s, src);
+goto error;
+  }
+
+  /* What we get from the kernel is a string foo\0bar\0baz of length
+   * len.
+   */
+  for (i = 0; i  (size_t) len; i += strlen (buf[i]) + 1) {
+CHROOT_IN;
+vlen = getxattr (src, buf[i], NULL, 0);
+CHROOT_OUT;
+if (vlen == -1) {
+  reply_with_perror (getxattr: %s, %s, src, buf[i]);
+  goto error;
+}
+
+if (vlen  XATTR_SIZE_MAX) {
+  /* The next call to getxattr will fail anyway, so ... */
+  reply_with_error (extended attribute is too large);
+  goto error;
+}
+
+if (vlen  attrval_len) {
+  char *new = realloc (attrval, vlen);
+  if (new == NULL) {
+reply_with_perror (realloc);
+goto error;
+  }
+  attrval = new;
+  attrval_len = vlen;
+}
+
+CHROOT_IN;
+vlen = getxattr (src, buf[i], attrval, vlen);
+CHROOT_OUT;
+if (vlen == -1) {
+  reply_with_perror (getxattr: %s, %s, src, buf[i]);
+  goto error;
+}
+
+CHROOT_IN;
+ret = setxattr (dest, buf[i], attrval, vlen, 0);
+CHROOT_OUT;
+if (vlen == -1) {
+  reply_with_perror (setxattr: %s, %s, dest, buf[i]);
+  goto error;
+}
+  }
+
+  return 1;
+
+ error:
+  return 0;
+#else
+  return 0;
+#endif
+}
+
 #else /* no xattr.h */
 
 OPTGROUP_LINUXXATTRS_NOT_AVAILABLE
 
+int
+copy_xattrs (const char *src, const char *dest)
+{
+  abort ();
+}
+
 #endif /* no xattr.h */
diff --git a/fish/Makefile.am b/fish/Makefile.am
index bd0485f..fb0e99e 100644
--- a/fish/Makefile.am
+++ b/fish/Makefile.am
@@ -279,6 +279,7 @@ if ENABLE_APPLIANCE
 TESTS += \
 	test-copy.sh \
 	test-edit.sh \
+	test-file-attrs.sh \
 	test-find0.sh \

Re: [Libguestfs] [Bug 1046905] New: RFE: add argument to virt-sysprep to disable individual default operations

2014-01-09 Thread Pino Toscano

On Tuesday 07 January 2014 15:24:51 Pino Toscano wrote:
 On Tuesday 07 January 2014 12:12:43 Richard W.M. Jones wrote:
  On Tue, Jan 07, 2014 at 11:16:08AM +0100, Pino Toscano wrote:
   On Friday 27 December 2013 10:58:15 you wrote:
virt-sysprep either runs with all default operations or a
selected
list of operations with the --enable argument.  A few times I've
found I'd like to use the default list, but minus one or two
operations in particular, however there's no easy way to specify
this.

A --disable argument that took the default operation list and
skipped
selected operations would be useful.
   
   A rough idea I had about this is adding a new --operations
   parameter,
   which would take a a comma-separated list of operations (just like
   the current --enable), but with the following differences:
   - a leading minus would disable the specified operation
   - it would recognize the meta-keywords all (for all the
   available
   
 operations) and defaults (for the ones enabled by default)
   
   Processing the list of operations would add/remove them from the
   current set of operations, bailing out whether the resulting set
   is
   empty (just like right now --enable rejects an empty string).
   
   This way, you could write e.g.:
   - --operations defaults,-hostname,user-account
   
 runs the default ones but not hostname, and the non-default
 user-account
   
   - --operations all
   
 easy shortcut to run all the available operations
   
   and so on.
   
   --enable could just be an alias for --operations, or this new
   syntax
   could be added directly to --enable directly.
   
   Maybe I'm over-engineering, but IMHO seems a better way than
   potentially adding a --disable argument and deal with the order of
   --enable  --disable and their interactions.
  
  Seems reasonable.

Attached there is a first PoC for --operations, implementing the idea I 
outlined with your hints too.

Possibly not the best ocaml code ever (my first long-ish ocaml code, 
actually), but seems to work fine; maybe the various functions added in 
Sysprep_operations could be simplified in one with a label as parameter 
(eg `All | `Defaults).

-- 
Pino Toscanodiff --git a/sysprep/main.ml b/sysprep/main.ml
index 689a394..328f93c 100644
--- a/sysprep/main.ml
+++ b/sysprep/main.ml
@@ -87,6 +87,40 @@ let debug_gc, operations, g, selinux_relabel, quiet, mount_opts =
   exit 1
 ) Sysprep_operation.empty_set ops in
 operations := Some opset
+  and set_operations op_string =
+let currentopset =
+  match (!operations) with
+  | Some x - x
+  | None - Sysprep_operation.empty_set
+in
+let ops = string_nsplit , op_string in
+let opset = List.fold_left (
+  fun opset op_name -
+let n = ref op_name in
+let remove = string_prefix op_name - in
+if remove then
+  n := String.sub op_name 1 (String.length op_name - 1);
+match !n with
+|  -
+  eprintf (f_%s: --operations: empty operation name\n)
+prog;
+  exit 1
+| defaults -
+  let f = if remove then Sysprep_operation.remove_defaults_from_set else Sysprep_operation.add_defaults_to_set in
+  f opset
+| all -
+  let f = if remove then Sysprep_operation.remove_all_from_set else Sysprep_operation.add_all_to_set in
+  f opset
+| _ -
+  let f = if remove then Sysprep_operation.remove_from_set else Sysprep_operation.add_to_set in
+  (try f !n opset with
+  | Not_found -
+eprintf (f_%s: --operations: '%s' is not a known operation\n)
+  prog !n;
+exit 1
+  )
+) currentopset ops in
+operations := Some opset
   and force_selinux_relabel () =
 selinux_relabel := `Force
   and no_force_selinux_relabel () =
@@ -114,6 +148,7 @@ let debug_gc, operations, g, selinux_relabel, quiet, mount_opts =
 --list-operations, Arg.Unit list_operations,   ^ s_List supported operations;
 --long-options, Arg.Unit display_long_options,   ^ s_List long options;
 --mount-options, Arg.Set_string mount_opts, s_opts ^   ^ s_Set mount options (eg /:noatime;/var:rw,noatime);
+--operations, Arg.String set_operations,   ^ s_Enable/disable specific operations;
 -q,Arg.Set quiet,   ^ s_Don't print log messages;
 --quiet,   Arg.Set quiet,   ^ s_Don't print log messages;
 --selinux-relabel, Arg.Unit force_selinux_relabel,   ^ s_Force SELinux relabel;
diff --git a/sysprep/sysprep_operation.ml b/sysprep/sysprep_operation.ml
index 1fb4f17..4d77ff2 100644
--- a/sysprep/sysprep_operation.ml
+++ b/sysprep/sysprep_operation.ml
@@ -68,10 +68,50 @@ type set = OperationSet.t
 
 let empty_set = OperationSet.empty
 
+let add_list_to_opset l s =
+  List.fold_left (
+fun acc elem -
+  OperationSet.add elem acc
+  ) s l
+
+let remove_list_from_opset l set

Re: [Libguestfs] RFC: copy-attributes command

2014-01-09 Thread Pino Toscano

On Tuesday 07 January 2014 21:04:36 Richard W.M. Jones wrote:
 On Tue, Jan 07, 2014 at 04:06:43PM +0100, Pino Toscano wrote:
  diff --git a/daemon/xattr.c b/daemon/xattr.c
  index af8bfd4..97a94d5 100644
  --- a/daemon/xattr.c
  +++ b/daemon/xattr.c
  @@ -545,8 +545,98 @@ do_lgetxattr (const char *path, const char
  *name, size_t *size_r) 
 return buf; /* caller frees */
   
   }
  
  +int
  +copy_xattrs (const char *src, const char *dest)
  +{
  +#if defined(HAVE_LISTXATTR)  defined(HAVE_GETXATTR) 
  defined(HAVE_SETXATTR)
 I wonder if there are any platforms that lack one of listxattr,
 getxattr and setxattr, but at the same time have one of these calls.
 The xattr code (in general) is incredibly complex because of all these
 tests.
 
 I guess Mac OS X probably has none of them, and RHEL 5 / Ubuntu 10.10
 probably had only some of them.

At least in the gnu/stubs-$bits.h of RHEL 5 they are not declared as 
__stub_*, so they should be implemented (and being Linux, it means that 
most probably there are the right syscalls for them).
So I would not think the change proposed below (and implemented) would 
cause regressions in such old Linux systems.

 We don't care about RHEL 5 since it
 now has its own branch (oldlinux), so the code might be made simpler
 by an accompanying patch which reduces all of the HAVE_*XATTR* macros
 down to a single one (HAVE_LINUX_XATTRS).

Sounds reasonable; attached a patch for it.

-- 
Pino ToscanoFrom fc4dd8cab10b1c3571c49c0ed8bd3de29e98bd6c Mon Sep 17 00:00:00 2001
From: Pino Toscano ptosc...@redhat.com
Date: Thu, 9 Jan 2014 17:21:31 +0100
Subject: [PATCH] daemon: xattr: simplify the enabling of the linuxxattrs
 features

Instead of enable them when having one of the two headers for it but
still checking for the HAVE_* availability of each *xattr() function
used, just enable the linuxxattrs as a whole when having any of the
needed headers (like before) and all the needed functions.

This might cause the linuxxattrs to not be available anymore on systems
without the whole set of *xattr() functions implemented, but OTOH it
simplifies the xattr.c implementations.
---
 daemon/xattr.c | 49 +++--
 1 file changed, 11 insertions(+), 38 deletions(-)

diff --git a/daemon/xattr.c b/daemon/xattr.c
index af8bfd4..b84cf3d 100644
--- a/daemon/xattr.c
+++ b/daemon/xattr.c
@@ -27,7 +27,15 @@
 #include actions.h
 #include optgroups.h
 
-#if defined(HAVE_ATTR_XATTR_H) || defined(HAVE_SYS_XATTR_H)
+#if (defined(HAVE_ATTR_XATTR_H) || defined(HAVE_SYS_XATTR_H))  \
+defined(HAVE_LISTXATTR)  defined(HAVE_LLISTXATTR)  \
+defined(HAVE_GETXATTR)  defined(HAVE_LGETXATTR)  \
+defined(HAVE_REMOVEXATTR)  defined(HAVE_LREMOVEXATTR)  \
+defined(HAVE_SETXATTR)  defined(HAVE_LSETXATTR)
+# define HAVE_LINUX_XATTRS
+#endif
+
+#ifdef HAVE_LINUX_XATTRS
 
 # ifdef HAVE_ATTR_XATTR_H
 #  include attr/xattr.h
@@ -50,67 +58,37 @@ static int _removexattr (const char *xattr, const char *path, int (*removexattr)
 guestfs_int_xattr_list *
 do_getxattrs (const char *path)
 {
-#if defined(HAVE_LISTXATTR)  defined(HAVE_GETXATTR)
   return getxattrs (path, listxattr, getxattr);
-#else
-  reply_with_error (no support for listxattr and getxattr);
-  return NULL;
-#endif
 }
 
 guestfs_int_xattr_list *
 do_lgetxattrs (const char *path)
 {
-#if defined(HAVE_LLISTXATTR)  defined(HAVE_LGETXATTR)
   return getxattrs (path, llistxattr, lgetxattr);
-#else
-  reply_with_error (no support for llistxattr and lgetxattr);
-  return NULL;
-#endif
 }
 
 int
 do_setxattr (const char *xattr, const char *val, int vallen, const char *path)
 {
-#if defined(HAVE_SETXATTR)
   return _setxattr (xattr, val, vallen, path, setxattr);
-#else
-  reply_with_error (no support for setxattr);
-  return -1;
-#endif
 }
 
 int
 do_lsetxattr (const char *xattr, const char *val, int vallen, const char *path)
 {
-#if defined(HAVE_LSETXATTR)
   return _setxattr (xattr, val, vallen, path, lsetxattr);
-#else
-  reply_with_error (no support for lsetxattr);
-  return -1;
-#endif
 }
 
 int
 do_removexattr (const char *xattr, const char *path)
 {
-#if defined(HAVE_REMOVEXATTR)
   return _removexattr (xattr, path, removexattr);
-#else
-  reply_with_error (no support for removexattr);
-  return -1;
-#endif
 }
 
 int
 do_lremovexattr (const char *xattr, const char *path)
 {
-#if defined(HAVE_LREMOVEXATTR)
   return _removexattr (xattr, path, lremovexattr);
-#else
-  reply_with_error (no support for lremovexattr);
-  return -1;
-#endif
 }
 
 static int
@@ -277,7 +255,6 @@ _removexattr (const char *xattr, const char *path,
 guestfs_int_xattr_list *
 do_internal_lxattrlist (const char *path, char *const *names)
 {
-#if defined(HAVE_LLISTXATTR)  defined(HAVE_LGETXATTR)
   guestfs_int_xattr_list *ret = NULL;
   size_t i, j;
   size_t k, m, nr_attrs;
@@ -443,10 +420,6 @@ do_internal_lxattrlist (const char *path, char *const *names)
 free (ret);
   }
   return NULL;
-#else
-  reply_with_error (no support

Re: [Libguestfs] RFC: copy-attributes command

2014-01-10 Thread Pino Toscano

On Tuesday 07 January 2014 21:04:36 Richard W.M. Jones wrote:
 On Tue, Jan 07, 2014 at 04:06:43PM +0100, Pino Toscano wrote:
  Hi,
  
  attached there is a prototype of patch for adding a new
  copy-attributes command. Such command would allow copy the
  attributes of a file to 
  another, so for example in guestfish:
copy-attributes foo bar permissions:true xattributes:false
  
  would only copy the permissions of foo to bar, not copying its
  extended attributes too.
 
 I think the general idea of the new API is fine.
 
 More comments about the code below.
 
  Just few notes:
  - my first daemon command, so possibly I could be missing something
  - copy_xattrs is in xattr.c to avoid spreading the usage of xattr
  API in 
many places
  
  - copy_xattrs does a bit of code repetition with other stuff in
  xattr.c, 
but I'm not sure how to avoid it without making the xattr listing
code (getxattrs) a bit more complex that what it is already
  
  Comments?
  
  
  diff --git a/daemon/daemon.h b/daemon/daemon.h
  index b77d764..6535658 100644
  --- a/daemon/daemon.h
  +++ b/daemon/daemon.h
  @@ -231,6 +231,9 @@ extern void journal_finalize (void);
  
   /*-- in proto.c --*/
   extern void main_loop (int sock) __attribute__((noreturn));
  
  +/*-- in xattr.c --*/
  +extern int copy_xattrs (const char *src, const char *dest);
  +
  
   /* ordinary daemon functions use these to indicate errors
   
* NB: you don't need to prefix the string with the current
command,
* it is added automatically by the client-side RPC stubs.
  
  diff --git a/daemon/file.c b/daemon/file.c
  index f348f87..fd6da71 100644
  --- a/daemon/file.c
  +++ b/daemon/file.c
  @@ -28,6 +28,7 @@
  
   #include guestfs_protocol.h
   #include daemon.h
   #include actions.h
  
  +#include optgroups.h
  
   GUESTFSD_EXT_CMD(str_file, file);
   GUESTFSD_EXT_CMD(str_zcat, zcat);
  
  @@ -584,3 +585,46 @@ do_filesize (const char *path)
  
 return buf.st_size;
   
   }
  
  +
  +int
  +do_copy_attributes (const char *src, const char *dest, int
  permissions, int xattributes) +{
  +  int r;
  +  struct stat srcstat, deststat;
  +
  +  CHROOT_IN;
  +  r = stat (src, srcstat);
  +  CHROOT_OUT;
  +
  +  if (r == -1) {
  +reply_with_perror (stat: %s, src);
  +return -1;
  +  }
  +
  +  CHROOT_IN;
  +  r = stat (dest, deststat);
  +  CHROOT_OUT;
  +
  +  if (r == -1) {
  +reply_with_perror (stat: %s, dest);
  +return -1;
  +  }
  +
  +  if (permissions  ((srcstat.st_mode  0777) != (deststat.st_mode
   0777))) { +CHROOT_IN;
  +r = chmod (dest, (srcstat.st_mode  0777));
 
 I suspect you want 0 in order to copy sticky/setuid/setgid bits.
 Perhaps those should be a separate flag, but we definitely want to
 copy them!

Right, fixed to be part of the permissions (or mode actually, see 
below).

  +  ssize_t len, vlen, ret;
  +  CLEANUP_FREE char *buf = NULL, *attrval = NULL;
  +  size_t i, attrval_len = 0;
  +
  +  CHROOT_IN;
  +  len = listxattr (src, NULL, 0);
  +  CHROOT_OUT;
  +  if (len == -1) {
  +reply_with_perror (listxattr: %s, src);
  +goto error;
  +  }
  +
  +  buf = malloc (len);
  +  if (buf == NULL) {
  +reply_with_perror (malloc);
  +goto error;
  +  }
  +
  +  CHROOT_IN;
  +  len = listxattr (src, buf, len);
  +  CHROOT_OUT;
  +  if (len == -1) {
  +reply_with_perror (listxattr: %s, src);
  +goto error;
  +  }

This two-pass snippet to do (l)listxattr is already elsewhere in 
xattr.c, I will move it to an own function.

  +  /* What we get from the kernel is a string foo\0bar\0baz of
  length +   * len.
  +   */
  +  for (i = 0; i  (size_t) len; i += strlen (buf[i]) + 1) {
  +CHROOT_IN;
  +vlen = getxattr (src, buf[i], NULL, 0);
  +CHROOT_OUT;
  +if (vlen == -1) {
  +  reply_with_perror (getxattr: %s, %s, src, buf[i]);
  +  goto error;
  +}
  +
  +if (vlen  XATTR_SIZE_MAX) {
  +  /* The next call to getxattr will fail anyway, so ... */
  +  reply_with_error (extended attribute is too large);
  +  goto error;
  +}
  +
  +if (vlen  attrval_len) {
  +  char *new = realloc (attrval, vlen);
  +  if (new == NULL) {
  +reply_with_perror (realloc);
  +goto error;
  +  }
  +  attrval = new;
  +  attrval_len = vlen;
  +}
  +
  +CHROOT_IN;
  +vlen = getxattr (src, buf[i], attrval, vlen);
  +CHROOT_OUT;
  +if (vlen == -1) {
  +  reply_with_perror (getxattr: %s, %s, src, buf[i]);
  +  goto error;
  +}
  +
  +CHROOT_IN;
  +ret = setxattr (dest, buf[i], attrval, vlen, 0);
  +CHROOT_OUT;
  +if (vlen == -1) {
  +  reply_with_perror (setxattr: %s, %s, dest, buf[i]);
  +  goto error;
  +}
  +  }
 
 This code looks as if it will copy the xattrs, but it won't
 remove any which don't exist in the source.  eg:
 
   source xattrs before:
 user.foo = 1
   dest xattrs before:
 user.bar = 2
   dest xattrs after:
 user.foo = 1

[Libguestfs] [PATCH] daemon: xattr: move the listxattrs code in an own function

2014-01-10 Thread Pino Toscano

Move in an own function the code that does the (l)listxattrs allocating
the buffer of the right legth, as it will be useful later.

No functional changes, just code motion.
---
 daemon/xattr.c | 64 --
 1 file changed, 44 insertions(+), 20 deletions(-)

diff --git a/daemon/xattr.c b/daemon/xattr.c
index b84cf3d..e01e9e2 100644
--- a/daemon/xattr.c
+++ b/daemon/xattr.c
@@ -54,6 +54,7 @@ optgroup_linuxxattrs_available (void)
 static guestfs_int_xattr_list *getxattrs (const char *path, ssize_t 
(*listxattr) (const char *path, char *list, size_t size), ssize_t (*getxattr) 
(const char *path, const char *name, void *value, size_t size));
 static int _setxattr (const char *xattr, const char *val, int vallen, const 
char *path, int (*setxattr) (const char *path, const char *name, const void 
*value, size_t size, int flags));
 static int _removexattr (const char *xattr, const char *path, int 
(*removexattr) (const char *path, const char *name));
+static char *_listxattrs (const char *path, ssize_t (*listxattr) (const char 
*path, char *list, size_t size), ssize_t *size);
 
 guestfs_int_xattr_list *
 do_getxattrs (const char *path)
@@ -111,27 +112,10 @@ getxattrs (const char *path,
   size_t i, j;
   guestfs_int_xattr_list *r = NULL;
 
-  CHROOT_IN;
-  len = listxattr (path, NULL, 0);
-  CHROOT_OUT;
-  if (len == -1) {
-reply_with_perror (listxattr: %s, path);
+  buf = _listxattrs (path, listxattr, len);
+  if (buf == NULL)
+/* _listxattrs issues reply_with_perror already. */
 goto error;
-  }
-
-  buf = malloc (len);
-  if (buf == NULL) {
-reply_with_perror (malloc);
-goto error;
-  }
-
-  CHROOT_IN;
-  len = listxattr (path, buf, len);
-  CHROOT_OUT;
-  if (len == -1) {
-reply_with_perror (listxattr: %s, path);
-goto error;
-  }
 
   r = calloc (1, sizeof (*r));
   if (r == NULL) {
@@ -252,6 +236,46 @@ _removexattr (const char *xattr, const char *path,
   return 0;
 }
 
+static char *
+_listxattrs (const char *path,
+ ssize_t (*listxattr) (const char *path, char *list, size_t size),
+ ssize_t *size)
+{
+  int r;
+  char *buf = NULL;
+  ssize_t len;
+
+  CHROOT_IN;
+  len = listxattr (path, NULL, 0);
+  CHROOT_OUT;
+  if (len == -1) {
+reply_with_perror (listxattr: %s, path);
+goto error;
+  }
+
+  buf = malloc (len);
+  if (buf == NULL) {
+reply_with_perror (malloc);
+goto error;
+  }
+
+  CHROOT_IN;
+  len = listxattr (path, buf, len);
+  CHROOT_OUT;
+  if (len == -1) {
+reply_with_perror (listxattr: %s, path);
+goto error;
+  }
+
+  if (size)
+*size = len;
+  return buf;
+
+ error:
+  free (buf);
+  return NULL;
+}
+
 guestfs_int_xattr_list *
 do_internal_lxattrlist (const char *path, char *const *names)
 {
-- 
1.8.3.1

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

Re: [Libguestfs] RFC: copy-attributes command

2014-01-10 Thread Pino Toscano

On Friday 10 January 2014 13:33:38 Richard W.M. Jones wrote:
 On Fri, Jan 10, 2014 at 02:17:48PM +0100, Pino Toscano wrote:
   This code looks as if it will copy the xattrs, but it won't
   
   remove any which don't exist in the source.  eg:
 source xattrs before:
   user.foo = 1
 
 dest xattrs before:
   user.bar = 2
 
 dest xattrs after:
   user.foo = 1
   user.bar = 2
   
   That may or may not be what we want, but I would say it's a bit
   unexpected.
  
  Yes, the current behaviour is done on purpose; my thought that,
  given
  the command is copy attributes, it would just copy what specified
  from the source to the destination.
  
  I see reasons for both the behaviours, so I'm not totally sure which
  one pick.
 
 On the basis that most users will be creating a new file (which will
 have no xattrs except in some very odd corner cases), just leave your
 implementation for now, but don't specify it in the documentation so
 we could change it later.

After all, the documentation says that it just copies the xattrs from 
the source to the destination, but it does not explicitly mention what 
is done with the xattrs in the destination not in the source ... :-)

On Friday 10 January 2014 13:36:06 Richard W.M. Jones wrote:
 On Fri, Jan 10, 2014 at 01:33:38PM +, Richard W.M. Jones wrote:
  The API is now pretty confusing.  Each OBool flag is really a
  tristate.  It can either be true, false or not specified.

I see, I did not pay much attention to the use of optargs_bitmask 
outside the auto-generated RPC stuff the daemon code.

  Therefore I think it should be:
xattributes:true# copies only xattrs and nothing else
all:true# copies everything
all:true xattributes:false  # copies everything except xattrs
  
  In other words, 'all' changes the default (ie. not specified)
  state
  of the other flags.

Given the above, this indeed becomes straightforward to have.

  To be clearer, the four OBool parameters would be:
[OBool all; OBool mode; OBool ownership; OBool
xattributes]
 
 So looking at your v2 patch a bit more, I think this is what you
 already did, except that skipmode is backwards from the other flags.
 I think we're in agreement, except I think skipmode should just
 become mode and not have negated meaning compared to the other
 flags.

OK, I will turn it back as it was before (into mode), but making use 
of the tristate information to have it true by default.

 We're allowed to extend the API later by adding optional flags (see
 Note about extending functions in generator/README for the
 complicated rules about extending APIs while preserving binary
 compatibility).

Yes, I read it earlier, that's why I'm not that concerned about adding 
all the potential attributes now.

Attached there is the v3 of the patch.

-- 
Pino Toscanodiff --git a/daemon/daemon.h b/daemon/daemon.h
index b77d764..6535658 100644
--- a/daemon/daemon.h
+++ b/daemon/daemon.h
@@ -231,6 +231,9 @@ extern void journal_finalize (void);
 /*-- in proto.c --*/
 extern void main_loop (int sock) __attribute__((noreturn));
 
+/*-- in xattr.c --*/
+extern int copy_xattrs (const char *src, const char *dest);
+
 /* ordinary daemon functions use these to indicate errors
  * NB: you don't need to prefix the string with the current command,
  * it is added automatically by the client-side RPC stubs.
diff --git a/daemon/file.c b/daemon/file.c
index f348f87..34ec95a 100644
--- a/daemon/file.c
+++ b/daemon/file.c
@@ -28,6 +28,7 @@
 #include guestfs_protocol.h
 #include daemon.h
 #include actions.h
+#include optgroups.h
 
 GUESTFSD_EXT_CMD(str_file, file);
 GUESTFSD_EXT_CMD(str_zcat, zcat);
@@ -584,3 +585,79 @@ do_filesize (const char *path)
 
   return buf.st_size;
 }
+
+int
+do_copy_attributes (const char *src, const char *dest, int all, int mode, int xattributes, int ownership)
+{
+  int r;
+  struct stat srcstat, deststat;
+
+  static const unsigned int file_mask = 0;
+
+  /* If it was specified to copy everything, manually enable all the flags
+   * not manually specified to avoid checking for flag || all everytime.
+   */
+  if (all) {
+if (!(optargs_bitmask  GUESTFS_COPY_ATTRIBUTES_MODE_BITMASK))
+  mode = 1;
+if (!(optargs_bitmask  GUESTFS_COPY_ATTRIBUTES_XATTRIBUTES_BITMASK))
+  xattributes = 1;
+if (!(optargs_bitmask  GUESTFS_COPY_ATTRIBUTES_OWNERSHIP_BITMASK))
+  ownership = 1;
+  } else if ((!(optargs_bitmask  GUESTFS_COPY_ATTRIBUTES_ALL_BITMASK)) 
+ (!(optargs_bitmask  GUESTFS_COPY_ATTRIBUTES_MODE_BITMASK))) {
+/* Neither all nor mode were specified, so make mode on by default.
+ */
+mode = 1;
+  }
+
+  CHROOT_IN;
+  r = stat (src, srcstat);
+  CHROOT_OUT;
+
+  if (r == -1) {
+reply_with_perror (stat: %s, src);
+return -1;
+  }
+
+  CHROOT_IN;
+  r = stat (dest, deststat);
+  CHROOT_OUT;
+
+  if (r == -1) {
+reply_with_perror (stat: %s, dest);
+return -1;
+  }
+
+  if (mode

Re: [Libguestfs] RFC: copy-attributes command

2014-01-13 Thread Pino Toscano

On Friday 10 January 2014 16:53:32 Richard W.M. Jones wrote:
 On Fri, Jan 10, 2014 at 05:36:39PM +0100, Pino Toscano wrote:
 [..]
 
 This still isn't quite what I meant.  My meaning was that mode
 would be disabled by default (unless all:true).

OK.

 How about:
 
 int copy_mode, copy_xattributes, copy_ownership;
 
 /* Set defaults. */
 if (all)
   copy_mode = copy_xattributes = copy_ownership = 1;
 else
   copy_mode = copy_xattributes = copy_ownership = 0;
 
 /* If set in the original struct, copy those settings overriding
  * the defaults.
  */
 if ((optargs_bitmask  GUESTFS_COPY_ATTRIBUTES_MODE_BITMASK))
   copy_mode = mode;
 if ((optargs_bitmask  GUESTFS_COPY_ATTRIBUTES_XATTRIBUTES_BITMASK))
   copy_xattributes = xattributes;
 if ((optargs_bitmask  GUESTFS_COPY_ATTRIBUTES_OWNERSHIP_BITMASK))
   copy_ownership = ownership;

Isn't this doing basically the same of the snippet I used (the «if (all) 
{ ... }» one), short of the part that enables mode if neither all nor 
mode were specified?

-- 
Pino Toscano

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

Re: [Libguestfs] [Bug 1046905] New: RFE: add argument to virt-sysprep to disable individual default operations

2014-01-13 Thread Pino Toscano

On Friday 10 January 2014 10:09:19 Richard W.M. Jones wrote:
 On Thu, Jan 09, 2014 at 03:45:54PM +, Richard W.M. Jones wrote:
  On Thu, Jan 09, 2014 at 04:21:10PM +0100, Pino Toscano wrote:
   +  and set_operations op_string =
   +let currentopset =
   +  match (!operations) with
  
  No need for parentheses around !operations.

Fixed.

   +let n = ref op_name in
   +let remove = string_prefix op_name - in
   +if remove then
   +  n := String.sub op_name 1 (String.length op_name - 1);
   +match !n with
  
  This can be written a bit more naturally as ...
  
let n, remove =
  if string_prefix op_name - then
String.sub op_name 1 (String.length op_name-1), true
  else
op_name, false in
match n with
...

This is nice even without the way below.

 An even more natural way to write this is:

Maybe for skilled OCaml programmers ;) not (yet) for me.

   let op =
 if string_prefix op_name - then
   `Remove (String.sub op_name 1 (String.length op_name-1))
 else
   `Add op_name
match op with
 
| `Add  | `Remove  - (* error *)
| `Add defaults - Sysprep_operation.add_defaults_to_set opset
| `Remove defaults - Sysprep_operation.remove_defaults_from_set
| opset etc
 
 The type of op will be [ `Add of string | `Remove of string ].
 Actually you never need to write that type out, as OCaml will infer it
 (and check it).  It will only appear in error messages if you get the
 code wrong.

Interesting way, made use of it, thanks.

  +  let f = if remove then
  Sysprep_operation.remove_defaults_from_set else
  Sysprep_operation.add_defaults_to_set in
  +  f opset
 
 You can actually write:
 
   (if remove then Sysprep_operation.remove_defaults_from_set
else Sysprep_operation.add_defaults_to_set) opset
 
 I don't know which way you think is clearer, but I would avoid the 80
 character lines.

Yes, I knew about that, just thought it was clearer to split the actual 
call on an own line, making the argument a bit less hidden by the 
whole instruction.

  +--operations, Arg.String set_operations,   ^
  s_Enable/disable specific operations;
 
 I'd also add an alias --operation (it would just do the same thing).

Added.

  +let add_list_to_opset l s =
  +  List.fold_left (
  +fun acc elem -
  +  OperationSet.add elem acc
  +  ) s l
  +
  +let remove_list_from_opset l set =
  +  OperationSet.fold (
  +fun elem opset -
  +  if List.exists (
  +fun li -
  +  li.name = elem.name
  +  ) l then
  +opset
  +  else
  +OperationSet.add elem opset
  +  ) set empty_set
 
 OCaml Set has subset, intersection, difference operations if you need
 them.  See /usr/lib64/ocaml/set.mli

True; I went with manual folding to avoid build new temporary sets when 
needed, but I guess doing this could be more natural.

-- 
Pino Toscano

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

[Libguestfs] [PATCH] sysprep: add --operations

2014-01-13 Thread Pino Toscano

Add a new --operation parameter which, similarly to --enable, can be
used to enable operations, but also to remove them, and to add/remove
the default operations and all the available ones.
---
 sysprep/main.ml   | 36 +++
 sysprep/sysprep_operation.ml  | 24 ++
 sysprep/sysprep_operation.mli | 21 
 sysprep/virt-sysprep.pod  | 57 +--
 4 files changed, 131 insertions(+), 7 deletions(-)

diff --git a/sysprep/main.ml b/sysprep/main.ml
index 689a394..49750a9 100644
--- a/sysprep/main.ml
+++ b/sysprep/main.ml
@@ -87,6 +87,40 @@ let debug_gc, operations, g, selinux_relabel, quiet, 
mount_opts =
   exit 1
 ) Sysprep_operation.empty_set ops in
 operations := Some opset
+  and set_operations op_string =
+let currentopset =
+  match !operations with
+  | Some x - x
+  | None - Sysprep_operation.empty_set
+in
+let ops = string_nsplit , op_string in
+let opset = List.fold_left (
+  fun opset op_name -
+let op =
+  if string_prefix op_name - then
+`Remove (String.sub op_name 1 (String.length op_name - 1))
+  else
+`Add op_name in
+match op with
+| `Add  | `Remove  -
+  eprintf (f_%s: --operations: empty operation name\n)
+prog;
+  exit 1
+| `Add defaults - Sysprep_operation.add_defaults_to_set opset
+| `Remove defaults - Sysprep_operation.remove_defaults_from_set 
opset
+| `Add all - Sysprep_operation.add_all_to_set opset
+| `Remove all - Sysprep_operation.remove_all_from_set opset
+| `Add n | `Remove n -
+  let f = match op with
+  | `Add n - Sysprep_operation.add_to_set
+  | `Remove n - Sysprep_operation.remove_from_set in
+  try f n opset with
+  | Not_found -
+eprintf (f_%s: --operations: '%s' is not a known operation\n)
+  prog n;
+exit 1
+) currentopset ops in
+operations := Some opset
   and force_selinux_relabel () =
 selinux_relabel := `Force
   and no_force_selinux_relabel () =
@@ -114,6 +148,8 @@ let debug_gc, operations, g, selinux_relabel, quiet, 
mount_opts =
 --list-operations, Arg.Unit list_operations,   ^ s_List supported 
operations;
 --long-options, Arg.Unit display_long_options,   ^ s_List long 
options;
 --mount-options, Arg.Set_string mount_opts, s_opts ^   ^ s_Set 
mount options (eg /:noatime;/var:rw,noatime);
+--operation,  Arg.String set_operations,   ^ s_Enable/disable 
specific operations;
+--operations, Arg.String set_operations,   ^ s_Enable/disable 
specific operations;
 -q,Arg.Set quiet,   ^ s_Don't print log messages;
 --quiet,   Arg.Set quiet,   ^ s_Don't print log messages;
 --selinux-relabel, Arg.Unit force_selinux_relabel,   ^ s_Force 
SELinux relabel;
diff --git a/sysprep/sysprep_operation.ml b/sysprep/sysprep_operation.ml
index 1fb4f17..572a65f 100644
--- a/sysprep/sysprep_operation.ml
+++ b/sysprep/sysprep_operation.ml
@@ -68,10 +68,34 @@ type set = OperationSet.t
 
 let empty_set = OperationSet.empty
 
+let opset_of_oplist li =
+  List.fold_left (
+fun acc elem -
+  OperationSet.add elem acc
+  ) empty_set li
+
 let add_to_set name set =
   let op = List.find (fun { name = n } - name = n) !all_operations in
   OperationSet.add op set
 
+let add_defaults_to_set set =
+  OperationSet.union set (opset_of_oplist !enabled_by_default_operations)
+
+let add_all_to_set set =
+  opset_of_oplist !all_operations
+
+let remove_from_set name set =
+  let name_filter = fun { name = n } - name = n in
+  if List.exists name_filter !all_operations  true then
+raise Not_found;
+  OperationSet.diff set (OperationSet.filter name_filter set)
+
+let remove_defaults_from_set set =
+  OperationSet.diff set (opset_of_oplist !enabled_by_default_operations)
+
+let remove_all_from_set set =
+  empty_set
+
 let register_operation op =
   all_operations := op :: !all_operations;
   if op.enabled_by_default then
diff --git a/sysprep/sysprep_operation.mli b/sysprep/sysprep_operation.mli
index 16eee64..61dde72 100644
--- a/sysprep/sysprep_operation.mli
+++ b/sysprep/sysprep_operation.mli
@@ -128,6 +128,27 @@ val add_to_set : string - set - set
 Note that this will raise [Not_found] if [name] is not
 a valid operation name. *)
 
+val add_defaults_to_set : set - set
+(** [add_defaults_to_set set] adds to [set] all the operations enabled
+by default. *)
+
+val add_all_to_set : set - set
+(** [add_all_to_set set] adds to [set] all the available operations. *)
+
+val remove_from_set : string - set - set
+(** [remove_from_set name set] remove the operation named [name] from [set].
+
+Note that this will raise [Not_found] if [name] is not
+a valid operation name. *)
+
+val remove_defaults_from_set : set - set
+(** [remove_defaults_from_set set] removes

[Libguestfs] [PATCH] New API: copy-attributes.

2014-01-13 Thread Pino Toscano

This allows one to copy attributes (like permissions, xattrs,
ownership) from a file to another.
---
 daemon/daemon.h |   3 +
 daemon/file.c   |  72 ++
 daemon/xattr.c  |  69 +
 fish/Makefile.am|   1 +
 fish/test-file-attrs.sh | 157 
 generator/actions.ml|  38 
 src/MAX_PROC_NR |   2 +-
 7 files changed, 341 insertions(+), 1 deletion(-)
 create mode 100755 fish/test-file-attrs.sh

diff --git a/daemon/daemon.h b/daemon/daemon.h
index b77d764..6535658 100644
--- a/daemon/daemon.h
+++ b/daemon/daemon.h
@@ -231,6 +231,9 @@ extern void journal_finalize (void);
 /*-- in proto.c --*/
 extern void main_loop (int sock) __attribute__((noreturn));
 
+/*-- in xattr.c --*/
+extern int copy_xattrs (const char *src, const char *dest);
+
 /* ordinary daemon functions use these to indicate errors
  * NB: you don't need to prefix the string with the current command,
  * it is added automatically by the client-side RPC stubs.
diff --git a/daemon/file.c b/daemon/file.c
index f348f87..856ab5f 100644
--- a/daemon/file.c
+++ b/daemon/file.c
@@ -28,6 +28,7 @@
 #include guestfs_protocol.h
 #include daemon.h
 #include actions.h
+#include optgroups.h
 
 GUESTFSD_EXT_CMD(str_file, file);
 GUESTFSD_EXT_CMD(str_zcat, zcat);
@@ -584,3 +585,74 @@ do_filesize (const char *path)
 
   return buf.st_size;
 }
+
+int
+do_copy_attributes (const char *src, const char *dest, int all, int mode, int 
xattributes, int ownership)
+{
+  int r;
+  struct stat srcstat, deststat;
+
+  static const unsigned int file_mask = 0;
+
+  /* If it was specified to copy everything, manually enable all the flags
+   * not manually specified to avoid checking for flag || all everytime.
+   */
+  if (all) {
+if (!(optargs_bitmask  GUESTFS_COPY_ATTRIBUTES_MODE_BITMASK))
+  mode = 1;
+if (!(optargs_bitmask  GUESTFS_COPY_ATTRIBUTES_XATTRIBUTES_BITMASK))
+  xattributes = 1;
+if (!(optargs_bitmask  GUESTFS_COPY_ATTRIBUTES_OWNERSHIP_BITMASK))
+  ownership = 1;
+  }
+
+  CHROOT_IN;
+  r = stat (src, srcstat);
+  CHROOT_OUT;
+
+  if (r == -1) {
+reply_with_perror (stat: %s, src);
+return -1;
+  }
+
+  CHROOT_IN;
+  r = stat (dest, deststat);
+  CHROOT_OUT;
+
+  if (r == -1) {
+reply_with_perror (stat: %s, dest);
+return -1;
+  }
+
+  if (mode 
+  ((srcstat.st_mode  file_mask) != (deststat.st_mode  file_mask))) {
+CHROOT_IN;
+r = chmod (dest, (srcstat.st_mode  file_mask));
+CHROOT_OUT;
+
+if (r == -1) {
+  reply_with_perror (chmod: %s, dest);
+  return -1;
+}
+  }
+
+  if (ownership 
+  (srcstat.st_uid != deststat.st_uid || srcstat.st_gid != 
deststat.st_gid)) {
+CHROOT_IN;
+r = chown (dest, srcstat.st_uid, srcstat.st_gid);
+CHROOT_OUT;
+
+if (r == -1) {
+  reply_with_perror (chown: %s, dest);
+  return -1;
+}
+  }
+
+  if (xattributes  optgroup_linuxxattrs_available ()) {
+if (!copy_xattrs (src, dest))
+  /* copy_xattrs replies with an error already. */
+  return -1;
+  }
+
+  return 0;
+}
diff --git a/daemon/xattr.c b/daemon/xattr.c
index ebacc02..abed5ff 100644
--- a/daemon/xattr.c
+++ b/daemon/xattr.c
@@ -541,8 +541,77 @@ do_lgetxattr (const char *path, const char *name, size_t 
*size_r)
   return buf; /* caller frees */
 }
 
+int
+copy_xattrs (const char *src, const char *dest)
+{
+  ssize_t len, vlen, ret, attrval_len = 0;
+  CLEANUP_FREE char *buf = NULL, *attrval = NULL;
+  size_t i;
+
+  buf = _listxattrs (src, listxattr, len);
+  if (buf == NULL)
+/* _listxattrs issues reply_with_perror already. */
+goto error;
+
+  /* What we get from the kernel is a string foo\0bar\0baz of length
+   * len.
+   */
+  for (i = 0; i  (size_t) len; i += strlen (buf[i]) + 1) {
+CHROOT_IN;
+vlen = getxattr (src, buf[i], NULL, 0);
+CHROOT_OUT;
+if (vlen == -1) {
+  reply_with_perror (getxattr: %s, %s, src, buf[i]);
+  goto error;
+}
+
+if (vlen  XATTR_SIZE_MAX) {
+  /* The next call to getxattr will fail anyway, so ... */
+  reply_with_error (extended attribute is too large);
+  goto error;
+}
+
+if (vlen  attrval_len) {
+  char *new = realloc (attrval, vlen);
+  if (new == NULL) {
+reply_with_perror (realloc);
+goto error;
+  }
+  attrval = new;
+  attrval_len = vlen;
+}
+
+CHROOT_IN;
+vlen = getxattr (src, buf[i], attrval, vlen);
+CHROOT_OUT;
+if (vlen == -1) {
+  reply_with_perror (getxattr: %s, %s, src, buf[i]);
+  goto error;
+}
+
+CHROOT_IN;
+ret = setxattr (dest, buf[i], attrval, vlen, 0);
+CHROOT_OUT;
+if (ret == -1) {
+  reply_with_perror (setxattr: %s, %s, dest, buf[i]);
+  goto error;
+}
+  }
+
+  return 1;
+
+ error:
+  return 0;
+}
+
 #else /* no HAVE_LINUX_XATTRS */
 
 OPTGROUP_LINUXXATTRS_NOT_AVAILABLE
 
+int
+copy_xattrs (const char *src, const char *dest)

[Libguestfs] [PATCH] builder, edit, fish: use copy-attributes

2014-01-14 Thread Pino Toscano

Make use of the new copy-attributes command to properly copy all file
attributes from a file to the new version of it.
---
 builder/perl_edit.ml | 29 +---
 edit/edit.c  | 49 ++-
 fish/edit.c  | 54 ++--
 3 files changed, 5 insertions(+), 127 deletions(-)

diff --git a/builder/perl_edit.ml b/builder/perl_edit.ml
index aa4c2e6..28e5dea 100644
--- a/builder/perl_edit.ml
+++ b/builder/perl_edit.ml
@@ -42,7 +42,7 @@ let rec edit_file ~debug (g : Guestfs.guestfs) file expr =
   g#upload tmpfile file;
 
   (* However like virt-edit we do need to copy attributes. *)
-  copy_attributes g file_old file;
+  g#copy_attributes ~all:true file_old file;
   g#rm file_old
 
 and do_perl_edit ~debug g file expr =
@@ -76,30 +76,3 @@ and do_perl_edit ~debug g file expr =
   );
 
   Unix.rename (file ^ .out) file
-
-and copy_attributes g src dest =
-  let has_linuxxattrs = g#feature_available [|linuxxattrs|] in
-
-  (* Get the mode. *)
-  let stat = g#stat src in
-
-  (* Get the SELinux context.  XXX Should we copy over other extended
-   * attributes too?
-   *)
-  let selinux_context =
-if has_linuxxattrs then (
-  try Some (g#getxattr src security.selinux) with _ - None
-) else None in
-
-  (* Set the permissions (inc. sticky and set*id bits), UID, GID. *)
-  let mode = Int64.to_int stat.G.mode
-  and uid = Int64.to_int stat.G.uid and gid = Int64.to_int stat.G.gid in
-  g#chmod (mode land 0o) dest;
-  g#chown uid gid dest;
-
-  (* Set the SELinux context. *)
-  match selinux_context with
-  | None - ()
-  | Some selinux_context -
-g#setxattr security.selinux
-  selinux_context (String.length selinux_context) dest
diff --git a/edit/edit.c b/edit/edit.c
index e5e3a29..07790be 100644
--- a/edit/edit.c
+++ b/edit/edit.c
@@ -56,7 +56,6 @@ static void edit_files (int argc, char *argv[]);
 static void edit (const char *filename, const char *root);
 static char *edit_interactively (const char *tmpfile);
 static char *edit_non_interactively (const char *tmpfile);
-static int copy_attributes (const char *src, const char *dest);
 static int is_windows (guestfs_h *g, const char *root);
 static char *windows_path (guestfs_h *g, const char *root, const char 
*filename);
 static char *generate_random_name (const char *filename);
@@ -361,7 +360,8 @@ edit (const char *filename, const char *root)
 /* Set the permissions, UID, GID and SELinux context of the new
  * file to match the old file (RHBZ#788641).
  */
-if (copy_attributes (filename, newname) == -1)
+if (guestfs_copy_attributes (g, filename, newname,
+GUESTFS_COPY_ATTRIBUTES_ALL, 1, -1) == -1)
   goto error;
 
 /* Backup or overwrite the file. */
@@ -510,51 +510,6 @@ edit_non_interactively (const char *tmpfile)
 }
 
 static int
-copy_attributes (const char *src, const char *dest)
-{
-  CLEANUP_FREE_STAT struct guestfs_stat *stat = NULL;
-  const char *linuxxattrs[] = { linuxxattrs, NULL };
-  int has_linuxxattrs;
-  CLEANUP_FREE char *selinux_context = NULL;
-  size_t selinux_context_size;
-
-  has_linuxxattrs = guestfs_feature_available (g, (char **) linuxxattrs);
-
-  /* Get the mode. */
-  stat = guestfs_stat (g, src);
-  if (stat == NULL)
-return -1;
-
-  /* Get the SELinux context.  XXX Should we copy over other extended
-   * attributes too?
-   */
-  if (has_linuxxattrs) {
-guestfs_push_error_handler (g, NULL, NULL);
-
-selinux_context = guestfs_getxattr (g, src, security.selinux,
-selinux_context_size);
-/* selinux_context could be NULL.  This isn't an error. */
-
-guestfs_pop_error_handler (g);
-  }
-
-  /* Set the permissions (inc. sticky and set*id bits), UID, GID. */
-  if (guestfs_chmod (g, stat-mode  0, dest) == -1)
-return -1;
-  if (guestfs_chown (g, stat-uid, stat-gid, dest) == -1)
-return -1;
-
-  /* Set the SELinux context. */
-  if (has_linuxxattrs  selinux_context) {
-if (guestfs_setxattr (g, security.selinux, selinux_context,
-  (int) selinux_context_size, dest) == -1)
-  return -1;
-  }
-
-  return 0;
-}
-
-static int
 is_windows (guestfs_h *g, const char *root)
 {
   int w;
diff --git a/fish/edit.c b/fish/edit.c
index 754a34a..bd02f4b 100644
--- a/fish/edit.c
+++ b/fish/edit.c
@@ -32,7 +32,6 @@
 #include fish.h
 
 static char *generate_random_name (const char *filename);
-static int copy_attributes (const char *src, const char *dest);
 
 /* guestfish edit command, suggested by Ján Ondrej, implemented by RWMJ */
 
@@ -135,7 +134,8 @@ run_edit (const char *cmd, size_t argc, char *argv[])
   /* Set the permissions, UID, GID and SELinux context of the new
* file to match the old file (RHBZ#788641).
*/
-  if (copy_attributes (remotefilename, newname) == -1)
+  if (guestfs_copy_attributes (g, remotefilename, newname,
+  GUESTFS_COPY_ATTRIBUTES_ALL, 1, -1) == -1)

[Libguestfs] [PATCH] builder: test-virt-builder: check some results

2014-01-14 Thread Pino Toscano

Check at least some basic modifications in the image created with
virt-builder.
---
 builder/test-virt-builder.sh | 47 +++-
 1 file changed, 46 insertions(+), 1 deletion(-)

diff --git a/builder/test-virt-builder.sh b/builder/test-virt-builder.sh
index 47d20a4..3c8eb60 100755
--- a/builder/test-virt-builder.sh
+++ b/builder/test-virt-builder.sh
@@ -65,6 +65,51 @@ $VG ./virt-builder phony-fedora \
 --firstboot Makefile --firstboot-command 'echo hello' \
 --firstboot-install minicom,inkscape
 
-# XXX Test that the modifications were made.
+# Check that some modifications were made.
+$VG ../fish/guestfish --ro -i -a $output  test.out EOF
+# Uploaded files
+is-file /etc/foo/bar/baz/Makefile
+cat /etc/foo/bar/baz/foo
+is-symlink /foo
+is-symlink /foo1
+is-symlink /foo2
+is-symlink /foo3
+
+echo -
+# Hostname
+cat /etc/sysconfig/network | grep HOSTNAME=
+
+echo -
+# Timezone
+is-file /usr/share/zoneinfo/Europe/London
+is-symlink /etc/localtime
+readlink /etc/localtime
+
+echo -
+# Password
+is-file /etc/shadow
+cat /etc/shadow | sed -r '/^root:/!d;s,^(root:\\\$6\\\$).*,\\1,g'
+EOF
+
+if [ $(cat test.out) != true
+Hello World
+true
+true
+true
+true
+-
+HOSTNAME=test.example.com
+-
+true
+true
+/usr/share/zoneinfo/Europe/London
+-
+true
+root:\$6\$ ]; then
+echo $0: unexpected output:
+cat test.out
+exit 1
+fi
 
 rm $output
+rm test.out
-- 
1.8.3.1

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

[Libguestfs] [PATCH] fuse: remove extra trailing \n in debug messages

2014-01-15 Thread Pino Toscano

debug() adds it already.
---
 src/fuse.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/fuse.c b/src/fuse.c
index d684c84..288c02a 100644
--- a/src/fuse.c
+++ b/src/fuse.c
@@ -68,7 +68,7 @@ gl_lock_define_initialized (static, mount_local_lock);
 #define DEBUG_CALL(fs,...)  \
   if (g-ml_debug_calls) {  \
 debug (g,   \
-   %s: %s ( fs )\n, \
+   %s: %s ( fs ),   \
g-localmountpoint, __func__, ## __VA_ARGS__);   \
   }
 
-- 
1.8.3.1

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

[Libguestfs] [PATCH] fuse: clear stat structs (RHBZ#660687).

2014-01-15 Thread Pino Toscano

Not all the fields of struct stat are actually filled by us. This caused
rubbish to appear in the microseconds fields, which were then used as
base when changing atime/ctime (with e.g. touch), triggering EINVAL by
futimens/utimensat when those rubbish values were out of the range
allowed for microseconds.
---
 src/fuse.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/fuse.c b/src/fuse.c
index 288c02a..dd4f139 100644
--- a/src/fuse.c
+++ b/src/fuse.c
@@ -175,6 +175,7 @@ mount_local_readdir (const char *path, void *buf, 
fuse_fill_dir_t filler,
 if (ss-val[i].ino = 0) {
   struct stat statbuf;
 
+  memset (statbuf, 0, sizeof statbuf);
   statbuf.st_dev = ss-val[i].dev;
   statbuf.st_ino = ss-val[i].ino;
   statbuf.st_mode = ss-val[i].mode;
@@ -255,6 +256,7 @@ mount_local_getattr (const char *path, struct stat *statbuf)
   if (r == NULL)
 RETURN_ERRNO;
 
+  memset (statbuf, 0, sizeof *statbuf);
   statbuf-st_dev = r-dev;
   statbuf-st_ino = r-ino;
   statbuf-st_mode = r-mode;
-- 
1.8.3.1

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

Re: [Libguestfs] [PATCH] fuse: provide a stub flush implementation (RHBZ#660687).

2014-01-15 Thread Pino Toscano

Hi,

On Thursday 12 December 2013 16:28:31 Pino Toscano wrote:
 It seems that FUSE can invoke flush to make sure the pending changes
 (e.g. to the attributes) of a file are set. Since a missing flush
 implementation is handled as if it were returning ENOSYS, this can
 cause issues later.
 
 To overcome this, just provide a stub implementation which does
 nothing, since we have nothing to do and don't want to have FUSE
 error out.
 
 Furthermore, uncomment the timestamp checks in test-fuse.sh, since now
 they should be working fine.

Apparently this way not the right fix for rh#660687 (although the 
addition of the patch did not harm). I should have hopefully found the 
right cause of it, will follow-up with the fix.

-- 
Pino Toscano

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

[Libguestfs] [PATCH 0/3] Add JSON output for virt-builder

2014-01-16 Thread Pino Toscano

Hi,

This small patch serie adds a JSON output for virt-builder.
This way it is possible to parse the list of available templates,
with no need to parse the unstructured and possibly changing short and
long outputs of virt-builder.


Pino Toscano (3):
  builder: small refactor of the list output
  builder: add --list-format
  builder: add a JSON output for --list

 builder/builder.ml|   4 +-
 builder/cmdline.ml|  19 +++--
 builder/list_entries.ml   | 142 --
 builder/list_entries.mli  |   2 +-
 builder/test-virt-builder-list.sh |  67 ++
 builder/virt-builder.pod  |  34 -
 6 files changed, 222 insertions(+), 46 deletions(-)

-- 
1.8.3.1

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

[Libguestfs] [PATCH 2/3] builder: add --list-format

2014-01-16 Thread Pino Toscano

Add a --list-format which allows to choose which in format should be the
output of --list.
---
 builder/cmdline.ml   | 11 ++-
 builder/virt-builder.pod | 24 +---
 2 files changed, 31 insertions(+), 4 deletions(-)

diff --git a/builder/cmdline.ml b/builder/cmdline.ml
index f199f03..6d6439f 100644
--- a/builder/cmdline.ml
+++ b/builder/cmdline.ml
@@ -132,6 +132,13 @@ let parse_cmdline () =
 
   let list_format = ref `Short in
   let list_set_long () = list_format := `Long in
+  let list_set_format arg =
+list_format := match arg with
+| short - `Short
+| long - `Long
+| fmt -
+  eprintf (f_%s: invalid --list-format type '%s', see the man page.\n) 
prog fmt;
+  exit 1 in
 
   let memsize = ref None in
   let set_memsize arg = memsize := Some arg in
@@ -256,7 +263,9 @@ let parse_cmdline () =
 --link,Arg.String add_link,   target:link.. ^   ^ s_Create 
symbolic links;
 -l,Arg.Unit list_mode,  ^ s_List available templates;
 --list,Arg.Unit list_mode,ditto;
---long,Arg.Unit list_set_long,  ^ s_List available templates, 
in long textual form;
+--long,Arg.Unit list_set_long,  ^ s_Shortcut for 
--list-format short;
+--list-format, Arg.String list_set_format,
+short|long ^   ^ s_Set the 
format for --list (default: short);
 --no-logfile, Arg.Set scrub_logfile,^ s_Scrub build log file;
 --long-options, Arg.Unit display_long_options,   ^ s_List long 
options;
 -m,Arg.Int set_memsize,   mb ^   ^ s_Set memory size;
diff --git a/builder/virt-builder.pod b/builder/virt-builder.pod
index 9cbfbab..05abcc6 100644
--- a/builder/virt-builder.pod
+++ b/builder/virt-builder.pod
@@ -31,7 +31,7 @@ virt-builder - Build virtual machine images quickly
 [--firstboot SCRIPT] [--firstboot-command 'CMD ARGS ...']
 [--firstboot-install PKG,[PKG...]]
 
- virt-builder -l|--list [--long]
+ virt-builder -l|--list [--long] [--list-format short|long]
 
  virt-builder --notes os-version
 
@@ -374,12 +374,30 @@ pointing at CTARGET.
 
 =item B--list
 
+=item B--list --format format
+
 =item B--list --long
 
 List available templates.
 
-The alternative I--list --long form shows lots more details about
-each operating system option.
+It is possible to choose with I--format the output format for the list
+templates:
+
+=over 4
+
+=item Bshort
+
+The default format, prints only the template identifier and, next to it,
+its short description.
+
+=item Blong
+
+Prints a textual list with the details of the available sources, followed
+by the details of the available templates.
+
+=back
+
+I--long is a shorthand for the Clong format.
 
 See also: I--source, I--notes, L/CREATING YOUR OWN TEMPLATES.
 
-- 
1.8.3.1

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

[Libguestfs] [PATCH] builder: add index-struct.h as dependency for index-parser-c.c

2014-01-21 Thread Pino Toscano

Just like with index-parse.h, also index-struct.h is a dependency of
index-parser-c.c which automake cannot generate correctly.
Thus, add it manually.
---
 builder/Makefile.am | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builder/Makefile.am b/builder/Makefile.am
index fc4c552..6565abb 100644
--- a/builder/Makefile.am
+++ b/builder/Makefile.am
@@ -231,7 +231,7 @@ CLEANFILES += \
 
 # Fix dependencies which automake doesn't generate correctly.
 if HAVE_OCAML
-index-parser-c.o: index-parse.h
+index-parser-c.o: index-parse.h index-struct.h
 index-scan.o: index-parse.h
 endif
 index-validate.o: index-parse.h
-- 
1.8.3.1

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

[Libguestfs] [PATCH] builder: read all the available notes from the index

2014-01-22 Thread Pino Toscano

Switch the internal storage for the notes of each entry to a sorted list
with all the subkeys available (which should represent the translations
to various languages).
The current outputs are the same (i.e. still the untranslated notes), so
this is just internal refactoring/preparation.
---
 builder/builder.ml   |  4 ++--
 builder/index_parser.ml  | 19 +++
 builder/index_parser.mli |  2 +-
 builder/list_entries.ml  | 10 +++---
 4 files changed, 25 insertions(+), 10 deletions(-)

diff --git a/builder/builder.ml b/builder/builder.ml
index bb0b108..19d1e42 100644
--- a/builder/builder.ml
+++ b/builder/builder.ml
@@ -200,9 +200,9 @@ let main () =
   (match mode with
   | `Notes -   (* --notes *)
 (match entry with
-| { Index_parser.notes = Some notes } -
+| { Index_parser.notes = (, notes) :: _ } -
   print_endline notes;
-| { Index_parser.notes = None } -
+| { Index_parser.notes = _ } -
   printf (f_There are no notes for %s\n) arg
 );
 exit 0
diff --git a/builder/index_parser.ml b/builder/index_parser.ml
index da44b21..961b91b 100644
--- a/builder/index_parser.ml
+++ b/builder/index_parser.ml
@@ -35,7 +35,7 @@ and entry = {
   compressed_size : int64 option;
   expand : string option;
   lvexpand : string option;
-  notes : string option;
+  notes : (string * string) list;
   hidden : bool;
 
   sigchecker : Sigchecker.t;
@@ -92,8 +92,8 @@ let print_entry chan (name, { printable_name = printable_name;
   | Some lvexpand - fp lvexpand=%s\n lvexpand
   );
   (match notes with
-  | None - ()
-  | Some notes - fp notes=%s\n notes
+  | (, notes) :: _ - fp notes=%s\n notes
+  | _ - ()
   );
   if hidden then fp hidden=true\n
 
@@ -219,7 +219,18 @@ let get_index ~prog ~debug ~downloader ~sigchecker source =
   let lvexpand =
 try Some (List.assoc (lvexpand, None) fields) with Not_found - 
None in
   let notes =
-try Some (List.assoc (notes, None) fields) with Not_found - 
None in
+let rec loop = function
+  | [] - []
+  | ((notes, subkey), value) :: xs -
+let subkey = match subkey with
+| None - 
+| Some v - v in
+(subkey, value) :: loop xs
+  | _ :: xs - loop xs in
+List.sort (
+  fun (k1, _) (k2, _) -
+String.compare k1 k2
+) (loop fields) in
   let hidden =
 try bool_of_string (List.assoc (hidden, None) fields)
 with
diff --git a/builder/index_parser.mli b/builder/index_parser.mli
index 54f1807..3c679b3 100644
--- a/builder/index_parser.mli
+++ b/builder/index_parser.mli
@@ -29,7 +29,7 @@ and entry = {
   compressed_size : int64 option;
   expand : string option;
   lvexpand : string option;
-  notes : string option;
+  notes : (string * string) list;
   hidden : bool;
 
   sigchecker : Sigchecker.t;
diff --git a/builder/list_entries.ml b/builder/list_entries.ml
index 7369e6c..742e43b 100644
--- a/builder/list_entries.ml
+++ b/builder/list_entries.ml
@@ -71,10 +71,10 @@ and list_entries_long ~sources index =
   printf %-24s %s\n (s_Download size:) (human_size size);
 );
 (match notes with
-| None - ()
-| Some notes -
+| (, notes) :: _ -
   printf \n;
   printf (f_Notes:\n\n%s\n) notes
+| _ - ()
 );
 printf \n
   )
@@ -108,6 +108,10 @@ and list_entries_json ~sources index =
 | None - ()
 | Some n -
   printf \%s\: \%Ld\,\n key n in
+  let print_notes = function
+| (, notes) :: _ -
+  printf \notes\: \%s\,\n (json_string_escape notes)
+| _ - () in
 
   printf {\n;
   printf   \version\: %d,\n 1;
@@ -132,7 +136,7 @@ and list_entries_json ~sources index =
   json_optional_printf_string full-name printable_name;
   printf \size\: %Ld,\n size;
   json_optional_printf_int64 compressed-size compressed_size;
-  json_optional_printf_string notes notes;
+  print_notes notes;
   printf \hidden\: %s\n (json_string_of_bool hidden);
   printf   }%s\n (trailing_comma i (List.length index))
   ) index;
-- 
1.8.3.1

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

[Libguestfs] [PATCH] builder: small code simplification

2014-01-22 Thread Pino Toscano

No actual behaviour changes, just remove extra match statements.
---
 builder/list_entries.ml | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/builder/list_entries.ml b/builder/list_entries.ml
index 742e43b..0d3d9b2 100644
--- a/builder/list_entries.ml
+++ b/builder/list_entries.ml
@@ -98,13 +98,11 @@ and list_entries_json ~sources index =
 | c - String.make 1 c)
 done;
 !res in
-  let json_optional_printf_string key value =
-match value with
+  let json_optional_printf_string key = function
 | None - ()
 | Some str -
   printf \%s\: \%s\,\n key (json_string_escape str) in
-  let json_optional_printf_int64 key value =
-match value with
+  let json_optional_printf_int64 key = function
 | None - ()
 | Some n -
   printf \%s\: \%Ld\,\n key n in
-- 
1.8.3.1

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

Re: [Libguestfs] [PATCH] builder: read all the available notes from the index

2014-01-23 Thread Pino Toscano

On Thursday 23 January 2014 12:01:02 Richard W.M. Jones wrote:
 Can you still change the patch to avoid the _ match
 case.  I think something like this should work:
   | { Index_parser.notes = (, notes) :: _ } - (* print notes *)
   | { Index_parser.notes = _ :: _ }
   | { Index_parser.notes = [] } - (* no untranslated notes *)
 
 ACK with that change.

OK for me. Modified accordingly, and pushed.

-- 
Pino Toscano

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

Re: [Libguestfs] [virt-builder] symbol lookup error: /lib64/libgnutls.so.28 - undefined symbol: nettle_secp_256r1

2014-01-27 Thread Pino Toscano

On Monday 27 January 2014 11:06:14 Kashyap Chamarthy wrote:
 Running virt-builder in a guest Fedora-20 guest hypervisor w/ Rawhide
 Kernel, throws the below:
 
   $ virt-builder fedora-20 --format qcow2 --size 20G
   virt-builder: symbol lookup error: /lib64/libgnutls.so.28: undefined
 symbol: nettle_secp_256r1

A couple of ideas:
1) libgnutls has been upgraded and it uses newer symbols from libnettle
   without a proper versioned dependency on it
2) libnettle has been upgraded, removing symbols used by libgnutls (that
   would be an ABI break)

So either there was an ABI break, or there's a too low dependency in 
those two packages.

-- 
Pino Toscano

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

[Libguestfs] [PATCH 1/2] tests: add a a simple libvirt-is-version test tool

2014-01-27 Thread Pino Toscano

libvirt-is-version returns successfully in case the available version of
libvirt is greater or equal than the specified major/minor/release
values.
---
 .gitignore |  1 +
 Makefile.am|  1 +
 configure.ac   |  1 +
 tests/libvirt/Makefile.am  | 32 
 tests/libvirt/libvirt-is-version.c | 76 ++
 5 files changed, 111 insertions(+)
 create mode 100644 tests/libvirt/Makefile.am
 create mode 100644 tests/libvirt/libvirt-is-version.c

diff --git a/.gitignore b/.gitignore
index f8e6c71..c7da2f2 100644
--- a/.gitignore
+++ b/.gitignore
@@ -485,6 +485,7 @@ Makefile.in
 /tests/guests/stamp-fedora-md.img
 /tests/guests/ubuntu.img
 /tests/guests/windows.img
+/tests/libvirt/libvirt-is-version
 /tests/mount-local/test-parallel-mount-local
 /tests/mountable/test-internal-parse-mountable
 /tests/parallel/test-parallel
diff --git a/Makefile.am b/Makefile.am
index e39d11f..dbffdda 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -34,6 +34,7 @@ SUBDIRS += appliance
 if ENABLE_APPLIANCE
 SUBDIRS += tests/qemu
 SUBDIRS += tests/guests
+SUBDIRS += tests/libvirt
 SUBDIRS += tests/c-api
 SUBDIRS += tests/tmpdirs
 SUBDIRS += tests/protocol
diff --git a/configure.ac b/configure.ac
index c07462e..a454570 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1756,6 +1756,7 @@ AC_CONFIG_FILES([Makefile
  tests/hotplug/Makefile
  tests/http/Makefile
  tests/journal/Makefile
+ tests/libvirt/Makefile
  tests/luks/Makefile
  tests/lvm/Makefile
  tests/md/Makefile
diff --git a/tests/libvirt/Makefile.am b/tests/libvirt/Makefile.am
new file mode 100644
index 000..bf5088c
--- /dev/null
+++ b/tests/libvirt/Makefile.am
@@ -0,0 +1,32 @@
+# libguestfs
+# Copyright (C) 2014 Red Hat Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+
+include $(top_srcdir)/subdir-rules.mk
+
+if HAVE_LIBVIRT
+noinst_PROGRAMS = libvirt-is-version
+
+libvirt_is_version_SOURCES = \
+   libvirt-is-version.c
+
+libvirt_is_version_LDADD = \
+   $(LIBVIRT_LIBS)
+
+libvirt_is_version_CFLAGS = \
+   $(WARN_CFLAGS) $(WERROR_CFLAGS) \
+   $(LIBVIRT_CFLAGS)
+endif
diff --git a/tests/libvirt/libvirt-is-version.c 
b/tests/libvirt/libvirt-is-version.c
new file mode 100644
index 000..47f2e25
--- /dev/null
+++ b/tests/libvirt/libvirt-is-version.c
@@ -0,0 +1,76 @@
+/* libguestfs
+ * Copyright (C) 2014 Red Hat Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#include config.h
+
+#include stdio.h
+#include stdlib.h
+#include string.h
+#include unistd.h
+#include errno.h
+
+#include libvirt/libvirt.h
+
+static unsigned int argtoint (const char *prog, const char *arg);
+
+int
+main (int argc, char *argv[])
+{
+  unsigned long ver;
+  unsigned int major = 0, minor = 0, release = 0;
+
+  switch (argc) {
+  case 4:
+release = argtoint (argv[0], argv[3]);
+/*FALLTHROUGH*/
+  case 3:
+minor = argtoint (argv[0], argv[2]);
+/*FALLTHROUGH*/
+  case 2:
+major = argtoint (argv[0], argv[1]);
+break;
+  case 1:
+fprintf (stderr, %s: not enough arguments: MAJOR [MINOR [PATCH]]\n,
+ argv[0]);
+exit (EXIT_FAILURE);
+break;
+  }
+
+  virInitialize ();
+
+  virGetVersion (ver, NULL, NULL);
+
+  return ver = (major * 100 + minor * 1000 + release)
+ ? EXIT_SUCCESS : EXIT_FAILURE;
+}
+
+static unsigned int
+argtoint (const char *prog, const char *arg)
+{
+  long int res;
+  char *endptr;
+
+  errno = 0;
+  res = strtol (arg, endptr, 10);
+  if (errno ||

[Libguestfs] [PATCH 0/2] Skip test-qemu-drive-libvirt.sh if libvirt is 1.1.3

2014-01-27 Thread Pino Toscano

Hi,

test-qemu-drive-libvirt.sh fails to run with libvirt  1.1.3, because
the test:runstate attribute (used to keep the domains shut off) has
been introduced in that libvirt version.

Create a small (uninstalled) C tool which just does this version check,
to be used in all the tests (just one, so far) written in
shell/scripting language.


Pino Toscano (2):
  tests: add a a simple libvirt-is-version test tool
  tests/disks: skip test-qemu-drive-libvirt.sh if libvirt is  1.1.3

 .gitignore |  1 +
 Makefile.am|  1 +
 configure.ac   |  1 +
 tests/disks/test-qemu-drive-libvirt.sh | 10 +
 tests/libvirt/Makefile.am  | 32 ++
 tests/libvirt/libvirt-is-version.c | 76 ++
 6 files changed, 121 insertions(+)
 create mode 100644 tests/libvirt/Makefile.am
 create mode 100644 tests/libvirt/libvirt-is-version.c

-- 
1.8.3.1

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

Re: [Libguestfs] [PATCH INCOMPLETE] Rewrite virt-make-fs in C (originally Perl).

2014-01-27 Thread Pino Toscano

On Monday 27 January 2014 13:29:54 Richard W.M. Jones wrote:
 +/* Execute a command, sending output to a file. */
 +static int
 +exec_command (char **argv, const char *file, int stderr_to_file)
 +{
 +  pid_t pid;
 +  int status, fd;
 +  FILE *fp;
 +  char line[256];
 +
 +  pid = fork ();
 +  if (pid == -1) {
 +perror (fork);
 +return -1;
 +  }
 +  if (pid  0) {
 +if (waitpid (pid, status, 0) == -1) {
 +  perror (waitpid);
 +  return -1;
 +}
 +if (!WIFEXITED (status) || WEXITSTATUS (status) != 0) {
 +  /* If the command failed, dump out the contents of tmpfile
 which +   * contains the exact error messages from qemu-img.
 +   */
 +  fprintf (stderr, _(%s: %s command failed\n), program_name,
 argv[0]); +
 +  if (stderr_to_file) {
 +fp = fopen (tmpfile, r);
 +if (fp != NULL) {
 +  while (fgets (line, sizeof line, fp) != NULL)
 +fprintf (stderr, %s, line);
 +  fclose (fp);
 +}
 +  }
 +
 +  return -1;
 +}
 +return 0;
 +  }
 +
 +  /* Child process. */
 +  fd = open (tmpfile, O_WRONLY|O_NOCTTY);
 +  if (fd == -1) {
 +perror (tmpfile);
 +_exit (EXIT_FAILURE);
 +  }
 +  dup2 (fd, 1);
 +  if (stderr_to_file)
 +dup2 (fd, 2);
 +  close (fd);
 +
 +  execvp (argv[0], argv);
 +  perror (execvp);
 +  _exit (EXIT_FAILURE);
 +}
 +
 +/* Execute a command in the background, sending output to a pipe. */
 +static int
 +bg_command (char **argv, char **pipef)
 +{

Reading this and other similar implementations, for example:
- fish/events.c, do_event_handler
- fish/fish.c, execute_and_inline and issue_command (which has a comment
  regarding pipe commands)
- src/command.c, run_command
- src/launch-direct.c, launch_direct
- src/launch-uml.c, launch_uml
- daemon/guestfsd.c, commandrf (maybe, since it is in the appliance);
  other popen usages in daemon/*

what about using libpipeline [1] to handle them? While it adds the 
overhead of a new shared library (although using just libc, and ~55kb in 
my f19/x86_64 installation), should help a bit in spawning commands and 
handling piping of them if needed.

Given you are starting a new tool in C [2], what about making use of it 
to check how it works?

[1] http://libpipeline.nongnu.org/
[2] as in, was not in C before

-- 
Pino Toscano

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

[Libguestfs] [PATCH] tests/regressions: remove C part of rhbz1044014

2014-01-27 Thread Pino Toscano

All it did was checking for a libvirt version, which is what
libvirt-is-version now does; hence remove the C part, and use guestfish,
ignoring the launch failure (as the C test did).
---
 .gitignore   |  1 -
 tests/regressions/Makefile.am| 21 
 tests/regressions/rhbz1044014.c  | 69 
 tests/regressions/rhbz1044014.sh | 12 ++-
 4 files changed, 11 insertions(+), 92 deletions(-)
 delete mode 100644 tests/regressions/rhbz1044014.c

diff --git a/.gitignore b/.gitignore
index f84e2cd..74661b0 100644
--- a/.gitignore
+++ b/.gitignore
@@ -492,7 +492,6 @@ Makefile.in
 /tests/regressions/rhbz501893
 /tests/regressions/rhbz790721
 /tests/regressions/rhbz914931
-/tests/regressions/rhbz1044014
 /tests/regressions/rhbz1044014.out
 /tests/regressions/rhbz1055452
 /tests/rsync/rsyncd.pid
diff --git a/tests/regressions/Makefile.am b/tests/regressions/Makefile.am
index d45a76c..8016e2f 100644
--- a/tests/regressions/Makefile.am
+++ b/tests/regressions/Makefile.am
@@ -84,10 +84,6 @@ check_PROGRAMS = \
rhbz914931 \
rhbz1055452
 
-if HAVE_LIBVIRT
-check_PROGRAMS += rhbz1044014
-endif
-
 rhbz501893_SOURCES = rhbz501893.c
 rhbz501893_CPPFLAGS = \
-I$(top_srcdir)/src -I$(top_builddir)/src
@@ -118,23 +114,6 @@ rhbz914931_CFLAGS = \
 rhbz914931_LDADD = \
$(top_builddir)/src/libguestfs.la
 
-if HAVE_LIBVIRT
-rhbz1044014_SOURCES = rhbz1044014.c
-rhbz1044014_CPPFLAGS = \
-   -I$(top_srcdir)/gnulib/lib -I$(top_builddir)/gnulib/lib \
-   -I$(top_srcdir)/src -I$(top_builddir)/src
-rhbz1044014_CFLAGS = \
-   $(WARN_CFLAGS) $(WERROR_CFLAGS) \
-   $(GPROF_CFLAGS) $(GCOV_CFLAGS) \
-   $(LIBVIRT_CFLAGS)
-rhbz1044014_LDADD = \
-$(top_builddir)/src/libutils.la \
-$(top_builddir)/src/libguestfs.la \
-$(LIBVIRT_LIBS) \
-$(LIBXML2_LIBS) \
-$(top_builddir)/gnulib/lib/libgnu.la
-endif
-
 rhbz1055452_SOURCES = rhbz1055452.c
 rhbz1055452_CPPFLAGS = \
-I$(top_srcdir)/src -I$(top_builddir)/src \
diff --git a/tests/regressions/rhbz1044014.c b/tests/regressions/rhbz1044014.c
deleted file mode 100644
index 18ce4a7..000
--- a/tests/regressions/rhbz1044014.c
+++ /dev/null
@@ -1,69 +0,0 @@
-/* libguestfs
- * Copyright (C) 2014 Red Hat Inc.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 
USA.
- */
-
-/* Regression test for RHBZ#1044014.
- *
- * The only reason to write this in C is so we can easily check the
- * version of libvirt = 1.2.1.  In the future when we can assume a
- * newer libvirt, we can just have the main rhbz1044014.sh script set
- * some environment variables and use guestfish.
- */
-
-#include config.h
-
-#include stdio.h
-#include stdlib.h
-#include string.h
-#include unistd.h
-#include errno.h
-
-#include libvirt/libvirt.h
-
-#include guestfs.h
-#include guestfs-internal-frontend.h
-
-int
-main (int argc, char *argv[])
-{
-  unsigned long ver;
-  guestfs_h *g;
-
-  virInitialize ();
-
-  /* Check that the version of libvirt we are linked against
-   * supports the new test-driver auth feature.
-   */
-  virGetVersion (ver, NULL, NULL);
-  if (ver  1002001) {
-fprintf (stderr, %s: test skipped because libvirt is too old (%lu)\n,
- argv[0], ver);
-exit (77);
-  }
-
-  g = guestfs_create ();
-  if (!g)
-exit (EXIT_FAILURE);
-
-  /* This will ask the user for credentials.  It will also fail
-   * (expectedly) because the test driver does not support qemu/KVM.
-   */
-  guestfs_launch (g);
-
-  guestfs_close (g);
-  exit (EXIT_SUCCESS);
-}
diff --git a/tests/regressions/rhbz1044014.sh b/tests/regressions/rhbz1044014.sh
index f1e458c..ce1be76 100755
--- a/tests/regressions/rhbz1044014.sh
+++ b/tests/regressions/rhbz1044014.sh
@@ -34,12 +34,22 @@ if [[ ! ( $backend =~ ^libvirt ) ]]; then
 exit 77
 fi
 
+if [ ! -x ../../src/libvirt-is-version ]; then
+echo $0: test skipped because libvirt-is-version is not built yet
+exit 77
+fi
+
+if ! ../../src/libvirt-is-version 1 2 1; then
+echo $0: test skipped because libvirt is too old ( 1.2.1)
+exit 77
+fi
+
 # Set the backend to the test driver.
 export LIBGUESTFS_BACKEND=libvirt:test://$(pwd)/$srcdir/rhbz1044014.xml
 
 rm -f rhbz1044014.out
 
-./rhbz1044014  $srcdir/rhbz1044014.in  rhbz1044014.out 21 || {

[Libguestfs] [PATCH] builder, sysprep: initialise the random generator

2014-01-28 Thread Pino Toscano

virt-builder and virt-sysprep may make use of
Common_utils.string_random8 (which uses Random.int) for constructing
temporary paths; not initialising the random generator means that every
invocation will reuse the same name used previously (!).
Thus just call Random.self_init, just like virt-sparsify already does.

Expand the test-virt-sysprep-script.sh test to ensure that virt-sysprep
is not affected again by this issue.
---
 .gitignore  |  1 +
 builder/builder.ml  |  2 ++
 sysprep/main.ml |  2 ++
 sysprep/script4.sh  | 21 +
 sysprep/test-virt-sysprep-script.sh | 22 +-
 5 files changed, 47 insertions(+), 1 deletion(-)
 create mode 100755 sysprep/script4.sh

diff --git a/.gitignore b/.gitignore
index 74661b0..d9bef99 100644
--- a/.gitignore
+++ b/.gitignore
@@ -428,6 +428,7 @@ Makefile.in
 /sysprep/.depend
 /sysprep/stamp-script1.sh
 /sysprep/stamp-script2.sh
+/sysprep/stamp-script4.sh
 /sysprep/stamp-virt-sysprep.pod
 /sysprep/sysprep-extra-options.pod
 /sysprep/sysprep-operations.pod
diff --git a/builder/builder.ml b/builder/builder.ml
index 3c45fa5..6dc172f 100644
--- a/builder/builder.ml
+++ b/builder/builder.ml
@@ -33,6 +33,8 @@ let quote = Filename.quote
 
 let prog = Filename.basename Sys.executable_name
 
+let () = Random.self_init ()
+
 let main () =
   (* Command line argument parsing - see cmdline.ml. *)
   let mode, arg,
diff --git a/sysprep/main.ml b/sysprep/main.ml
index c1ce3c7..9431e88 100644
--- a/sysprep/main.ml
+++ b/sysprep/main.ml
@@ -31,6 +31,8 @@ let () = Sysprep_operation.bake ()
 (* Command line argument parsing. *)
 let prog = Filename.basename Sys.executable_name
 
+let () = Random.self_init ()
+
 let debug_gc, operations, g, selinux_relabel, quiet, mount_opts =
   let debug_gc = ref false in
   let domain = ref None in
diff --git a/sysprep/script4.sh b/sysprep/script4.sh
new file mode 100755
index 000..fe377e0
--- /dev/null
+++ b/sysprep/script4.sh
@@ -0,0 +1,21 @@
+#!/bin/bash -
+# libguestfs virt-sysprep test --script option
+# Copyright (C) 2014 Red Hat Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+
+# This is used by test-virt-sysprep-script.sh.
+
+pwd  $abs_builddir/stamp-script4.sh
diff --git a/sysprep/test-virt-sysprep-script.sh 
b/sysprep/test-virt-sysprep-script.sh
index 98ecb96..a62cfce 100755
--- a/sysprep/test-virt-sysprep-script.sh
+++ b/sysprep/test-virt-sysprep-script.sh
@@ -31,7 +31,7 @@ if [ ! -w /dev/fuse ]; then
 fi
 
 # Check that multiple scripts can run.
-rm -f stamp-script1.sh stamp-script2.sh
+rm -f stamp-script1.sh stamp-script2.sh stamp-script4.sh
 if ! ./virt-sysprep -q -n -a ../tests/guests/fedora.img --enable script \
 --script $abs_srcdir/script1.sh --script $abs_srcdir/script2.sh; then
 echo $0: virt-sysprep wasn't expected to exit with error.
@@ -48,3 +48,23 @@ if ./virt-sysprep -q -n -a ../tests/guests/fedora.img 
--enable script \
 echo $0: virt-sysprep didn't exit with an error.
 exit 1
 fi
+
+# Check that virt-sysprep uses a new temporary directory every time.
+if ! ./virt-sysprep -q -n -a ../tests/guests/fedora.img --enable script \
+--script $abs_srcdir/script4.sh; then
+echo $0: virt-sysprep (script4.sh, try #1) wasn't expected to exit with 
error.
+exit 1
+fi
+if ! ./virt-sysprep -q -n -a ../tests/guests/fedora.img --enable script \
+--script $abs_srcdir/script4.sh; then
+echo $0: virt-sysprep (script4.sh, try #2) wasn't expected to exit with 
error.
+exit 1
+fi
+if [ x`wc -l stamp-script4.sh | awk '{print $1}'` != x2 ]; then
+echo $0: stamp-script4.sh does not contain two lines.
+exit 1
+fi
+if [ x`head -n1 stamp-script4.sh` == x`tail -n1 stamp-script4.sh` ]; then
+echo $0: stamp-script4.sh does not contain different paths.
+exit 1
+fi
-- 
1.8.3.1

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

Re: [Libguestfs] [PATCH] run: Actually use timeout --foreground option (RHBZ#1025269).

2014-01-28 Thread Pino Toscano

On Tuesday 28 January 2014 16:21:38 Richard W.M. Jones wrote:
 The following commit managed to not actually add the --foreground
 option to the timeout command, just test for it.  Add it this time.
 
   commit 681488877456b83f039dc518861f29ab4e1857f0
   Author: Richard W.M. Jones rjo...@redhat.com
   Date:   Thu Dec 19 08:21:53 2013 +
 
 run: Use timeout --foreground option.
 
 If timeout doesn't have this option (RHEL 6) don't use timeout at
 all.
 
 Attempt to fix RHBZ#1025269.
 ---
  run.in | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/run.in b/run.in
 index d4b13fe..93c50d2 100755
 --- a/run.in
 +++ b/run.in
 @@ -231,7 +231,7 @@ if timeout --help /dev/null 21; then
  if timeout --foreground 2 sleep 0 /dev/null 21; then
  # Does this version of timeout have the -k option?  (Not on
 RHEL 6) if timeout -k 10s 10s true /dev/null 21; then
 -timeout=timeout -k $timeout_kill $timeout_period
 +timeout=timeout --foreground -k $timeout_kill
 $timeout_period fi
  fi
  fi

Ah! I've wondered why it would require a version of timeout with that 
specific feature, without actually making use of it... just forgot to 
ask when I noticed that few days ago. :)

-- 
Pino Toscano

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

Re: [Libguestfs] [PATCH 1/2] daemon: If /selinux exists in the guest, bind-mount /sys/fs/selinux to there.

2014-01-28 Thread Pino Toscano

On Tuesday 28 January 2014 16:21:09 Richard W.M. Jones wrote:
 Commit 72afcf450a78b7e58f65b4a7aaf94d71cd25fca5 was partially
 incorrect.  If the guest userspace is expecting /selinux to exist,
 then we should bind-mount /sys/fs/selinux from the appliance kernel
 there.
 ---
  daemon/command.c | 16 +---
  1 file changed, 13 insertions(+), 3 deletions(-)
 
 diff --git a/daemon/command.c b/daemon/command.c
 index 1aa1a52..939bf87 100644
 --- a/daemon/command.c
 +++ b/daemon/command.c
 @@ -47,9 +47,10 @@ struct bind_state {
char *sysroot_dev;
char *sysroot_dev_pts;
char *sysroot_proc;
 +  char *sysroot_selinux;
char *sysroot_sys;
char *sysroot_sys_fs_selinux;
 -  bool dev_ok, dev_pts_ok, proc_ok, sys_ok, sys_fs_selinux_ok;
 +  bool dev_ok, dev_pts_ok, proc_ok, selinux_ok, sys_ok,
 sys_fs_selinux_ok; };
 
  struct resolver_state {
 @@ -76,16 +77,18 @@ bind_mount (struct bind_state *bs)
bs-sysroot_dev = sysroot_path (/dev);
bs-sysroot_dev_pts = sysroot_path (/dev/pts);
bs-sysroot_proc = sysroot_path (/proc);
 +  bs-sysroot_selinux = sysroot_path (/selinux);
bs-sysroot_sys = sysroot_path (/sys);
bs-sysroot_sys_fs_selinux = sysroot_path (/sys/fs/selinux);
 
if (bs-sysroot_dev == NULL || bs-sysroot_dev_pts == NULL ||
 -  bs-sysroot_proc == NULL || bs-sysroot_sys == NULL ||
 -  bs-sysroot_sys_fs_selinux == NULL) {
 +  bs-sysroot_proc == NULL || bs-sysroot_selinux == NULL ||
 +  bs-sysroot_sys == NULL || bs-sysroot_sys_fs_selinux == NULL)
 { reply_with_perror (malloc);
  free (bs-sysroot_dev);
  free (bs-sysroot_dev_pts);
  free (bs-sysroot_proc);
 +free (bs-sysroot_selinux);
  free (bs-sysroot_sys);
  free (bs-sysroot_sys_fs_selinux);
  return -1;
 @@ -97,6 +100,11 @@ bind_mount (struct bind_state *bs)
bs-dev_pts_ok = r != -1;
r = command (NULL, NULL, str_mount, --bind, /proc, bs-sysroot_proc, 
 NULL);
bs-proc_ok = r != -1;
 +  /* Note on the next line we have to bind-mount /sys/fs/selinux (appliance
 +   * kernel) on top of /selinux (where guest is expecting selinux).
 +   */
 +  r = command (NULL, NULL, str_mount, --bind, /sys/fs/selinux, 
 bs-sysroot_selinux, NULL);
 +  bs-selinux_ok = r != -1;
r = command (NULL, NULL, str_mount, --bind, /sys, bs-sysroot_sys, 
 NULL);
bs-sys_ok = r != -1;
r = command (NULL, NULL, str_mount, --bind, /sys/fs/selinux, 
 bs-sysroot_sys_fs_selinux, NULL);

Possibly I'm missing something, but... given that later /sys/fs/selinux
of the appliance is bind-mounted as /sys/fs/selinux into the sysroot,
couldn't /selinux be created just as a /syslinux - sys/fs/selinux
symlink, to have a bind mount less?

-- 
Pino Toscano

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

Re: [Libguestfs] [PATCH 05/10] examples: Update various examples to use new disk-create API.

2014-01-28 Thread Pino Toscano

On Tuesday 28 January 2014 16:24:52 Richard W.M. Jones wrote:
 --- a/ocaml/examples/create_disk.ml
 +++ b/ocaml/examples/create_disk.ml
 @@ -9,9 +9,7 @@ let () =
let g = new Guestfs.guestfs () in
 
(* Create a raw-format sparse disk image, 512 MB in size. *)
 -  let fd = openfile output [O_WRONLY;O_CREAT;O_TRUNC;O_NOCTTY] 0o666 in
 -  ftruncate fd (512 * 1024 * 1024);
 -  close fd;
 +  g#disk_create output raw 536870912L;

Minor niptick: I'd leave the multiplication, as it was before and as
the other examples do (easier to spot and to change).

-- 
Pino Toscano

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

[Libguestfs] [PATCH] builder: remove unused variables

2014-01-30 Thread Pino Toscano

Leftovers of the list_entries_short+list_entries_long split done in
91aae893c70b3877b31803800ba77836fd7a45e8.
---
 builder/list_entries.ml | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/builder/list_entries.ml b/builder/list_entries.ml
index 2f47231..80a309d 100644
--- a/builder/list_entries.ml
+++ b/builder/list_entries.ml
@@ -47,9 +47,6 @@ let rec list_entries ~list_format ~sources index =
 and list_entries_short index =
   List.iter (
 fun (name, { Index_parser.printable_name = printable_name;
- size = size;
- compressed_size = compressed_size;
- notes = notes;
  hidden = hidden }) -
   if not hidden then (
 printf %-24s name;
-- 
1.8.3.1

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

[Libguestfs] [PATCH] builder: output translated notes

2014-01-30 Thread Pino Toscano

Output all the translations available for the notes in the verbose
output and the JSON output, while trying to match the system langauge in
the show notes output.

The JSON output is slightly changed to handle translations, with the
untranslated notes being matched as C. The version is not bumped
though, since there have been no stable releases with the former output
yet.
---
 builder/Makefile.am   |  5 
 builder/index_parser.ml   | 11 
 builder/list_entries.ml   | 55 +++-
 builder/setlocale-c.c | 59 +++
 builder/setlocale.ml  | 29 +++
 builder/setlocale.mli | 30 
 builder/test-virt-builder-list.sh | 28 ++-
 po/POTFILES   |  1 +
 po/POTFILES-ml|  1 +
 9 files changed, 200 insertions(+), 19 deletions(-)
 create mode 100644 builder/setlocale-c.c
 create mode 100644 builder/setlocale.ml
 create mode 100644 builder/setlocale.mli

diff --git a/builder/Makefile.am b/builder/Makefile.am
index 4777619..2be495b 100644
--- a/builder/Makefile.am
+++ b/builder/Makefile.am
@@ -51,6 +51,9 @@ SOURCES = \
pxzcat.ml \
pxzcat.mli \
pxzcat-c.c \
+   setlocale.ml \
+   setlocale.mli \
+   setlocale-c.c \
sigchecker.mli \
sigchecker.ml
 
@@ -83,6 +86,8 @@ OBJECTS = \
index-parser-c.o \
pxzcat-c.o \
pxzcat.cmx \
+   setlocale-c.o \
+   setlocale.cmx \
get_kernel.cmx \
downloader.cmx \
sigchecker.cmx \
diff --git a/builder/index_parser.ml b/builder/index_parser.ml
index d5b48ae..2d4a642 100644
--- a/builder/index_parser.ml
+++ b/builder/index_parser.ml
@@ -91,11 +91,12 @@ let print_entry chan (name, { printable_name = 
printable_name;
   | None - ()
   | Some lvexpand - fp lvexpand=%s\n lvexpand
   );
-  (match notes with
-  | (, notes) :: _ - fp notes=%s\n notes
-  | _ :: _
-  | [] - ()
-  );
+  List.iter (
+fun (lang, notes) -
+  match lang with
+  |  - fp notes=%s\n notes
+  | lang - fp notes[%s]=%s\n lang notes
+  ) notes;
   if hidden then fp hidden=true\n
 
 (* Types returned by the C index parser. *)
diff --git a/builder/list_entries.ml b/builder/list_entries.ml
index b947cc8..2f47231 100644
--- a/builder/list_entries.ml
+++ b/builder/list_entries.ml
@@ -21,6 +21,23 @@ open Common_utils
 
 open Printf
 
+let split_locale loc =
+  let regex = Str.regexp 
^\\([A-Za-z]+\\)\\(_\\([A-Za-z]+\\)\\)?\\(\\.\\([A-Za-z0-9-]+\\)\\)?\\(@\\([A-Za-z]+\\)\\)?$
 in
+  let l = ref [] in
+  if Str.string_match regex loc 0 then (
+let match_or_empty n =
+  try Str.matched_group n loc with
+  | Not_found -  in
+let lang = Str.matched_group 1 loc in
+let territory = match_or_empty 3 in
+(match territory with
+|  - ()
+| territory - l := (lang ^ _ ^ territory) :: !l);
+l := lang :: !l;
+  );
+  l :=  :: !l;
+  List.rev !l
+
 let rec list_entries ~list_format ~sources index =
   match list_format with
   | `Short - list_entries_short index
@@ -45,6 +62,10 @@ and list_entries_short index =
   ) index
 
 and list_entries_long ~sources index =
+  let langs = match Setlocale.setlocale Setlocale.LC_MESSAGES None with
+  | None - []
+  | Some locale - split_locale locale in
+
   List.iter (
 fun (source, fingerprint) -
   printf (f_Source URI: %s\n) source;
@@ -70,11 +91,21 @@ and list_entries_long ~sources index =
 | Some size -
   printf %-24s %s\n (s_Download size:) (human_size size);
 );
-(match notes with
-| (, notes) :: _ -
+let notes = List.fold_left (
+  fun acc lang -
+match List.filter (
+  fun (langkey, _) -
+match langkey with
+| C - lang = 
+| langkey - langkey = lang
+) notes with
+| (_, noteskey) :: _ - noteskey :: acc
+| [] - acc
+) [] langs in
+(match List.rev notes with
+| notes :: _ -
   printf \n;
   printf (f_Notes:\n\n%s\n) notes
-| _ :: _
 | [] - ()
 );
 printf \n
@@ -108,10 +139,20 @@ and list_entries_json ~sources index =
 | Some n -
   printf \%s\: \%Ld\,\n key n in
   let print_notes = function
-| (, notes) :: _ -
-  printf \notes\: \%s\,\n (json_string_escape notes)
-| _ :: _
-| _ - () in
+| [] - ()
+| notes -
+  printf \notes\: {\n;
+  iteri (
+fun i (lang, langnotes) -
+  let lang =
+match lang with
+|  - C
+| x - x in
+  printf   \%s\: \%s\%s\n
+(json_string_escape lang) (json_string_escape langnotes)
+(trailing_comma i (List.length notes))
+  ) notes;
+  printf },\n in
 
   printf {\n;
   printf   \version\: %d,\n 1;
diff --git

Re: [Libguestfs] [PATCH] builder: output translated notes

2014-01-30 Thread Pino Toscano

On Thursday 30 January 2014 15:52:05 Richard W.M. Jones wrote:
 On Thu, Jan 30, 2014 at 02:44:12PM +0100, Pino Toscano wrote:
  +  let l = ref [] in
 
 'xs' is a bit more natural for lists of things than 'l'.
 
  +  if Str.string_match regex loc 0 then (
  +let match_or_empty n =
  +  try Str.matched_group n loc with
  +  | Not_found -  in
 
 My preference is to put 'in' on a separate line if you're defining a
 function (as here), and 'in' at the end of the line if you're defining
 a value.
 
 So I would have written this:
 
   let match_or_empty n =
 try Str.matched_group n loc with Not_found - 
in

Ah OK.

  +let notes = List.fold_left (
  +  fun acc lang -
  +match List.filter (
  +  fun (langkey, _) -
  +match langkey with
  +| C - lang = 
  +| langkey - langkey = lang
  +) notes with
  +| (_, noteskey) :: _ - noteskey :: acc
  +| [] - acc
  +) [] langs in
 
 I've noticed you like to put huge expressions between match .. with!

You are right, it started with less stuff, and then I didn't realize how 
big it became :) I just split as you hinted above, and also for the 
List.rev notes.

 Rest seems fine so ACK.

Fixed and pushed, thanks.

-- 
Pino Toscano

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

[Libguestfs] [PATCH] resize: properly restore GPT partition types

2014-02-03 Thread Pino Toscano

If there is a GPT partition layout, then what should be read and
restored for each partition is the GPT type and not the MBR ID.

Related to RHBZ#1060404.
---
 resize/resize.ml | 46 +++---
 1 file changed, 35 insertions(+), 11 deletions(-)

diff --git a/resize/resize.ml b/resize/resize.ml
index 8683df7..a2670e5 100644
--- a/resize/resize.ml
+++ b/resize/resize.ml
@@ -48,6 +48,7 @@ type partition = {
   p_part : G.partition;  (* SOURCE partition data from libguestfs. *)
   p_bootable : bool; (* Is it bootable? *)
   p_mbr_id : int option; (* MBR ID, if it has one. *)
+  p_gpt_type : string option;(* GPT ID, if it has one. *)
   p_type : partition_content;(* Content type and content size. *)
 
   (* What we're going to do: *)
@@ -75,7 +76,14 @@ let rec debug_partition p =
 p.p_part.G.part_size;
   eprintf \tbootable: %b\n p.p_bootable;
   eprintf \tpartition ID: %s\n
-(match p.p_mbr_id with None - (none) | Some i - sprintf 0x%x i);
+(match p.p_mbr_id, p.p_gpt_type with
+| None, None - (none)
+| Some i, None - sprintf 0x%x i
+| None, Some i - i
+| Some _, Some _ -
+  (* This should not happen. *)
+  assert false
+);
   eprintf \tcontent: %s\n (string_of_partition_content p.p_type)
 and string_of_partition_content = function
   | ContentUnknown - unknown data
@@ -440,15 +448,21 @@ read the man page virt-resize(1).
   let part_num = Int32.to_int part_num in
   let name = sprintf /dev/sda%d part_num in
   let bootable = g#part_get_bootable /dev/sda part_num in
-  let mbr_id =
-try Some (g#part_get_mbr_id /dev/sda part_num)
-with G.Error _ - None in
+  let mbr_id, gpt_type =
+match parttype with
+| GPT -
+  None, (try Some (g#part_get_gpt_type /dev/sda part_num)
+ with G.Error _ - None)
+| MBR -
+  (try Some (g#part_get_mbr_id /dev/sda part_num)
+  with G.Error _ - None), None in
   let typ =
 if is_extended_partition mbr_id then ContentExtendedPartition
 else get_partition_content name in
 
   { p_name = name; p_part = part;
-p_bootable = bootable; p_mbr_id = mbr_id; p_type = typ;
+p_bootable = bootable; p_mbr_id = mbr_id; p_gpt_type = gpt_type;
+p_type = typ;
 p_operation = OpCopy; p_target_partnum = 0;
 p_target_start = 0L; p_target_end = 0L }
   ) parts in
@@ -1025,7 +1039,8 @@ read the man page virt-resize(1).
 p_name = ;
 p_part = { G.part_num = 0l; part_start = 0L; part_end = 0L;
part_size = 0L };
-p_bootable = false; p_mbr_id = None; p_type = ContentUnknown;
+p_bootable = false; p_mbr_id = None; p_gpt_type = None;
+p_type = ContentUnknown;
 
 (* Target information is meaningful. *)
 p_operation = OpIgnore;
@@ -1103,11 +1118,20 @@ read the man page virt-resize(1).
   if p.p_bootable then
 g#part_set_bootable /dev/sdb p.p_target_partnum true;
 
-  (match p.p_mbr_id with
-  | None - ()
-  | Some mbr_id -
-g#part_set_mbr_id /dev/sdb p.p_target_partnum mbr_id
-  );
+  match parttype with
+  | GPT -
+(match p.p_gpt_type with
+| None - ()
+| Some gpt_type -
+  g#part_set_gpt_type /dev/sdb p.p_target_partnum gpt_type
+)
+  | MBR -
+(match p.p_mbr_id with
+| None - ()
+| Some mbr_id -
+  g#part_set_mbr_id /dev/sdb p.p_target_partnum mbr_id
+)
+
   ) partitions;
 
   (* Fix the bootloader if we aligned the first partition. *)
-- 
1.8.3.1

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

[Libguestfs] [PATCH 0/3] virt-resize: preserve GPT partitions label

2014-02-04 Thread Pino Toscano

Hi,

attached there are few patches to implement a way to get the label of
GPT partitions (refactoring an existing function and adding a new
daemon API) and using it in virt-resize to restore them when copying
partitions.

Thanks,

Pino Toscano (3):
  daemon: parted: refactor sgdisk info parsing code
  New API: part-get-name (RHBZ#593511).
  resize: preserve GPT partition names (RHBZ#1060404).

 daemon/parted.c  | 86 ++--
 generator/actions.ml | 13 
 resize/resize.ml | 19 +++-
 src/MAX_PROC_NR  |  2 +-
 4 files changed, 102 insertions(+), 18 deletions(-)

-- 
1.8.3.1

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

[Libguestfs] [PATCH 3/3] resize: preserve GPT partition names (RHBZ#1060404).

2014-02-04 Thread Pino Toscano

Save the partition names/labels of the source partitions, and restore
them after the partition copy.
---
 resize/resize.ml | 19 ++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/resize/resize.ml b/resize/resize.ml
index 191be83..c1794ed 100644
--- a/resize/resize.ml
+++ b/resize/resize.ml
@@ -49,6 +49,7 @@ type partition = {
   p_bootable : bool; (* Is it bootable? *)
   p_id : partition_id;   (* Partition (MBR/GPT) ID. *)
   p_type : partition_content;(* Content type and content size. *)
+  p_label : string option;   (* Label/name. *)
 
   (* What we're going to do: *)
   mutable p_operation : partition_operation;
@@ -84,7 +85,12 @@ let rec debug_partition p =
 | MBR_ID i - sprintf 0x%x i
 | GPT_Type i - i
 );
-  eprintf \tcontent: %s\n (string_of_partition_content p.p_type)
+  eprintf \tcontent: %s\n (string_of_partition_content p.p_type);
+  eprintf \tlabel: %s\n
+(match p.p_label with
+| Some label - label
+| None - (none)
+)
 and string_of_partition_content = function
   | ContentUnknown - unknown data
   | ContentPV sz - sprintf LVM PV (%Ld bytes) sz
@@ -459,9 +465,13 @@ read the man page virt-resize(1).
   let typ =
 if is_extended_partition id then ContentExtendedPartition
 else get_partition_content name in
+  let label =
+try Some (g#part_get_name /dev/sda part_num)
+with G.Error _ - None in
 
   { p_name = name; p_part = part;
 p_bootable = bootable; p_id = id; p_type = typ;
+p_label = label;
 p_operation = OpCopy; p_target_partnum = 0;
 p_target_start = 0L; p_target_end = 0L }
   ) parts in
@@ -1040,6 +1050,7 @@ read the man page virt-resize(1).
 p_part = { G.part_num = 0l; part_start = 0L; part_end = 0L;
part_size = 0L };
 p_bootable = false; p_id = No_ID; p_type = ContentUnknown;
+p_label = None;
 
 (* Target information is meaningful. *)
 p_operation = OpIgnore;
@@ -1117,6 +1128,12 @@ read the man page virt-resize(1).
   if p.p_bootable then
 g#part_set_bootable /dev/sdb p.p_target_partnum true;
 
+  (match p.p_label with
+  | Some label -
+g#part_set_name /dev/sdb p.p_target_partnum label;
+  | None - ()
+  );
+
   match parttype, p.p_id with
   | GPT, GPT_Type gpt_type -
 g#part_set_gpt_type /dev/sdb p.p_target_partnum gpt_type
-- 
1.8.3.1

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

[Libguestfs] [PATCH 1/3] daemon: parted: refactor sgdisk info parsing code

2014-02-04 Thread Pino Toscano

Isolate in an own function the code that runs sgdisk and parse a field
of it (using an extraction function passed as parameter), using it for
the GUID type.

This is just code motion, no actual behaviour changes.
---
 daemon/parted.c | 53 +
 1 file changed, 37 insertions(+), 16 deletions(-)

diff --git a/daemon/parted.c b/daemon/parted.c
index bd81986..5282adb 100644
--- a/daemon/parted.c
+++ b/daemon/parted.c
@@ -784,8 +784,9 @@ do_part_set_gpt_type(const char *device, int partnum, const 
char *guid)
   return 0;
 }
 
-char *
-do_part_get_gpt_type(const char *device, int partnum)
+static char *
+sgdisk_info_extract_field (const char *device, int partnum, const char *field,
+   char *(*extract) (const char *path))
 {
   if (partnum = 0) {
 reply_with_error (partition number must be = 1);
@@ -814,6 +815,8 @@ do_part_get_gpt_type(const char *device, int partnum)
 return NULL;
   }
 
+  int fieldlen = strlen (field);
+
   /* Parse the output of sgdisk -i:
* Partition GUID code: 21686148-6449-6E6F-744E-656564454649 (BIOS boot 
partition)
* Partition unique GUID: 19AEC5FE-D63A-4A15-9D37-6FCBFB873DC0
@@ -832,28 +835,22 @@ do_part_get_gpt_type(const char *device, int partnum)
 /* Split the line in 2 at the colon */
 char *colon = strchr (line, ':');
 if (colon) {
-#define SEARCH Partition GUID code
-  if (colon - line == strlen(SEARCH) 
-  memcmp (line, SEARCH, strlen(SEARCH)) == 0)
+  if (colon - line == fieldlen 
+  memcmp (line, field, fieldlen) == 0)
   {
-#undef SEARCH
 /* The value starts after the colon */
 char *value = colon + 1;
 
 /* Skip any leading whitespace */
 value += strspn (value,  \t);
 
-/* The value contains only valid GUID characters */
-size_t value_len = strspn (value, -0123456789ABCDEF);
-
-char *ret = malloc (value_len + 1);
+/* Extract the actual information from the field. */
+char *ret = extract (value);
 if (ret == NULL) {
-  reply_with_perror (malloc);
+  /* The extraction function already sends the error. */
   return NULL;
 }
 
-memcpy (ret, value, value_len);
-ret[value_len] = '\0';
 return ret;
   }
 } else {
@@ -866,8 +863,32 @@ do_part_get_gpt_type(const char *device, int partnum)
 }
   }
 
-  /* If we got here it means we didn't find the Partition GUID code */
-  reply_with_error (sgdisk output did not contain Partition GUID code. 
-See LIBGUESTFS_DEBUG output for more details);
+  /* If we got here it means we didn't find the field */
+  reply_with_error (sgdisk output did not contain '%s'. 
+See LIBGUESTFS_DEBUG output for more details, field);
   return NULL;
 }
+
+static char *
+extract_uuid (const char *value)
+{
+  /* The value contains only valid GUID characters */
+  size_t value_len = strspn (value, -0123456789ABCDEF);
+
+  char *ret = malloc (value_len + 1);
+  if (ret == NULL) {
+reply_with_perror (malloc);
+return NULL;
+  }
+
+  memcpy (ret, value, value_len);
+  ret[value_len] = '\0';
+  return ret;
+}
+
+char *
+do_part_get_gpt_type (const char *device, int partnum)
+{
+  return sgdisk_info_extract_field (device, partnum,
+Partition GUID code, extract_uuid);
+}
-- 
1.8.3.1

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

[Libguestfs] [PATCH 2/3] New API: part-get-name (RHBZ#593511).

2014-02-04 Thread Pino Toscano

Counterpart of part-set-name, it uses sgdisk (hence needs the gdisk
feature) to query for the label/name of a partition in a GPT layout.
---
 daemon/parted.c  | 33 +
 generator/actions.ml | 13 +
 src/MAX_PROC_NR  |  2 +-
 3 files changed, 47 insertions(+), 1 deletion(-)

diff --git a/daemon/parted.c b/daemon/parted.c
index 5282adb..83f2411 100644
--- a/daemon/parted.c
+++ b/daemon/parted.c
@@ -886,9 +886,42 @@ extract_uuid (const char *value)
   return ret;
 }
 
+static char *
+extract_optionally_quoted (const char *value)
+{
+  size_t value_len = strlen (value);
+
+  if (value_len = 2 
+  ((value[0] == '\''  value[value_len - 1] == '\'') ||
+   (value[0] == ''  value[value_len - 1] == ''))) {
+value_len -= 2;
+++value;
+  }
+
+  char *ret = strndup (value, value_len);
+  if (ret == NULL) {
+reply_with_perror (strndup);
+return NULL;
+  }
+
+  return ret;
+}
+
 char *
 do_part_get_gpt_type (const char *device, int partnum)
 {
   return sgdisk_info_extract_field (device, partnum,
 Partition GUID code, extract_uuid);
 }
+
+char *
+do_part_get_name (const char *device, int partnum)
+{
+  char *parttype = do_part_get_parttype (device);
+  if (STREQ (parttype, gpt))
+return sgdisk_info_extract_field (device, partnum,
+  Partition name, 
extract_optionally_quoted);
+
+  reply_with_error (cannot get the partition name from '%s' layouts, 
parttype);
+  return NULL;
+}
diff --git a/generator/actions.ml b/generator/actions.ml
index 176de98..b665149 100644
--- a/generator/actions.ml
+++ b/generator/actions.ml
@@ -11766,6 +11766,19 @@ enables all the other flags, if they are not specified 
already.
 
 =back };
 
+  { defaults with
+name = part_get_name;
+style = RString name, [Device device; Int partnum], [];
+proc_nr = Some 416;
+optional = Some gdisk;
+shortdesc = get partition name;
+longdesc = \
+This gets the partition name on partition numbered Cpartnum on
+device Cdevice.  Note that partitions are numbered from 1.
+
+The partition name can only be read on certain types of partition
+table.  This works on Cgpt but not on Cmbr partitions. };
+
 ]
 
 (* Non-API meta-commands available only in guestfish.
diff --git a/src/MAX_PROC_NR b/src/MAX_PROC_NR
index 21c8d99..1c105f1 100644
--- a/src/MAX_PROC_NR
+++ b/src/MAX_PROC_NR
@@ -1 +1 @@
-415
+416
-- 
1.8.3.1

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

Re: [Libguestfs] [PATCH 2/3] New API: part-get-name (RHBZ#593511).

2014-02-04 Thread Pino Toscano

On Tuesday 04 February 2014 15:59:36 Richard W.M. Jones wrote:
 On Tue, Feb 04, 2014 at 04:01:32PM +0100, Pino Toscano wrote:
  +static char *
  +extract_optionally_quoted (const char *value)
  +{
  +  size_t value_len = strlen (value);
  +
  +  if (value_len = 2 
  +  ((value[0] == '\''  value[value_len - 1] == '\'') ||
  +   (value[0] == ''  value[value_len - 1] == ''))) {
  +value_len -= 2;
  +++value;
  +  }
  +
  +  char *ret = strndup (value, value_len);
  +  if (ret == NULL) {
  +reply_with_perror (strndup);
  +return NULL;
  +  }
  +
  +  return ret;
  +}
 
 My spidey sense asks what happens if the value contains quote
 characters?  I wonder if sgdisk escapes them.
 
 Rest of the patch looks fine, but I think you should try setting the
 partition name to a few weird values and see what sgdisk does.

It seems sgdisk just picks whatever it is being passed to it:

  rescue sgdisk -c 3:test /dev/sda
  [  129.149627]  sda: sda1 sda2 sda3
  The operation has completed successfully.
  rescue sgdisk -i 3 /dev/sda | grep name
  Partition name: 'test'
  rescue sgdisk -c 3:test /dev/sda
  [  146.992530]  sda: sda1 sda2 sda3
  The operation has completed successfully.
  rescue sgdisk -i 3 /dev/sda | grep name
  Partition name: 'test'
  rescue sgdisk -c 3:test /dev/sda
  [  194.584171]  sda: sda1 sda2 sda3
  The operation has completed successfully.
  rescue sgdisk -i 3 /dev/sda | grep name
  Partition name: 'test'
  rescue sgdisk -c 3:'test' /dev/sda
  [  202.042122]  sda: sda1 sda2 sda3
  The operation has completed successfully.
  rescue sgdisk -i 3 /dev/sda | grep name
  Partition name: 'test'
  rescue sgdisk -c 3:'test' /dev/sda
  [  211.002185]  sda: sda1 sda2 sda3
  The operation has completed successfully.
  rescue sgdisk -i 3 /dev/sda | grep name
  Partition name: ''test''
  rescue sgdisk -c 3:'test' /dev/sda
  [  217.441540]  sda: sda1 sda2 sda3
  The operation has completed successfully.
  rescue sgdisk -i 3 /dev/sda | grep name
  Partition name: 'test'

So the output is always single-quoted, with other quote characters just 
appearing as they were in the partition label.

-- 
Pino Toscano

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

Re: [Libguestfs] [PATCH 1/3] daemon: parted: refactor sgdisk info parsing code

2014-02-04 Thread Pino Toscano

On Tuesday 04 February 2014 15:56:54 Richard W.M. Jones wrote:
 On Tue, Feb 04, 2014 at 04:01:31PM +0100, Pino Toscano wrote:
  @@ -832,28 +835,22 @@ do_part_get_gpt_type(const char *device, int
  partnum) 
   /* Split the line in 2 at the colon */
   char *colon = strchr (line, ':');
   if (colon) {
  
  -#define SEARCH Partition GUID code
  -  if (colon - line == strlen(SEARCH) 
  -  memcmp (line, SEARCH, strlen(SEARCH)) == 0)
  +  if (colon - line == fieldlen 
  +  memcmp (line, field, fieldlen) == 0)
 
 Maybe use STRPREFIX macro here?

STRPREFIX calls strlen every time it is used, and uses strncmp. While 
not a big deal, the code above is used in a loop, so doing strlen once 
and comparing bits using memcmp should be faster.

Yes, I know it is not performance-critical code, but since it was 
already there and working fine, IMHO could continue to do so :)

-- 
Pino Toscano

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

Re: [Libguestfs] libguestfs and zfs-fuse

2014-02-05 Thread Pino Toscano

On Tuesday 04 February 2014 09:54:58 Richard W.M. Jones wrote:
 On Mon, Feb 03, 2014 at 09:14:09PM +, Richard W.M. Jones wrote:
  On Mon, Feb 03, 2014 at 03:29:37PM -0500, Andre Goree wrote:
   Ahhh, understood.  I do have have an image I can provide that was
   installed onto and MBR layout (seemed the easiest for libguestfs
   to
   understand, GPT is preferred if libguestfs works well with it, but
   that's another matter I can look into later).  Standby let me
   clean
   it up a bit and I'll get it to you -- I'll put it on my box and
   send
   ya a link offlist.
  
  Yes it would be useful to have this image.  Either MBR or GPT
  should work equally well.
 
 FYI Andre sent me a disk image:
 
 $ qemu-img info FreeBSD_ZFS-test.qcow2
 image: FreeBSD_ZFS-test.qcow2
 file format: qcow2
 virtual size: 1.0G (1073741824 bytes)
 disk size: 1.0G
 cluster_size: 65536
 
 $ guestfish --ro -a FreeBSD_ZFS-test.qcow2
 
 Welcome to guestfish, the guest filesystem shell for
 editing virtual machine filesystems and disk images.
 
 Type: 'help' for help on commands
   'man' to read the manual
   'quit' to quit the shell
 
 fs run
 fs list-filesystems
 fs list-partitions
 
 /dev/sda1
 
 fs file /dev/sda1
 
 ; partition 4: ID=0xa5, active, starthead 0, startsector 0, 5
 sectors
 fs vfs-type /dev/sda1
 
 zfs_member
 
 I tried opening it with virt-rescue, but for some reason the zfs-fuse
 daemon would not start, and hence other commands such as zpool didn't
 work.

I've taken a quick look at this, and got a bit more with:
  rescue ln -s ../run/lock /var/lock
  rescue zfs-fuse-helper start
  Starting zfs-fuse: [  OK  ]
  Immunizing zfs-fuse against OOM kills[  OK  ]
  Mounting zfs partitions: [  OK  ]
and after this the zpool/zfs commands were usable.

Just note that:
a) the first command is not needed now with the newly released
   development version libguestfs 1.25.33
b) I couldn't mount the image you provided, since it was created with
   versions of zpool and zfs greater than what zfs-fuse currently
   supports (it seems not having been updated in a while...)
so even if we automate somehow the starting of the zfs-fuse helper and 
the handling of zpool/zfs, I'm not sure it could be actually useful with 
a zfs-fuse not supporting recent versions.
(To have a test partition with ZFS, I had to create it on a 
Debian/kFreeBSD 6.0 (oldstable), as even Debian/kFreeBSD 7 (stable) had 
too new zfs stuff for zfs-fuse.)

-- 
Pino Toscano

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

[Libguestfs] [PATCH 0/4] add GUID validation (RHBZ#1008417)

2014-02-10 Thread Pino Toscano

Hi,

this patch serie adds a new GUID type in the generator, which would do
the same as String, but also validating (just in the C output) the
passed GUID string.
This allows to reject invalid GUIDs before passing them to low-level
tools.


Pino Toscano (4):
  utils: add a function to validate a GUID string
  generator: add a GUID parameter type
  generator: generate code for parameter validation
  actions/part_set_gpt_type: set type of guid parameter as GUID
(RHBZ#1008417).

 generator/actions.ml | 11 +-
 generator/bindtests.ml   |  3 ++-
 generator/c.ml   | 54 
 generator/csharp.ml  |  6 --
 generator/daemon.ml  |  4 ++--
 generator/erlang.ml  |  3 ++-
 generator/fish.ml| 11 ++
 generator/gobject.ml |  8 ---
 generator/golang.ml  |  9 +---
 generator/haskell.ml |  8 +++
 generator/java.ml| 15 +-
 generator/lua.ml |  6 +++---
 generator/ocaml.ml   |  9 +---
 generator/perl.ml|  8 +++
 generator/php.ml | 12 ++-
 generator/python.ml  | 12 +--
 generator/ruby.ml|  4 ++--
 generator/tests_c_api.ml |  6 --
 generator/types.ml   |  6 ++
 generator/utils.ml   |  3 ++-
 generator/xdr.ml |  2 +-
 src/guestfs-internal.h   |  3 +++
 src/test-utils.c | 14 +
 src/utils.c  | 44 +++
 24 files changed, 204 insertions(+), 57 deletions(-)

-- 
1.8.3.1

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

[Libguestfs] [PATCH 4/4] actions/part_set_gpt_type: set type of guid parameter as GUID (RHBZ#1008417).

2014-02-10 Thread Pino Toscano

Switch the type of the guid parameter from String to GUID; this
adds the validation of the GUID as such, rejecting straight away invalid
GUIDs which otherwise could be handled badly by low-level tools (such as
sgdisk).

Add a couple of easy tests (taken from RHBZ#1008417) to
part_set_gpt_type about this.
---
 generator/actions.ml | 11 ++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/generator/actions.ml b/generator/actions.ml
index b665149..a6ef194 100644
--- a/generator/actions.ml
+++ b/generator/actions.ml
@@ -11339,9 +11339,18 @@ group with GUID Cdiskgroup. };
 
   { defaults with
 name = part_set_gpt_type;
-style = RErr, [Device device; Int partnum; String guid], [];
+style = RErr, [Device device; Int partnum; GUID guid], [];
 proc_nr = Some 392;
 optional = Some gdisk;
+tests = [
+  InitGPT, Always, TestLastFail (
+[[part_set_gpt_type; /dev/sda; 1; f]]), [];
+  InitGPT, Always, TestResultString (
+[[part_set_gpt_type; /dev/sda; 1;
+  01234567-89AB-CDEF-0123-456789ABCDEF];
+ [part_get_gpt_type; /dev/sda; 1]],
+01234567-89AB-CDEF-0123-456789ABCDEF), [];
+];
 shortdesc = set the type GUID of a GPT partition;
 longdesc = \
 Set the type GUID of numbered GPT partition Cpartnum to Cguid. Return an
-- 
1.8.3.1

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

[Libguestfs] [PATCH 3/4] generator: generate code for parameter validation

2014-02-10 Thread Pino Toscano

Implemented only in the C output, since every binding uses it anyway,
and just for the GUID type (since its format is well-known).
---
 generator/c.ml | 43 +++
 1 file changed, 43 insertions(+)

diff --git a/generator/c.ml b/generator/c.ml
index e2d5754..c3185be 100644
--- a/generator/c.ml
+++ b/generator/c.ml
@@ -1329,6 +1329,47 @@ and generate_client_actions hash () =
 pr \n;
   in
 
+  (* Generate code to check for parameter validation (where supported
+   * for the type).
+   *)
+  let check_args_validity c_name (ret, args, optargs) =
+let pr_newline = ref false in
+List.iter (
+  function
+  | GUID n -
+  pr   if (!guestfs___validate_guid (%s)) {\n n;
+  pr error (g, \%%s: %%s: parameter is not a valid GUID\,\n;
+  pr\%s\, \%s\);\n c_name n;
+  let errcode =
+match errcode_of_ret ret with
+| `CannotReturnError - assert false
+| (`ErrorIsMinusOne |`ErrorIsNULL) as e - e in
+  pr return %s;\n (string_of_errcode errcode);
+  pr   }\n;
+  pr_newline := true
+
+  (* not applicable *)
+  | String _
+  | Device _
+  | Mountable _
+  | Pathname _
+  | Dev_or_Path _ | Mountable_or_Path _
+  | FileIn _
+  | FileOut _
+  | BufferIn _
+  | StringList _
+  | DeviceList _
+  | Key _
+  | Pointer (_, _)
+  | OptString _
+  | Bool _
+  | Int _
+  | Int64 _ - ()
+) args;
+
+if !pr_newline then pr \n;
+  in
+
   (* Generate code to generate guestfish call traces. *)
   let trace_call name c_name (ret, args, optargs) =
 pr   if (trace_flag) {\n;
@@ -1547,6 +1588,7 @@ and generate_client_actions hash () =
 enter_event name;
 check_null_strings c_name style;
 reject_unknown_optargs c_name style;
+check_args_validity c_name style;
 trace_call name c_name style;
 pr   r = guestfs__%s  c_name;
 generate_c_call_args ~handle:g ~implicit_size_ptr:size_r style;
@@ -1649,6 +1691,7 @@ and generate_client_actions hash () =
 enter_event name;
 check_null_strings c_name style;
 reject_unknown_optargs c_name style;
+check_args_validity c_name style;
 trace_call name c_name style;
 
 (* Calculate the total size of all FileIn arguments to pass
-- 
1.8.3.1

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

[Libguestfs] [PATCH 0/3] virt-builder: copy local files instead of using curl

2014-02-11 Thread Pino Toscano

Hi,

this patch serie does a small optimisation in virt-builder, i.e. make
its internal Downloader just copy files when the source is a local URI,
instead of spawn curl in this case too.


Pino Toscano (3):
  builder: isolate C libraries in an own OCAMLCLIBS
  builder: prepare for different per-protocol download actions
  builder: do a copy when downloading local files

 builder/Makefile.am   | 13 ++-
 builder/downloader.ml | 95 +++
 2 files changed, 70 insertions(+), 38 deletions(-)

-- 
1.8.3.1

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

[Libguestfs] [PATCH 2/3] builder: prepare for different per-protocol download actions

2014-02-11 Thread Pino Toscano

Small refactor of Downloader.download_to to allow different download
actions depending on the protocol of the URI (which is now parsed).

No actual behaviour changes, just mostly code motion.
---
 builder/Makefile.am   |  6 
 builder/downloader.ml | 84 ---
 2 files changed, 53 insertions(+), 37 deletions(-)

diff --git a/builder/Makefile.am b/builder/Makefile.am
index 78a9e72..9d2dbc5 100644
--- a/builder/Makefile.am
+++ b/builder/Makefile.am
@@ -80,6 +80,9 @@ OBJECTS = \
$(top_builddir)/mllib/password.cmx \
$(top_builddir)/mllib/planner.cmx \
$(top_builddir)/mllib/config.cmx \
+   $(top_builddir)/fish/guestfish-uri.o \
+   $(top_builddir)/mllib/uri-c.o \
+   $(top_builddir)/mllib/uRI.cmx \
index-scan.o \
index-struct.o \
index-parse.o \
@@ -115,6 +118,9 @@ OCAMLOPTFLAGS = $(OCAMLCFLAGS)
 
 OCAMLCLIBS  = \
$(LIBLZMA_LIBS) \
+   $(LIBXML2_LIBS) \
+   -L../src/.libs -lutils \
+   -L../gnulib/lib/.libs -lgnu \
-pthread -lpthread \
-lncurses -lcrypt
 
diff --git a/builder/downloader.ml b/builder/downloader.ml
index 77f48ae..95b5817 100644
--- a/builder/downloader.ml
+++ b/builder/downloader.ml
@@ -69,50 +69,60 @@ let rec download ~prog t ?template ?progress_bar uri =
   (filename, false)
 
 and download_to ~prog t ?(progress_bar = false) uri filename =
-  (* Get the status code first to ensure the file exists. *)
-  let cmd = sprintf %s%s -g -o /dev/null -I -w '%%{http_code}' %s
-t.curl
-(if t.debug then  else  -s -S)
-(quote uri) in
-  if t.debug then eprintf %s\n%! cmd;
-  let lines = external_command ~prog cmd in
-  if List.length lines  1 then (
-eprintf (f_%s: unexpected output from curl command, enable debug and look 
at previous messages\n) prog;
-exit 1
-  );
-  let status_code = List.hd lines in
-  let bad_status_code = function
-|  - true
-| s when s.[0] = '4' - true (* 4xx *)
-| s when s.[0] = '5' - true (* 5xx *)
-| _ - false
-  in
-  if bad_status_code status_code then (
-eprintf (f_%s: failed to download %s: HTTP status code %s\n)
-  prog uri status_code;
-exit 1
-  );
+  let parseduri =
+try URI.parse_uri uri
+with Invalid_argument URI.parse_uri -
+  eprintf (f_Error parsing URI '%s'. Look for error messages printed 
above.\n) uri;
+  exit 1 in
 
-  (* Now download the file.
-   * 
-   * Note because there may be parallel virt-builder instances running
+  (* Note because there may be parallel virt-builder instances running
* and also to avoid partial downloads in the cachedir if the network
* fails, we download to a random name in the cache and then
* atomically rename it to the final filename.
*)
   let filename_new = filename ^ . ^ string_random8 () in
   unlink_on_exit filename_new;
-  let cmd = sprintf %s%s -g -o %s %s
-t.curl
-(if t.debug then  else if progress_bar then  -# else  -s -S)
-(quote filename_new) (quote uri) in
-  if t.debug then eprintf %s\n%! cmd;
-  let r = Sys.command cmd in
-  if r  0 then (
-eprintf (f_%s: curl (download) command failed downloading '%s'\n)
-  prog uri;
-exit 1
+
+  (match parseduri.URI.protocol with
+  | _ - (* Any other protocol. *)
+(* Get the status code first to ensure the file exists. *)
+let cmd = sprintf %s%s -g -o /dev/null -I -w '%%{http_code}' %s
+  t.curl
+  (if t.debug then  else  -s -S)
+  (quote uri) in
+if t.debug then eprintf %s\n%! cmd;
+let lines = external_command ~prog cmd in
+if List.length lines  1 then (
+  eprintf (f_%s: unexpected output from curl command, enable debug and 
look at previous messages\n)
+prog;
+  exit 1
+);
+let status_code = List.hd lines in
+let bad_status_code = function
+  |  - true
+  | s when s.[0] = '4' - true (* 4xx *)
+  | s when s.[0] = '5' - true (* 5xx *)
+  | _ - false
+in
+if bad_status_code status_code then (
+  eprintf (f_%s: failed to download %s: HTTP status code %s\n)
+prog uri status_code;
+  exit 1
+);
+
+(* Now download the file. *)
+let cmd = sprintf %s%s -g -o %s %s
+  t.curl
+  (if t.debug then  else if progress_bar then  -# else  -s -S)
+  (quote filename_new) (quote uri) in
+if t.debug then eprintf %s\n%! cmd;
+let r = Sys.command cmd in
+if r  0 then (
+  eprintf (f_%s: curl (download) command failed downloading '%s'\n)
+prog uri;
+  exit 1
+)
   );
 
-  (* Rename the file if curl was successful. *)
+  (* Rename the file if the download was successful. *)
   rename filename_new filename
-- 
1.8.3.1

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

[Libguestfs] [PATCH 1/3] builder: isolate C libraries in an own OCAMLCLIBS

2014-02-11 Thread Pino Toscano

Just moving stuff within Makefile.am, no functional changes.
---
 builder/Makefile.am | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/builder/Makefile.am b/builder/Makefile.am
index 2be495b..78a9e72 100644
--- a/builder/Makefile.am
+++ b/builder/Makefile.am
@@ -113,10 +113,15 @@ endif
 OCAMLCFLAGS = -g -warn-error CDEFLMPSUVYZX $(OCAMLPACKAGES)
 OCAMLOPTFLAGS = $(OCAMLCFLAGS)
 
+OCAMLCLIBS  = \
+   $(LIBLZMA_LIBS) \
+   -pthread -lpthread \
+   -lncurses -lcrypt
+
 virt-builder: $(OBJECTS)
$(OCAMLFIND) ocamlopt $(OCAMLOPTFLAGS) \
  mlguestfs.cmxa -linkpkg $^ \
- -cclib '-pthread $(LIBLZMA_LIBS) -lncurses -lcrypt -lpthread' \
+ -cclib '$(OCAMLCLIBS)' \
  $(OCAML_GCOV_LDFLAGS) \
  -o $@
 
-- 
1.8.3.1

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

[Libguestfs] [PATCH 3/3] builder: do a copy when downloading local files

2014-02-11 Thread Pino Toscano

Instead of spawning curl even to download file:// URIs, just copy
them.
---
 builder/downloader.ml | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/builder/downloader.ml b/builder/downloader.ml
index 95b5817..e386c06 100644
--- a/builder/downloader.ml
+++ b/builder/downloader.ml
@@ -84,6 +84,17 @@ and download_to ~prog t ?(progress_bar = false) uri filename 
=
   unlink_on_exit filename_new;
 
   (match parseduri.URI.protocol with
+  | file -
+let path = parseduri.URI.path in
+let cmd = sprintf cp%s %s %s
+  (if t.debug then  -v else )
+  (quote path) (quote filename_new) in
+let r = Sys.command cmd in
+if r  0 then (
+  eprintf (f_%s: cp (download) command failed copying '%s'\n)
+prog path;
+  exit 1
+)
   | _ - (* Any other protocol. *)
 (* Get the status code first to ensure the file exists. *)
 let cmd = sprintf %s%s -g -o /dev/null -I -w '%%{http_code}' %s
-- 
1.8.3.1

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

[Libguestfs] [PATCH 2/2] mllib: hostname: add a newline in /etc/hostname

2014-02-12 Thread Pino Toscano

/etc/hostname usually has an ending newline, so add it when changing it.
---
 mllib/hostname.ml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mllib/hostname.ml b/mllib/hostname.ml
index e1c5d4b..cffba6b 100644
--- a/mllib/hostname.ml
+++ b/mllib/hostname.ml
@@ -79,7 +79,7 @@ and replace_line_in_file g filename key value =
   g#write filename content
 
 and update_etc_hostname g hostname =
-  g#write /etc/hostname hostname
+  g#write /etc/hostname (hostname ^ \n)
 
 and update_etc_machine_info g hostname =
   replace_line_in_file g /etc/machine-info PRETTY_HOSTNAME hostname
-- 
1.8.3.1

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

[Libguestfs] [PATCH 1/2] mllib: hostname: replace the hostname on Debian also in /etc/hosts (RHBZ#953907).

2014-02-12 Thread Pino Toscano

In Debian/Ubuntu systems, read the previous hostname from /etc/hostname
before replacing it, and try to carefully replace it in /etc/hosts with
the new hostname.

Since Perl_edit to edit /etc/hosts, it is added/changed as dependency
for Hostname.
---
 builder/Makefile.am |  2 +-
 mllib/Makefile.am   |  2 +-
 mllib/hostname.ml   | 22 ++
 sysprep/Makefile.am |  1 +
 4 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/builder/Makefile.am b/builder/Makefile.am
index 9d2dbc5..0880e03 100644
--- a/builder/Makefile.am
+++ b/builder/Makefile.am
@@ -69,10 +69,10 @@ OBJECTS = \
$(top_builddir)/mllib/common_utils.cmx \
$(top_builddir)/mllib/urandom.cmx \
$(top_builddir)/mllib/random_seed.cmx \
+   $(top_builddir)/mllib/perl_edit.cmx \
$(top_builddir)/mllib/hostname.cmx \
$(top_builddir)/mllib/timezone.cmx \
$(top_builddir)/mllib/firstboot.cmx \
-   $(top_builddir)/mllib/perl_edit.cmx \
$(top_builddir)/mllib/crypt-c.o \
$(top_builddir)/mllib/crypt.cmx \
$(top_builddir)/mllib/fsync-c.o \
diff --git a/mllib/Makefile.am b/mllib/Makefile.am
index 67f0a0a..fc328d6 100644
--- a/mllib/Makefile.am
+++ b/mllib/Makefile.am
@@ -80,10 +80,10 @@ OBJECTS = \
common_utils.cmx \
urandom.cmx \
random_seed.cmx \
+   perl_edit.cmx \
hostname.cmx \
timezone.cmx \
firstboot.cmx \
-   perl_edit.cmx \
tTY.cmx \
fsync.cmx \
progress.cmx \
diff --git a/mllib/hostname.ml b/mllib/hostname.ml
index 6702f29..e1c5d4b 100644
--- a/mllib/hostname.ml
+++ b/mllib/hostname.ml
@@ -42,7 +42,12 @@ let rec set_hostname (g : Guestfs.guestfs) root hostname =
 true
 
   | linux, (debian|ubuntu), _ -
+let old_hostname = read_etc_hostname g in
 update_etc_hostname g hostname;
+(match old_hostname with
+| Some old_hostname - replace_host_in_etc_hosts g old_hostname hostname
+| None - ()
+);
 true
 
   | linux, (fedora|rhel|centos|scientificlinux|redhat-based), _ -
@@ -78,3 +83,20 @@ and update_etc_hostname g hostname =
 
 and update_etc_machine_info g hostname =
   replace_line_in_file g /etc/machine-info PRETTY_HOSTNAME hostname
+
+and read_etc_hostname g =
+  let filename = /etc/hostname in
+  if g#is_file filename then (
+let lines = Array.to_list (g#read_lines filename) in
+match lines with
+| hd :: _ - Some hd
+| [] - None
+  ) else
+None
+
+and replace_host_in_etc_hosts g oldhost newhost =
+  let filename = /etc/hosts in
+  if g#is_file filename then (
+Perl_edit.edit_file ~debug:false g filename
+  (s,(\\s) ^ (Str.quote oldhost) ^ (\\s|\\$),\\1 ^ (Str.quote newhost) 
^ \\2, if ($_ !~ /^#/))
+  )
diff --git a/sysprep/Makefile.am b/sysprep/Makefile.am
index 4c03c7f..25cdaa5 100644
--- a/sysprep/Makefile.am
+++ b/sysprep/Makefile.am
@@ -95,6 +95,7 @@ OBJECTS = \
$(top_builddir)/mllib/urandom.cmx \
$(top_builddir)/mllib/password.cmx \
$(top_builddir)/mllib/random_seed.cmx \
+   $(top_builddir)/mllib/perl_edit.cmx \
$(top_builddir)/mllib/hostname.cmx \
$(top_builddir)/mllib/timezone.cmx \
$(top_builddir)/mllib/firstboot.cmx \
-- 
1.8.3.1

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

[Libguestfs] [PATCH] sysprep: remove RH subscription manager files

2014-02-13 Thread Pino Toscano

Add a new operation for it, which should do what
`subscription-manager clean` does.

Part of RHBZ#1063374.
---
 po/POTFILES-ml |  1 +
 sysprep/Makefile.am|  1 +
 .../sysprep_operation_rh_subscription_manager.ml   | 42 ++
 3 files changed, 44 insertions(+)
 create mode 100644 sysprep/sysprep_operation_rh_subscription_manager.ml

diff --git a/po/POTFILES-ml b/po/POTFILES-ml
index cd869fb..7937728 100644
--- a/po/POTFILES-ml
+++ b/po/POTFILES-ml
@@ -57,6 +57,7 @@ sysprep/sysprep_operation_pam_data.ml
 sysprep/sysprep_operation_password.ml
 sysprep/sysprep_operation_puppet_data_log.ml
 sysprep/sysprep_operation_random_seed.ml
+sysprep/sysprep_operation_rh_subscription_manager.ml
 sysprep/sysprep_operation_rhn_systemid.ml
 sysprep/sysprep_operation_rpm_db.ml
 sysprep/sysprep_operation_samba_db_log.ml
diff --git a/sysprep/Makefile.am b/sysprep/Makefile.am
index 4c03c7f..2600477 100644
--- a/sysprep/Makefile.am
+++ b/sysprep/Makefile.am
@@ -60,6 +60,7 @@ operations = \
password \
puppet_data_log \
random_seed \
+   rh_subscription_manager \
rhn_systemid \
rpm_db \
samba_db_log \
diff --git a/sysprep/sysprep_operation_rh_subscription_manager.ml 
b/sysprep/sysprep_operation_rh_subscription_manager.ml
new file mode 100644
index 000..d6c38b2
--- /dev/null
+++ b/sysprep/sysprep_operation_rh_subscription_manager.ml
@@ -0,0 +1,42 @@
+(* virt-sysprep
+ * Copyright (C) 2014 Red Hat Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ *)
+
+open Sysprep_operation
+open Common_gettext.Gettext
+
+module G = Guestfs
+
+let rh_subscription_manager_perform g root side_effects =
+  let typ = g#inspect_get_type root in
+  let distro = g#inspect_get_distro root in
+
+  match typ, distro with
+  | linux, rhel -
+Array.iter g#rm_rf (g#glob_expand /etc/pki/consumer/*);
+Array.iter g#rm_rf (g#glob_expand /etc/pki/entitlement/*)
+  | _ - ()
+
+let op = {
+  defaults with
+name = rh-subscription-manager;
+enabled_by_default = true;
+heading = s_Remove the RH subscription manager files;
+perform_on_filesystems = Some rh_subscription_manager_perform;
+}
+
+let () = register_operation op
-- 
1.8.3.1

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

[Libguestfs] [PATCH] sysprep: remove RH subscription manager log files

2014-02-13 Thread Pino Toscano

Part of RHBZ#1063374.
---
 sysprep/sysprep_operation_logfiles.ml | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/sysprep/sysprep_operation_logfiles.ml 
b/sysprep/sysprep_operation_logfiles.ml
index f154b4d..4c3eb8c 100644
--- a/sysprep/sysprep_operation_logfiles.ml
+++ b/sysprep/sysprep_operation_logfiles.ml
@@ -98,6 +98,9 @@ let globs = List.sort compare [
   /etc/Pegasus/*.csr;
   /etc/Pegasus/*.pem;
   /etc/Pegasus/*.srl;
+
+  (* Red Hat subscription manager log files *)
+  /var/log/rhsm/*;
 ]
 let globs_as_pod = String.concat \n (List.map ((^)  ) globs)
 
-- 
1.8.3.1

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

[Libguestfs] [PATCH] mllib: hostname: replace the hostname on Debian also in /etc/hosts (RHBZ#953907).

2014-02-13 Thread Pino Toscano

In Debian/Ubuntu systems, read the previous hostname from /etc/hostname
before replacing it, and replace it in /etc/hosts with the new hostname.
---
 mllib/hostname.ml | 27 +++
 1 file changed, 27 insertions(+)

diff --git a/mllib/hostname.ml b/mllib/hostname.ml
index fce16ff..0d5b5d4 100644
--- a/mllib/hostname.ml
+++ b/mllib/hostname.ml
@@ -42,7 +42,12 @@ let rec set_hostname (g : Guestfs.guestfs) root hostname =
 true
 
   | linux, (debian|ubuntu), _ -
+let old_hostname = read_etc_hostname g in
 update_etc_hostname g hostname;
+(match old_hostname with
+| Some old_hostname - replace_host_in_etc_hosts g old_hostname hostname
+| None - ()
+);
 true
 
   | linux, (fedora|rhel|centos|scientificlinux|redhat-based), _ -
@@ -78,3 +83,25 @@ and update_etc_hostname g hostname =
 
 and update_etc_machine_info g hostname =
   replace_line_in_file g /etc/machine-info PRETTY_HOSTNAME hostname
+
+and read_etc_hostname g =
+  let filename = /etc/hostname in
+  if g#is_file filename then (
+let lines = Array.to_list (g#read_lines filename) in
+match lines with
+| hd :: _ - Some hd
+| [] - None
+  ) else
+None
+
+and replace_host_in_etc_hosts g oldhost newhost =
+  if g#is_file /etc/hosts then (
+let expr = /files/etc/hosts/*[label() != '#comment']/*[label() != 
'ipaddr'][. = ' ^ oldhost ^ '] in
+g#aug_init / 0;
+let matches = Array.to_list (g#aug_match expr) in
+List.iter (
+  fun m -
+g#aug_set m newhost
+) matches;
+g#aug_save ()
+  )
-- 
1.8.3.1

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

Re: [Libguestfs] [PATCH] mllib: hostname: replace the hostname on Debian also in /etc/hosts (RHBZ#953907).

2014-02-13 Thread Pino Toscano

On Thursday 13 February 2014 13:33:16 Richard W.M. Jones wrote:
 On Thu, Feb 13, 2014 at 02:15:31PM +0100, Pino Toscano wrote:
  +let expr = /files/etc/hosts/*[label() != '#comment']/*[label()
  != 'ipaddr'][. = ' ^ oldhost ^ '] in
 Quoting?  If oldhost contains a ' character + some Augeas code, this
 might be exploitable.

Hm right. Gone back in manually checking the values.

 I thought it might be possible to iterate over the Augeas tree.  I'm
 fairly sure I used to have some code that did this, but I can't find
 it at the moment.

At least in libguestfs, the two files which do augeas match+iteration 
are sysprep/sysprep_operation_user_account.ml (which you mentioned 
earlier) and src/inspect-fs-unix.c.

-- 
Pino Toscano

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

[Libguestfs] [PATCH] mllib: hostname: on Debian replace it also in /etc/hosts (RHBZ#953907).

2014-02-13 Thread Pino Toscano

In Debian/Ubuntu systems, read the previous hostname from /etc/hostname
before replacing it, and replace it in /etc/hosts with the new hostname.
---
 mllib/hostname.ml | 30 ++
 1 file changed, 30 insertions(+)

diff --git a/mllib/hostname.ml b/mllib/hostname.ml
index fce16ff..70ca934 100644
--- a/mllib/hostname.ml
+++ b/mllib/hostname.ml
@@ -42,7 +42,12 @@ let rec set_hostname (g : Guestfs.guestfs) root hostname =
 true
 
   | linux, (debian|ubuntu), _ -
+let old_hostname = read_etc_hostname g in
 update_etc_hostname g hostname;
+(match old_hostname with
+| Some old_hostname - replace_host_in_etc_hosts g old_hostname hostname
+| None - ()
+);
 true
 
   | linux, (fedora|rhel|centos|scientificlinux|redhat-based), _ -
@@ -78,3 +83,28 @@ and update_etc_hostname g hostname =
 
 and update_etc_machine_info g hostname =
   replace_line_in_file g /etc/machine-info PRETTY_HOSTNAME hostname
+
+and read_etc_hostname g =
+  let filename = /etc/hostname in
+  if g#is_file filename then (
+let lines = Array.to_list (g#read_lines filename) in
+match lines with
+| hd :: _ - Some hd
+| [] - None
+  ) else
+None
+
+and replace_host_in_etc_hosts g oldhost newhost =
+  if g#is_file /etc/hosts then (
+let expr = /files/etc/hosts/*[label() != '#comment']/*[label() != 
'ipaddr'] in
+g#aug_init / 0;
+let matches = Array.to_list (g#aug_match expr) in
+List.iter (
+  fun m -
+let value = g#aug_get m in
+if value = oldhost then (
+  g#aug_set m newhost
+)
+) matches;
+g#aug_save ()
+  )
-- 
1.8.3.1

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

[Libguestfs] Handling different architectures in virt-builder

2014-02-14 Thread Pino Toscano

Hi,

currently virt-builder's index contains only x86_64/amd64 images, so 
asking virt-builder to produce an image always produce a x86_64 image, 
regardless of the host. This also mean that trying to produce, say, a 
new fedora-20 image from an i686 not only would produce an unexpected 
result, but also would prevent to e.g. install packages on it.

So virt-builder and its index need to be able to distinguish the 
architecture; choosing the architecture could be a --arch parameter for 
virt-builder, but what about the keys in the index since it needs to 
preserve compatibility with former versions?
Maybe a solution could be:
a) adding arch=.. keys in entries
b) rename (or just copy, to avoid breaking older virt-builders) keys to
   $distro-$version-$arch
c) to not break compatibility with user input virt-builder joins
   $user_selection + $arch = $user_selection-$arch, and looks in the
   index
d) default $arch to `uname -m/p`, if --arch is not specified

Or maybe an even idea could be to give the index file a suffix with the 
architecture name... although that could break users specifying an 
absolute URL for an own index of distros...

Also, two side notes related to the different architecture handling 
issue:
1) the naming of architectures can change between distros (e.g. amd64 vs
   x86_64 vs x64, powerpc vs ppc) -- a simple hardcoded mapping in
   virt-builder should do the job, I guess?
2) what should be done with commands/operations (e.g. --install) that
   try to run stuff from the guest, when the host and guest
   architectures are different? Always deny, deny but allow with a
   configure switch, or don't bother and let the user get their failure?

-- 
Pino Toscano

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

Re: [Libguestfs] [PATCH] builder: add index-struct.h as dependency for index-parser-c.c

2014-02-17 Thread Pino Toscano

On Sunday 16 February 2014 22:32:52 Richard W.M. Jones wrote:
 BTW, although there is nothing wrong with this patch, it doesn't seem
 to completely resolve the problem.  It looks like there could be
 another missing dependency.
 
 When I did:
 
   make distclean  ./configure  make -j5
 
 the build failed as below (not always reproducible).  As you can see
 'index-parse.h' is updating some time after it is used.
 [...]
 make[2]: Entering directory `/home/rjones/d/libguestfs/builder'
   YACC index-parse.c
   LEX  index-scan.c
   CC   virt_index_validate-index-struct.o
   CC   virt_index_validate-index-validate.o
   CC   index-struct.o
 ocamlfind ocamlc -g -warn-error CDEFLMPSUVYZX -package str,unix -I
 ../src/.libs -I ../ocaml -I ../mllib -package gettext-stub -c
 pxzcat.mli -o pxzcat.cmi CC   pxzcat-c.o
   CC   setlocale-c.o
 ocamlfind ocamlc -g -warn-error CDEFLMPSUVYZX -package str,unix -I
 ../src/.libs -I ../ocaml -I ../mllib -package gettext-stub -c
 setlocale.mli -o setlocale.cmi index-validate.c:34:25: fatal error:
 index-parse.h: No such file or directory #include index-parse.h
  ^
 compilation terminated.
 [...]
 make[2]: *** [virt_index_validate-index-validate.o] Error 1
 make[2]: *** Waiting for unfinished jobs

I see.
While I don't have a deep knowledge of it, my guess would be the way 
automake 1.14 handles outputs for different targets in the same build 
directory. As you can see, it prefixes them by the target name, so our 
custom rules won't apply basically.

Curiously, I hit that last weekend when rebuilding on my Debian testing 
(which has automake 1.14, unlike my current F19 which has 1.13), but 
didn't have the occasion to debug it yet.

-- 
Pino Toscano

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

Re: [Libguestfs] Handling different architectures in virt-builder

2014-02-17 Thread Pino Toscano

On Friday 14 February 2014 16:08:10 Richard W.M. Jones wrote:
 On Fri, Feb 14, 2014 at 03:48:34PM +0100, Pino Toscano wrote:
  Hi,
  
  currently virt-builder's index contains only x86_64/amd64 images, so
  asking virt-builder to produce an image always produce a x86_64
  image, regardless of the host. This also mean that trying to
  produce, say, a new fedora-20 image from an i686 not only would
  produce an unexpected result, but also would prevent to e.g.
  install packages on it.
  
  So virt-builder and its index need to be able to distinguish the
  architecture; choosing the architecture could be a --arch parameter
  for virt-builder, but what about the keys in the index since it
  needs to preserve compatibility with former versions?
  Maybe a solution could be:
  a) adding arch=.. keys in entries
 
 An observation: All current images are x86-64, so you can assume if
 arch= is missing then it means arch=x86_64.

Sounds fair.

  b) rename (or just copy, to avoid breaking older virt-builders) keys
  to 
 $distro-$version-$arch
  
  c) to not break compatibility with user input virt-builder joins
  
 $user_selection + $arch = $user_selection-$arch, and looks in the
 index
  
  d) default $arch to `uname -m/p`, if --arch is not specified
 
 I think (d) for the following reasons.

The list above was not actually a choice of possibilities, but a list of 
steps to do (IMHO) ;)

 We ought to keep:
 
   virt-builder fedora-20
 
 doing the Right Thing, which for the vast majority of users, on x86-64
 host, means they want an x86-64 guest.

True, that's what I want to achieve.
 
 If aarch64 becomes popular, then:
 
   virt-builder fedora-20
 
 should build an aarch64 image.  Reason: building an x86-64 image isn't
 very useful, because the user wouldn't immediately be able to run it
 (or: it would run very very slowly).

I'm not sure to understand: are you implying that at some point the 
default output might be aarch64 (or whatever is the cool arch at that 
time)?

 If people want to do strange stuff, they can use the --arch option.

Right.

  Or maybe an even idea could be to give the index file a suffix with
  the architecture name... although that could break users specifying
  an absolute URL for an own index of distros...
 
 Yup, sounds complicated.
 
 For 1.26 I would like to change (again) how virt-builder finds its
 index.
 
 The idea is to have /etc/virt-builder.d/... file(s) which each list
 an upstream source, so for example:
 
   /etc/virt-builder.d/fedora.conf
 
 would contain:
 
   name=Fedora
   source=http://cloud.fedoraproject.org/metadata/index.asc
   fingerprint=blah blah
 
 which would list all Fedora cloud images (assuming we publish ARM
 cloud images in future, they would be in the same place).
 
 As these are configuration files, people could add their own.

This sounds like a better idea indeed -- I guess also our current 
hardcoded remote location would become just a config file shipped by 
default? (This way distros can even not use it, if they feel to.)

  Also, two side notes related to the different architecture
  handling
  issue:
  1) the naming of architectures can change between distros (e.g.
  amd64 vs 
 x86_64 vs x64, powerpc vs ppc) -- a simple hardcoded mapping in
 virt-builder should do the job, I guess?
 
 Even better, list what architectures we support in the man page, and
 refuse to process index files that contain other names.

Hm, not sure about this; while hardcoding does not seem nice (although 
would be a small mapping), rejecting unknown architectures that might 
work OOTB might be worse.

  2) what should be done with commands/operations (e.g. --install)
  that
  
 try to run stuff from the guest, when the host and guest
 architectures are different? Always deny, deny but allow with a
 configure switch, or don't bother and let the user get their
 failure?
 
 I would say: if host arch != requested guest arch, then deny.  It's
 the principle of least surprise for users, and people can always use a
 firstboot script.
 
 We can fix it later, as it is theoretically possible to run qemu in
 TCG to emulate other architectures, just real slow.

Reasonable enough, I'd say.

-- 
Pino Toscano

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

Re: [Libguestfs] Handling different architectures in virt-builder

2014-02-17 Thread Pino Toscano

On Friday 14 February 2014 16:19:39 Richard W.M. Jones wrote:
 On Fri, Feb 14, 2014 at 03:48:34PM +0100, Pino Toscano wrote:
  a) adding arch=.. keys in entries
  b) rename (or just copy, to avoid breaking older virt-builders) keys
  to 
 $distro-$version-$arch
  
  c) to not break compatibility with user input virt-builder joins
  
 $user_selection + $arch = $user_selection-$arch, and looks in the
 index
  
  d) default $arch to `uname -m/p`, if --arch is not specified
 
 os-version is a unique key in the index today, but does it need to be?
 You could have multiple os-version entries differing only by at least
 the following fields:
 
 . revision
 . arch
 . format
 
 So your unique key internally in virt-builder would be (os-version,
 revision, arch, format) ...

Sure, changing the key is not actually an issue by itself; the issue 
comes up whether we want the new indexes to be usable by older
virt-builders, which want os-version as unique key.

OTOH, if we switch to the virt-builder.d/*.conf configuration files, 
those would need to be written anew, so could automatically gain new 
features incompatible with older virt-builders.

-- 
Pino Toscano

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

[Libguestfs] [PATCH 2/2] fish: change order of config files being read

2014-02-17 Thread Pino Toscano

First read the global configuration and then the local one in user's
HOME, so the latter can really override system settings.
---
 fish/config.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/fish/config.c b/fish/config.c
index 02d850b..9e5da87 100644
--- a/fish/config.c
+++ b/fish/config.c
@@ -83,7 +83,10 @@ parse_config (void)
 {
   const char *home;
 
-  /* Try $HOME first. */
+  /* Try the global configuration first. */
+  read_config_from_file (etc_filename);
+
+  /* Read the configuration from $HOME, to override system settings. */
   home = getenv (HOME);
   if (home != NULL) {
 CLEANUP_FREE char *path = NULL;
@@ -95,8 +98,6 @@ parse_config (void)
 
 read_config_from_file (path);
   }
-
-  read_config_from_file (etc_filename);
 }
 
 #else /* !HAVE_LIBCONFIG */
-- 
1.8.3.1

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

[Libguestfs] [PATCH 1/2] fish: small refactor of config reading code

2014-02-17 Thread Pino Toscano

Even though so far there is just one possible setting to read, isolate
in an own function the code to parse a configuration file and read the
settings out of it.

Now there's a new config_t handle used every time, but since config_read
would reset an handle completely, there is no behaviour change.
---
 fish/config.c | 88 +--
 1 file changed, 31 insertions(+), 57 deletions(-)

diff --git a/fish/config.c b/fish/config.c
index e9f437a..02d850b 100644
--- a/fish/config.c
+++ b/fish/config.c
@@ -42,87 +42,61 @@ static const char *etc_filename = 
/etc/libguestfs-tools.conf;
  * global handle 'g' is opened.
  */
 
-void
-parse_config (void)
+static void
+read_config_from_file (const char *filename)
 {
-  const char *home;
   FILE *fp;
-  config_t conf;
-
-  config_init (conf);
-
-  /* Try $HOME first. */
-  home = getenv (HOME);
-  if (home != NULL) {
-CLEANUP_FREE char *path = NULL;
 
-if (asprintf (path, %s/%s, home, home_filename) == -1) {
-  perror (asprintf);
-  exit (EXIT_FAILURE);
-}
+  fp = fopen (filename, r);
+  if (fp != NULL) {
+config_t conf;
 
-fp = fopen (path, r);
-if (fp != NULL) {
-  /*
-  if (verbose)
-fprintf (stderr, %s: reading configuration from %s\n,
- program_name, path);
-  */
-
-  if (config_read (conf, fp) == CONFIG_FALSE) {
-fprintf (stderr,
- _(%s: %s: line %d: error parsing configuration file: %s\n),
- program_name, path, config_error_line (conf),
- config_error_text (conf));
-exit (EXIT_FAILURE);
-  }
-
-  if (fclose (fp) == -1) {
-perror (path);
-exit (EXIT_FAILURE);
-  }
-
-  /* Notes:
-   *
-   * (1) It's not obvious from the documentation, that config_read
-   * completely resets the 'conf' structure.  This means we cannot
-   * call config_read twice on the two possible configuration
-   * files, but instead have to copy out settings into our
-   * variables between calls.
-   *
-   * (2) If the next call fails then 'read_only' variable is not
-   * updated.  Failure could happen just because the setting is
-   * missing from the configuration file, so we ignore it here.
-   */
-  config_lookup_bool (conf, read_only, read_only);
-}
-  }
+config_init (conf);
 
-  fp = fopen (etc_filename, r);
-  if (fp != NULL) {
 /*
 if (verbose)
   fprintf (stderr, %s: reading configuration from %s\n,
-   program_name, etc_filename);
+   program_name, filename);
 */
 
 if (config_read (conf, fp) == CONFIG_FALSE) {
   fprintf (stderr,
_(%s: %s: line %d: error parsing configuration file: %s\n),
-   program_name, etc_filename, config_error_line (conf),
+   program_name, filename, config_error_line (conf),
config_error_text (conf));
   exit (EXIT_FAILURE);
 }
 
 if (fclose (fp) == -1) {
-  perror (etc_filename);
+  perror (filename);
   exit (EXIT_FAILURE);
 }
 
 config_lookup_bool (conf, read_only, read_only);
+
+config_destroy (conf);
+  }
+}
+
+void
+parse_config (void)
+{
+  const char *home;
+
+  /* Try $HOME first. */
+  home = getenv (HOME);
+  if (home != NULL) {
+CLEANUP_FREE char *path = NULL;
+
+if (asprintf (path, %s/%s, home, home_filename) == -1) {
+  perror (asprintf);
+  exit (EXIT_FAILURE);
+}
+
+read_config_from_file (path);
   }
 
-  config_destroy (conf);
+  read_config_from_file (etc_filename);
 }
 
 #else /* !HAVE_LIBCONFIG */
-- 
1.8.3.1

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

[Libguestfs] [PATCH] fish: use XDG paths for the config file

2014-02-17 Thread Pino Toscano

Read the configuration file from XDG paths for both global and
user-local locations, keeping the old paths as fallback.
---
 fish/config.c  | 68 ++
 fish/guestfish.pod |  9 --
 fish/libguestfs-tools.conf.pod | 52 ++--
 fuse/guestmount.pod|  4 +++
 rescue/virt-rescue.pod |  4 +++
 5 files changed, 119 insertions(+), 18 deletions(-)

diff --git a/fish/config.c b/fish/config.c
index 9e5da87..7d319ad 100644
--- a/fish/config.c
+++ b/fish/config.c
@@ -34,8 +34,9 @@
 
 #ifdef HAVE_LIBCONFIG
 
+#define GLOBAL_CONFIG_FILENAME libguestfs-tools.conf
 static const char *home_filename = /* $HOME/ */ .libguestfs-tools.rc;
-static const char *etc_filename = /etc/libguestfs-tools.conf;
+static const char *etc_filename = /etc/ GLOBAL_CONFIG_FILENAME;
 
 /* Note that parse_config is called very early, before command line
  * parsing, before the verbose flag has been set, even before the
@@ -86,17 +87,72 @@ parse_config (void)
   /* Try the global configuration first. */
   read_config_from_file (etc_filename);
 
+  {
+/* Then read the configuration from XDG system paths. */
+const char *xdg_env, *var;
+CLEANUP_FREE_STRING_LIST char **xdg_config_dirs = NULL;
+size_t xdg_config_dirs_count;
+
+xdg_env = getenv (XDG_CONFIG_DIRS);
+var = xdg_env != NULL  xdg_env[0] != 0 ? xdg_env : /etc/xdg;
+xdg_config_dirs = guestfs___split_string (':', var);
+xdg_config_dirs_count = guestfs___count_strings (xdg_config_dirs);
+for (size_t i = xdg_config_dirs_count; i  0; --i) {
+  CLEANUP_FREE char *path = NULL;
+  const char *dir = xdg_config_dirs[i - 1];
+
+  if (asprintf (path, %s/libguestfs/ GLOBAL_CONFIG_FILENAME, dir) == 
-1) {
+perror (asprintf);
+exit (EXIT_FAILURE);
+  }
+
+  read_config_from_file (path);
+}
+  }
+
   /* Read the configuration from $HOME, to override system settings. */
   home = getenv (HOME);
   if (home != NULL) {
-CLEANUP_FREE char *path = NULL;
+{
+  /* Old-style configuration file first. */
+  CLEANUP_FREE char *path = NULL;
 
-if (asprintf (path, %s/%s, home, home_filename) == -1) {
-  perror (asprintf);
-  exit (EXIT_FAILURE);
+  if (asprintf (path, %s/%s, home, home_filename) == -1) {
+perror (asprintf);
+exit (EXIT_FAILURE);
+  }
+
+  read_config_from_file (path);
 }
 
-read_config_from_file (path);
+{
+  /* Then, XDG_CONFIG_HOME path. */
+  CLEANUP_FREE char *path = NULL;
+  CLEANUP_FREE char *home_copy = strdup (home);
+  const char *xdg_env;
+
+  if (home_copy == NULL) {
+perror (strdup);
+exit (EXIT_FAILURE);
+  }
+
+  xdg_env = getenv (XDG_CONFIG_HOME);
+  if (xdg_env == NULL) {
+if (asprintf (path, %s/.config/libguestfs/ GLOBAL_CONFIG_FILENAME,
+  home_copy) == -1) {
+  perror (asprintf);
+  exit (EXIT_FAILURE);
+}
+  } else {
+if (asprintf (path, %s/libguestfs/ GLOBAL_CONFIG_FILENAME,
+  xdg_env) == -1) {
+  perror (asprintf);
+  exit (EXIT_FAILURE);
+}
+  }
+
+  read_config_from_file (path);
+}
   }
 }
 
diff --git a/fish/guestfish.pod b/fish/guestfish.pod
index bdfe64b..31add50 100644
--- a/fish/guestfish.pod
+++ b/fish/guestfish.pod
@@ -516,9 +516,8 @@ then you will cause irreversible disk corruption.
 In a future libguestfs we intend to change the default the other way.
 Disk images will be opened read-only.  You will have to either specify
 Iguestfish --rw, Iguestmount --rw, Ivirt-rescue --rw, or change
-the configuration file C/etc/libguestfs-tools.conf in order to get
-write access for disk images specified by those other command line
-options.
+the configuration file in order to get write access for disk images
+specified by those other command line options.
 
 This version of guestfish, guestmount and virt-rescue has a I--rw
 option which does nothing (it is already the default).  However it is
@@ -1541,8 +1540,12 @@ See L/LIBGUESTFS_CACHEDIR, L/LIBGUESTFS_TMPDIR.
 
 =over 4
 
+=item $XDG_CONFIG_HOME/libguestfs/libguestfs-tools.conf
+
 =item $HOME/.libguestfs-tools.rc
 
+=item $XDG_CONFIG_DIRS/libguestfs/libguestfs-tools.conf
+
 =item /etc/libguestfs-tools.conf
 
 This configuration file controls the default read-only or read-write
diff --git a/fish/libguestfs-tools.conf.pod b/fish/libguestfs-tools.conf.pod
index 771eb50..bba00e3 100644
--- a/fish/libguestfs-tools.conf.pod
+++ b/fish/libguestfs-tools.conf.pod
@@ -2,18 +2,22 @@
 
 =head1 NAME
 
-/etc/libguestfs-tools.conf - configuration file for guestfish, guestmount, 
virt-rescue
+libguestfs-tools.conf - configuration file for guestfish, guestmount, 
virt-rescue
 
 =head1 SYNOPSIS
 
  /etc/libguestfs-tools.conf
 
+ $XDG_CONFIG_DIRS/libguestfs/libguestfs-tools.conf
+
  $HOME/.libguestfs-tools.rc
 
+

[Libguestfs] [PATCH] builder: move the XDG path handling in an own file

2014-02-18 Thread Pino Toscano

Just code motion and renaming, no actual behaviour changes.
---
 builder/Makefile.am |  2 ++
 builder/cmdline.ml  |  9 +
 builder/paths.ml| 26 ++
 po/POTFILES-ml  |  1 +
 4 files changed, 30 insertions(+), 8 deletions(-)
 create mode 100644 builder/paths.ml

diff --git a/builder/Makefile.am b/builder/Makefile.am
index 6313bad..23336c0 100644
--- a/builder/Makefile.am
+++ b/builder/Makefile.am
@@ -48,6 +48,7 @@ SOURCES = \
index-parser-c.c \
list_entries.mli \
list_entries.ml \
+   paths.ml \
pxzcat.ml \
pxzcat.mli \
pxzcat-c.c \
@@ -91,6 +92,7 @@ OBJECTS = \
pxzcat.cmx \
setlocale-c.o \
setlocale.cmx \
+   paths.cmx \
get_kernel.cmx \
downloader.cmx \
sigchecker.cmx \
diff --git a/builder/cmdline.ml b/builder/cmdline.ml
index a6cb6c5..e9e47ae 100644
--- a/builder/cmdline.ml
+++ b/builder/cmdline.ml
@@ -30,13 +30,6 @@ open Printf
 
 let prog = Filename.basename Sys.executable_name
 
-let default_cachedir =
-  try Some (Sys.getenv XDG_CACHE_HOME // virt-builder)
-  with Not_found -
-try Some (Sys.getenv HOME // .cache // virt-builder)
-with Not_found -
-  None (* no cache directory *)
-
 let default_source = http://libguestfs.org/download/builder/index.asc;
 
 let parse_cmdline () =
@@ -61,7 +54,7 @@ let parse_cmdline () =
   in
   let attach_disk s = attach := (!attach_format, s) :: !attach in
 
-  let cache = ref default_cachedir in
+  let cache = ref Paths.xdg_cache_home in
   let set_cache arg = cache := Some arg in
   let no_cache () = cache := None in
 
diff --git a/builder/paths.ml b/builder/paths.ml
new file mode 100644
index 000..66e8922
--- /dev/null
+++ b/builder/paths.ml
@@ -0,0 +1,26 @@
+(* virt-builder
+ * Copyright (C) 2014 Red Hat Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ *)
+
+open Common_utils
+
+let xdg_cache_home =
+  try Some (Sys.getenv XDG_CACHE_HOME // virt-builder)
+  with Not_found -
+try Some (Sys.getenv HOME // .cache // virt-builder)
+with Not_found -
+  None (* no cache directory *)
diff --git a/po/POTFILES-ml b/po/POTFILES-ml
index 7937728..301fe81 100644
--- a/po/POTFILES-ml
+++ b/po/POTFILES-ml
@@ -4,6 +4,7 @@ builder/downloader.ml
 builder/get_kernel.ml
 builder/index_parser.ml
 builder/list_entries.ml
+builder/paths.ml
 builder/pxzcat.ml
 builder/setlocale.ml
 builder/sigchecker.ml
-- 
1.8.3.1

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

[Libguestfs] [PATCH] builder: accept also '_' in group names

2014-02-19 Thread Pino Toscano

---
 builder/index-scan.l | 2 +-
 builder/index-validate.c | 8 
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/builder/index-scan.l b/builder/index-scan.l
index b120590..832ea51 100644
--- a/builder/index-scan.l
+++ b/builder/index-scan.l
@@ -52,7 +52,7 @@ extern void yyerror (const char *);
 ^\n { return EMPTY_LINE; }
 
   /* [...] marks beginning of a section. */
-^[[-A-Za-z0-9.]+]\n {
+^[[-A-Za-z0-9._]+]\n {
   yylval.str = strndup (yytext+1, yyleng-3);
   return SECTION_HEADER;
 }
diff --git a/builder/index-validate.c b/builder/index-validate.c
index 26abaa8..7f02ffb 100644
--- a/builder/index-validate.c
+++ b/builder/index-validate.c
@@ -128,6 +128,14 @@ main (int argc, char *argv[])
 int seen_sig = 0;
 struct field *fields;
 
+if (compat_1_24_0) {
+  if (strchr (sections-name, '_')) {
+fprintf (stderr, _(%s: %s: section [%s] has invalid characters which 
will not work with virt-builder 1.24.0\n),
+ program_name, input, sections-name);
+exit (EXIT_FAILURE);
+  }
+}
+
 for (fields = sections-fields; fields != NULL; fields = fields-next) {
   if (compat_1_24_0) {
 if (strchr (fields-key, '[') ||
-- 
1.8.3.1

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

[Libguestfs] [PATCH 2/2] builder: use a disposable GPG keyring for every Sigchecker

2014-02-20 Thread Pino Toscano

Create a temporary directory and tell gpg to use it as homedir, so
imported keys do not get into the user's keyring. This also avoid
importing the default key when a different one is needed to check the
signature.

The only exception is when a non-default fingerprint is used: in this
case, that key is read from the user's keyring, since it is where it is.
---
 builder/Makefile.am   |  5 +
 builder/mkdtemp-c.c   | 56 ++
 builder/mkdtemp.ml| 19 
 builder/mkdtemp.mli   | 20 +
 builder/sigchecker.ml | 61 ---
 po/POTFILES   |  1 +
 po/POTFILES-ml|  1 +
 7 files changed, 150 insertions(+), 13 deletions(-)
 create mode 100644 builder/mkdtemp-c.c
 create mode 100644 builder/mkdtemp.ml
 create mode 100644 builder/mkdtemp.mli

diff --git a/builder/Makefile.am b/builder/Makefile.am
index 23336c0..d668377 100644
--- a/builder/Makefile.am
+++ b/builder/Makefile.am
@@ -48,6 +48,9 @@ SOURCES = \
index-parser-c.c \
list_entries.mli \
list_entries.ml \
+   mkdtemp.mli \
+   mkdtemp.ml \
+   mkdtemp-c.c \
paths.ml \
pxzcat.ml \
pxzcat.mli \
@@ -92,6 +95,8 @@ OBJECTS = \
pxzcat.cmx \
setlocale-c.o \
setlocale.cmx \
+   mkdtemp-c.o \
+   mkdtemp.cmx \
paths.cmx \
get_kernel.cmx \
downloader.cmx \
diff --git a/builder/mkdtemp-c.c b/builder/mkdtemp-c.c
new file mode 100644
index 000..2284e3a
--- /dev/null
+++ b/builder/mkdtemp-c.c
@@ -0,0 +1,56 @@
+/* virt-builder
+ * Copyright (C) 2014 Red Hat Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 
USA.
+ */
+
+#include config.h
+
+#include stdlib.h
+#include errno.h
+#include string.h
+
+#include caml/alloc.h
+#include caml/fail.h
+#include caml/memory.h
+#include caml/mlvalues.h
+
+#ifdef HAVE_CAML_UNIXSUPPORT_H
+#include caml/unixsupport.h
+#else
+#define Nothing ((value) 0)
+extern void unix_error (int errcode, char * cmdname, value arg) Noreturn;
+#endif
+
+value
+virt_builder_mkdtemp (value val_pattern)
+{
+  CAMLparam1 (val_pattern);
+  CAMLlocal1 (rv);
+  char *pattern, *ret;
+
+  pattern = strdup (String_val (val_pattern));
+  if (pattern == NULL)
+unix_error (errno, (char *) strdup, val_pattern);
+
+  ret = mkdtemp (pattern);
+  if (ret == NULL)
+unix_error (errno, (char *) mkdtemp, val_pattern);
+
+  rv = caml_copy_string (ret);
+  free (pattern);
+
+  CAMLreturn (rv);
+}
diff --git a/builder/mkdtemp.ml b/builder/mkdtemp.ml
new file mode 100644
index 000..2e64862
--- /dev/null
+++ b/builder/mkdtemp.ml
@@ -0,0 +1,19 @@
+(* virt-builder
+ * Copyright (C) 2014 Red Hat Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ *)
+
+external mkdtemp : string - string = virt_builder_mkdtemp
diff --git a/builder/mkdtemp.mli b/builder/mkdtemp.mli
new file mode 100644
index 000..674e6f2
--- /dev/null
+++ b/builder/mkdtemp.mli
@@ -0,0 +1,20 @@
+(* virt-builder
+ * Copyright (C) 2014 Red Hat Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General

[Libguestfs] [PATCH 1/2] mllib: add an hook to cleanup directories on exit

2014-02-20 Thread Pino Toscano

Much similar to unlink_on_exit, but recursively cleaning directories.
---
 mllib/common_utils.ml | 29 +
 1 file changed, 29 insertions(+)

diff --git a/mllib/common_utils.ml b/mllib/common_utils.ml
index 3943417..f49ede6 100644
--- a/mllib/common_utils.ml
+++ b/mllib/common_utils.ml
@@ -386,6 +386,35 @@ let unlink_on_exit =
   registered_handlers := true
 )
 
+(* Remove a temporary directory on exit. *)
+let rmdir_on_exit =
+  let dirs = ref [] in
+  let registered_handlers = ref false in
+
+  let rec unlink_dirs () =
+let rec recursive_rmdir fn =
+  if Sys.is_directory fn then (
+let names = Array.map (fun d - fn // d) (Sys.readdir fn) in
+Array.iter recursive_rmdir names;
+Unix.rmdir fn
+  ) else
+Unix.unlink fn
+in
+List.iter (
+  fun dir - try recursive_rmdir dir with _ - ()
+) !dirs
+  and register_handlers () =
+(* Remove on exit. *)
+at_exit unlink_dirs
+  in
+
+  fun dir -
+dirs := dir :: !dirs;
+if not !registered_handlers then (
+  register_handlers ();
+  registered_handlers := true
+)
+
 (* Using the libguestfs API, recursively remove only files from the
  * given directory.  Useful for cleaning /var/cache etc in sysprep
  * without removing the actual directory structure.  Also if 'dir' is
-- 
1.8.3.1

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

Re: [Libguestfs] [PATCH 1/2] mllib: add an hook to cleanup directories on exit

2014-02-20 Thread Pino Toscano

On Thursday 20 February 2014 13:08:40 Richard W.M. Jones wrote:
 On Thu, Feb 20, 2014 at 11:53:16AM +0100, Pino Toscano wrote:
  Much similar to unlink_on_exit, but recursively cleaning
  directories.
  ---
  
   mllib/common_utils.ml | 29 +
   1 file changed, 29 insertions(+)
  
  diff --git a/mllib/common_utils.ml b/mllib/common_utils.ml
  index 3943417..f49ede6 100644
  --- a/mllib/common_utils.ml
  +++ b/mllib/common_utils.ml
  @@ -386,6 +386,35 @@ let unlink_on_exit =
  
 registered_handlers := true
   
   )
  
  +(* Remove a temporary directory on exit. *)
  +let rmdir_on_exit =
  +  let dirs = ref [] in
  +  let registered_handlers = ref false in
  +
  +  let rec unlink_dirs () =
  +let rec recursive_rmdir fn =
  +  if Sys.is_directory fn then (
 
 I suspect this will follow symlinks, so that if the temporary
 directory contains a link like:
 
   /tmp/tmpdir/foo - /
 
 it will proceed to wipe out other bits of your filesystem.

Uh duh, sorry for the oversight. Fixed it checking the file type using 
lstat.

 Especially line 105 ff. but the rest of the function may be
 interesting too in the context of patch 2/2.

Hm, it is kind of duplicating what mkdtemp already does, so I would 
rather keep using it. Pity OCaml does not offer it at all...

-- 
Pino Toscano

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

[Libguestfs] [PATCH 1/2] mllib: add an hook to cleanup directories on exit

2014-02-20 Thread Pino Toscano

Much similar to unlink_on_exit, but recursively cleaning directories.
---
 mllib/common_utils.ml | 35 +++
 1 file changed, 35 insertions(+)

diff --git a/mllib/common_utils.ml b/mllib/common_utils.ml
index 3943417..d02a2d3 100644
--- a/mllib/common_utils.ml
+++ b/mllib/common_utils.ml
@@ -386,6 +386,41 @@ let unlink_on_exit =
   registered_handlers := true
 )
 
+(* Remove a temporary directory on exit. *)
+let rmdir_on_exit =
+  let dirs = ref [] in
+  let registered_handlers = ref false in
+
+  let rec unlink_dirs () =
+let rec recursive_rmdir fn =
+  match (Unix.lstat fn).Unix.st_kind with
+  | Unix.S_DIR -
+let names = Array.map (fun d - fn // d) (Sys.readdir fn) in
+Array.iter recursive_rmdir names;
+Unix.rmdir fn
+  | Unix.S_REG
+  | Unix.S_CHR
+  | Unix.S_BLK
+  | Unix.S_LNK
+  | Unix.S_FIFO
+  | Unix.S_SOCK -
+Unix.unlink fn
+in
+List.iter (
+  fun dir - try recursive_rmdir dir with _ - ()
+) !dirs
+  and register_handlers () =
+(* Remove on exit. *)
+at_exit unlink_dirs
+  in
+
+  fun dir -
+dirs := dir :: !dirs;
+if not !registered_handlers then (
+  register_handlers ();
+  registered_handlers := true
+)
+
 (* Using the libguestfs API, recursively remove only files from the
  * given directory.  Useful for cleaning /var/cache etc in sysprep
  * without removing the actual directory structure.  Also if 'dir' is
-- 
1.8.3.1

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

[Libguestfs] [PATCH 1/2] mllib: add an hook to cleanup directories on exit

2014-02-20 Thread Pino Toscano

Much similar to unlink_on_exit, but recursively cleaning directories.
---
 mllib/common_utils.ml | 23 +++
 1 file changed, 23 insertions(+)

diff --git a/mllib/common_utils.ml b/mllib/common_utils.ml
index 3943417..de3bd40 100644
--- a/mllib/common_utils.ml
+++ b/mllib/common_utils.ml
@@ -386,6 +386,29 @@ let unlink_on_exit =
   registered_handlers := true
 )
 
+(* Remove a temporary directory on exit. *)
+let rmdir_on_exit =
+  let dirs = ref [] in
+  let registered_handlers = ref false in
+
+  let rec rmdirs () =
+List.iter (
+  fun dir -
+let cmd = sprintf rm -rf %s (Filename.quote dir) in
+ignore (Sys.command cmd)
+) !dirs
+  and register_handlers () =
+(* Remove on exit. *)
+at_exit rmdirs
+  in
+
+  fun dir -
+dirs := dir :: !dirs;
+if not !registered_handlers then (
+  register_handlers ();
+  registered_handlers := true
+)
+
 (* Using the libguestfs API, recursively remove only files from the
  * given directory.  Useful for cleaning /var/cache etc in sysprep
  * without removing the actual directory structure.  Also if 'dir' is
-- 
1.8.3.1

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

[Libguestfs] [PATCH] builder: allow Sigchecker to import keys from file

2014-02-21 Thread Pino Toscano

Extend Sigchecker so it allows both fingerprints (to be imported from
user's keyring, as before) and keys stored in files. To simplify this
process (and have the fingerprint always around), the key is imported
on Sigchecker.create time, instead of lazily at the first verification.
---
 builder/builder.ml |   3 +-
 builder/sigchecker.ml  | 120 -
 builder/sigchecker.mli |   6 ++-
 3 files changed, 76 insertions(+), 53 deletions(-)

diff --git a/builder/builder.ml b/builder/builder.ml
index 7fa3cc7..80ccef7 100644
--- a/builder/builder.ml
+++ b/builder/builder.ml
@@ -141,7 +141,8 @@ let main () =
   List.map (
 fun (source, fingerprint) -
   let sigchecker =
-Sigchecker.create ~debug ~gpg ~fingerprint ~check_signature in
+Sigchecker.create ~debug ~gpg ~check_signature
+  ~gpgkey:(Sigchecker.Fingerprint fingerprint) in
   Index_parser.get_index ~prog ~debug ~downloader ~sigchecker source
   ) sources
 ) in
diff --git a/builder/sigchecker.ml b/builder/sigchecker.ml
index 10e342e..7459e4b 100644
--- a/builder/sigchecker.ml
+++ b/builder/sigchecker.ml
@@ -96,41 +96,97 @@ ZvXkQ3FVJwZoLmHw47vvlVpLD/4gi1SuHWieRvZ+UdDq00E348pm
 -END PGP PUBLIC KEY BLOCK-
 
 
+type gpgkey_type =
+  | Fingerprint of string
+  | KeyFile of string
+
 type t = {
   debug : bool;
   gpg : string;
   fingerprint : string;
   check_signature : bool;
   gpghome : string;
-  mutable key_imported : bool;
 }
 
-let create ~debug ~gpg ~fingerprint ~check_signature =
-  (* Create a temporary directory for gnupg. *)
-  let tmpdir = Mkdtemp.mkdtemp (Filename.temp_dir_name // vb.gpghome.XX) 
in
-  rmdir_on_exit tmpdir;
-  (* Run gpg once, so it can setup its own home directory, failing
-   * if it cannot.
-   *)
-  let cmd = sprintf %s --homedir %s --list-keys%s
-gpg tmpdir (if debug then  else  /dev/null 21) in
+(* Import the specified key file. *)
+let import_keyfile ~gpg ~gpghome ~debug keyfile =
+  let status_file = Filename.temp_file vbstat .txt in
+  unlink_on_exit status_file;
+  let cmd = sprintf %s --homedir %s --status-file %s --import %s%s
+gpg gpghome (quote status_file) (quote keyfile)
+(if debug then  else  /dev/null 21) in
   if debug then eprintf %s\n%! cmd;
   let r = Sys.command cmd in
   if r  0 then (
-eprintf (f_virt-builder: error: GPG failure: could not run GPG the first 
time\nUse the '-v' option and look for earlier error messages.\n);
+eprintf (f_virt-builder: error: could not import public key\nUse the '-v' 
option and look for earlier error messages.\n);
 exit 1
   );
+  status_file
+
+let rec create ~debug ~gpg ~gpgkey ~check_signature =
+  (* Create a temporary directory for gnupg. *)
+  let tmpdir = Mkdtemp.mkdtemp (Filename.temp_dir_name // vb.gpghome.XX) 
in
+  rmdir_on_exit tmpdir;
+  let fingerprint =
+if check_signature then (
+  (* Run gpg so it can setup its own home directory, failing if it
+   * cannot.
+   *)
+  let cmd = sprintf %s --homedir %s --list-keys%s
+gpg tmpdir (if debug then  else  /dev/null 21) in
+  if debug then eprintf %s\n%! cmd;
+  let r = Sys.command cmd in
+  if r  0 then (
+eprintf (f_virt-builder: error: GPG failure: could not run GPG the 
first time\nUse the '-v' option and look for earlier error messages.\n);
+exit 1
+  );
+  match gpgkey with
+  | KeyFile kf -
+let status_file = import_keyfile gpg tmpdir debug kf in
+let status = read_whole_file status_file in
+let status = string_nsplit \n status in
+let fingerprint = ref  in
+List.iter (
+  fun line -
+let line = string_nsplit   line in
+match line with
+| [GNUPG:] :: IMPORT_OK :: _ :: fp :: _ - fingerprint := fp
+| _ - ()
+) status;
+!fingerprint
+  | Fingerprint fp when equal_fingerprints default_fingerprint fp -
+let filename, chan = Filename.open_temp_file vbpubkey .asc in
+unlink_on_exit filename;
+output_string chan default_pubkey;
+close_out chan;
+ignore (import_keyfile gpg tmpdir debug filename);
+fp
+  | Fingerprint fp -
+let filename = Filename.temp_file vbpubkey .asc in
+unlink_on_exit filename;
+let cmd = sprintf %s --yes --armor --output %s --export %s%s
+  gpg (quote filename) (quote fp)
+  (if debug then  else  /dev/null 21) in
+if debug then eprintf %s\n%! cmd;
+let r = Sys.command cmd in
+if r  0 then (
+  eprintf (f_virt-builder: error: could not export public key\nUse 
the '-v' option and look for earlier error messages.\n);
+  exit 1
+);
+ignore (import_keyfile gpg tmpdir debug filename);
+fp
+) else
+   in
   {
 debug = debug;
 gpg = gpg;
 fingerprint = fingerprint;
 check_signature =

[Libguestfs] [PATCH] builder: add an arch field to sources read from indexes

2014-02-21 Thread Pino Toscano

Add an architecture field for all the entries in each index, so we know
which architecture they are (not used right now, but will be in the
future).

The problematic part here is properly marking with the correct
architecture: since we only know the current index on libguestfs.org
contains x86_64/amd64 images, entries coming from it are marked that
way; images in all the other indexes (user-provided ones) are not known
to us, so assume they are using the same architecture as virt-builder,
hence the special @same.
---
 builder/builder.ml   |  5 +++--
 builder/cmdline.ml   | 16 +---
 builder/index_parser.ml  |  6 +-
 builder/index_parser.mli |  3 ++-
 4 files changed, 19 insertions(+), 11 deletions(-)

diff --git a/builder/builder.ml b/builder/builder.ml
index 80ccef7..d6d7570 100644
--- a/builder/builder.ml
+++ b/builder/builder.ml
@@ -42,7 +42,7 @@ let main () =
 edit, firstboot, run, format, gpg, hostname, install, list_format, links,
 memsize, mkdirs,
 network, output, password_crypto, quiet, root_password, scrub,
-scrub_logfile, selinux_relabel, size, smp, sources, sync, timezone,
+scrub_logfile, selinux_relabel, size, smp, sources, indexarch, sync, 
timezone,
 update, upload, writes =
 parse_cmdline () in
 
@@ -143,7 +143,8 @@ let main () =
   let sigchecker =
 Sigchecker.create ~debug ~gpg ~check_signature
   ~gpgkey:(Sigchecker.Fingerprint fingerprint) in
-  Index_parser.get_index ~prog ~debug ~downloader ~sigchecker source
+  Index_parser.get_index ~prog ~debug ~downloader ~sigchecker
+~arch:indexarch source
   ) sources
 ) in
 
diff --git a/builder/cmdline.ml b/builder/cmdline.ml
index e9e47ae..8959429 100644
--- a/builder/cmdline.ml
+++ b/builder/cmdline.ml
@@ -31,6 +31,8 @@ open Printf
 let prog = Filename.basename Sys.executable_name
 
 let default_source = http://libguestfs.org/download/builder/index.asc;
+(* So far images in the default_source index were x86_64/amd64. *)
+let default_source_architecture = x86_64
 
 let parse_cmdline () =
   let display_version () =
@@ -408,18 +410,18 @@ read the man page virt-builder(1).
   ) in
 
   (* Check source(s) and fingerprint(s), or use environment or default. *)
-  let sources =
+  let sources, indexarch =
 let list_split = function  - [] | str - string_nsplit , str in
 let rec repeat x = function
   | 0 - [] | 1 - [x]
   | n - x :: repeat x (n-1)
 in
 
-let sources =
-  if sources  [] then sources
+let sources, indexarch =
+  if sources  [] then sources, @same
   else (
-try list_split (Sys.getenv VIRT_BUILDER_SOURCE)
-with Not_found - [ default_source ]
+try list_split (Sys.getenv VIRT_BUILDER_SOURCE), @same
+with Not_found - [ default_source ], default_source_architecture
   ) in
 let fingerprints =
   if fingerprints  [] then fingerprints
@@ -447,12 +449,12 @@ read the man page virt-builder(1).
 assert (nr_sources  0);
 
 (* Combine the sources and fingerprints into a single list of pairs. *)
-List.combine sources fingerprints in
+List.combine sources fingerprints, indexarch in
 
   mode, arg,
   attach, cache, check_signature, curl, debug, delete, delete_on_failure,
   edit, firstboot, run, format, gpg, hostname, install, list_format, links,
   memsize, mkdirs,
   network, output, password_crypto, quiet, root_password, scrub,
-  scrub_logfile, selinux_relabel, size, smp, sources, sync, timezone,
+  scrub_logfile, selinux_relabel, size, smp, sources, indexarch, sync, 
timezone,
   update, upload, writes
diff --git a/builder/index_parser.ml b/builder/index_parser.ml
index 2d4a642..0b9a1b9 100644
--- a/builder/index_parser.ml
+++ b/builder/index_parser.ml
@@ -37,6 +37,7 @@ and entry = {
   lvexpand : string option;
   notes : (string * string) list;
   hidden : bool;
+  arch : string;
 
   sigchecker : Sigchecker.t;
 }
@@ -53,6 +54,7 @@ let print_entry chan (name, { printable_name = printable_name;
   expand = expand;
   lvexpand = lvexpand;
   notes = notes;
+  arch = arch;
   hidden = hidden }) =
   let fp fs = fprintf chan fs in
   fp [%s]\n name;
@@ -91,6 +93,7 @@ let print_entry chan (name, { printable_name = printable_name;
   | None - ()
   | Some lvexpand - fp lvexpand=%s\n lvexpand
   );
+  fp arch=%s\n arch;
   List.iter (
 fun (lang, notes) -
   match lang with
@@ -108,7 +111,7 @@ and field = string * string option * string(* key + 
subkey + value *)
 (* Calls yyparse in the C code. *)
 external parse_index : string - sections = virt_builder_parse_index
 
-let get_index ~prog ~debug ~downloader ~sigchecker source =
+let get_index ~prog ~debug ~downloader ~sigchecker ~arch source =
   let corrupt_file () =
 eprintf (f_\nThe index file downloaded from '%s' is

Re: [Libguestfs] [PATCH] builder: add an arch field to sources read from indexes

2014-02-21 Thread Pino Toscano

On Friday 21 February 2014 16:29:23 Richard W.M. Jones wrote:
 On Fri, Feb 21, 2014 at 05:17:59PM +0100, Pino Toscano wrote:
  Add an architecture field for all the entries in each index, so we
  know which architecture they are (not used right now, but will be
  in the future).
  
  The problematic part here is properly marking with the correct
  architecture: since we only know the current index on libguestfs.org
  contains x86_64/amd64 images, entries coming from it are marked that
  way; images in all the other indexes (user-provided ones) are not
  known to us, so assume they are using the same architecture as
  virt-builder, hence the special @same.
 
 A few questions:
 
  - Would it be better/easier if we ditched the whole idea of
VIRT_BUILDER_SOURCE being a list?  It wasn't really a great idea in
 hindsight, and we could change it now.  It's even possible to not
 have VIRT_BUILDER_SOURCE at all, everything is configured through the
 config files.

I was following the keep compatibility principle we have so far in 
libguestfs, so part of the current patch was also in that sense. (Of 
course the additions of arch to Index_parser would still apply.)
Ditching VIRT_BUILDER_SOURCE (and VIRT_BUILDER_FINGERPRINT with it) 
would be okay for me as well, that could indeed simplify some stuff that 
I'm currently working on.

  - Can we force people to put an arch= field in the index, and if it
is missing, assume x86_64?

My idea/implementation so far had arch= in the .conf files: this way, 
one could in principle keep an existing (optionally signed) index 
somewhere, adding a .conf file pointing to it, and describing which 
architecture would have the images in that index.
Hmm, now that I think about it,
- adding arch= in indexes is backward compatible
- an user-supplied index could also have images with different 
  architectures
so I will move arch= to indexes.

Regarding requiring arch= in indexes: this would mean users providing 
indexes right now would need to update them (and re-sign, in case) to 
have them working with future virt-builder 1.26. While not a big deal, 
it would prevent the ship this .conf file pointing to my index to work 
OOTB.
Having arch= optional (with implicit @same) would not be a big deal 
either, at least IMHO.

-- 
Pino Toscano

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

[Libguestfs] [PATCH] builder: add a mandatory 'arch' key in index files

2014-02-24 Thread Pino Toscano

Introduce a mandatory arch= key in all the entries of index files, to
identify which architecture is each. Adapt the long and JSON list
outputs to print also this new field.

This introduces an incompatibility with index files created with
virt-builder  1.26, as they will be rejected until entries will have
the arch= key added (which is ignored by older virt-builder, so adding
it will not create backward-compatibility issues).
---
 builder/index_parser.ml   |  9 +
 builder/index_parser.mli  |  1 +
 builder/list_entries.ml   |  4 
 builder/test-index.in |  7 +++
 builder/test-virt-builder-list.sh | 14 ++
 5 files changed, 35 insertions(+)

diff --git a/builder/index_parser.ml b/builder/index_parser.ml
index 2d4a642..de4d72e 100644
--- a/builder/index_parser.ml
+++ b/builder/index_parser.ml
@@ -27,6 +27,7 @@ and entry = {
   printable_name : string option;   (* the name= field *)
   osinfo : string option;
   file_uri : string;
+  arch : string;
   signature_uri : string option;(* deprecated, will be removed in 1.26 
*)
   checksum_sha512 : string option;
   revision : int;
@@ -43,6 +44,7 @@ and entry = {
 
 let print_entry chan (name, { printable_name = printable_name;
   file_uri = file_uri;
+  arch = arch;
   osinfo = osinfo;
   signature_uri = signature_uri;
   checksum_sha512 = checksum_sha512;
@@ -65,6 +67,7 @@ let print_entry chan (name, { printable_name = printable_name;
   | Some id - fp osinfo=%s\n id
   );
   fp file=%s\n file_uri;
+  fp arch=%s\n arch;
   (match signature_uri with
   | None - ()
   | Some uri - fp sig=%s\n uri
@@ -179,6 +182,11 @@ let get_index ~prog ~debug ~downloader ~sigchecker source =
 with Not_found -
   eprintf (f_virt-builder: no 'file' (URI) entry for '%s'\n) n;
 corrupt_file () in
+  let arch =
+try List.assoc (arch, None) fields
+with Not_found -
+  eprintf (f_virt-builder: no 'arch' entry for '%s'\n) n;
+corrupt_file () in
   let signature_uri =
 try Some (make_absolute_uri (List.assoc (sig, None) fields))
 with Not_found - None in
@@ -245,6 +253,7 @@ let get_index ~prog ~debug ~downloader ~sigchecker source =
   let entry = { printable_name = printable_name;
 osinfo = osinfo;
 file_uri = file_uri;
+arch = arch;
 signature_uri = signature_uri;
 checksum_sha512 = checksum_sha512;
 revision = revision;
diff --git a/builder/index_parser.mli b/builder/index_parser.mli
index 3c679b3..0575dc4 100644
--- a/builder/index_parser.mli
+++ b/builder/index_parser.mli
@@ -21,6 +21,7 @@ and entry = {
   printable_name : string option;   (* the name= field *)
   osinfo : string option;
   file_uri : string;
+  arch : string;
   signature_uri : string option;(* deprecated, will be removed in 1.26 
*)
   checksum_sha512 : string option;
   revision : int;
diff --git a/builder/list_entries.ml b/builder/list_entries.ml
index 27ea95e..edf7dfb 100644
--- a/builder/list_entries.ml
+++ b/builder/list_entries.ml
@@ -73,6 +73,7 @@ and list_entries_long ~sources index =
 
   List.iter (
 fun (name, { Index_parser.printable_name = printable_name;
+ arch = arch;
  size = size;
  compressed_size = compressed_size;
  notes = notes;
@@ -83,6 +84,7 @@ and list_entries_long ~sources index =
 | None - ()
 | Some name - printf %-24s %s\n (s_Full name:) name;
 );
+printf %-24s %s\n (s_Architecture:) arch;
 printf %-24s %s\n (s_Minimum/default size:) (human_size size);
 (match compressed_size with
 | None - ()
@@ -168,6 +170,7 @@ and list_entries_json ~sources index =
   printf   \templates\: [\n;
   iteri (
 fun i (name, { Index_parser.printable_name = printable_name;
+   arch = arch;
size = size;
compressed_size = compressed_size;
notes = notes;
@@ -175,6 +178,7 @@ and list_entries_json ~sources index =
   printf   {\n;
   printf \os-version\: \%s\,\n name;
   json_optional_printf_string full-name printable_name;
+  printf \arch\: \%s\,\n arch;
   printf \size\: %Ld,\n size;
   json_optional_printf_int64 compressed-size compressed_size;
   print_notes notes;
diff --git a/builder/test-index.in b/builder/test-index.in
index 1bca6b8..3efebc4 100644
--- a/builder/test-index.in
+++ b/builder/test-index.in
@@ -1,5 +1,6 @@
 [phony-debian]
 name=Phony Debian
+arch=x86_64
 file=debian.xz
 format=raw
 size=536870912
@@ -9,6 +10,7 @@ notes=Phony Debian look-alike used for

[Libguestfs] [PATCH] sysprep: use Mkdtemp to create the temporary directory

2014-02-24 Thread Pino Toscano

Use the safer mkdtemp instead of manually creating a path.
---
 sysprep/Makefile.am | 2 ++
 sysprep/sysprep_operation_script.ml | 4 +---
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/sysprep/Makefile.am b/sysprep/Makefile.am
index 2600477..9c9ab14 100644
--- a/sysprep/Makefile.am
+++ b/sysprep/Makefile.am
@@ -100,6 +100,8 @@ OBJECTS = \
$(top_builddir)/mllib/timezone.cmx \
$(top_builddir)/mllib/firstboot.cmx \
$(top_builddir)/mllib/config.cmx \
+   $(top_builddir)/mllib/mkdtemp-c.o \
+   $(top_builddir)/mllib/mkdtemp.cmx \
sysprep_operation.cmx \
$(patsubst %,sysprep_operation_%.cmx,$(operations)) \
main.cmx
diff --git a/sysprep/sysprep_operation_script.ml 
b/sysprep/sysprep_operation_script.ml
index 518207e..614910f 100644
--- a/sysprep/sysprep_operation_script.ml
+++ b/sysprep/sysprep_operation_script.ml
@@ -44,9 +44,7 @@ let rec script_perform (g : Guestfs.guestfs) root 
side_effects =
   match !scriptdir with
   | Some dir - dir, false
   | None -
-let tmpdir = Filename.temp_dir_name in
-let tmpdir = tmpdir // string_random8 () in
-mkdir tmpdir 0o755;
+let tmpdir = Mkdtemp.mkdtemp (Filename.temp_dir_name // 
virt-sysprep.XX) in
 tmpdir, true in
 
 (* Mount the directory locally. *)
-- 
1.8.3.1

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

[Libguestfs] [PATCH] builder: split INI C - OCaml glue code in own module

2014-02-24 Thread Pino Toscano

Move in an own module the code which calls the C
virt_builder_parse_index and does the array - list conversion of the
result. This way this code can be easily called also in places different
than Index_parser without the need to copy the types mapping, etc.

Just code motion, no actual behaviour changes.
---
 builder/Makefile.am |  3 +++
 builder/index_parser.ml | 17 +
 builder/ini_reader.ml   | 38 ++
 builder/ini_reader.mli  | 24 
 po/POTFILES-ml  |  1 +
 5 files changed, 67 insertions(+), 16 deletions(-)
 create mode 100644 builder/ini_reader.ml
 create mode 100644 builder/ini_reader.mli

diff --git a/builder/Makefile.am b/builder/Makefile.am
index 8c48be5..a72b7ac 100644
--- a/builder/Makefile.am
+++ b/builder/Makefile.am
@@ -46,6 +46,8 @@ SOURCES = \
index_parser.mli \
index_parser.ml \
index-parser-c.c \
+   ini_reader.mli \
+   ini_reader.ml \
list_entries.mli \
list_entries.ml \
paths.ml \
@@ -94,6 +96,7 @@ OBJECTS = \
pxzcat.cmx \
setlocale-c.o \
setlocale.cmx \
+   ini_reader.cmx \
paths.cmx \
get_kernel.cmx \
downloader.cmx \
diff --git a/builder/index_parser.ml b/builder/index_parser.ml
index de4d72e..9fd9cda 100644
--- a/builder/index_parser.ml
+++ b/builder/index_parser.ml
@@ -102,15 +102,6 @@ let print_entry chan (name, { printable_name = 
printable_name;
   ) notes;
   if hidden then fp hidden=true\n
 
-(* Types returned by the C index parser. *)
-type sections = section array
-and section = string * fields   (* [name] + fields *)
-and fields = field array
-and field = string * string option * string(* key + subkey + value *)
-
-(* Calls yyparse in the C code. *)
-external parse_index : string - sections = virt_builder_parse_index
-
 let get_index ~prog ~debug ~downloader ~sigchecker source =
   let corrupt_file () =
 eprintf (f_\nThe index file downloaded from '%s' is corrupt.\nYou need to 
ask the supplier of this file to fix it and upload a fixed version.\n)
@@ -128,16 +119,10 @@ let get_index ~prog ~debug ~downloader ~sigchecker source 
=
 Sigchecker.verify sigchecker tmpfile;
 
 (* Try parsing the file. *)
-let sections = parse_index tmpfile in
+let sections = Ini_reader.read_ini tmpfile in
 if delete_tmpfile then
   (try Unix.unlink tmpfile with _ - ());
 
-let sections = Array.to_list sections in
-let sections = List.map (
-  fun (n, fields) -
-n, Array.to_list fields
-) sections in
-
 (* Check for repeated os-version names. *)
 let nseen = Hashtbl.create 13 in
 List.iter (
diff --git a/builder/ini_reader.ml b/builder/ini_reader.ml
new file mode 100644
index 000..fbd4d2f
--- /dev/null
+++ b/builder/ini_reader.ml
@@ -0,0 +1,38 @@
+(* virt-builder
+ * Copyright (C) 2013-2014 Red Hat Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ *)
+
+type sections = section list
+and section = string * fields(* [name] + fields *)
+and fields = field list
+and field = string * string option * string  (* key + subkey + value *)
+
+(* Types returned by the C index parser. *)
+type c_sections = c_section array
+and c_section = string * c_fields (* [name] + fields *)
+and c_fields = field array
+
+(* Calls yyparse in the C code. *)
+external parse_index : string - c_sections = virt_builder_parse_index
+
+let read_ini file =
+  let sections = parse_index file in
+  let sections = Array.to_list sections in
+  List.map (
+fun (n, fields) -
+  n, Array.to_list fields
+  ) sections
diff --git a/builder/ini_reader.mli b/builder/ini_reader.mli
new file mode 100644
index 000..992a1cb
--- /dev/null
+++ b/builder/ini_reader.mli
@@ -0,0 +1,24 @@
+(* virt-builder
+ * Copyright (C) 2013-2014 Red Hat Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS

Re: [Libguestfs] [PATCH] builder: add a mandatory 'arch' key in index files

2014-02-24 Thread Pino Toscano

On Monday 24 February 2014 15:15:11 Richard W.M. Jones wrote:
 On Mon, Feb 24, 2014 at 11:36:29AM +0100, Pino Toscano wrote:
  Introduce a mandatory arch= key in all the entries of index files,
  to
  identify which architecture is each. Adapt the long and JSON list
  outputs to print also this new field.
  
  This introduces an incompatibility with index files created with
  virt-builder  1.26, as they will be rejected until entries will
  have
  the arch= key added (which is ignored by older virt-builder, so
  adding it will not create backward-compatibility issues).
  ---
  
   builder/index_parser.ml   |  9 +
   builder/index_parser.mli  |  1 +
   builder/list_entries.ml   |  4 
   builder/test-index.in |  7 +++
   builder/test-virt-builder-list.sh | 14 ++
   5 files changed, 35 insertions(+)
  
  diff --git a/builder/index_parser.ml b/builder/index_parser.ml
  index 2d4a642..de4d72e 100644
  --- a/builder/index_parser.ml
  +++ b/builder/index_parser.ml
  @@ -27,6 +27,7 @@ and entry = {
  
 printable_name : string option;   (* the name= field *)
 osinfo : string option;
 file_uri : string;
  
  +  arch : string;
  
 signature_uri : string option;(* deprecated, will be
 removed in 1.26 *) checksum_sha512 : string option;
 revision : int;
  
  @@ -43,6 +44,7 @@ and entry = {
  
   let print_entry chan (name, { printable_name = printable_name;
   
 file_uri = file_uri;
  
  +  arch = arch;
  
 osinfo = osinfo;
 signature_uri = signature_uri;
 checksum_sha512 = checksum_sha512;
  
  @@ -65,6 +67,7 @@ let print_entry chan (name, { printable_name =
  printable_name; 
 | Some id - fp osinfo=%s\n id
 
 );
 fp file=%s\n file_uri;
  
  +  fp arch=%s\n arch;
  
 (match signature_uri with
 
 | None - ()
 | Some uri - fp sig=%s\n uri
  
  @@ -179,6 +182,11 @@ let get_index ~prog ~debug ~downloader
  ~sigchecker source = 
   with Not_found -
   
 eprintf (f_virt-builder: no 'file' (URI) entry for
 '%s'\n) n;
   
   corrupt_file () in
  
  +  let arch =
  +try List.assoc (arch, None) fields
  +with Not_found -
  +  eprintf (f_virt-builder: no 'arch' entry for
  '%s'\n) n; +corrupt_file () in
  
 let signature_uri =
 
   try Some (make_absolute_uri (List.assoc (sig, None)
   fields))
   with Not_found - None in
  
  @@ -245,6 +253,7 @@ let get_index ~prog ~debug ~downloader
  ~sigchecker source = 
 let entry = { printable_name = printable_name;
 
   osinfo = osinfo;
   file_uri = file_uri;
  
  +arch = arch;
  
   signature_uri = signature_uri;
   checksum_sha512 = checksum_sha512;
   revision = revision;
  
  diff --git a/builder/index_parser.mli b/builder/index_parser.mli
  index 3c679b3..0575dc4 100644
  --- a/builder/index_parser.mli
  +++ b/builder/index_parser.mli
  @@ -21,6 +21,7 @@ and entry = {
  
 printable_name : string option;   (* the name= field *)
 osinfo : string option;
 file_uri : string;
  
  +  arch : string;
  
 signature_uri : string option;(* deprecated, will be
 removed in 1.26 *) checksum_sha512 : string option;
 revision : int;
  
  diff --git a/builder/list_entries.ml b/builder/list_entries.ml
  index 27ea95e..edf7dfb 100644
  --- a/builder/list_entries.ml
  +++ b/builder/list_entries.ml
  @@ -73,6 +73,7 @@ and list_entries_long ~sources index =
  
 List.iter (
 
   fun (name, { Index_parser.printable_name = printable_name;
  
  + arch = arch;
  
size = size;
compressed_size = compressed_size;
notes = notes;
  
  @@ -83,6 +84,7 @@ and list_entries_long ~sources index =
  
   | None - ()
   | Some name - printf %-24s %s\n (s_Full name:) name;
   
   );
  
  +printf %-24s %s\n (s_Architecture:) arch;
  
   printf %-24s %s\n (s_Minimum/default size:) (human_size
   size);
   (match compressed_size with
   
   | None - ()
  
  @@ -168,6 +170,7 @@ and list_entries_json ~sources index =
  
 printf   \templates\: [\n;
 iteri (
 
   fun i (name, { Index_parser.printable_name = printable_name;
  
  +   arch = arch;
  
  size = size;
  compressed_size = compressed_size;
  notes = notes;
  
  @@ -175,6 +178,7 @@ and list_entries_json ~sources index =
  
 printf

[Libguestfs] [PATCH 4/8] builder: extract the default key to file

2014-02-25 Thread Pino Toscano

This is basically default_pubkey from sigchecker.ml, just extracted as
file. Not used right now, but will be in the future.
---
 builder/libguestfs.gpg | 64 ++
 1 file changed, 64 insertions(+)
 create mode 100644 builder/libguestfs.gpg

diff --git a/builder/libguestfs.gpg b/builder/libguestfs.gpg
new file mode 100644
index 000..306a234
--- /dev/null
+++ b/builder/libguestfs.gpg
@@ -0,0 +1,64 @@
+-BEGIN PGP PUBLIC KEY BLOCK-
+Version: GnuPG v1.4.14 (GNU/Linux)
+
+mQINBE6UMMEBEADM811hfTulaF4JpkVpAI10FImyb4ArvOiu8NdcUwTFo+cyWno3
+U85B86H1Bsk/LgLTYtthSrTgsCtdxy+i5OaMjxZDIwKQ2+IYI3FCn9T3Mn28Idyh
+kLHzrO9ph0Dv0BNfrlDZhQEC53aAFe/QxN7+A49BNBV7D1VAOOCsHjxMEDzcZkCa
+oCrtXw1aNm2vkkj5ukbfukHAyLcQL7kow0qKPSVa1G4lfQP0WiG259Ydy+sUmbVb
+TGdb6MEC84PQRDuw6/ZeoV04tn7ZNtQEMOS0uiciHOGfr2hBxQf9VIPNrHg42yaL
+dOv51D99GuaxZ9E0HSoH/RwB1oXgd6rFdqVNYaBIQnnkwJANUEeGBArtIOZNCADT
+Bt8vkSDm+lLEAFS+V8CACyW/LMIrGCvLdHeqtoAv0GDVyR2GPxldYfdtEmCUMWcb
+Jlf71V9iAse2gUdoiHp5FfpGMkA5j7idKuxIws11XxRZJXXbBqiBqmVEAQ/v0m6p
+kdo0MYTHydmecLuUK2bAGhpysfX97EfTSrxfrYphYWjTfKRD9GrADeZNfuz1DbKs
+7LSqVaQJSjQrfgAwcnZLRaU0V4P5zxiz50gz1Aj3AZRL+Y3meZenzZTXcLFdnusg
+wUfhhCuL3tluMtEh6tznumyxb43WO1yLwj6J6LtveiuJN1Z+KSQ6OieZcwARAQAB
+tCVSaWNoYXJkIFcuTS4gSm9uZXMgPHJpY2hAYW5uZXhpYS5vcmc+iQI4BBMBAgAi
+BQJOlDDBAhsDBgsJCAcDAgYVCAIJCgsEFgIDAQIeAQIXgAAKCRCRc49z4bdooHQY
+D/wJLklSZNyXIW+rG5sUbg7j9cTIF5p/lB9kI2yx6KodJp/2knKyvnmzz0gBw/OE
+HL4E4UW26oWKo+36I8wkBnuGa6UtANeITcJqFE19VpHEXHsxre64jNQnO8/w748W
+1ROW+Ry43xmrlRWKuCm4oPYUzlp0fq9ATAne8eblfG+NOs8DYuA8xZNQzFaI2kDC
+QLD4YoXLoNsP27Koga36b0KwxPFD9tyVZiu9XDH/3hMN7Nb15B66PFr+HcMmQ67G
+nUIN5ulcIwj38i40cyaTs1VRheOzTHXE/a6Q2AhMKiKqOoEjQ73/mV7cAVoPtM3o
+83Q/8aVKBH0bVRwAeV1tju6b14fqKoG0zNBEcXdlSkht6ScxJYIc/LPUxAMDwgSE
+OWshjmeRzKXypBbHn/DP8QVyM2gk5wY+mMSH7MpR0p/hgj+rFO8H9L7pC4dCog3E
+qzrYhRN+TaP6MPH3WkOwPH4d4IfQRFnHp+VPYPijKEiLrUl/o8k3DyAanAPBpJ/x
+na4wXAjlFBctOq6g+SrCUiHpwk7b2YNwGgr5Vl3GmZELzK/G8gg3uJYKQ9Bpv16t
+WWOz+IFiOFa0UULeo0QPmFAIMZiDojNsY1SwBKB3ZL1YWZezgMdQAbpze/IXoSt7
+zxWJoKH2jK7q9mvFiaY12l2YnKuCcegWVAViLxRpBnrbz7QmUmljaGFyZCBXLk0u
+IEpvbmVzIDxyam9uZXNAcmVkaGF0LmNvbT6JAjgEEwECACIFAk6UOQsCGwMGCwkI
+BwMCBhUIAgkKCwQWAgMBAh4BAheAAAoJEJFzj3Pht2igIUYQAKomI0edLakahsUQ
+MxOZuhBbXJ4/VWF8bXYChDNPKvJp5nB7fBXujJ+39cIUM5fe2ViO6qSDpFC29imx
+F5pPbAqspZBPBkLLiZLji8R42hGarntdtTW0UWSBpq+nC5+G1psrnATI3uXGNxKQ
+R99c5HoMY7dBC2Y8TCGE64NINZ/XVh472s6IGLPn8MTn26YdRKC9BrVkCFMP2OBr
+6D4IprnyTAWAzb68ew20QmyWO+NBi9MplaDNQVl8PIOgfpyWlkgX1z9m67pcSDkw
+46hksp0yuOD1VwR4iVZ2/CmIsGRUlx41vWD6BIp9KxKyDIU1CYTRhq72dahHsl/8
+BjCndV5PO0GphqfCzmCv4DXjUwmrMTbH/GFnt5rfwcMcXUgcK0vV9vQ2SOU56Zd1
+fb27ZCFJKZc0Fu8krwFldCp/NYILf6ogUL/C1hfuCGSSuyDVY16Gg3dla1x+6zpF
+asnWQlaw8xT5LlMWvTZs5WsoSVHu7dVZWlgxINP++hlZrTz/S8l38yyQ15YFFl3W
+9M7dzkegOeDTPfx6B89WgfvfJjA/D0/FYxxWPXEtrn9DlJ4daEJqNsrvfLErz9R8
+4IQmfmhR93j+rdotner+6keC/wVByEfbW1wmXtmFKXQ6srdpj8VKRFrvkyXVgepM
+DypLgRH2v7lL2kdWhUu2y4EAgrwzuQINBE6UMMEBEADxQxMgUuDrw5GT4tqARTPI
+SSdNcUsRxRhVA8srYOyECliE+B3TwcRDFBs+MyPFJVEuX8fi4eGj/AK5t1GHerfk
+orUGlz72q4c7LLhkfZrsuJbk2dgkjvldKJnIazQJa6epGLqdsE5RlmSgwedIbtMd
+naGJBQH8aKP/Wi1+wUxsm5N3p7+R2WRx48VfpEhYB+Zf/FkFm1Ycjwh57KQ0+OHw
+ykf8VfMisxuH30tDxOCV+VptWKfOF2rDNdaNPWhij2YIjhJXRpkuRR+1PpI4jLaD
+JxcVZmG/0zucacupUN2g5OUH59ySU/totD6YMnmp3FONoyF1uIEJo6Vs30npHGkO
+XgBo3Pxt7oLJeykLPtdSLgm3cwXIYMWarVsAkKNXitQIVGpVRLeaK373VwmXFqoi
+M2SMHeawTUdOORFjpQzkknlJWM1TmUVtHHKt8Pl9+/5+wXKyt2IDdcUkMrB6K5qF
+fb7EwVhoI8ehJQK+eeDCjFwCAiwB3iV8JlyW+tEU7JuyXOQlwY1VWm/WqMD8gaRi
+rT+RFDFliZ3tQbW2pqUoZBROV5HN4tieDfwxGKCvk6Tsdb30zA9DPQp93+238bYf
+312sg9R+CD0AqxoxFG5FJu4HShcPRrPnYtRZqKRe40GDWvBEArXZprwL1qrP+Kl/
+mRrEQpxAGIoFG8HbVvD3EQARAQABiQIfBBgBAgAJBQJOlDDBAhsMAAoJEJFzj3Ph
+t2igSLQP/2uIrAY2CDr0kWBJiD3TztiHy8IdxwUpyTBTebwmAbi44/EvtJfIisrG
+YjKIEv/w0E61gO7O1JBG4+IG93W+v9fTT/e39JMyxsYqoZZHUhP11Okx5grDS5b0
+O8VXOmXVRMdVNfstRBr10HD9uNDq7ruKD18TxYTwN0GPD4gj1dbHQDR77Tr5cyBs
+6Ou5PBOH4r3qcqf/cJUSMeUUu75xLwixux6E7tD2S+t6F07wlWxntUcPtzyAHj20
+J89orUC+dT6r6MypBoI0jdJCp9JPGtR7i+fE5Gm4E5+AUSubLPtZGRY9Um2eMoS2
+DnQpGOKx1VvsixR/Kw44j2tRAvmYMS4iDKcuZU+nZ+xokAgObILj/b9n/Qe2/fXy
+CFdcgSvbm+dV1fZxsdMF/P9OU8aqdT9A9Fv5y+cDMEg4DVnhwMJTxGh/TCkw/H+A
+frHEtRc98lSQN5odpITNG17mG6JOdHM+wA57qHH0uy4+5RsbyAJahcdBcmObK/RF
+i4WZlThpbHftX5O/LH98aYQ2fJayIxv1EAjzOBOQ0MfBHI0KCJR1pysEisX28sJA
+Ic73gnJJ3BLZbqfBRgxjNMNroxC+5Tw6uPGFHa3YnuIAxxw0HcDVZ9vnTWBWFPGw
+ZvXkQ3FVJwZoLmHw47vvlVpLD/4gi1SuHWieRvZ+UdDq00E348pm
+=neBW
+-END PGP PUBLIC KEY BLOCK-
-- 
1.8.3.1

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

1 2 3 4 5 6 7 8 9 10 >

1 - 100 of 2718 matches

Mail list logo