Am 25.02.26 um 1:06 PM schrieb Markus Ebner:
> The previous file-read implementation in the Proxmox API always
> attempted to read a fixed 16 MiB. On busy or resource-constrainted
> hosts, that often caused request timeouts, with no option to
> reduce the number of read bytes to at least read something.
>
> The new parameter count now allows specifying the exact number of
> bytes that should be read from the given file. To be backwards
> compatible with the previous behavior, it defaults to 16 MiB.
> These 16 MiB are further also the maximum allowed value.
>
> Signed-off-by: Markus Ebner <[email protected]>
> ---
> src/PVE/API2/Qemu/Agent.pm | 27 ++++++++++++++++++++-------
> 1 file changed, 20 insertions(+), 7 deletions(-)
>
> diff --git a/src/PVE/API2/Qemu/Agent.pm b/src/PVE/API2/Qemu/Agent.pm
> index de36ce1e..f32e1dc2 100644
> --- a/src/PVE/API2/Qemu/Agent.pm
> +++ b/src/PVE/API2/Qemu/Agent.pm
> @@ -464,6 +464,14 @@ __PACKAGE__->register_method({
> 'pve-vmid',
> { completion => \&PVE::QemuServer::complete_vmid_running },
> ),
> + count => {
> + type => 'integer',
> + optional => 1,
> + minimum => 0,
Is allowing 0 intentional?
> + maximum => $MAX_READ_SIZE,
> + default => $MAX_READ_SIZE,
> + description => "Number of bytes to read.",
> + },
> file => {
> type => 'string',
> description => 'The path to the file',
> @@ -487,6 +495,7 @@ __PACKAGE__->register_method({
> },
> code => sub {
> my ($param) = @_;
> + my $count = int($param->{count} // $MAX_READ_SIZE);
Nit: The int() is superfluous.
>
> my $vmid = $param->{vmid};
> my $conf = PVE::QemuConfig->load_config($vmid);
> @@ -494,18 +503,20 @@ __PACKAGE__->register_method({
> my $qgafh =
> agent_cmd($vmid, $conf, "file-open", { path => $param->{file} },
> "can't open file");
>
> - my $bytes_left = $MAX_READ_SIZE;
> + my $bytes_read = 0;
> my $eof = 0;
> my $read_size = 1024 * 1024;
> my $content = "";
>
> - while ($bytes_left > 0 && !$eof) {
> + while ($bytes_read < $count && !$eof) {
> + my $bytes_left = $count - $bytes_read;
> + my $chunk_size = $bytes_left < $read_size ? $bytes_left :
> $read_size;
> my $read =
> - mon_cmd($vmid, "guest-file-read", handle => $qgafh, count =>
> int($read_size));
> + mon_cmd($vmid, "guest-file-read", handle => $qgafh, count =>
> int($chunk_size));
> check_agent_error($read, "can't read from file");
>
> $content .= decode_base64($read->{'buf-b64'});
> - $bytes_left -= $read->{count};
> + $bytes_read += $read->{count};
> $eof = $read->{eof} // 0;
> }
>
> @@ -514,12 +525,14 @@ __PACKAGE__->register_method({
>
> my $result = {
> content => $content,
> - 'bytes-read' => ($MAX_READ_SIZE - $bytes_left),
> + 'bytes-read' => $bytes_read,
> };
>
> if (!$eof) {
> - warn
> - "agent file-read: reached maximum read size: $MAX_READ_SIZE
> bytes. output might be truncated.\n";
> + if (!defined $param->{count}) {
Style nit: please use parentheses for definedness checks:
if (!defined($param->{count})) {
> + warn "agent file-read: reached maximum read size:
> $MAX_READ_SIZE bytes."
> + . " output might be truncated.\n";
> + }
> $result->{truncated} = 1;
The description for 'truncated' is:
"If set to 1, the output is truncated and not complete"
But if I request 1 byte and get 1 byte, why should it be considered
"truncated and not complete"?
I do think it's useful information to have 'truncated' for the API
users, but the description should be updated to better convey that it's
about whether EOF was reached or not.
