"Steve M. Robbins" <[EMAIL PROTECTED]> writes:
> On Wed, Jul 05, 2006 at 11:51:37PM +0100, Roger Leigh wrote:
>> "Steve M. Robbins" <[EMAIL PROTECTED]> writes:
>>
>> > On Mon, Jul 03, 2006 at 11:10:46PM +0100, Roger Leigh wrote:
>> >> "Steve M. Robbins" <[EMAIL PROTECTED]> writes:
>> >
>> >> Because the automatic mounting is being used by a few people, and it
>> >> is a convenient way to do things, I'm going to introduce a new chroot
>> >> type which will be "plain+mounts".
>> >
>> > Doesn't that leave you open to someone requesting both "file" and
>> > "file+mounts" chroot types?
>>
>> No. "plain" is special, since it's just a plain directory in the
>> filesystem. All the other types involve some element of
>> mounting/copying/unpacking and the mounting is part of the chroot
>> setup (if enabled with run-setup-scripts).
>
> Again, my ignorance will show through, but in my mind the "type" just
> specifies the source of the filesystem. I don't see an unpacked
> filesystem ("plain") as radically different than one packed up in a
> tarball or zip file ("file"). The only difference I see is in speed
> (no unzipping required) and that any modification to a "plain" chroot
> is persistent.You are right about the meaning of the type, which is effectively the "source" of the chroot data. The chroots can be split into two distinct types: those which are modified directly (plain, block-device) and those which create a copy (file, lvm-snapshot). Those which create a copy are also "source chroots" which give direct access to the original data so the master copy can be updated. I agree that they aren't radically different. After the changes I've mentioned below, the stage is this: “directory” Source is a directory “file” Source is a tarfile/zipfile “block-device”Source is a block device “lvm-snapshot”Source is an LVM LV snapshot block device All these types will mount the filesystems (10mount) when run-setup-scripts=true. “plain” Source is a directory The same as "directory", but will never mount additional filesystems. This acts as a verbatim copy of a pre-existing filesystem, and is a special case. >> "plain" defaults to /not/ running the setup scripts, where the >> other types all run them by default. > > Sure, but that's just saying "plain is special because I deem it to be > special". Yes, that's pretty much the current situation. >> > At the risk of displaying my ignorance of schroot: I'm having a hard >> > time understanding why automatic mounting is deemed always useful for >> > "non-plain" chroots and (currently) deemed not useful for "plain" >> > chroots. As I understand it, the chroot type describes the source of >> > the chroot filesystem. To me, that seems orthogonal to the question >> > of whether you want "/home" mounted. >> >> That's a good point. In addition to my comment above, there are some >> additional points: >> >> - Currently, having run-setup-scripts=true runs all the scripts. >> There's no way to skip the /home mounting script, or indeed any >> other script. It would be nice to be able to customise which >> scripts are run (or have an effect, at least) on a per-chroot basis. > > Exactly. That's what I had in mind by the new "auto-mount" option: it > would specify to run or not run 10mount. I would like to make the way the scripts run rather more flexible, and customisable by the user. However, I would like to make sure it doesn't lead to an explosion in the number of options. The setup script directory may also have additional scripts installed by other packages or by the sysadmin, so it has to be able to cope with them as well. Something like: setup-options=check,mount,network,chrootname,sbuild exec-options=check would enable all the standard scripts, and be extensible for additional scripts. However, I would like it to be easier to disable individual scripts, e.g. setup-options=no-network to allow all but the networking setup. Any suggestions or thoughts here would be great. >> - "plain" is supposed to be plain. It's the default because it's used >> to be compatible with dchroot(1), dchroot-dsa(1) and chroot(8). It >> doesn't do anything fancy for that reason. >> >> I think I'll create a "directory" chroot type; this will be similar to >> plain, but will enable all the features plain lacks in comparision >> with the other chroot types. >> >> > So: rather than a new chroot type, why not introduce a new option, say >> > "auto-mount", for this? Personally, I'd default it to true; but >> > you're the boss. >> >> The main reason not to do this is for compatiblity with other tools, >> and to do the least surprising thing. > > What are you not going to do? Not introduce the option, or not > default it to true? It depends exactly on how the option will interact with run-setup-scripts. The current approach is to make all useful options available, but default them to off so the admin has to actively enable them. This is just for safety, since some can cause dataloss if used inappropriately. If it modifies the behaviour of the scripts, run-setup-scripts will be the main switch to turn all scripts on (which defaults to off), and so it is then safe to default the automount behaviour to on. > So is your intention to create "directory" type that works just like, > say, "file"? And leave "plain" to be like the old "dchroot"? That > works for me. Yes. I've committed the attached patch into SVN to implement a "directory" type. Regards, Roger -- .''`. Roger Leigh : :' : Debian GNU/Linux http://people.debian.org/~rleigh/ `. `' Printing on GNU/Linux? http://gutenprint.sourceforge.net/ `- GPG Public Key: 0x25BFB848 Please sign and encrypt your mail.
Index: schroot/schroot.conf.5.in
===================================================================
--- schroot/schroot.conf.5.in (revision 832)
+++ schroot/schroot.conf.5.in (working copy)
@@ -43,9 +43,9 @@
This is then followed by several key-value pairs:
.TP
.B type=\fItype\fP
-The type of the chroot. Valid types are \[lq]plain\[rq], \[lq]file\[rq],
-\[lq]block\-device\[rq] and \[lq]lvm\-snapshot\[rq]. If empty or omitted, the
-default type is \[lq]plain\[rq].
+The type of the chroot. Valid types are \[lq]plain\[rq], \[lq]directory\[rq],
+\[lq]file\[rq], \[lq]block\-device\[rq] and \[lq]lvm\-snapshot\[rq]. If empty
+or omitted, the default type is \[lq]plain\[rq].
.TP
.B description=\fIdescription\fP
A short description of the chroot. This may be localised for different
@@ -92,11 +92,12 @@
scripts (\[lq]false\[rq]), for safety reasons (so it won't clobber your passwd
and other critical files). The default for session-managed chroots
(\[lq]file\[rq] and \[lq]lvm\-snapshot\[rq]) is to run setup scripts. If
-\fItype\fP is set to a value other than \[lq]plain\[rq], setup scripts are
-\fBrequired\fP to mount and configure the chroot environment. If enabled for a
-\[lq]plain\[rq] chroot, the chroot will support simple session management (not
-true session management, because it does not make a copy of the chroot). If
-your chroots are exclusively controlled by schroot, set to \[lq]true\[rq].
+\fItype\fP is set to a value other than \[lq]plain\[rq] or \[lq]directory\[rq],
+setup scripts are \fBrequired\fP to mount and configure the chroot environment.
+If enabled for a \[lq]plain\[rq] or \[lq]directory\[rq] chroot, the chroot will
+support simple session management (not true session management, because it does
+not make a copy of the chroot). If your chroots are exclusively controlled by
+schroot, set to \[lq]true\[rq].
.TP
.B run\-exec\-scripts=\fItrue\fP|\fIfalse\fP
Set whether chroot execution scripts will be run. The default is the same as
@@ -120,10 +121,13 @@
\[lq]linux32\[rq], is the option required. The only valid option for non-Linux
systems is \[lq]undefined\[rq]. The default value is \[lq]undefined\[rq].
.SS
-Plain chroots
+Plain and directory chroots
.PP
-Chroots of type \[lq]plain\[rq] are directories accessible in the filesystem.
-They have an additional (mandatory) configuration option:
+Chroots of type \[lq]plain\[rq] or \[lq]directory\[rq] are directories
+accessible in the filesystem. The two types are equivalent except for the fact
+that if \fIrun-setup-scripts\fP is set to \[lq]true\[rq] for plain chroots,
+filesystem mounting is disabled. They have an additional (mandatory)
+configuration option:
.TP
.B location=\fIdirectory\fP
The directory containing the chroot environment. This is where the root will
Index: schroot/exec/00check
===================================================================
--- schroot/exec/00check (revision 832)
+++ schroot/exec/00check (working copy)
@@ -16,7 +16,7 @@
echo "CHROOT_LOCATION=$CHROOT_LOCATION"
echo "CHROOT_PATH=$CHROOT_PATH"
echo "CHROOT_MOUNT_DEVICE=$CHROOT_MOUNT_DEVICE"
- if [ "$CHROOT_TYPE" = "plain" ]; then
+ if [ "$CHROOT_TYPE" = "plain" ] || [ "$CHROOT_TYPE" = "directory" ]; then
:
elif [ "$CHROOT_TYPE" = "file" ]; then
echo "CHROOT_FILE=$CHROOT_FILE"
Index: schroot/schroot-setup.5.in
===================================================================
--- schroot/schroot-setup.5.in (revision 832)
+++ schroot/schroot-setup.5.in (working copy)
@@ -104,7 +104,7 @@
CHROOT_MOUNT_DEVICE
The device containing the chroot root filesystem. This is only set for chroots
which require mounting.
-.SS Plain chroot variables
+.SS Plain and directory chroot variables
.TP
CHROOT_LOCATION
The location of the chroot. This is useful for creating files, or copying
Index: schroot/setup/10mount
===================================================================
--- schroot/setup/10mount (revision 832)
+++ schroot/setup/10mount (working copy)
@@ -41,9 +41,9 @@
# FSCK_VERBOSE="-V"
fi
-if [ "$CHROOT_TYPE" = "plain" ] || [ "$CHROOT_TYPE" = "file" ] || [ "$CHROOT_TYPE" = "block-device" ] || [ "$CHROOT_TYPE" = "lvm-snapshot" ]; then
+if [ "$CHROOT_TYPE" = "plain" ] || [ "$CHROOT_TYPE" = "directory" ] || [ "$CHROOT_TYPE" = "file" ] || [ "$CHROOT_TYPE" = "block-device" ] || [ "$CHROOT_TYPE" = "lvm-snapshot" ]; then
- if [ "$CHROOT_TYPE" = "plain" ]; then
+ if [ "$CHROOT_TYPE" = "plain" ] || [ "$CHROOT_TYPE" = "directory" ]; then
CHROOT_MOUNT_OPTIONS="--rbind"
CHROOT_MOUNT_DEVICE="$CHROOT_LOCATION"
fi
Index: schroot/setup/00check
===================================================================
--- schroot/setup/00check (revision 832)
+++ schroot/setup/00check (working copy)
@@ -16,7 +16,7 @@
echo "CHROOT_LOCATION=$CHROOT_LOCATION"
echo "CHROOT_PATH=$CHROOT_PATH"
echo "CHROOT_MOUNT_DEVICE=$CHROOT_MOUNT_DEVICE"
- if [ "$CHROOT_TYPE" = "plain" ]; then
+ if [ "$CHROOT_TYPE" = "plain" ] || [ "$CHROOT_TYPE" = "directory" ]; then
:
elif [ "$CHROOT_TYPE" = "file" ]; then
echo "CHROOT_FILE=$CHROOT_FILE"
@@ -33,7 +33,7 @@
fi
case "$CHROOT_TYPE" in
- plain)
+ plain | directory)
if [ ! -d "$CHROOT_LOCATION" ]; then
echo "Directory '$CHROOT_LOCATION' does not exist"
exit 1
Index: sbuild/sbuild-chroot.cc
===================================================================
--- sbuild/sbuild-chroot.cc (revision 832)
+++ sbuild/sbuild-chroot.cc (working copy)
@@ -20,6 +20,7 @@
#include <config.h>
#include "sbuild-chroot.h"
+#include "sbuild-chroot-directory.h"
#include "sbuild-chroot-plain.h"
#include "sbuild-chroot-file.h"
#include "sbuild-chroot-block-device.h"
@@ -118,7 +119,9 @@
{
chroot *new_chroot = 0;
- if (type == "plain")
+ if (type == "directory")
+ new_chroot = new chroot_directory();
+ else if (type == "plain")
new_chroot = new chroot_plain();
else if (type == "file")
new_chroot = new chroot_file();
Index: sbuild/sbuild-chroot-directory.h
===================================================================
--- sbuild/sbuild-chroot-directory.h (revision 828)
+++ sbuild/sbuild-chroot-directory.h (working copy)
@@ -17,8 +17,8 @@
*
*********************************************************************/
-#ifndef SBUILD_CHROOT_PLAIN_H
-#define SBUILD_CHROOT_PLAIN_H
+#ifndef SBUILD_CHROOT_DIRECTORY_H
+#define SBUILD_CHROOT_DIRECTORY_H
#include <sbuild/sbuild-chroot.h>
@@ -28,17 +28,17 @@
/**
* A chroot located on a mounted filesystem.
*/
- class chroot_plain : virtual public chroot
+ class chroot_directory : virtual public chroot
{
protected:
/// The constructor.
- chroot_plain ();
+ chroot_directory ();
friend class chroot;
public:
/// The destructor.
- virtual ~chroot_plain ();
+ virtual ~chroot_directory ();
virtual chroot::ptr
clone () const;
@@ -89,7 +89,7 @@
}
-#endif /* SBUILD_CHROOT_PLAIN_H */
+#endif /* SBUILD_CHROOT_DIRECTORY_H */
/*
* Local Variables:
Index: sbuild/sbuild-chroot-plain.cc
===================================================================
--- sbuild/sbuild-chroot-plain.cc (revision 832)
+++ sbuild/sbuild-chroot-plain.cc (working copy)
@@ -48,32 +48,6 @@
}
std::string const&
-chroot_plain::get_location () const
-{
- return chroot::get_location();
-}
-
-void
-chroot_plain::set_location (std::string const& location)
-{
- if (!is_absname(location))
- throw error(location, LOCATION_ABS);
-
- chroot::set_location(location);
-}
-
-std::string
-chroot_plain::get_path () const
-{
- // When running setup scripts, we are session-capable, so the path
- // is the bind-mounted location, rather than the original location.
- if (get_run_setup_scripts() == true)
- return get_mount_location();
- else
- return get_location();
-}
-
-std::string const&
chroot_plain::get_chroot_type () const
{
static const std::string type("plain");
@@ -81,66 +55,6 @@
return type;
}
-void
-chroot_plain::setup_env (environment& env)
-{
- this->chroot::setup_env(env);
-
- env.add("CHROOT_LOCATION", get_location());
-}
-
-void
-chroot_plain::setup_lock (setup_type type,
- bool lock,
- int status)
-{
- /* By default, plain chroots do no locking. */
- /* Create or unlink session information. */
- if (get_run_setup_scripts() == true)
- {
- if ((type == SETUP_START && lock == true) ||
- (type == SETUP_STOP && lock == false && status == 0))
- {
- bool start = (type == SETUP_START);
- setup_session_info(start);
- }
- }
-}
-
-sbuild::chroot::session_flags
-chroot_plain::get_session_flags () const
-{
- if (get_run_setup_scripts() == true)
- return SESSION_CREATE;
- else
- return static_cast<session_flags>(0);
-}
-
-void
-chroot_plain::get_details (format_detail& detail) const
-{
- chroot::get_details(detail);
-}
-
-void
-chroot_plain::get_keyfile (keyfile& keyfile) const
-{
- chroot::get_keyfile(keyfile);
-
- keyfile::set_object_value(*this, &chroot_plain::get_location,
- keyfile, get_name(), "location");
-}
-
-void
-chroot_plain::set_keyfile (keyfile const& keyfile)
-{
- chroot::set_keyfile(keyfile);
-
- keyfile::get_object_value(*this, &chroot_plain::set_location,
- keyfile, get_name(), "location",
- keyfile::PRIORITY_REQUIRED);
-}
-
/*
* Local Variables:
* mode:C++
Index: sbuild/sbuild-chroot-directory.cc
===================================================================
--- sbuild/sbuild-chroot-directory.cc (revision 828)
+++ sbuild/sbuild-chroot-directory.cc (working copy)
@@ -19,7 +19,7 @@
#include <config.h>
-#include "sbuild-chroot-plain.h"
+#include "sbuild-chroot-directory.h"
#include "sbuild-format-detail.h"
#include "sbuild-lock.h"
@@ -32,29 +32,29 @@
using namespace sbuild;
-chroot_plain::chroot_plain ():
+chroot_directory::chroot_directory ():
chroot()
{
}
-chroot_plain::~chroot_plain ()
+chroot_directory::~chroot_directory ()
{
}
sbuild::chroot::ptr
-chroot_plain::clone () const
+chroot_directory::clone () const
{
- return ptr(new chroot_plain(*this));
+ return ptr(new chroot_directory(*this));
}
std::string const&
-chroot_plain::get_location () const
+chroot_directory::get_location () const
{
return chroot::get_location();
}
void
-chroot_plain::set_location (std::string const& location)
+chroot_directory::set_location (std::string const& location)
{
if (!is_absname(location))
throw error(location, LOCATION_ABS);
@@ -63,7 +63,7 @@
}
std::string
-chroot_plain::get_path () const
+chroot_directory::get_path () const
{
// When running setup scripts, we are session-capable, so the path
// is the bind-mounted location, rather than the original location.
@@ -74,15 +74,15 @@
}
std::string const&
-chroot_plain::get_chroot_type () const
+chroot_directory::get_chroot_type () const
{
- static const std::string type("plain");
+ static const std::string type("directory");
return type;
}
void
-chroot_plain::setup_env (environment& env)
+chroot_directory::setup_env (environment& env)
{
this->chroot::setup_env(env);
@@ -90,11 +90,11 @@
}
void
-chroot_plain::setup_lock (setup_type type,
- bool lock,
- int status)
+chroot_directory::setup_lock (setup_type type,
+ bool lock,
+ int status)
{
- /* By default, plain chroots do no locking. */
+ /* By default, directory chroots do no locking. */
/* Create or unlink session information. */
if (get_run_setup_scripts() == true)
{
@@ -108,7 +108,7 @@
}
sbuild::chroot::session_flags
-chroot_plain::get_session_flags () const
+chroot_directory::get_session_flags () const
{
if (get_run_setup_scripts() == true)
return SESSION_CREATE;
@@ -117,26 +117,26 @@
}
void
-chroot_plain::get_details (format_detail& detail) const
+chroot_directory::get_details (format_detail& detail) const
{
chroot::get_details(detail);
}
void
-chroot_plain::get_keyfile (keyfile& keyfile) const
+chroot_directory::get_keyfile (keyfile& keyfile) const
{
chroot::get_keyfile(keyfile);
- keyfile::set_object_value(*this, &chroot_plain::get_location,
+ keyfile::set_object_value(*this, &chroot_directory::get_location,
keyfile, get_name(), "location");
}
void
-chroot_plain::set_keyfile (keyfile const& keyfile)
+chroot_directory::set_keyfile (keyfile const& keyfile)
{
chroot::set_keyfile(keyfile);
- keyfile::get_object_value(*this, &chroot_plain::set_location,
+ keyfile::get_object_value(*this, &chroot_directory::set_location,
keyfile, get_name(), "location",
keyfile::PRIORITY_REQUIRED);
}
Index: sbuild/sbuild-chroot-plain.h
===================================================================
--- sbuild/sbuild-chroot-plain.h (revision 832)
+++ sbuild/sbuild-chroot-plain.h (working copy)
@@ -20,15 +20,15 @@
#ifndef SBUILD_CHROOT_PLAIN_H
#define SBUILD_CHROOT_PLAIN_H
-#include <sbuild/sbuild-chroot.h>
+#include <sbuild/sbuild-chroot-directory.h>
namespace sbuild
{
/**
- * A chroot located on a mounted filesystem.
+ * A chroot located on a mounted filesystem (mounts disabled).
*/
- class chroot_plain : virtual public chroot
+ class chroot_plain : public chroot_directory
{
protected:
/// The constructor.
@@ -43,48 +43,8 @@
virtual chroot::ptr
clone () const;
- /**
- * Get the directory location of the chroot.
- *
- * @returns the location.
- */
virtual std::string const&
- get_location () const;
-
- /**
- * Set the directory location of the chroot.
- *
- * @param location the location.
- */
- virtual void
- set_location (std::string const& location);
-
- virtual std::string
- get_path () const;
-
- virtual std::string const&
get_chroot_type () const;
-
- virtual void
- setup_env (environment& env);
-
- virtual session_flags
- get_session_flags () const;
-
- protected:
- virtual void
- setup_lock (setup_type type,
- bool lock,
- int status);
-
- virtual void
- get_details (format_detail& detail) const;
-
- virtual void
- get_keyfile (keyfile& keyfile) const;
-
- virtual void
- set_keyfile (keyfile const& keyfile);
};
}
Index: sbuild/Makefile.am
===================================================================
--- sbuild/Makefile.am (revision 832)
+++ sbuild/Makefile.am (working copy)
@@ -37,6 +37,7 @@
sbuild-auth-message.h \
sbuild-chroot.h \
sbuild-chroot-block-device.h \
+ sbuild-chroot-directory.h \
sbuild-chroot-file.h \
sbuild-chroot-lvm-snapshot.h \
sbuild-chroot-plain.h \
@@ -69,6 +70,7 @@
sbuild-auth-message.cc \
sbuild-chroot.cc \
sbuild-chroot-block-device.cc \
+ sbuild-chroot-directory.cc \
sbuild-chroot-file.cc \
sbuild-chroot-lvm-snapshot.cc \
sbuild-chroot-plain.cc \
Index: ChangeLog
===================================================================
--- ChangeLog (revision 832)
+++ ChangeLog (working copy)
@@ -1,5 +1,30 @@
2006-07-09 Roger Leigh <[EMAIL PROTECTED]>
+ * NEWS: Document the new "directory" chroot type.
+
+ * schroot/schroot.conf.5.in, schroot/schroot-setup.5.in: Document
+ new "directory" chroot type.
+
+ * schroot/setup/00check, schroot/setup/10mount,
+ schroot/setup/00check: Add logic for "directory" CHROOT_TYPE. The
+ "directory" type will mount the chroot location with --rbind, like
+ "plain", but will additionally mount filesystems like all other
+ chroot types.
+
+ * sbuild/Makefile.am: Add sbuild-chroot-directory.(cc|h).
+
+ * sbuild/sbuild-chroot.cc (create): Add support for the
+ "directory" chroot type.
+
+ * sbuild/sbuild-chroot-plain.(cc|h): chroot_plain derives from
+ chroot_directory. It is exactly the same as directory except for
+ the chroot name.
+
+ * sbuild/sbuild-chroot-directory.(cc|h): New files. The directory
+ class is based upon the "plain" chroot type.
+
+2006-07-09 Roger Leigh <[EMAIL PROTECTED]>
+
* sbuild/sbuild-dirstream.h: The dirstream extraction operator is
declared outside the class, in addition to the friend declaration.
This fixes a compilation error with GCC 4.2. Thanks to Martin
Index: NEWS
===================================================================
--- NEWS (revision 832)
+++ NEWS (working copy)
@@ -9,6 +9,10 @@
* Major changes in 0.99.3:
+ 1) A new chroot type, "directory", has been added. This is the same
+ as the "plain" type, but additionally allows filesystem mounting
+ when setup scripts are enabled.
+
* Major changes in 0.99.2:
1) A --debug option has been added to all programs. Its use is
pgpgYmmTrHLt8.pgp
Description: PGP signature
