From: "Fabian Grünbichler" <f.gruenbichler@proxmox.com>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
Thomas Lamprecht <t.lamprecht@proxmox.com>
Subject: Re: [pve-devel] [RFC PATCH storage] pvesm export: convert dd's \r in status=progress output to \n
Date: Thu, 16 Jan 2025 09:51:04 +0100 [thread overview]
Message-ID: <1737017396.ggw2z4w5gj.astroid@yuna.none> (raw)
In-Reply-To: <27ec7af2-cfa8-4202-9137-eb9182b80e52@proxmox.com>
On January 16, 2025 9:13 am, Dominik Csapak wrote:
> On 1/15/25 12:28, Thomas Lamprecht wrote:
>> Am 15.01.25 um 10:59 schrieb 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@proxmox.com>
>>> ---
>>> Not sure if we want to take this approach because we lose the
>>> functionalty of overwriting progress on the terminal.
>>
>> FWIW, you could test with `-t STDERR` if the std error FD is a terminal
>> and differ between replacing \r or not.
>>
>
> not sure if that would work here since we do quite some redirection for
> the worker task (to be able to display + putting it in the task log at
> the same time), but yes, I'll try that
>
>>>
>>> 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.
>>
>> Wouldn't be one alternative also be to do that in the UI?
>
> thats what i meant in that sentence. (replacing in yew widget toolkit)
one issue with that though would be that the line (in the task log/API
response) can get really long? not sure whether/where that might cause
problems..
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
next prev parent reply other threads:[~2025-01-16 8:51 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-01-15 9:59 Dominik Csapak
2025-01-15 10:04 ` Dominik Csapak
2025-01-15 11:28 ` Thomas Lamprecht
2025-01-16 8:13 ` Dominik Csapak
2025-01-16 8:51 ` Fabian Grünbichler [this message]
2025-01-16 9:21 ` Thomas Lamprecht
2025-01-16 9:38 ` Dominik Csapak
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1737017396.ggw2z4w5gj.astroid@yuna.none \
--to=f.gruenbichler@proxmox.com \
--cc=pve-devel@lists.proxmox.com \
--cc=t.lamprecht@proxmox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.