Libguestfs - May 2015 - [PATCH] api: Don't truncate /dev/stdout or /dev/stderr when used as FileOut.

In APIs such as guestfs_download, when the FileOut parameter exactly
matches "/dev/stdout" or "/dev/stderr", don't reopen the
possibly
redirected output file with O_TRUNC (truncate).  Instead dup the file
descriptor.

This magic behaviour doesn't happen for /dev/fd/* (or any other output
file) allowing callers the choice of using /dev/stderr or /dev/fd/2
depending on whether or not they want truncation.

This works around an annoying virt-builder bug.  If you do:

  $ virt-builder fedora-21 --install no_such_package -v -x >& /tmp/log

then when the `--install' command fails, virt-builder will download
the log file using `guestfs_download (g, log, "/dev/stderr")'. 
Since
this truncates the redirected /dev/stderr, the final log file is
truncated and corrupted.

With this patch the log file is no longer corrupted.
---
 customize/customize_run.ml |  7 +------
 src/proto.c                | 13 +++++++++++--
 2 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/customize/customize_run.ml b/customize/customize_run.ml
index 5a7e209..d9547a0 100644
--- a/customize/customize_run.ml
+++ b/customize/customize_run.ml
@@ -45,12 +45,7 @@ let run (g : Guestfs.guestfs) root (ops : ops)  
   (* Function to cat the log file, for debugging and error messages. *)
   let debug_logfile () -    try
-      (* XXX If stderr is redirected this actually truncates the
-       * redirection file, which is pretty annoying to say the
-       * least.
-       *)
-      g#download logfile "/dev/stderr"
+    try g#download logfile "/dev/stderr"
     with exn ->
       warning (f_"log file %s: %s (ignored)") logfile
(Printexc.to_string exn) in
 
diff --git a/src/proto.c b/src/proto.c
index a019625..a46a382 100644
--- a/src/proto.c
+++ b/src/proto.c
@@ -756,9 +756,18 @@ guestfs_int_recv_file (guestfs_h *g, const char *filename)
 
   g->user_cancel = 0;
 
-  fd = open (filename, O_WRONLY|O_CREAT|O_TRUNC|O_NOCTTY|O_CLOEXEC, 0666);
+  /* If downloading to /dev/stdout or /dev/stderr, dup the file
+   * descriptor instead of reopening the file, so that redirected
+   * stdout/stderr work properly.
+   */
+  if (STREQ (filename, "/dev/stdout"))
+    fd = dup (1);
+  else if (STREQ (filename, "/dev/stderr"))
+    fd = dup (2);
+  else
+    fd = open (filename, O_WRONLY|O_CREAT|O_TRUNC|O_NOCTTY|O_CLOEXEC, 0666);
   if (fd == -1) {
-    perrorf (g, "open: %s", filename);
+    perrorf (g, "%s", filename);
     goto cancel;
   }
 
-- 
2.3.1

Hi Rich,

     I am developing a patch for btrfs enhancement: btrfs-convert. When 
I do test, run: make check tests, in dir libguestfs/tests/c-api, I got 
these error:

           0/516 test_btrfs_convert_0
     libguestfs: error: btrfs_convert: /dev/sda1: block size is too small
     conversion aborted.
     FAIL: test_btrfs_convert_0

     I also ran "btrfs-convert" command to deal with a 100MB img file 
outside the guestfish, I got the same err msg, so it is a litmitation of 
btrfs-convert

     So my question is:
         How to changed the default disk size, when do: make check tests


Yours sincerely,

Pino

On Tue, May 26, 2015 at 05:13:55PM +0800, Pino wrote:
> Hi Rich,
> 
>     I am developing a patch for btrfs enhancement: btrfs-convert.
> When I do test, run: make check tests, in dir
> libguestfs/tests/c-api, I got these error:
> 
>           0/516 test_btrfs_convert_0
>     libguestfs: error: btrfs_convert: /dev/sda1: block size is too small
>     conversion aborted.
>     FAIL: test_btrfs_convert_0
> 
>     I also ran "btrfs-convert" command to deal with a 100MB img
file
> outside the guestfish, I got the same err msg, so it is a
> litmitation of btrfs-convert
> 
>     So my question is:
>         How to changed the default disk size, when do: make check tests
The tests/c-api/ tests use certain fixed size disks, to make those
tests run quickly (if we had to create new disks for every test,
they'd run very slowly indeed).

The fixed disk sizes are described here:

  http://libguestfs.org/guestfs.3.html#adding-tests-for-an-api-action

If you want to write a more complicated test that requires a larger
disk, then you should put it into tests/btrfs/ instead.

In generator/actions.ml, leave the `tests' field out, and add a
`tests_excuse' line explaining that the tests are located in
tests/btrfs/.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v