Re: [Spacewalk-devel] [PATCH] package-push: Avoid opening the system rpmdb
James Bowes wrote: On Sun, Mar 22, 2009 at 01:52:15PM -0400, Pradeep Kilambi wrote: Ah nice timing. We hit this recently and were investigating the same as stated boiled down to rpm and httpd's db4. See this bug#491119. Panu was going to spend sometime and see if he could get rpm to upgrade to db4.7, which apparently has number of advantages and he wanted to do it anyway irrespective of this issue. Of course if we have a patch then even better, we dont have to wait for his efforts to get spacewalk on f10 out. I'll test the attached patch and apply if all is well. Thanks for the patch. Yeah, I saw some discussion of it in the spacewalk irc channel, and after looking at the bug it sounded pretty crazy/fun, so I gave it a go :) Patch tested and applied. Thank you! ~ Prad -James ___ Spacewalk-devel mailing list Spacewalk-devel@redhat.com https://www.redhat.com/mailman/listinfo/spacewalk-devel -- -- Pradeep Kilambi RHN Satellite Engineering pkila...@redhat.com Phone: +1 919 754 4285 RHCE # 805008680430554 ___ Spacewalk-devel mailing list Spacewalk-devel@redhat.com https://www.redhat.com/mailman/listinfo/spacewalk-devel
Re: [Spacewalk-devel] [PATCH] package-push: Avoid opening the system rpmdb
On Sun, Mar 22, 2009 at 01:52:15PM -0400, Pradeep Kilambi wrote: > Ah nice timing. We hit this recently and were investigating the same as > stated boiled down to rpm and httpd's db4. See this bug#491119. Panu was > going to spend sometime and see if he could get rpm to upgrade to db4.7, > which apparently has number of advantages and he wanted to do it anyway > irrespective of this issue. Of course if we have a patch then even > better, we dont have to wait for his efforts to get spacewalk on f10 > out. I'll test the attached patch and apply if all is well. > > Thanks for the patch. Yeah, I saw some discussion of it in the spacewalk irc channel, and after looking at the bug it sounded pretty crazy/fun, so I gave it a go :) -James pgpPk2ahhph9J.pgp Description: PGP signature ___ Spacewalk-devel mailing list Spacewalk-devel@redhat.com https://www.redhat.com/mailman/listinfo/spacewalk-devel
Re: [Spacewalk-devel] [PATCH] package-push: Avoid opening the system rpmdb
Ah nice timing. We hit this recently and were investigating the same as stated boiled down to rpm and httpd's db4. See this bug#491119. Panu was going to spend sometime and see if he could get rpm to upgrade to db4.7, which apparently has number of advantages and he wanted to do it anyway irrespective of this issue. Of course if we have a patch then even better, we dont have to wait for his efforts to get spacewalk on f10 out. I'll test the attached patch and apply if all is well. Thanks for the patch. ~ Prad James Bowes wrote: httpd and rpm both use the db4 library. When running code in httpd that uses rpmlib, the db4 so loaded by httpd will trump whatever may be loaded by rpm. If the versions linked against by the two code bases are different, any api/abi changes can cause breakage in the rpm code. Spacewalk imports rpms into its database via an rpm read-only transaction and hdrFromFdno, which initializes the system rpmdb, even though it's not strictly required for reading the rpm's header information. Instead, use rpm.readHeaderFromFD (after seeking to the header's start in the rpm file), which does not try to open the rpmdb. --- This patch should fix up the segfaults rhnpush has been causing server-side on F10. There may be other places where such changes could be made, but I've yet to look for them. Tested with a few x86_64 and noarch packages, both signed and unsigned. -James backend/common/rhn_rpm.py | 61 -- backend/satellite_tools/satComputePkgHeaders.py |4 +- backend/server/rhnPackageUpload.py | 54 + 3 files changed, 58 insertions(+), 61 deletions(-) diff --git a/backend/common/rhn_rpm.py b/backend/common/rhn_rpm.py index a7a9baf..93b87d5 100644 --- a/backend/common/rhn_rpm.py +++ b/backend/common/rhn_rpm.py @@ -189,6 +189,57 @@ class RPM_Header: 'signature' : ret, }) +def get_header_byte_range(package_file): +""" +Return the start and end bytes of the rpm header object. + +For details of the rpm file format, see: +http://www.rpm.org/max-rpm/s1-rpm-file-format-rpm-file-format.html +""" + +lead_size = 96 + +# Move past the rpm lead +package_file.seek(lead_size) + +sig_size = get_header_struct_size(package_file) + +# Now we can find the start of the actual header. +header_start = lead_size + sig_size + +package_file.seek(header_start) + +header_size = get_header_struct_size(package_file) + +header_end = header_start + header_size + +return (header_start, header_end) + +def get_header_struct_size(package_file): +""" +Compute the size in bytes of the rpm header struct starting at the current +position in package_file. +""" +# Move past the header preamble +package_file.seek(8, 1) + +# Read the number of index entries +header_index = package_file.read(4) +(header_index_value, ) = struct.unpack('>I', header_index) + +# Read the the size of the header data store +header_store = package_file.read(4) +(header_store_value, ) = struct.unpack('>I', header_store) + +# The total size of the header. Each index entry is 16 bytes long. +header_size = 8 + 4 + 4 + header_index_value * 16 + header_store_value + +# Headers end on an 8-byte boundary. Round out the extra data. +round_out = header_size % 8 +if round_out != 0: +header_size = header_size + (8 - round_out) + +return header_size # Loads the package header from a file / stream / file descriptor # Raises rpm.error if an error is found, or InvalidPacageError if package is @@ -217,12 +268,10 @@ def get_package_header(filename=None, file=None, fd=None): if hdr is None: raise InvalidPackageError else: -ts = RPMReadOnlyTransaction() -nomd5 = getattr(rpm, 'RPMVSF_NOMD5') -needpayload = getattr(rpm, 'RPMVSF_NEEDPAYLOAD') -ts.pushVSFlags(~(nomd5 | needpayload)) -hdr = RPMReadOnlyTransaction().hdrFromFdno(file_desc) -ts.popVSFlags() +header_start, header_end = \ +get_header_byte_range(os.fdopen(os.dup(file_desc))) +os.lseek(file_desc, header_start, 0) +hdr, offset = rpm.readHeaderFromFD(file_desc) if hdr is None: raise InvalidPackageError is_source = hdr[getattr(rpm, 'RPMTAG_SOURCEPACKAGE')] diff --git a/backend/satellite_tools/satComputePkgHeaders.py b/backend/satellite_tools/satComputePkgHeaders.py index 7aca36d..015a658 100644 --- a/backend/satellite_tools/satComputePkgHeaders.py +++ b/backend/satellite_tools/satComputePkgHeaders.py @@ -55,7 +55,7 @@ sys.modules["_apache"] = sys.modules["__main__"] from server import rhnSQL -from server import rhnPackageUpload +from common import rhn_rpm options_table = [ Option("-v", "--verbose", action="count", @@ -205,7 +205,7 @@ class Runner: continue
[Spacewalk-devel] [PATCH] package-push: Avoid opening the system rpmdb
httpd and rpm both use the db4 library. When running code in httpd that uses rpmlib, the db4 so loaded by httpd will trump whatever may be loaded by rpm. If the versions linked against by the two code bases are different, any api/abi changes can cause breakage in the rpm code. Spacewalk imports rpms into its database via an rpm read-only transaction and hdrFromFdno, which initializes the system rpmdb, even though it's not strictly required for reading the rpm's header information. Instead, use rpm.readHeaderFromFD (after seeking to the header's start in the rpm file), which does not try to open the rpmdb. --- This patch should fix up the segfaults rhnpush has been causing server-side on F10. There may be other places where such changes could be made, but I've yet to look for them. Tested with a few x86_64 and noarch packages, both signed and unsigned. -James backend/common/rhn_rpm.py | 61 -- backend/satellite_tools/satComputePkgHeaders.py |4 +- backend/server/rhnPackageUpload.py | 54 + 3 files changed, 58 insertions(+), 61 deletions(-) diff --git a/backend/common/rhn_rpm.py b/backend/common/rhn_rpm.py index a7a9baf..93b87d5 100644 --- a/backend/common/rhn_rpm.py +++ b/backend/common/rhn_rpm.py @@ -189,6 +189,57 @@ class RPM_Header: 'signature' : ret, }) +def get_header_byte_range(package_file): +""" +Return the start and end bytes of the rpm header object. + +For details of the rpm file format, see: +http://www.rpm.org/max-rpm/s1-rpm-file-format-rpm-file-format.html +""" + +lead_size = 96 + +# Move past the rpm lead +package_file.seek(lead_size) + +sig_size = get_header_struct_size(package_file) + +# Now we can find the start of the actual header. +header_start = lead_size + sig_size + +package_file.seek(header_start) + +header_size = get_header_struct_size(package_file) + +header_end = header_start + header_size + +return (header_start, header_end) + +def get_header_struct_size(package_file): +""" +Compute the size in bytes of the rpm header struct starting at the current +position in package_file. +""" +# Move past the header preamble +package_file.seek(8, 1) + +# Read the number of index entries +header_index = package_file.read(4) +(header_index_value, ) = struct.unpack('>I', header_index) + +# Read the the size of the header data store +header_store = package_file.read(4) +(header_store_value, ) = struct.unpack('>I', header_store) + +# The total size of the header. Each index entry is 16 bytes long. +header_size = 8 + 4 + 4 + header_index_value * 16 + header_store_value + +# Headers end on an 8-byte boundary. Round out the extra data. +round_out = header_size % 8 +if round_out != 0: +header_size = header_size + (8 - round_out) + +return header_size # Loads the package header from a file / stream / file descriptor # Raises rpm.error if an error is found, or InvalidPacageError if package is @@ -217,12 +268,10 @@ def get_package_header(filename=None, file=None, fd=None): if hdr is None: raise InvalidPackageError else: -ts = RPMReadOnlyTransaction() -nomd5 = getattr(rpm, 'RPMVSF_NOMD5') -needpayload = getattr(rpm, 'RPMVSF_NEEDPAYLOAD') -ts.pushVSFlags(~(nomd5 | needpayload)) -hdr = RPMReadOnlyTransaction().hdrFromFdno(file_desc) -ts.popVSFlags() +header_start, header_end = \ +get_header_byte_range(os.fdopen(os.dup(file_desc))) +os.lseek(file_desc, header_start, 0) +hdr, offset = rpm.readHeaderFromFD(file_desc) if hdr is None: raise InvalidPackageError is_source = hdr[getattr(rpm, 'RPMTAG_SOURCEPACKAGE')] diff --git a/backend/satellite_tools/satComputePkgHeaders.py b/backend/satellite_tools/satComputePkgHeaders.py index 7aca36d..015a658 100644 --- a/backend/satellite_tools/satComputePkgHeaders.py +++ b/backend/satellite_tools/satComputePkgHeaders.py @@ -55,7 +55,7 @@ sys.modules["_apache"] = sys.modules["__main__"] from server import rhnSQL -from server import rhnPackageUpload +from common import rhn_rpm options_table = [ Option("-v", "--verbose", action="count", @@ -205,7 +205,7 @@ class Runner: continue try: -(header_start, header_end) = rhnPackageUpload.get_header_byte_range(p_file); +(header_start, header_end) = rhn_rpm.get_header_byte_range(p_file); except Exception, e: print "Error reading header size from file %s: %s" % (path, e) diff --git a/backend/server/rhnPackageUpload.py b/backend/server/rhnPackageUpload.py index b81e98d..fce9b56 100644 --- a/backend/server/rhnPackageUpload.py +++ b/backend/server/rhnPackageUpload.py @@ -16,11 +16,11 @@ # $Id$ import os