Libguestfs - Sep 2022 - [libnbd PATCH] copy: Write progress to stderr, not /dev/tty

dd writes progress messages to stderr, with no complaints.  Having
nbdcopy open /dev/tty instead of reusing stderr to write messages
litters the output of a terminal during an interactive 'make check',
which is rather distracting, even though that test also ran a second
copy using a different fd for inspecting the output.

Fixes: 1df1b05a ("New nbdcopy tool", v1.3.8)
---
 copy/copy-progress-bar.sh |  4 ++--
 copy/progress.c           | 12 ++----------
 2 files changed, 4 insertions(+), 12 deletions(-)

diff --git a/copy/copy-progress-bar.sh b/copy/copy-progress-bar.sh
index 00380d17..4a0b9a3b 100755
--- a/copy/copy-progress-bar.sh
+++ b/copy/copy-progress-bar.sh
@@ -1,6 +1,6 @@
 #!/usr/bin/env bash
 # nbd client library in userspace
-# Copyright (C) 2020 Red Hat Inc.
+# Copyright (C) 2020-2022 Red Hat Inc.
 #
 # This library is free software; you can redistribute it and/or
 # modify it under the terms of the GNU Lesser General Public
@@ -32,7 +32,7 @@ cleanup_fn rm -f $file $file2 $file3
 dd if=/dev/urandom of=$file bs=512 count=1

 # Check that a regular progress bar works.
-# This writes to /dev/tty :-)
+# This writes to stderr
 $VG nbdcopy --progress $file $file2

 # Check that a machine-readable progress bar works.
diff --git a/copy/progress.c b/copy/progress.c
index a8b1a470..c1a873c9 100644
--- a/copy/progress.c
+++ b/copy/progress.c
@@ -1,5 +1,5 @@
 /* NBD client library in userspace.
- * Copyright (C) 2020 Red Hat Inc.
+ * Copyright (C) 2020-2022 Red Hat Inc.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -37,8 +37,6 @@
 static void
 do_progress_bar (off_t pos, int64_t size)
 {
-  static int tty = -1;
-
   /* Note the spinner is covered with the cursor which usually makes
    * it appear inverse video.
    */
@@ -50,12 +48,6 @@ do_progress_bar (off_t pos, int64_t size)
   char msg[80];
   size_t n, i;

-  if (tty == -1) {
-    tty = open ("/dev/tty", O_WRONLY);
-    if (tty == -1)
-      return;
-  }
-
   if (frac < 0) frac = 0; else if (frac > 1) frac = 1;

   if (frac == 1) {
@@ -75,7 +67,7 @@ do_progress_bar (off_t pos, int64_t size)

 #pragma GCC diagnostic push
 #pragma GCC diagnostic ignored "-Wunused-result"
-  write (tty, msg, strlen (msg));
+  write (fileno (stderr), msg, strlen (msg));
 #pragma GCC diagnostic pop
 }

-- 
2.37.2

On Fri, Sep 02, 2022 at 10:25:38AM -0500, Eric Blake
wrote:
> dd writes progress messages to stderr, with no complaints.  Having
> nbdcopy open /dev/tty instead of reusing stderr to write messages
> litters the output of a terminal during an interactive 'make
check',
> which is rather distracting, even though that test also ran a second
> copy using a different fd for inspecting the output.
> 
> Fixes: 1df1b05a ("New nbdcopy tool", v1.3.8)
>
>  copy/copy-progress-bar.sh |  4 ++--
>  copy/progress.c           | 12 ++----------
>  2 files changed, 4 insertions(+), 12 deletions(-)
> 
> diff --git a/copy/copy-progress-bar.sh b/copy/copy-progress-bar.sh
> index 00380d17..4a0b9a3b 100755
> --- a/copy/copy-progress-bar.sh
> +++ b/copy/copy-progress-bar.sh
> @@ -1,6 +1,6 @@
>  #!/usr/bin/env bash
>  # nbd client library in userspace
> -# Copyright (C) 2020 Red Hat Inc.
> +# Copyright (C) 2020-2022 Red Hat Inc.
>  #
>  # This library is free software; you can redistribute it and/or
>  # modify it under the terms of the GNU Lesser General Public
> @@ -32,7 +32,7 @@ cleanup_fn rm -f $file $file2 $file3
>  dd if=/dev/urandom of=$file bs=512 count=1
> 
>  # Check that a regular progress bar works.
> -# This writes to /dev/tty :-)
> +# This writes to stderr
>  $VG nbdcopy --progress $file $file2
> 
>  # Check that a machine-readable progress bar works.
> diff --git a/copy/progress.c b/copy/progress.c
> index a8b1a470..c1a873c9 100644
> --- a/copy/progress.c
> +++ b/copy/progress.c
> @@ -1,5 +1,5 @@
>  /* NBD client library in userspace.
> - * Copyright (C) 2020 Red Hat Inc.
> + * Copyright (C) 2020-2022 Red Hat Inc.
>   *
>   * This library is free software; you can redistribute it and/or
>   * modify it under the terms of the GNU Lesser General Public
> @@ -37,8 +37,6 @@
>  static void
>  do_progress_bar (off_t pos, int64_t size)
>  {
> -  static int tty = -1;
> -
>    /* Note the spinner is covered with the cursor which usually makes
>     * it appear inverse video.
>     */
> @@ -50,12 +48,6 @@ do_progress_bar (off_t pos, int64_t size)
>    char msg[80];
>    size_t n, i;
> 
> -  if (tty == -1) {
> -    tty = open ("/dev/tty", O_WRONLY);
> -    if (tty == -1)
> -      return;
> -  }
> -
>    if (frac < 0) frac = 0; else if (frac > 1) frac = 1;
> 
>    if (frac == 1) {
> @@ -75,7 +67,7 @@ do_progress_bar (off_t pos, int64_t size)
> 
>  #pragma GCC diagnostic push
>  #pragma GCC diagnostic ignored "-Wunused-result"
> -  write (tty, msg, strlen (msg));
> +  write (fileno (stderr), msg, strlen (msg));
>  #pragma GCC diagnostic pop
>  }
ACK

Thanks for looking at this,

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top