[gentoo-catalyst] Fix the recent python sem_open failure

[gentoo-catalyst] Fix the recent python sem_open failure

[Brian Dolbec]

These 2 patches I've just bumped up in the pending branch order.  They are needed
to fix recent changes to python build system.

The SOURCE_MOUNTS_DEFAULTS is to compliment the TARGET_MOUNTS_DEFAULTS and
simplifies it's use through the code.  It also is one step closer to having them 
configurable.

The shm patch is a merge of 3 patches into this final one.
The changes have all been tested.   These jsut need testing due to the rebase.

I hope to merge these patches this weekend along with teh previous 6 that fix
other commits already applied.  They have had lots of scrutiny and review already.

[gentoo-catalyst] [PATCH 1/2] modules/generic_stage_target.py, Create SOURCE_MOUNTS_DEFAULTS

[Brian Dolbec]

Similarly to TARGET_MOUNTS_DEFAULTS this is a temporary location.
This will simplify the migration to being fully configurable.
It also simplifies initialization somewhat.
---
 modules/generic_stage_target.py | 39 ++++++++++++++++++++++-----------------
 1 file changed, 22 insertions(+), 17 deletions(-)

diff --git a/modules/generic_stage_target.py b/modules/generic_stage_target.py
index 7919f07..790e4da 100644
--- a/modules/generic_stage_target.py
+++ b/modules/generic_stage_target.py
@@ -22,6 +22,15 @@ TARGET_MOUNTS_DEFAULTS = {
 	"proc": "/proc",
 	}
 
+SOURCE_MOUNTS_DEFAULTS = {
+	"dev": "/dev",
+	"devpts": "/dev/pts",
+	"distdir": "/usr/portage/distfiles",
+	"portdir": "/usr/portage",
+	"port_tmpdir": "tmpfs",
+	"proc": "/proc",
+	}
+
 
 class generic_stage_target(generic_target):
 	"""
@@ -194,23 +203,19 @@ class generic_stage_target(generic_target):
 		""" Setup our mount points """
 		# initialize our target mounts.
 		self.target_mounts = TARGET_MOUNTS_DEFAULTS.copy()
-		if "SNAPCACHE" in self.settings:
-			self.mounts = ["proc", "dev", "portdir", "distdir", "port_tmpdir"]
-			self.mountmap = {
-				"dev": "/dev",
-				"devpts": "/dev/pts",
-				"distdir": self.settings["distdir"],
-				"portdir": normpath("/".join([
-					self.settings["snapshot_cache_path"],
-					self.settings["repo_name"],
-					])),
-				"port_tmpdir": "tmpfs",
-				"proc": "/proc",
-				}
-		else:
-			self.mounts = ["proc", "dev", "distdir", "port_tmpdir"]
-			self.mountmap = {"proc":"/proc", "dev":"/dev", "devpts":"/dev/pts",
-				"distdir":self.settings["distdir"], "port_tmpdir":"tmpfs"}
+
+		self.mounts = ["proc", "dev", "portdir", "distdir", "port_tmpdir"]
+		# initialize our source mounts
+		self.mountmap = SOURCE_MOUNTS_DEFAULTS.copy()
+		# update them from settings
+		self.mountmap["distdir"] = self.settings["distdir"]
+		self.mountmap["portdir"] = normpath("/".join([
+			self.settings["snapshot_cache_path"],
+			self.settings["repo_name"],
+			]))
+		if "SNAPCACHE" not in self.settings:
+			self.mounts.remove("portdir")
+			#self.mountmap["portdir"] = None
 		if os.uname()[0] == "Linux":
 			self.mounts.append("devpts")
 
-- 
1.8.3.2

Re: [gentoo-catalyst] [PATCH 1/2] modules/generic_stage_target.py, Create SOURCE_MOUNTS_DEFAULTS

[W. Trevor King]

[-- Attachment #1: Type: text/plain, Size: 1856 bytes --]

I shift quoted lines around for easier comparison.

On Thu, Jan 02, 2014 at 10:00:43PM -0800, Brian Dolbec wrote:
> +SOURCE_MOUNTS_DEFAULTS = {
> …
> +	"distdir": "/usr/portage/distfiles",
> +	"portdir": "/usr/portage",
> …
> -				"distdir": self.settings["distdir"],
> -				"portdir": normpath("/".join([
> -					self.settings["snapshot_cache_path"],
> -					self.settings["repo_name"],
> -					])),
> …
> +		# initialize our source mounts
> +		self.mountmap = SOURCE_MOUNTS_DEFAULTS.copy()
> +		# update them from settings
> +		self.mountmap["distdir"] = self.settings["distdir"]
> +		self.mountmap["portdir"] = normpath("/".join([
> +			self.settings["snapshot_cache_path"],
> +			self.settings["repo_name"],
> +			]))

Why create dummy initial values and then blow them away?  Wouldn't:

  SOURCE_MOUNTS_DEFAULTS = {
    …
    'distdir': None,  # initialized from settings
    'portdir': None,  # initialized from settings
    }

make more sense?  We'll blow away this default dict once we're loading
it via ConfigParser, so this doesn't have to be super elegant, but
adding values just to clobber them seems misleading.

> -		if "SNAPCACHE" in self.settings:
> -			self.mounts = ["proc", "dev", "portdir", "distdir", "port_tmpdir"]
> …
> -		else:
> -			self.mounts = ["proc", "dev", "distdir", "port_tmpdir"]
> …
> +		self.mounts = ["proc", "dev", "portdir", "distdir", "port_tmpdir"]
> …
> +		if "SNAPCACHE" not in self.settings:
> +			self.mounts.remove("portdir")

I'd prefer:

  self.mounts = ["proc", "dev", "distdir", "port_tmpdir"]
  if "SNAPCACHE" in self.settings:
      self.mounts.append("portdir")

Cheers,
Trevor

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [gentoo-catalyst] [PATCH 1/2] modules/generic_stage_target.py, Create SOURCE_MOUNTS_DEFAULTS

[Brian Dolbec]

[-- Attachment #1: Type: text/plain, Size: 2218 bytes --]

On Thu, 2014-01-02 at 22:20 -0800, W. Trevor King wrote:
> I shift quoted lines around for easier comparison.
> 
> On Thu, Jan 02, 2014 at 10:00:43PM -0800, Brian Dolbec wrote:
> > +SOURCE_MOUNTS_DEFAULTS = {
> > …
> > +	"distdir": "/usr/portage/distfiles",
> > +	"portdir": "/usr/portage",
> > …
> > -				"distdir": self.settings["distdir"],
> > -				"portdir": normpath("/".join([
> > -					self.settings["snapshot_cache_path"],
> > -					self.settings["repo_name"],
> > -					])),
> > …
> > +		# initialize our source mounts
> > +		self.mountmap = SOURCE_MOUNTS_DEFAULTS.copy()
> > +		# update them from settings
> > +		self.mountmap["distdir"] = self.settings["distdir"]
> > +		self.mountmap["portdir"] = normpath("/".join([
> > +			self.settings["snapshot_cache_path"],
> > +			self.settings["repo_name"],
> > +			]))
> 
> Why create dummy initial values and then blow them away?  Wouldn't:
> 
>   SOURCE_MOUNTS_DEFAULTS = {
>     …
>     'distdir': None,  # initialized from settings
>     'portdir': None,  # initialized from settings
>     }
> 
> make more sense?  We'll blow away this default dict once we're loading
> it via ConfigParser, so this doesn't have to be super elegant, but
> adding values just to clobber them seems misleading.
> 

We probably could, they are defaulted in config_defaults.  But at the
time I was just thinking that catalyst.conf or a spec file may not have
the value declared, so was better to give it a default here, just in
case.


> > -		if "SNAPCACHE" in self.settings:
> > -			self.mounts = ["proc", "dev", "portdir", "distdir", "port_tmpdir"]
> > …
> > -		else:
> > -			self.mounts = ["proc", "dev", "distdir", "port_tmpdir"]
> > …
> > +		self.mounts = ["proc", "dev", "portdir", "distdir", "port_tmpdir"]
> > …
> > +		if "SNAPCACHE" not in self.settings:
> > +			self.mounts.remove("portdir")
> 
> I'd prefer:
> 
>   self.mounts = ["proc", "dev", "distdir", "port_tmpdir"]
>   if "SNAPCACHE" in self.settings:
>       self.mounts.append("portdir")
> 
> Cheers,
> Trevor
> 

I didn't do that because I didn't know if the mount order was important,
so erred on the side of the current order.


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 620 bytes --]

Re: [gentoo-catalyst] [PATCH 1/2] modules/generic_stage_target.py, Create SOURCE_MOUNTS_DEFAULTS

[W. Trevor King]

[-- Attachment #1: Type: text/plain, Size: 2972 bytes --]

On Thu, Jan 02, 2014 at 10:52:00PM -0800, Brian Dolbec wrote:
> On Thu, 2014-01-02 at 22:20 -0800, W. Trevor King wrote:
> > On Thu, Jan 02, 2014 at 10:00:43PM -0800, Brian Dolbec wrote:
> > > +SOURCE_MOUNTS_DEFAULTS = {
> > > …
> > > +	"distdir": "/usr/portage/distfiles",
> > > +	"portdir": "/usr/portage",
> > > …
> > > -				"distdir": self.settings["distdir"],
> > > -				"portdir": normpath("/".join([
> > > -					self.settings["snapshot_cache_path"],
> > > -					self.settings["repo_name"],
> > > -					])),
> > > …
> > > +		# initialize our source mounts
> > > +		self.mountmap = SOURCE_MOUNTS_DEFAULTS.copy()
> > > +		# update them from settings
> > > +		self.mountmap["distdir"] = self.settings["distdir"]
> > > +		self.mountmap["portdir"] = normpath("/".join([
> > > +			self.settings["snapshot_cache_path"],
> > > +			self.settings["repo_name"],
> > > +			]))
> > 
> > Why create dummy initial values and then blow them away?  Wouldn't:
> > 
> >   SOURCE_MOUNTS_DEFAULTS = {
> >     …
> >     'distdir': None,  # initialized from settings
> >     'portdir': None,  # initialized from settings
> >     }
> > 
> > make more sense?  We'll blow away this default dict once we're loading
> > it via ConfigParser, so this doesn't have to be super elegant, but
> > adding values just to clobber them seems misleading.
> 
> We probably could, they are defaulted in config_defaults.  But at the
> time I was just thinking that catalyst.conf or a spec file may not have
> the value declared, so was better to give it a default here, just in
> case.

I'm fine with that, but then we should either use this to setup
settings' defaults, or check to make sure the key is in settings
before clobbering the version from SOURCE_MOUNTS_DEFAULTS.

> > > -		if "SNAPCACHE" in self.settings:
> > > -			self.mounts = ["proc", "dev", "portdir", "distdir", "port_tmpdir"]
> > > …
> > > -		else:
> > > -			self.mounts = ["proc", "dev", "distdir", "port_tmpdir"]
> > > …
> > > +		self.mounts = ["proc", "dev", "portdir", "distdir", "port_tmpdir"]
> > > …
> > > +		if "SNAPCACHE" not in self.settings:
> > > +			self.mounts.remove("portdir")
> > 
> > I'd prefer:
> > 
> >   self.mounts = ["proc", "dev", "distdir", "port_tmpdir"]
> >   if "SNAPCACHE" in self.settings:
> >       self.mounts.append("portdir")
> 
> I didn't do that because I didn't know if the mount order was important,
> so erred on the side of the current order.

Ah, good point.  Mounting portdir after distdir would probably not
work well ;).  I now think the preloaded mounts is a good idea, and
that we should fill in the missing keys here too for consistency
('devpts', 'ccache', 'icecream', …).  Then remove entries as
appropriate for the OS/settings.

Cheers,
Trevor

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[gentoo-catalyst] [PATCH 2/2] catalyst/targets/generic_stage_target.py: mount /dev/shm on linux

[Brian Dolbec]

Add shm targets defaults. Anthony G. Basile <blueness@gentoo.org>
Some build systems require /dev/shm to be mounted, like python's
build system.  We make sure that on Linux systems, /dev/shm is
mounted in the stage chroots.  See bug #496328.

Douglas Freed <dwfreed@mtu.edu> :
Mount /dev/shm in the chroot with the right options
Bind mounting /dev/shm into the chroot isn't a good idea, as there may
be collisions and result in weird side effects.  Instead, we can just
mount a new tmpfs there, with the right options to ensure security.

(Forward ported to pending branch from 2.X Brian Dolbec)
Conflicts:
	catalyst/targets/generic_stage_target.py
---
 modules/generic_stage_target.py | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/modules/generic_stage_target.py b/modules/generic_stage_target.py
index 790e4da..3d66231 100644
--- a/modules/generic_stage_target.py
+++ b/modules/generic_stage_target.py
@@ -20,6 +20,7 @@ TARGET_MOUNTS_DEFAULTS = {
 	"port_tmpdir": "/var/tmp/portage",
 	"port_logdir": "/var/log/portage",
 	"proc": "/proc",
+	"shm": "/dev/shm",
 	}
 
 SOURCE_MOUNTS_DEFAULTS = {
@@ -29,6 +30,7 @@ SOURCE_MOUNTS_DEFAULTS = {
 	"portdir": "/usr/portage",
 	"port_tmpdir": "tmpfs",
 	"proc": "/proc",
+	"shm": "shmfs",
 	}
 
 
@@ -218,6 +220,7 @@ class generic_stage_target(generic_target):
 			#self.mountmap["portdir"] = None
 		if os.uname()[0] == "Linux":
 			self.mounts.append("devpts")
+			self.mounts.append("shm")
 
 		self.set_mounts()
 
@@ -938,7 +941,7 @@ class generic_stage_target(generic_target):
 				os.makedirs(target, 0755)
 
 			if not os.path.exists(self.mountmap[x]):
-				if not self.mountmap[x] == "tmpfs":
+				if self.mountmap[x] not in ["tmpfs", "shmfs"]:
 					os.makedirs(self.mountmap[x], 0755)
 
 			src=self.mountmap[x]
@@ -959,6 +962,9 @@ class generic_stage_target(generic_target):
 							self.settings["var_tmpfs_portage"] + "G " + \
 							src + " " + target
 						retval=os.system(cmd)
+				elif src == "shmfs":
+					cmd = "mount -t tmpfs -o noexec,nosuid,nodev shm " + target
+					retval=os.system(cmd)
 				else:
 					cmd = "mount --bind " + src + " " + target
 					#print "bind(); cmd =", cmd
-- 
1.8.3.2

Re: [gentoo-catalyst] [PATCH 2/2] catalyst/targets/generic_stage_target.py: mount /dev/shm on linux

[W. Trevor King]

[-- Attachment #1: Type: text/plain, Size: 688 bytes --]

>  		if os.uname()[0] == "Linux":
>  			self.mounts.append("devpts")
> +			self.mounts.append("shm")

Besides avoiding collisions, a fresh mount is better than --bind
mounting for Debian-based distributions which symlink into /run:

  # ls -ld /dev/shm
  lrwxrwxrwx 1 root root 8 Jan  1 16:42 /dev/shm -> /run/shm
  # cat /etc/os-release 
  PRETTY_NAME="Debian GNU/Linux jessie/sid"
  …

Since binding the host's /run into the container would clearly be
crazy ;).

The patch looks good to me.

Cheers,
Trevor

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]