[pve-devel] [RFC PATCH storage] pvesm export: convert dd's \r in status=progress output to \n

[Dominik Csapak]

pvesm export is mostly used for (remote) migrations, where the
status progress output lands in a task log. For task logs we want to
have line based output (since it's not a terminal), but dd uses \r
to overwrite the same line which does not work in every situation, e.g.
browsers sometimes simply don't show them, making the dd output a long
line instead of separate ones.

To fix this, use run_command's `errfunc` to log the lines. run_command
will split also on \r, but with warn we print a \n so this does the
conversion.

This fixes an issue where the remote migration task log on PDM does not
display that part in new lines. (ExtJS works because it does things
differently and some browser quirks convert \r to \n)

Signed-off-by: Dominik Csapak <d.csapak at proxmox.com>
---
Not sure if we want to take this approach because we lose the
functionalty of overwriting progress on the terminal.

AFAICS there is no easy way to only do this for the task log, since
we simply pipe the output fh of the worker task to the task log fh.

Alternatively we could patch the task log api to parse \r as newlines or
patch the yew widget toolkit to replace \r with \n.

No real preference from my side, but this patch fixes the task log file
without touching our central worker task code, so it seemed sensible.

 src/PVE/Storage/ISCSIPlugin.pm |  6 +++++-
 src/PVE/Storage/LVMPlugin.pm   |  6 +++++-
 src/PVE/Storage/Plugin.pm      | 10 ++++++++--
 3 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/src/PVE/Storage/ISCSIPlugin.pm b/src/PVE/Storage/ISCSIPlugin.pm
index eb70453..33604cd 100644
--- a/src/PVE/Storage/ISCSIPlugin.pm
+++ b/src/PVE/Storage/ISCSIPlugin.pm
@@ -631,7 +631,11 @@ sub volume_export {
 	$size = int($1);
     });
     PVE::Storage::Plugin::write_common_header($fh, $size);
-    run_command(['dd', "if=$file", "bs=64k", "status=progress"], output => '>&'.fileno($fh));
+    run_command(['dd', "if=$file", "bs=64k", "status=progress"], output => '>&'.fileno($fh), errfunc => sub {
+	# convert dd's \r to \n
+	my ($line) = @_;
+	warn "$line\n";
+    });
     return;
 }
 
diff --git a/src/PVE/Storage/LVMPlugin.pm b/src/PVE/Storage/LVMPlugin.pm
index 38f7fa1..d41647b 100644
--- a/src/PVE/Storage/LVMPlugin.pm
+++ b/src/PVE/Storage/LVMPlugin.pm
@@ -647,7 +647,11 @@ sub volume_export {
 	$size = int($1);
     });
     PVE::Storage::Plugin::write_common_header($fh, $size);
-    run_command(['dd', "if=$file", "bs=64k", "status=progress"], output => '>&'.fileno($fh));
+    run_command(['dd', "if=$file", "bs=64k", "status=progress"], output => '>&'.fileno($fh), errfunc => sub {
+	# convert dd's \r to \n
+	my ($line) = @_;
+	warn "$line\n";
+    });
 }
 
 sub volume_import_formats {
diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm
index 65cf43f..c42a675 100644
--- a/src/PVE/Storage/Plugin.pm
+++ b/src/PVE/Storage/Plugin.pm
@@ -1703,11 +1703,17 @@ sub volume_export {
 	my $file_format = ($class->parse_volname($volname))[6];
 	my $size = file_size_info($file, undef, $file_format);
 
+	# convert dd's \r to \n
+	my $errfunc = sub {
+	    my ($line) = @_;
+	    warn "$line\n";
+	};
+
 	if ($format eq 'raw+size') {
 	    die $err_msg if $with_snapshots || $file_format eq 'subvol';
 	    write_common_header($fh, $size);
 	    if ($file_format eq 'raw') {
-		run_command(['dd', "if=$file", "bs=4k", "status=progress"], output => '>&'.fileno($fh));
+		run_command(['dd', "if=$file", "bs=64k", "status=progress"], output => '>&'.fileno($fh), errfunc => $errfunc);
 	    } else {
 		run_command(['qemu-img', 'convert', '-f', $file_format, '-O', 'raw', $file, '/dev/stdout'],
 		            output => '>&'.fileno($fh));
@@ -1717,7 +1723,7 @@ sub volume_export {
 	    my $data_format = $1;
 	    die $err_msg if !$with_snapshots || $file_format ne $data_format;
 	    write_common_header($fh, $size);
-	    run_command(['dd', "if=$file", "bs=4k", "status=progress"], output => '>&'.fileno($fh));
+	    run_command(['dd', "if=$file", "bs=64k", "status=progress"], output => '>&'.fileno($fh), errfunc => $errfunc);
 	    return;
 	} elsif ($format eq 'tar+size') {
 	    die $err_msg if $file_format ne 'subvol';
-- 
2.39.5