From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [45.144.208.40]) by lore.proxmox.com (Postfix) with ESMTPS id 47B861FF0A3 for ; Thu, 01 Oct 2026 14:04:43 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 66F2421722; Thu, 01 Oct 2026 14:04:25 +0200 (CEST) Message-ID: Date: Thu, 1 Oct 2026 14:04:06 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH guest-common v6 08/18] tests: add tests for used-guest-ids max file size handling To: =?UTF-8?Q?Michael_K=C3=B6ppl?= , pve-devel@lists.proxmox.com References: <20260924161510.847362-1-m.koeppl@proxmox.com> <20260924161510.847362-9-m.koeppl@proxmox.com> Content-Language: en-US From: Fiona Ebner In-Reply-To: <20260924161510.847362-9-m.koeppl@proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1790856246758 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.491 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: ZT753WBKRQOAB43DJACXRDLBDVLZUUYA X-Message-ID-Hash: ZT753WBKRQOAB43DJACXRDLBDVLZUUYA X-MailFrom: f.ebner@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: Am 24.09.26 um 6:15 PM schrieb Michael Köppl: > The test cases cover whether a file exceeding the thresholds is > correctly detected and also cover multiple scenarios for merging ranges > with gaps between them until file size is below the threshold again. > > Signed-off-by: Michael Köppl > --- > src/tests/guest-id-tests.pl | 102 ++++++++++++++++++++++++++++++++++++ > 1 file changed, 102 insertions(+) > > diff --git a/src/tests/guest-id-tests.pl b/src/tests/guest-id-tests.pl > index 3be492a..6482f64 100755 > --- a/src/tests/guest-id-tests.pl > +++ b/src/tests/guest-id-tests.pl > @@ -4,11 +4,16 @@ use v5.36; > > use lib qw(..); > > +use List::Util qw(all min sum0); > use Test::MockModule; > use Test::More; > > my $FILE = '/etc/pve/virtual-guest/used-guest-ids'; > > +# must match the thresholds in PVE::GuestID > +my $SIZE_WARN = 512 * 1024; > +my $SIZE_MAX = 768 * 1024; > + > my ($parse, $write); > my $raw; # contents of the stubbed used-guest-ids file > my $writes = 0; > @@ -42,6 +47,38 @@ sub guests(@ids) { > return { ids => { map { $_ => { node => 'localhost', type => 'qemu' } } @ids } }; > } > > +# single IDs from 100 upwards, with the gaps between them cycling through @gaps > +sub single_ids($count, @gaps) { > + my @lines; > + my $id = 100; > + for my $i (0 .. $count - 1) { > + push @lines, "$id\n"; > + $id += 1 + $gaps[$i % @gaps]; Style nit: I would prefer an explicit scalar(@gaps) > + } > + return join('', @lines); > +} > + > +sub gap_sizes($ranges) { > + my @gap_sizes; > + for my $i (1 .. $#$ranges) { > + push @gap_sizes, $ranges->[$i]->[0] - $ranges->[$i - 1]->[1] - 1; > + } > + return @gap_sizes; > +} > + > +# whether every range in $inner lies within a range in $outer, both sorted > +sub covers($outer, $inner) { > + my $i = 0; > + for my $range ($inner->@*) { > + while ($i < @$outer && $outer->[$i]->[1] < $range->[0]) { Style nit: similar here > + $i++; > + } > + return 0 if $i >= @$outer; and here > + return 0 if $outer->[$i]->[0] > $range->[0] || $outer->[$i]->[1] < $range->[1]; > + } > + return 1; > +} > + > subtest 'parse' => sub { > my $tests = [ > ['canonical file', "100-250\n300\n900-1200\n", [[100, 250], [300, 300], [900, 1200]]], > @@ -101,6 +138,71 @@ subtest 'write' => sub { > } > }; > > +subtest 'write size limit' => sub { > + my sub write_with_warnings($input) { > + my @warnings; > + local $SIG{__WARN__} = sub { push @warnings, $_[0] }; > + my $output = $write->($FILE, $parse->($FILE, $input)); > + return ($output, \@warnings); > + } > + > + my $small = single_ids(50_000, 1); > + ok(length($small) < $SIZE_WARN, 'small list is below the warning threshold'); > + my ($output, $warnings) = write_with_warnings($small); > + is($output, $small, 'below the warning threshold: written unchanged'); > + is_deeply($warnings, [], 'below the warning threshold: no warning'); > + > + my $medium = single_ids(90_000, 1); > + ok(length($medium) > $SIZE_WARN && length($medium) < $SIZE_MAX, > + 'medium list is in between'); > + ($output, $warnings) = write_with_warnings($medium); > + is($output, $medium, 'above the warning threshold: written unchanged'); > + is(scalar($warnings->@*), 1, 'above the warning threshold: one warning'); > + like( > + $warnings->[0], > + qr/^\Q$FILE\E is \d+ KiB, .* closed beyond 768 KiB\n\z/, > + 'above the warning threshold: size is warned about', > + ); > + > + # above the hard limit, all gaps are equal, so they get closed from > + # the lowest ID upwards > + my $large = single_ids(150_000, 1); > + ok(length($large) > $SIZE_MAX, 'large list is above the limit'); > + my $input = $parse->($FILE, $large); > + ($output, $warnings) = write_with_warnings($large); > + my $closed = $parse->($FILE, $output); > + ok(length($output) <= $SIZE_MAX, 'above the limit: output fits'); > + ok(covers($closed, $input), 'above the limit: every used ID is still recorded'); > + is(scalar($warnings->@*), 1, 'above the limit: one warning'); > + my ($count) = > + ($warnings->[0] // '') =~ m/would exceed 768 KiB, recorded (\d+) unused guest IDs? /; > + is( > + $count, > + sum0(gap_sizes($input)) - sum0(gap_sizes($closed)), > + 'above the limit: number of IDs marked as used is reported', > + ); > + is($closed->[0]->[0], 100, 'equal gaps: the lowest ID starts the joined range'); This does not really test something about equal gaps, or even closing (except that the lowest ID is not completely dropped and sorting), so either remove the check or at least the "equal gaps" bit. > + ok( > + (all { $_->[0] == $_->[1] } $closed->@[1 .. $#$closed]), > + 'equal gaps: only the lowest ones are closed', Nit: not sure we should test this. This is an implementation detail and we could've sorted larger indices first to close just as well. > + ); > + > + # above the hard limit, mixed gaps > + my $mixed = single_ids(200_000, 1, 2, 5, 50); > + $input = $parse->($FILE, $mixed); > + ($output, $warnings) = write_with_warnings($mixed); > + $closed = $parse->($FILE, $output); > + my @remaining = gap_sizes($closed); > + ok(length($output) <= $SIZE_MAX, 'mixed gaps: output fits'); > + ok(covers($closed, $input), 'mixed gaps: every used ID is still recorded'); > + is(min(@remaining), 5, 'mixed gaps: all gaps of 1 and 2 are closed first'); > + is( > + scalar(grep { $_ == 50 } @remaining), > + scalar(grep { $_ == 50 } gap_sizes($input)), > + 'mixed gaps: the largest gaps are kept', > + ); > +}; > + > subtest 'get_next_unused_id' => sub { > my $tests = [ > # name, file contents, existing guests, input ID, expected ID