[buildd-tools-devel] [PATCH 09/17] [chroot_mountable] Refactor as partial interface

Jan-Marek Glogowski glogow at fbihome.de
Tue Jun 30 18:05:47 UTC 2009


chroot_mountable now contains virtual functions.  This cleans up
ambiguity in getting the mount device.
---
 sbuild/sbuild-chroot-block-device.cc |   19 ++++++++++++-------
 sbuild/sbuild-chroot-block-device.h  |    7 +++++++
 sbuild/sbuild-chroot-loopback.cc     |   13 ++++++++++++-
 sbuild/sbuild-chroot-loopback.h      |    7 +++++++
 sbuild/sbuild-chroot-lvm-snapshot.cc |   13 ++++++++++++-
 sbuild/sbuild-chroot-lvm-snapshot.h  |    7 +++++++
 sbuild/sbuild-chroot-mountable.cc    |    2 +-
 sbuild/sbuild-chroot-mountable.h     |    8 ++++----
 8 files changed, 62 insertions(+), 14 deletions(-)

diff --git a/sbuild/sbuild-chroot-block-device.cc b/sbuild/sbuild-chroot-block-device.cc
index 98991bf..be0bc5f 100644
--- a/sbuild/sbuild-chroot-block-device.cc
+++ b/sbuild/sbuild-chroot-block-device.cc
@@ -75,13 +75,6 @@ chroot_block_device::set_device (std::string const& device)
 
   set_mount_uuid_autodetection();
   this->device = device;
-  /** @todo When using LVM snapshots, this it is incorrect to call
-   * set_mount_device here, since it is not the mount device.  This
-   * currently works due to the order we call in, but could
-   * potentially break.  Make chroot_mountable a pure virtual
-   * interface to fix this.
-   */
-  chroot_mountable::set_mount_device(device);
 }
 
 std::string
@@ -90,6 +83,18 @@ chroot_block_device::get_path () const
   return chroot_mountable::get_path();
 }
 
+void
+chroot_block_device::set_mount_device (std::string const& mount_device)
+{
+  // Setting the mount device is not permitted for block_device chroots.
+}
+
+std::string const&
+chroot_block_device::get_mount_device () const
+{
+  return this->device;
+}
+
 std::string const&
 chroot_block_device::get_chroot_type () const
 {
diff --git a/sbuild/sbuild-chroot-block-device.h b/sbuild/sbuild-chroot-block-device.h
index f83cad3..7cef8f6 100644
--- a/sbuild/sbuild-chroot-block-device.h
+++ b/sbuild/sbuild-chroot-block-device.h
@@ -80,6 +80,13 @@ namespace sbuild
     virtual session_flags
     get_session_flags () const;
 
+    // Specialisation of the chroot_mountable interface
+    virtual void
+    set_mount_device (std::string const& mount_device);
+
+    virtual std::string const&
+    get_mount_device () const;
+
   protected:
     virtual void
     setup_lock (chroot::setup_type type,
diff --git a/sbuild/sbuild-chroot-loopback.cc b/sbuild/sbuild-chroot-loopback.cc
index 92400fd..f1c1d68 100644
--- a/sbuild/sbuild-chroot-loopback.cc
+++ b/sbuild/sbuild-chroot-loopback.cc
@@ -74,7 +74,6 @@ chroot_loopback::set_file (std::string const& file)
     throw chroot::error(file, FILE_ABS);
 
   this->file = file;
-  chroot_mountable::set_mount_device(file);
 }
 
 std::string
@@ -83,6 +82,18 @@ chroot_loopback::get_path () const
   return chroot_mountable::get_path();
 }
 
+void
+chroot_loopback::set_mount_device (std::string const& mount_device)
+{
+  // Setting the mount device is not permitted for loopback chroots.
+}
+
+std::string const&
+chroot_loopback::get_mount_device () const
+{
+  return this->file;
+}
+
 std::string const&
 chroot_loopback::get_chroot_type () const
 {
diff --git a/sbuild/sbuild-chroot-loopback.h b/sbuild/sbuild-chroot-loopback.h
index 818a26d..98b5295 100644
--- a/sbuild/sbuild-chroot-loopback.h
+++ b/sbuild/sbuild-chroot-loopback.h
@@ -77,6 +77,13 @@ namespace sbuild
     virtual session_flags
     get_session_flags () const;
 
+    // Implementation of the chroot_mountable interface
+    virtual void
+    set_mount_device (std::string const& mount_device);
+
+    virtual std::string const&
+    get_mount_device () const;
+
   protected:
     virtual void
     setup_lock (chroot::setup_type type,
diff --git a/sbuild/sbuild-chroot-lvm-snapshot.cc b/sbuild/sbuild-chroot-lvm-snapshot.cc
index a8846de..8670add 100644
--- a/sbuild/sbuild-chroot-lvm-snapshot.cc
+++ b/sbuild/sbuild-chroot-lvm-snapshot.cc
@@ -71,7 +71,6 @@ chroot_lvm_snapshot::set_snapshot_device (std::string const& snapshot_device)
     throw chroot::error(snapshot_device, DEVICE_ABS);
 
   this->snapshot_device = snapshot_device;
-  chroot_mountable::set_mount_device(snapshot_device);
 }
 
 std::string const&
@@ -86,6 +85,18 @@ chroot_lvm_snapshot::set_snapshot_options (std::string const& snapshot_options)
   this->snapshot_options = snapshot_options;
 }
 
+void
+chroot_lvm_snapshot::set_mount_device (std::string const& mount_device)
+{
+  // Setting the mount device is not permitted for lvm_snapshot chroots.
+}
+
+std::string const&
+chroot_lvm_snapshot::get_mount_device () const
+{
+  return this->snapshot_device;
+}
+
 std::string const&
 chroot_lvm_snapshot::get_chroot_type () const
 {
diff --git a/sbuild/sbuild-chroot-lvm-snapshot.h b/sbuild/sbuild-chroot-lvm-snapshot.h
index 433a12d..c4daa44 100644
--- a/sbuild/sbuild-chroot-lvm-snapshot.h
+++ b/sbuild/sbuild-chroot-lvm-snapshot.h
@@ -92,6 +92,13 @@ namespace sbuild
     virtual session_flags
     get_session_flags () const;
 
+    // Implementation of the chroot_mountable interface
+    virtual void
+    set_mount_device (std::string const& mount_device);
+
+    virtual std::string const&
+    get_mount_device () const;
+
   protected:
     virtual void
     setup_lock (chroot::setup_type type,
diff --git a/sbuild/sbuild-chroot-mountable.cc b/sbuild/sbuild-chroot-mountable.cc
index 6c27b3e..cd205be 100644
--- a/sbuild/sbuild-chroot-mountable.cc
+++ b/sbuild/sbuild-chroot-mountable.cc
@@ -142,7 +142,7 @@ chroot_mountable::get_details (format_detail& detail) const
 {
   chroot::get_details(detail);
 
-  if (!mount_options.empty())
+  if (!get_mount_options().empty())
     detail.add(_("Mount Options"), get_mount_options());
   if (!get_location().empty())
     detail.add(_("Location"), get_location());
diff --git a/sbuild/sbuild-chroot-mountable.h b/sbuild/sbuild-chroot-mountable.h
index 8ac7de2..cfa6144 100644
--- a/sbuild/sbuild-chroot-mountable.h
+++ b/sbuild/sbuild-chroot-mountable.h
@@ -46,7 +46,7 @@ namespace sbuild
      *
      * @returns the mount device.
      */
-    std::string const&
+    virtual std::string const&
     get_mount_device () const;
 
     /**
@@ -54,7 +54,7 @@ namespace sbuild
      *
      * @param mount_device the mount device.
      */
-    void
+    virtual void
     set_mount_device (std::string const& mount_device);
 
     /**
@@ -62,7 +62,7 @@ namespace sbuild
      *
      * @returns the mount options.
      */
-    std::string const&
+    virtual std::string const&
     get_mount_options () const;
 
     /**
@@ -70,7 +70,7 @@ namespace sbuild
      *
      * @param mount_options the mount options.
      */
-    void
+    virtual void
     set_mount_options (std::string const& mount_options);
 
     /**
-- 
1.6.3.2




More information about the Buildd-tools-devel mailing list