Re: [PATCH pmg-api] fix #7407: cluster: report: increase maximal filesize to 2M

[Stoiko Ivanov <s.ivanov@proxmox.com>]

Thanks for the feedback!

On Mon, 16 Mar 2026 11:53:58 +0100
Dominik Csapak <d.csapak@proxmox.com> wrote:

> just my thoughts:
> 
> if we're already increasing the limit here, and the file is
> already a 1.4M, wouldn't it possibly make sense to increase
> the allowed size a bit more? (e.g. 4/8M ?)
I think that choice was the reason for sending it to list instead of
direclty applying - as I wasn't sure either if 2M or something larger...

> 
> It's not an operation that happens too often (every few minutes)
> so the additional memory usage should not matter much.
As it's not pre-allocated it matters even less - and if an installation
needs it, it's easier to raise the PMG installation's memory than to
have to patch the source again...

> 
> Though I guess this doesn't happen often either way,
> but setting the limit just a bit over the reported
> size, I'm afraid we'll likely run into that again rather
> sooner than later.
> 
> IMO in an ideal world we'd never fail the sync due to file size
> especially because the INotify code doesn't care about it either.
Would consider this once we run into the issue with the increased size of
4M - as then we should probably also rework the diff-detection (currently
it's a string-comparison, which might be a bit expensive) in that case
(for the cluster-sync).

will resent with 4M for now.


> 
> On 3/13/26 9:13 PM, Stoiko Ivanov wrote:
> > the postfix configuration files are read via PVE::INotify in our
> > stack and have not implicit limit.
> > 
> > PMG::Cluster and PMG::Report do read the same file's contents using
> > `file_get_contents` with a limit (explicitly set, or implicit) of 1M.
> > 
> > This causes the cluster-sync to fail in one reported deployment
> > hosting ~40k domains, with explicit transport entries and comments
> > (resulting in a 1.4M /etc/pmg/transport).
> > 
> > As it's the first occurrence of this causing an issue doubling the
> > limit to 2M, instead of increasing it further seems appropriate.
> > 
> > Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
> > ---
> >   src/PMG/Cluster.pm | 4 ++--
> >   src/PMG/Report.pm  | 2 +-
> >   2 files changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/src/PMG/Cluster.pm b/src/PMG/Cluster.pm
> > index 02d727ee..380cfa6e 100644
> > --- a/src/PMG/Cluster.pm
> > +++ b/src/PMG/Cluster.pm
> > @@ -266,10 +266,10 @@ my $cond_commit_synced_file = sub {
> >           return;
> >       }
> >   
> > -    my $new = PVE::Tools::file_get_contents($srcfn, 1024 * 1024);
> > +    my $new = PVE::Tools::file_get_contents($srcfn, 2048 * 1024);
> >   
> >       if (-f $dstfn) {
> > -        my $old = PVE::Tools::file_get_contents($dstfn, 1024 * 1024);
> > +        my $old = PVE::Tools::file_get_contents($dstfn, 2048 * 1024);
> >           return 0 if $new eq $old;
> >       }
> >   
> > diff --git a/src/PMG/Report.pm b/src/PMG/Report.pm
> > index 84d1b887..20e4055d 100644
> > --- a/src/PMG/Report.pm
> > +++ b/src/PMG/Report.pm
> > @@ -72,7 +72,7 @@ sub dir2text {
> >           sub {
> >               my ($file) = @_;
> >               $report .= "\n# cat $target_dir$file\n";
> > -            $report .= PVE::Tools::file_get_contents($target_dir . $file) . "\n";
> > +            $report .= PVE::Tools::file_get_contents($target_dir . $file, 2048 * 1024) . "\n";
> >           },
> >       );
> >   }  
>