From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <pve-devel-bounces@lists.proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9])
by lore.proxmox.com (Postfix) with ESMTPS id 8FB0E1FF165
for <inbox@lore.proxmox.com>; Wed, 15 Jan 2025 11:04:42 +0100 (CET)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
by firstgate.proxmox.com (Proxmox) with ESMTP id CAAC6716D;
Wed, 15 Jan 2025 11:04:39 +0100 (CET)
Message-ID: <08ff130d-c525-4b51-8b31-e93a4cea492b@proxmox.com>
Date: Wed, 15 Jan 2025 11:04:25 +0100
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird Beta
From: Dominik Csapak <d.csapak@proxmox.com>
To: pve-devel@lists.proxmox.com
References: <20250115095901.809989-1-d.csapak@proxmox.com>
Content-Language: en-US
In-Reply-To: <20250115095901.809989-1-d.csapak@proxmox.com>
X-SPAM-LEVEL: Spam detection results: 0
AWL 0.014 Adjusted score from AWL reputation of From: address
BAYES_00 -1.9 Bayes spam probability is 0 to 1%
DMARC_MISSING 0.1 Missing DMARC policy
KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to
Validity was blocked. See
https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more
information.
RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to
Validity was blocked. See
https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more
information.
RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to
Validity was blocked. See
https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more
information.
SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record
SPF_PASS -0.001 SPF: sender matches SPF record
URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See
http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more
information. [lvmplugin.pm, iscsiplugin.pm, plugin.pm]
Subject: Re: [pve-devel] [RFC PATCH storage] pvesm export: convert dd's \r
in status=progress output to \n
X-BeenThere: pve-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox VE development discussion <pve-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pve-devel>,
<mailto:pve-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pve-devel/>
List-Post: <mailto:pve-devel@lists.proxmox.com>
List-Help: <mailto:pve-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel>,
<mailto:pve-devel-request@lists.proxmox.com?subject=subscribe>
Reply-To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Content-Transfer-Encoding: 7bit
Content-Type: text/plain; charset="us-ascii"; Format="flowed"
Errors-To: pve-devel-bounces@lists.proxmox.com
Sender: "pve-devel" <pve-devel-bounces@lists.proxmox.com>
oops, i accidentally committed a change from 4k -> 64k in the bs parameter that i added for testing
for comments, please disregard this change, this was unrelated and only for me to test
if we could get more performance out of the export (did not do too much testing there
yet)
On 1/15/25 10:59, Dominik Csapak wrote:
> 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@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';
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel