Re: [Spacewalk-devel] [PATCH] package-push: Avoid opening the system rpmdb

2009-03-23 Thread Pradeep Kilambi

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

2009-03-23 Thread James Bowes
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

2009-03-22 Thread Pradeep Kilambi
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

2009-03-22 Thread James Bowes
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