tags 557197 + fixed-upstream patch pending
thanks

On Fri, Nov 20, 2009 at 11:36:00AM -0500, Evan Broder wrote:
> On Fri, Nov 20, 2009 at 6:40 AM, Roger Leigh <rle...@codelibre.net> wrote:
> > On Fri, Nov 20, 2009 at 02:46:47AM -0500, Evan Broder wrote:
> >> Package: schroot
> >> Version: 1.3.1-1
> >> Severity: important
> >>
> >> If I use schroot -b to create a session of a chroot with
> >> type=block-device, I get a valid chroot session name back, and the
> >> chroot is setup, but it's not recorded in /var/lib/schroot/session,
> >> and I can't then later run it.
> >
> > Just to double check, this is definitely version 1.3.1 from
> > experimental, not the stable/unstable version?
> >
> > I thought this was fixed in 1.3.1, but I'll double check.
> > It /should/ be creating a session file, but obviously isn't
> > in your case.

Fixed in git with the following patch; I'll make a new release
shortly.


Regards,
Roger


diff --git a/sbuild/sbuild-chroot-block-device-base.cc 
b/sbuild/sbuild-chroot-block-device-base.cc
index 7010a49..d02a4f0 100644
--- a/sbuild/sbuild-chroot-block-device-base.cc
+++ b/sbuild/sbuild-chroot-block-device-base.cc
@@ -114,75 +114,6 @@ chroot_block_device_base::setup_env (chroot const& chroot,
   env.add("CHROOT_DEVICE", get_device());
 }
 
-void
-chroot_block_device_base::setup_lock (chroot::setup_type type,
-                                     bool               lock,
-                                     int                status)
-{
-  /* Only lock during setup, not exec. */
-  if (type == EXEC_START || type == EXEC_STOP)
-    return;
-
-  /* Lock is preserved through the entire session. */
-  if ((type == SETUP_START && lock == false) ||
-      (type == SETUP_STOP && lock == true))
-    return;
-
-  try
-    {
-      if (!stat(this->device).is_block())
-       {
-         throw error(get_device(), DEVICE_NOTBLOCK);
-       }
-      else
-       {
-#ifdef SBUILD_FEATURE_UNION
-         /* We don't lock the device if union is configured. */
-         const chroot *base = dynamic_cast<const chroot *>(this);
-         assert(base);
-         chroot_facet_union::const_ptr puni
-           (base->get_facet<chroot_facet_union>());
-         assert(puni);
-         if (puni->get_union_configured())
-           return;
-#endif
-
-         sbuild::device_lock dlock(this->device);
-         if (lock)
-           {
-             try
-               {
-                 dlock.set_lock(lock::LOCK_EXCLUSIVE, 15);
-               }
-             catch (sbuild::lock::error const& e)
-               {
-                 throw error(get_device(), DEVICE_LOCK, e);
-               }
-           }
-         else
-           {
-             try
-               {
-                 dlock.unset_lock();
-               }
-             catch (sbuild::lock::error const& e)
-               {
-                 throw error(get_device(), DEVICE_UNLOCK, e);
-               }
-           }
-       }
-    }
-  catch (sbuild::stat::error const& e) // Failed to stat
-    {
-      // Don't throw if stopping a session and the device stat
-      // failed.  This is because the setup scripts shouldn't fail
-      // to be run if the block device no longer exists, which
-      // would prevent the session from being ended.
-      if (type != SETUP_STOP)
-       throw;
-    }
-}
-
 sbuild::chroot::session_flags
 chroot_block_device_base::get_session_flags (chroot const& chroot) const
 {
diff --git a/sbuild/sbuild-chroot-block-device-base.h 
b/sbuild/sbuild-chroot-block-device-base.h
index ff5cf49..bba299a 100644
--- a/sbuild/sbuild-chroot-block-device-base.h
+++ b/sbuild/sbuild-chroot-block-device-base.h
@@ -83,11 +83,6 @@ namespace sbuild
 
   protected:
     virtual void
-    setup_lock (chroot::setup_type type,
-               bool               lock,
-               int                status);
-
-    virtual void
     get_details (chroot const& chroot,
                 format_detail& detail) const;
 
@@ -100,7 +95,6 @@ namespace sbuild
                 keyfile const& keyfile,
                 string_list&   used_keys);
 
-  private:
     /// The block device to use.
     std::string device;
   };
diff --git a/sbuild/sbuild-chroot-block-device.cc 
b/sbuild/sbuild-chroot-block-device.cc
index 2730e5e..680131a 100644
--- a/sbuild/sbuild-chroot-block-device.cc
+++ b/sbuild/sbuild-chroot-block-device.cc
@@ -111,6 +111,83 @@ chroot_block_device::setup_env (chroot const& chroot,
   chroot_block_device_base::setup_env(chroot, env);
 }
 
+void
+chroot_block_device::setup_lock (chroot::setup_type type,
+                                bool               lock,
+                                int                status)
+{
+  /* Only lock during setup, not exec. */
+  if (type == EXEC_START || type == EXEC_STOP)
+    return;
+
+  /* Lock is preserved through the entire session. */
+  if ((type == SETUP_START && lock == false) ||
+      (type == SETUP_STOP && lock == true))
+    return;
+
+  try
+    {
+      if (!stat(this->get_device()).is_block())
+       {
+         throw error(get_device(), DEVICE_NOTBLOCK);
+       }
+      else
+       {
+#ifdef SBUILD_FEATURE_UNION
+         /* We don't lock the device if union is configured. */
+         const chroot *base = dynamic_cast<const chroot *>(this);
+         assert(base);
+         chroot_facet_union::const_ptr puni
+           (base->get_facet<chroot_facet_union>());
+         assert(puni);
+         if (puni->get_union_configured())
+           return;
+#endif
+
+         sbuild::device_lock dlock(this->device);
+         if (lock)
+           {
+             try
+               {
+                 dlock.set_lock(lock::LOCK_EXCLUSIVE, 15);
+               }
+             catch (sbuild::lock::error const& e)
+               {
+                 throw error(get_device(), DEVICE_LOCK, e);
+               }
+           }
+         else
+           {
+             try
+               {
+                 dlock.unset_lock();
+               }
+             catch (sbuild::lock::error const& e)
+               {
+                 throw error(get_device(), DEVICE_UNLOCK, e);
+               }
+           }
+       }
+    }
+  catch (sbuild::stat::error const& e) // Failed to stat
+    {
+      // Don't throw if stopping a session and the device stat
+      // failed.  This is because the setup scripts shouldn't fail
+      // to be run if the block device no longer exists, which
+      // would prevent the session from being ended.
+      if (type != SETUP_STOP)
+       throw;
+    }
+
+  /* Create or unlink session information. */
+  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_block_device::get_session_flags (chroot const& chroot) const
 {
diff --git a/sbuild/sbuild-chroot-block-device.h 
b/sbuild/sbuild-chroot-block-device.h
index eb88865..9592268 100644
--- a/sbuild/sbuild-chroot-block-device.h
+++ b/sbuild/sbuild-chroot-block-device.h
@@ -76,6 +76,13 @@ namespace sbuild
     virtual session_flags
     get_session_flags (chroot const& chroot) const;
 
+  protected:
+    virtual void
+    setup_lock (chroot::setup_type type,
+               bool               lock,
+               int                status);
+
+
     virtual void
     get_details (chroot const&  chroot,
                 format_detail& detail) const;

-- 
  .''`.  Roger Leigh
 : :' :  Debian GNU/Linux             http://people.debian.org/~rleigh/
 `. `'   Printing on GNU/Linux?       http://gutenprint.sourceforge.net/
   `-    GPG Public Key: 0x25BFB848   Please GPG sign your mail.

Attachment: signature.asc
Description: Digital signature

Reply via email to