100 ms is quite plenty, while we would be better of using a event
based wait, i.e., dropping the WNOHANG, that would also mean handling
the time out via alarm, EINTR checking and quite a bit other stuff
making this more convoluted, so for now just go faster..
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Using full seconds as a granularity for sleeping between waitpid()'s is
way too much and unnecessarily slows down the installation a lot. Most
processes take a few moments after closing their stdin/stdout to
actually exit fully, which means that we would sleep a second in most
cases.
Lower it to 0.1 second, which immensely improves the situation.
Some values for comparison; tui-installer on the same bog-standard
2-core, SeaBIOS, ext4, virtio VM (roughly averaged over multiple runs):
* 8.0 ISO (baseline): ~2:30 min
* w/o patch: ~9:00 min
* w/ patch: ~2:30 min
Values measured are from pressing the 'Install' button until the
autoreboot dialog (aka. install finished) popped up.
Fixes: 152bbef ("sys: command: factor out kill() + waitpid() from run_command()")
Reported-by: Stoiko Ivanov <s.ivanov@proxmox.com>
Reported-by: Filip Schauer <f.schauer@proxmox.com>
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
In Perl, the last expression of a block (e.g. of a method, eval) gets
returned if there's no explicit return statement. Quite often that is
truthy, i.e., 1.
As that was chosen as the special value for the CMD_FINISHED flag it
had quite a few false positives, causing weird effects and
installation failure.
Reserve that overly problematic value and chose 2 as new CMD_FINISHED
value, albeit it could be better to signal this even more explicitly,
like with a structured hash reference, but for now this is a good stop
gap.
Fixes: 23c5fbe ("sys: command: allow terminating the process early from log subroutine")
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Graceful process termination can need a bit of time, so wait one
second between sending the (catchable) TERM signal and the
(uncatchable) KILL one.
Makes the code shorter as a side benefit.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
If $noprint is set, the output of the command won't be printed to stdout
of the parent process.
Fully backwards-compatible again, only takes effect if the new argument
is actually specified.
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
If the logging subroutine $func returns CMD_FINISHED after processing a
line, the running subprocess is killed early.
This mechanism can be used when e.g. only a certain part of the output
of a (long-running) command is needed, avoiding the extra time it would
take the command to finish properly.
This is done in a entirely backwards-compatible way, i.e. existing
usages don't need any modification.
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
Previously, the I/O loop would continue endlessly until the subprocess
exited.
This explicit handling allows run_command() to be used with e.g.
alarm().
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
This moves the kill() + waitpid() combo into a separate subroutine,
avoiding open-coding that sequence. wait_for_process() also handles
properly unkillable process (e.g. in D-state) and avoids completely
locking up the installer in such cases. See [0].
For the latter case, a timeout exists (with a default of 5 seconds) in
which to wait for the process to exit after sending an optional
TERM/KILL signal.
Also while at it, add a few basic tests for run_command().
[0] https://lists.proxmox.com/pipermail/pve-devel/2024-February/061697.html
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
