<tt><font size=2>Lubos Kardos <lkardos@redhat.com> wrote on 04/29/2016

08:33:58 AM:<br></font></tt><br><tt><font size=2>> <br>> Here is a backtrace for rpmfilesPopulate() called during the first

phase:<br>> <br>>     rpmfilesPopulate() at rpmfi.c:1427<br>>     rpmfilesNew() at rpmfi.c:1585<br>>     getFiles() at rpmte.c:110<br>>     addTE() at rpmte.c:173<br>>     rpmteNew() at rpmte.c:241<br>>     addPackage() at depends.c:438<br>>     rpmtsAddInstallElementat() depends.c:493<br>>     rpmInstall() at rp minstall.c:583<br>>     main() at rpmqv.c:295<br>> <br>> Here is a backtrace for rpmfilesPopulate() called during the second

phase:<br>> <br>>     rpmfilesPopulate() at rpmfi.c:1427<br>>     rpmfilesNew() at rpmfi.c:1585<br>>     getFiles() at rpmte.c:110<br>>     rpmteOpen() at rpmte.c:571<br>>     rpmteProcess() at rpmte.c:755<br>>     rpmtsProcess() at transaction.c:1362<br>>     rpmtsRun() at transaction.c:1542<br>>     rpmcliTransaction() at rpminstall.c:289<br>>     rpmInstall() at rpminstall.c:617<br>>     main() at rpmqv.c:295</font></tt><br><br><tt><font size=2>I am not sure whether this patch is entirely correct:</font></tt><br><br><tt><font size=2>diff --git a/lib/rpmte.c b/lib/rpmte.c</font></tt><br><tt><font size=2>index 248956c..5134e9b 100644</font></tt><br><tt><font size=2>--- a/lib/rpmte.c</font></tt><br><tt><font size=2>+++ b/lib/rpmte.c</font></tt><br><tt><font size=2>@@ -88,12 +88,15 @@ void rpmteCleanDS(rpmte te)</font></tt><br><tt><font size=2>     te->order = rpmdsFree(te->order);</font></tt><br><tt><font size=2> }</font></tt><br><tt><font size=2> </font></tt><br><tt><font size=2>-static rpmfiles getFiles(rpmte p, Header h)</font></tt><br><tt><font size=2>+static rpmfiles getFiles(rpmte p, Header h, int in_phase2)</font></tt><br><tt><font size=2> {</font></tt><br><tt><font size=2>     rpmfiFlags fiflags;</font></tt><br><tt><font size=2>     fiflags = (p->type == TR_ADDED)

? (RPMFI_NOHEADER | RPMFI_FLAGS_INSTALL) :</font></tt><br><tt><font size=2>               

(RPMFI_NOHEADER | RPMFI_FLAGS_ERASE);</font></tt><br><tt><font size=2> </font></tt><br><tt><font size=2>+    if (!in_phase2)</font></tt><br><tt><font size=2>+        fiflags |= RPMFI_NOFILESIGNATURES;</font></tt><br><tt><font size=2>+</font></tt><br><tt><font size=2>     /* relocate stuff in header if

necessary */</font></tt><br><tt><font size=2>     if (rpmteType(p) == TR_ADDED &&

rpmfsFC(p->fs) > 0) {</font></tt><br><tt><font size=2>        if (!headerIsEntry(h,

RPMTAG_ORIGBASENAMES)) {</font></tt><br><tt><font size=2>@@ -170,7 +173,7 @@ static int addTE(rpmte p, Header

h, fnpyKey key, rpmRelocation * relocs)</font></tt><br><tt><font size=2>     p->fs = rpmfsNew(rpmtdCount(&bnames),

(p->type == TR_ADDED));</font></tt><br><tt><font size=2>     rpmtdFreeData(&bnames);</font></tt><br><tt><font size=2> </font></tt><br><tt><font size=2>-    p->files = getFiles(p, h);</font></tt><br><tt><font size=2>+    p->files = getFiles(p, h, 0);</font></tt><br><tt><font size=2> </font></tt><br><tt><font size=2>     /* Packages with no files return

an empty file info set, NULL is an error */</font></tt><br><tt><font size=2>     if (p->files == NULL)</font></tt><br><tt><font size=2>@@ -568,7 +571,7 @@ static int rpmteOpen(rpmte te,

int reload_fi)</font></tt><br><tt><font size=2>     if (h != NULL) {</font></tt><br><tt><font size=2>        if (reload_fi) {</font></tt><br><tt><font size=2>            /* This

can fail if we get a different, bad header from callback */</font></tt><br><tt><font size=2>-           te->files

= getFiles(te, h);</font></tt><br><tt><font size=2>+           te->files

= getFiles(te, h, 1);</font></tt><br><tt><font size=2>            rc = (te->files

!= NULL);</tt> <tt><font size=2>        } else {</tt> <tt><font size=2>            rc = 1;</tt> <tt><font size=2>rpmteOpen only has one caller, which is rpmteProcess.

But rpmteProcess has 2 callers from lib/transaction.c : runTransScripts

and rpmtsProcess. I didn't follow those further.</font></tt><br><br><tt><font size=2>addTE has just one caller, which is rpmteNew, which

in turn has 1 possible caller from lib/verify.c::rpmVerifyScript, which

also only has one caller from lib/verify.c::showVerifyPackage. So the '0'

to getFiles is likely correct.</font></tt><br><br><tt><font size=2>    Stefan</font></tt><br><tt><font size=2><br>> <br>> One solution how to detect phase could be adding another argument

to function<br>> getFiles() and this argument will indicate if getFiles() was called

from<br>> addTE() i. e. the first phase or from rpmteOpen() i. e. the  second

phase then<br>> flags could be set according to this argument in function getFiles().<br>> <br>> Lubos<br>> <br>> <br>> ----- Original Message -----<br>> > From: "Stefan Berger" <stefanb@us.ibm.com><br>> > To: "Lubos Kardos" <lkardos@redhat.com><br>> > Cc: "Florian Festi" <ffesti@redhat.com>, rpm-maint@lists.rpm.org<br>> > Sent: Friday, April 29, 2016 1:35:35 PM<br>> > Subject: Re: [Rpm-maint] [PATCH 4/5] Extend header size to 64MB

<br>> due to file signatures<br>> > <br>> > Lubos Kardos <lkardos@redhat.com> wrote on 04/29/2016 06:40:16

AM:<br>> > <br>> > > <br>> > > It is just a thought. Rpm transaction can be divided in

two phases.<br>> > > In the first phase in the beginning of transaction rpm loads

all file<br>> > infos to<br>> > > perform transactions checks and then releases them. In the

second phase<br>> > rpm<br>> > > reloads single file infos to install single packages in

row. The memory > > peak > > > happens in the first phase when all file infos are loaded.

These file<br>> > infos<br>> > > contain also file signatures but in the first phase they

needn't to<br>> > contain<br>> > > them because the signature checking is performed only in

the second<br>> > phase.<br>> > > <br>> > > So if the file signatures blow up the file infos so much

so we need<br>> > > to increase<br>> > > maximum header size then maybe it would be nice not to load

file<br>> > signatures<br>> > > into file infos during the first phase of transaction when

the rpm > > memory peak > > > happens. > >  > > I agreed and it would be a separate patch. > >  > > I didn't look very deeply, but how does one detect the phases?

I suppose<br>> > the part<br>> > to skip would be in lib/trpmfi.c::rpmfilePopulate where the flag<br>> > RPMFI_NOFILESIGNATURES is<br>> > checked. If that's the case, maybe the 1st phase could call this

function<br>> > with this<br>> > flag always set?<br>> > <br>> >     Stefan<br>> > <br>> > > <br>> > > Lubos<br>> > > <br>> > > ----- Original Message -----<br>> > > > From: "Florian Festi" <ffesti@redhat.com><br>> > > > To: "Stefan Berger" <stefanb@us.ibm.com><br>> > > > Cc: rpm-maint@lists.rpm.org<br>> > > > Sent: Friday, April 29, 2016 10:27:39 AM<br>> > > > Subject: Re: [Rpm-maint] [PATCH 4/5] Extend header

size to 64MB<br>> > > due to file signatures<br>> > > > <br>> > > > On 04/27/2016 09:47 PM, Stefan Berger wrote:<br>> > > > > "Rpm-maint" <rpm-maint-bounces@lists.rpm.org>

wrote on 04/27/2016<br>> > > > > 05:50:54 AM:<br>> > > > > <br>> > > > > <br>> > > > >><br>> > > > >> Well changing header size limit needs a bit

more thought. The main<br>> > > > >> problem is that packages with bigger header

will look broken on<br>> > older<br>> > > > >> rpm versions and the usual way of dealing

with this (adding<br>> > rpmlib()<br>> > > > >> Requires) won't work it needs reading the

header.<br>> > > > > <br>> > > > > These huge headers are only occurring in a few

very large packages<br>> > and<br>> > > > > only if one applies the per-file signatures. So

most users probably<br>> > > > > won't notice.<br>> > > > > <br>> > > > >><br>> > > > >> Also I wonder if we should increase the header

size even more, to<br>> > get<br>> > > > >> rid of this topic for a longer time. I thought

about 256MB which<br>> > gives a<br>> > > > >> 4 times increase over the 16MB. I am kinda

tempted to go even<br>> > further.<br>> > > > >> Otoh the limit is there for a reason. And

having rpm chew through<br>> > one GB<br>> > > > >> of broken data doesn't sound like a pleasant

experience.<br>> > > > > <br>> > > > > Anything >=16 MB works with signed files for

all packages in Fedora<br>> > 23.<br>> > > > > Let me know if you want me to resubmit the patch

with a higher<br>> > limit.<br>> > > > <br>> > > > Yes, please. 256MB is probably the way to go. Let's

hope we don't<br>> > reach<br>> > > > that any time soon.<br>> > > > <br>> > > > Florian<br>> > > > <br>> > > > --<br>> > > > <br>> > > > Red Hat GmbH, </font></tt><a href=http://www.de.redhat.com/><tt><font size=2>http://www.de.redhat.com/</font></tt></a><tt><font size=2>,

Registered seat: Grasbrunn,<br>> > > > Commercial register: Amtsgericht Muenchen, HRB 153243,<br>> > > > Managing Directors: Paul Argiry, Charles Cachera, Michael

Cunningham,<br>> > > > Michael O'Neill<br>> > > > _______________________________________________<br>> > > > Rpm-maint mailing list<br>> > > > Rpm-maint@lists.rpm.org<br>> > > > </font></tt><a href="http://lists.rpm.org/mailman/listinfo/rpm-maint"><tt><font size=2>http://lists.rpm.org/mailman/listinfo/rpm-maint</font></tt></a><tt><font size=2><br>> > > > <br>> > > <br>> > <br>> > <br>> > <br>> <br></font></tt><BR>