[PATCH] Refactor disk/mount classes to allow multi-partition/fs layouts

[David Huff]

From: Daniel P. Berrange <berrange(a)redhat.com&gt;

---
 imgcreate/creator.py |   17 ++--
 imgcreate/fs.py      |  247 ++++++++++++++++++++++++++++++++-----------------
 imgcreate/live.py    |    6 +-
 3 files changed, 174 insertions(+), 96 deletions(-)

diff --git a/imgcreate/creator.py b/imgcreate/creator.py
index 0d22b56..979e1b9 100644
--- a/imgcreate/creator.py
+++ b/imgcreate/creator.py
@@ -208,7 +208,11 @@ class ImageCreator(object):
 
         """
         s =  "/dev/root  /         %s    defaults,noatime 0 0\n"
%(self._fstype)
-        s += "devpts     /dev/pts  devpts  gid=5,mode=620   0 0\n"
+        s += self._get_fstab_special()
+        return s
+
+    def _get_fstab_special(self):
+        s = "devpts     /dev/pts  devpts  gid=5,mode=620   0 0\n"
         s += "tmpfs      /dev/shm  tmpfs   defaults         0 0\n"
         s += "proc       /proc     proc    defaults         0 0\n"
         s += "sysfs      /sys      sysfs   defaults         0 0\n"
@@ -817,12 +821,11 @@ class LoopImageCreator(ImageCreator):
         if not base_on is None:
             shutil.copyfile(base_on, self._image)
 
-        self.__instloop = SparseExtLoopbackMount(self._image,
-                                                 self._instroot,
-                                                 self.__image_size,
-                                                 self.__fstype,
-                                                 self.__blocksize,
-                                                 self.fslabel)
+        self.__instloop = ExtDiskMount(SparseLoopbackDisk(self._image,
self.__image_size),
+                                       self._instroot,
+                                       self.__fstype,
+                                       self.__blocksize,
+                                       self.fslabel)
 
         try:
             self.__instloop.mount()
diff --git a/imgcreate/fs.py b/imgcreate/fs.py
index 9ca3a3e..e53cfa9 100644
--- a/imgcreate/fs.py
+++ b/imgcreate/fs.py
@@ -24,6 +24,7 @@ import stat
 import subprocess
 import random
 import string
+import logging
 
 from imgcreate.errors import *
 
@@ -86,42 +87,51 @@ class BindChrootMount:
         subprocess.call(["/bin/umount", self.dest])
         self.mounted = False
 
-class LoopbackMount:
-    def __init__(self, lofile, mountdir, fstype = None):
-        self.lofile = lofile
-        self.mountdir = mountdir
-        self.fstype = fstype
+class Disk:
+    def __init__(self, size, device = None):
+        self._device = device
+        self._size = size
 
-        self.mounted = False
-        self.losetup = False
-        self.rmdir   = False
-        self.loopdev = None
+    def create(self):
+        pass
 
     def cleanup(self):
-        self.unmount()
-        self.lounsetup()
+        pass
 
-    def unmount(self):
-        if self.mounted:
-            rc = subprocess.call(["/bin/umount", self.mountdir])
-            if rc == 0:
-                self.mounted = False
+    def get_device(self):
+        return self._device
+    def set_device(self, path):
+        self._device = path
+    device = property(get_device, set_device)
 
-        if self.rmdir and not self.mounted:
-            try:
-                os.rmdir(self.mountdir)
-            except OSError, e:
-                pass
-            self.rmdir = False
+    def get_size(self):
+        return self._size
+    size = property(get_size)
+
+
+class RawDisk(Disk):
+    def __init__(self, size, device):
+        Disk.__init__(self, size, device)
+
+    def fixed(self):
+        return True
+
+    def exists(self):
+        return True
+
+class LoopbackDisk(Disk):
+    def __init__(self, lofile, size):
+        Disk.__init__(self, size)
+        self.lofile = lofile
+
+    def fixed(self):
+        return False
 
-    def lounsetup(self):
-        if self.losetup:
-            rc = subprocess.call(["/sbin/losetup", "-d",
self.loopdev])
-            self.losetup = False
-            self.loopdev = None
+    def exists(self):
+        return os.path.exists(self.lofile)
 
-    def loopsetup(self):
-        if self.losetup:
+    def create(self):
+        if self.device is not None:
             return
 
         losetupProc = subprocess.Popen(["/sbin/losetup", "-f"],
@@ -132,40 +142,27 @@ class LoopbackMount:
             raise MountError("Failed to allocate loop device for '%s'"
%
                              self.lofile)
 
-        self.loopdev = losetupOutput.split()[0]
+        device = losetupOutput.split()[0]
 
-        rc = subprocess.call(["/sbin/losetup", self.loopdev, self.lofile])
+        logging.debug("Losetup add %s mapping to %s"  % (device, self.lofile))
+        rc = subprocess.call(["/sbin/losetup", device, self.lofile])
         if rc != 0:
             raise MountError("Failed to allocate loop device for '%s'"
%
                              self.lofile)
+        self.device = device
 
-        self.losetup = True
-
-    def mount(self):
-        if self.mounted:
+    def cleanup(self):
+        if self.device is None:
             return
+        logging.debug("Losetup remove %s" % self.device)
+        rc = subprocess.call(["/sbin/losetup", "-d", self.device])
+        self.device = None
 
-        self.loopsetup()
 
-        if not os.path.isdir(self.mountdir):
-            os.makedirs(self.mountdir)
-            self.rmdir = True
 
-        args = [ "/bin/mount", self.loopdev, self.mountdir ]
-        if self.fstype:
-            args.extend(["-t", self.fstype])
-
-        rc = subprocess.call(args)
-        if rc != 0:
-            raise MountError("Failed to mount '%s' to '%s'" %
-                             (self.loopdev, self.mountdir))
-
-        self.mounted = True
-
-class SparseLoopbackMount(LoopbackMount):
-    def __init__(self, lofile, mountdir, size, fstype = None):
-        LoopbackMount.__init__(self, lofile, mountdir, fstype)
-        self.size = size
+class SparseLoopbackDisk(LoopbackDisk):
+    def __init__(self, lofile, size):
+        LoopbackDisk.__init__(self, lofile, size)
 
     def expand(self, create = False, size = None):
         flags = os.O_WRONLY
@@ -176,6 +173,7 @@ class SparseLoopbackMount(LoopbackMount):
         if size is None:
             size = self.size
 
+        logging.debug("Extending sparse file %s to %d" % (self.lofile, size))
         fd = os.open(self.lofile, flags)
 
         os.lseek(fd, size, 0)
@@ -185,36 +183,107 @@ class SparseLoopbackMount(LoopbackMount):
     def truncate(self, size = None):
         if size is None:
             size = self.size
+
+        logging.debug("Truncating sparse file %s to %d" % (self.lofile, size))
         fd = os.open(self.lofile, os.O_WRONLY)
         os.ftruncate(fd, size)
         os.close(fd)
 
     def create(self):
         self.expand(create = True)
+        LoopbackDisk.create(self)
+
+class Mount:
+    def __init__(self, mountdir):
+        self.mountdir = mountdir
+
+    def cleanup(self):
+        self.unmount()
+
+    def mount(self):
+        pass
+
+    def unmount(self):
+        pass
+
+class DiskMount(Mount):
+    def __init__(self, disk, mountdir, fstype = None, rmmountdir = True):
+        Mount.__init__(self, mountdir)
+
+        self.disk = disk
+        self.fstype = fstype
+        self.rmmountdir = rmmountdir
+
+        self.mounted = False
+        self.rmdir   = False
+
+    def cleanup(self):
+        Mount.cleanup(self)
+        self.disk.cleanup()
+
+    def unmount(self):
+        if self.mounted:
+            logging.debug("Unmounting directory %s" % self.mountdir)
+            rc = subprocess.call(["/bin/umount", self.mountdir])
+            if rc == 0:
+                self.mounted = False
+
+        if self.rmdir and not self.mounted:
+            try:
+                os.rmdir(self.mountdir)
+            except OSError, e:
+                pass
+            self.rmdir = False
+
+
+    def __create(self):
+        self.disk.create()
+
 
-class SparseExtLoopbackMount(SparseLoopbackMount):
-    def __init__(self, lofile, mountdir, size, fstype, blocksize, fslabel):
-        SparseLoopbackMount.__init__(self, lofile, mountdir, size, fstype)
+    def mount(self):
+        if self.mounted:
+            return
+
+        if not os.path.isdir(self.mountdir):
+            logging.debug("Creating mount point %s" % self.mountdir)
+            os.makedirs(self.mountdir)
+            self.rmdir = self.rmmountdir
+
+        self.__create()
+
+        logging.debug("Mounting %s at %s" % (self.disk.device, self.mountdir))
+        args = [ "/bin/mount", self.disk.device, self.mountdir ]
+        if self.fstype:
+            args.extend(["-t", self.fstype])
+
+        rc = subprocess.call(args)
+        if rc != 0:
+            raise MountError("Failed to mount '%s' to '%s'" %
+                             (self.disk.device, self.mountdir))
+
+        self.mounted = True
+
+class ExtDiskMount(DiskMount):
+    def __init__(self, disk, mountdir, fstype, blocksize, fslabel, rmmountdir=True):
+        DiskMount.__init__(self, disk, mountdir, fstype, rmmountdir)
         self.blocksize = blocksize
         self.fslabel = fslabel
 
     def __format_filesystem(self):
+        logging.debug("Formating %s filesystem on %s" % (self.fstype,
self.disk.device))
         rc = subprocess.call(["/sbin/mkfs." + self.fstype,
                               "-F", "-L", self.fslabel,
                               "-m", "1", "-b",
str(self.blocksize),
-                              self.lofile,
-                              str(self.size / self.blocksize)])
+                              self.disk.device])
+        #                      str(self.disk.size / self.blocksize)])
         if rc != 0:
             raise MountError("Error creating %s filesystem" % (self.fstype,))
+        logging.debug("Tuning filesystem on %s" % self.disk.device)
         subprocess.call(["/sbin/tune2fs", "-c0", "-i0",
"-Odir_index",
-                         "-ouser_xattr,acl", self.lofile])
-
-    def create(self):
-        SparseLoopbackMount.create(self)
-        self.__format_filesystem()
+                         "-ouser_xattr,acl", self.disk.device])
 
-    def resize(self, size = None):
-        current_size = os.stat(self.lofile)[stat.ST_SIZE]
+    def __resize_filesystem(self, size = None):
+        current_size = os.stat(self.disk.lofile)[stat.ST_SIZE]
 
         if size is None:
             size = self.size
@@ -227,21 +296,28 @@ class SparseExtLoopbackMount(SparseLoopbackMount):
 
         self.__fsck()
 
-        resize2fs(self.lofile, size)
-
-        if size < current_size:
-            self.truncate(size)
+        resize2fs(self.disk.lofile, size)
         return size
 
-    def mount(self):
-        if not os.path.isfile(self.lofile):
-            self.create()
+    def __create(self):
+        resize = False
+        if not self.disk.fixed() and self.disk.exists():
+            resize = True
+
+        self.disk.create()
+
+        if resize:
+            self.__resize_filesystem()
         else:
-            self.resize()
-        return SparseLoopbackMount.mount(self)
+            self.__format_filesystem()
+
+    def mount(self):
+        self.__create()
+        DiskMount.mount(self)
 
     def __fsck(self):
-        subprocess.call(["/sbin/e2fsck", "-f", "-y",
self.lofile])
+        logging.debug("Checking filesystem %s" % self.disk.lofile)
+        subprocess.call(["/sbin/e2fsck", "-f", "-y",
self.disk.lofile])
 
     def __get_size_from_filesystem(self):
         def parse_field(output, field):
@@ -253,7 +329,7 @@ class SparseExtLoopbackMount(SparseLoopbackMount):
 
         dev_null = os.open("/dev/null", os.O_WRONLY)
         try:
-            out = subprocess.Popen(['/sbin/dumpe2fs', '-h',
self.lofile],
+            out = subprocess.Popen(['/sbin/dumpe2fs', '-h',
self.disk.lofile],
                                    stdout = subprocess.PIPE,
                                    stderr = dev_null).communicate()[0]
         finally:
@@ -273,7 +349,7 @@ class SparseExtLoopbackMount(SparseLoopbackMount):
         while top != (bot + 1):
             t = bot + ((top - bot) / 2)
 
-            if not resize2fs(self.lofile, t):
+            if not resize2fs(self.disk.lofile, t):
                 top = t
             else:
                 bot = t
@@ -306,8 +382,8 @@ class DeviceMapperSnapshot(object):
         if self.__created:
             return
 
-        self.imgloop.loopsetup()
-        self.cowloop.loopsetup()
+        self.imgloop.create()
+        self.cowloop.create()
 
         self.__name = "imgcreate-%d-%d" % (os.getpid(),
                                            random.randint(0, 2**16))
@@ -315,8 +391,8 @@ class DeviceMapperSnapshot(object):
         size = os.stat(self.imgloop.lofile)[stat.ST_SIZE]
 
         table = "0 %d snapshot %s %s p 8" % (size / 512,
-                                             self.imgloop.loopdev,
-                                             self.cowloop.loopdev)
+                                             self.imgloop.device,
+                                             self.cowloop.device)
 
         args = ["/sbin/dmsetup", "create", self.__name,
"--table", table]
         if subprocess.call(args) != 0:
@@ -382,15 +458,14 @@ class DeviceMapperSnapshot(object):
 #   8) Create a squashfs of the COW
 #
 def create_image_minimizer(path, image, minimal_size):
-    imgloop = LoopbackMount(image, "None")
+    imgloop = LoopbackDisk(image, None) # Passing bogus size - doesn't matter
 
-    cowloop = SparseLoopbackMount(os.path.join(os.path.dirname(path),
"osmin"),
-                                  None, 64L * 1024L * 1024L)
+    cowloop = SparseLoopbackDisk(os.path.join(os.path.dirname(path), "osmin"),
+                                 64L * 1024L * 1024L)
 
     snapshot = DeviceMapperSnapshot(imgloop, cowloop)
 
     try:
-        cowloop.create()
         snapshot.create()
 
         resize2fs(snapshot.path, minimal_size)
diff --git a/imgcreate/live.py b/imgcreate/live.py
index 03a5466..e25e8cd 100644
--- a/imgcreate/live.py
+++ b/imgcreate/live.py
@@ -131,7 +131,7 @@ class LiveImageCreatorBase(LoopImageCreator):
     #
     def __base_on_iso(self, base_on):
         """helper function to extract ext3 file system from a live CD
ISO"""
-        isoloop = LoopbackMount(base_on, self._mkdtemp())
+        isoloop = Mount(LoopbackDisk(base_on), self._mkdtemp())
 
         try:
             isoloop.mount()
@@ -145,10 +145,10 @@ class LiveImageCreatorBase(LoopImageCreator):
         else:
             squashimg = isoloop.mountdir + "/LiveOS/squashfs.img"
             
-        squashloop = LoopbackMount(squashimg, self._mkdtemp(), "squashfs")
+        squashloop = Mount(LoopbackDisk(squashimg), self._mkdtemp(),
"squashfs")
 
         try:
-            if not os.path.exists(squashloop.lofile):
+            if not squashloop.disk.exists():
                 raise CreatorError("'%s' is not a valid live CD ISO :
"
                                    "squashfs.img doesn't exist" % base_on)
 
-- 
1.5.4.3

[David Huff]

I have broken this patch in to two separate patches:

Refactor-disk-mount-classes-to-allow-multi-partitio.patch
fs.py-kickstart.py-chages-for-mulit-partitions.patch

Both depend on the python logging patch and the later, multi partition patch, depends on
the Refactor patch.  I have tested each incrementally against the latest version of
livecd-creaotr with no known issues.

[PATCH] -Refactor-disk-mount-classes-to-allow-multi-partitions

[David Huff]

---
 imgcreate/creator.py |   17 ++--
 imgcreate/fs.py      |  250 ++++++++++++++++++++++++++++++++------------------
 imgcreate/live.py    |    6 +-
 3 files changed, 175 insertions(+), 98 deletions(-)

diff --git a/imgcreate/creator.py b/imgcreate/creator.py
index febb847..7ccbef9 100644
--- a/imgcreate/creator.py
+++ b/imgcreate/creator.py
@@ -208,7 +208,11 @@ class ImageCreator(object):
 
         """
         s =  "/dev/root  /         %s    defaults,noatime 0 0\n"
%(self._fstype)
-        s += "devpts     /dev/pts  devpts  gid=5,mode=620   0 0\n"
+        s += self._get_fstab_special()
+        return s
+
+    def _get_fstab_special(self):
+        s = "devpts     /dev/pts  devpts  gid=5,mode=620   0 0\n"
         s += "tmpfs      /dev/shm  tmpfs   defaults         0 0\n"
         s += "proc       /proc     proc    defaults         0 0\n"
         s += "sysfs      /sys      sysfs   defaults         0 0\n"
@@ -840,12 +844,11 @@ class LoopImageCreator(ImageCreator):
         if not base_on is None:
             shutil.copyfile(base_on, self._image)
 
-        self.__instloop = SparseExtLoopbackMount(self._image,
-                                                 self._instroot,
-                                                 self.__image_size,
-                                                 self.__fstype,
-                                                 self.__blocksize,
-                                                 self.fslabel)
+        self.__instloop = ExtDiskMount(SparseLoopbackDisk(self._image,
self.__image_size),
+                                       self._instroot,
+                                       self.__fstype,
+                                       self.__blocksize,
+                                       self.fslabel)
 
         try:
             self.__instloop.mount()
diff --git a/imgcreate/fs.py b/imgcreate/fs.py
index 98c0db4..314a776 100644
--- a/imgcreate/fs.py
+++ b/imgcreate/fs.py
@@ -24,6 +24,7 @@ import stat
 import subprocess
 import random
 import string
+import logging
 
 from imgcreate.errors import *
 
@@ -86,42 +87,51 @@ class BindChrootMount:
         subprocess.call(["/bin/umount", self.dest])
         self.mounted = False
 
-class LoopbackMount:
-    def __init__(self, lofile, mountdir, fstype = None):
-        self.lofile = lofile
-        self.mountdir = mountdir
-        self.fstype = fstype
+class Disk:
+    def __init__(self, size, device = None):
+        self._device = device
+        self._size = size
 
-        self.mounted = False
-        self.losetup = False
-        self.rmdir   = False
-        self.loopdev = None
+    def create(self):
+        pass
 
     def cleanup(self):
-        self.unmount()
-        self.lounsetup()
+        pass
 
-    def unmount(self):
-        if self.mounted:
-            rc = subprocess.call(["/bin/umount", self.mountdir])
-            if rc == 0:
-                self.mounted = False
+    def get_device(self):
+        return self._device
+    def set_device(self, path):
+        self._device = path
+    device = property(get_device, set_device)
 
-        if self.rmdir and not self.mounted:
-            try:
-                os.rmdir(self.mountdir)
-            except OSError, e:
-                pass
-            self.rmdir = False
+    def get_size(self):
+        return self._size
+    size = property(get_size)
+
+
+class RawDisk(Disk):
+    def __init__(self, size, device):
+        Disk.__init__(self, size, device)
+
+    def fixed(self):
+        return True
+
+    def exists(self):
+        return True
+
+class LoopbackDisk(Disk):
+    def __init__(self, lofile, size):
+        Disk.__init__(self, size)
+        self.lofile = lofile
+
+    def fixed(self):
+        return False
 
-    def lounsetup(self):
-        if self.losetup:
-            rc = subprocess.call(["/sbin/losetup", "-d",
self.loopdev])
-            self.losetup = False
-            self.loopdev = None
+    def exists(self):
+        return os.path.exists(self.lofile)
 
-    def loopsetup(self):
-        if self.losetup:
+    def create(self):
+        if self.device is not None:
             return
 
         losetupProc = subprocess.Popen(["/sbin/losetup", "-f"],
@@ -132,40 +142,27 @@ class LoopbackMount:
             raise MountError("Failed to allocate loop device for '%s'"
%
                              self.lofile)
 
-        self.loopdev = losetupOutput.split()[0]
+        device = losetupOutput.split()[0]
 
-        rc = subprocess.call(["/sbin/losetup", self.loopdev, self.lofile])
+        logging.debug("Losetup add %s mapping to %s"  % (device, self.lofile))
+        rc = subprocess.call(["/sbin/losetup", device, self.lofile])
         if rc != 0:
             raise MountError("Failed to allocate loop device for '%s'"
%
                              self.lofile)
+        self.device = device
 
-        self.losetup = True
-
-    def mount(self):
-        if self.mounted:
+    def cleanup(self):
+        if self.device is None:
             return
+        logging.debug("Losetup remove %s" % self.device)
+        rc = subprocess.call(["/sbin/losetup", "-d", self.device])
+        self.device = None
 
-        self.loopsetup()
 
-        if not os.path.isdir(self.mountdir):
-            os.makedirs(self.mountdir)
-            self.rmdir = True
 
-        args = [ "/bin/mount", self.loopdev, self.mountdir ]
-        if self.fstype:
-            args.extend(["-t", self.fstype])
-
-        rc = subprocess.call(args)
-        if rc != 0:
-            raise MountError("Failed to mount '%s' to '%s'" %
-                             (self.loopdev, self.mountdir))
-
-        self.mounted = True
-
-class SparseLoopbackMount(LoopbackMount):
-    def __init__(self, lofile, mountdir, size, fstype = None):
-        LoopbackMount.__init__(self, lofile, mountdir, fstype)
-        self.size = size
+class SparseLoopbackDisk(LoopbackDisk):
+    def __init__(self, lofile, size):
+        LoopbackDisk.__init__(self, lofile, size)
 
     def expand(self, create = False, size = None):
         flags = os.O_WRONLY
@@ -176,6 +173,7 @@ class SparseLoopbackMount(LoopbackMount):
         if size is None:
             size = self.size
 
+        logging.debug("Extending sparse file %s to %d" % (self.lofile, size))
         fd = os.open(self.lofile, flags)
 
         os.lseek(fd, size, 0)
@@ -185,36 +183,107 @@ class SparseLoopbackMount(LoopbackMount):
     def truncate(self, size = None):
         if size is None:
             size = self.size
+
+        logging.debug("Truncating sparse file %s to %d" % (self.lofile, size))
         fd = os.open(self.lofile, os.O_WRONLY)
         os.ftruncate(fd, size)
         os.close(fd)
 
     def create(self):
         self.expand(create = True)
+        LoopbackDisk.create(self)
+
+class Mount:
+    def __init__(self, mountdir):
+        self.mountdir = mountdir
+
+    def cleanup(self):
+        self.unmount()
+
+    def mount(self):
+        pass
+
+    def unmount(self):
+        pass
+
+class DiskMount(Mount):
+    def __init__(self, disk, mountdir, fstype = None, rmmountdir = True):
+        Mount.__init__(self, mountdir)
+
+        self.disk = disk
+        self.fstype = fstype
+        self.rmmountdir = rmmountdir
+
+        self.mounted = False
+        self.rmdir   = False
+
+    def cleanup(self):
+        Mount.cleanup(self)
+        self.disk.cleanup()
+
+    def unmount(self):
+        if self.mounted:
+            logging.debug("Unmounting directory %s" % self.mountdir)
+            rc = subprocess.call(["/bin/umount", self.mountdir])
+            if rc == 0:
+                self.mounted = False
+
+        if self.rmdir and not self.mounted:
+            try:
+                os.rmdir(self.mountdir)
+            except OSError, e:
+                pass
+            self.rmdir = False
+
+
+    def __create(self):
+        self.disk.create()
+
 
-class SparseExtLoopbackMount(SparseLoopbackMount):
-    def __init__(self, lofile, mountdir, size, fstype, blocksize, fslabel):
-        SparseLoopbackMount.__init__(self, lofile, mountdir, size, fstype)
+    def mount(self):
+        if self.mounted:
+            return
+
+        if not os.path.isdir(self.mountdir):
+            logging.debug("Creating mount point %s" % self.mountdir)
+            os.makedirs(self.mountdir)
+            self.rmdir = self.rmmountdir
+
+        self.__create()
+
+        logging.debug("Mounting %s at %s" % (self.disk.device, self.mountdir))
+        args = [ "/bin/mount", self.disk.device, self.mountdir ]
+        if self.fstype:
+            args.extend(["-t", self.fstype])
+
+        rc = subprocess.call(args)
+        if rc != 0:
+            raise MountError("Failed to mount '%s' to '%s'" %
+                             (self.disk.device, self.mountdir))
+
+        self.mounted = True
+
+class ExtDiskMount(DiskMount):
+    def __init__(self, disk, mountdir, fstype, blocksize, fslabel, rmmountdir=True):
+        DiskMount.__init__(self, disk, mountdir, fstype, rmmountdir)
         self.blocksize = blocksize
         self.fslabel = fslabel
 
     def __format_filesystem(self):
+        logging.debug("Formating %s filesystem on %s" % (self.fstype,
self.disk.device))
         rc = subprocess.call(["/sbin/mkfs." + self.fstype,
                               "-F", "-L", self.fslabel,
                               "-m", "1", "-b",
str(self.blocksize),
-                              self.lofile,
-                              str(self.size / self.blocksize)])
+                              self.disk.device])
+        #                      str(self.disk.size / self.blocksize)])
         if rc != 0:
             raise MountError("Error creating %s filesystem" % (self.fstype,))
+        logging.debug("Tuning filesystem on %s" % self.disk.device)
         subprocess.call(["/sbin/tune2fs", "-c0", "-i0",
"-Odir_index",
-                         "-ouser_xattr,acl", self.lofile])
-
-    def create(self):
-        SparseLoopbackMount.create(self)
-        self.__format_filesystem()
+                         "-ouser_xattr,acl", self.disk.device])
 
-    def resize(self, size = None):
-        current_size = os.stat(self.lofile)[stat.ST_SIZE]
+    def __resize_filesystem(self, size = None):
+        current_size = os.stat(self.disk.lofile)[stat.ST_SIZE]
 
         if size is None:
             size = self.size
@@ -227,21 +296,28 @@ class SparseExtLoopbackMount(SparseLoopbackMount):
 
         self.__fsck()
 
-        resize2fs(self.lofile, size)
-
-        if size < current_size:
-            self.truncate(size)
+        resize2fs(self.disk.lofile, size)
         return size
 
-    def mount(self):
-        if not os.path.isfile(self.lofile):
-            self.create()
+    def __create(self):
+        resize = False
+        if not self.disk.fixed() and self.disk.exists():
+            resize = True
+
+        self.disk.create()
+
+        if resize:
+            self.__resize_filesystem()
         else:
-            self.resize()
-        return SparseLoopbackMount.mount(self)
+            self.__format_filesystem()
+
+    def mount(self):
+        self.__create()
+        DiskMount.mount(self)
 
     def __fsck(self):
-        subprocess.call(["/sbin/e2fsck", "-f", "-y",
self.lofile])
+        logging.debug("Checking filesystem %s" % self.disk.lofile)
+        subprocess.call(["/sbin/e2fsck", "-f", "-y",
self.disk.lofile])
 
     def __get_size_from_filesystem(self):
         def parse_field(output, field):
@@ -253,7 +329,7 @@ class SparseExtLoopbackMount(SparseLoopbackMount):
 
         dev_null = os.open("/dev/null", os.O_WRONLY)
         try:
-            out = subprocess.Popen(['/sbin/dumpe2fs', '-h',
self.lofile],
+            out = subprocess.Popen(['/sbin/dumpe2fs', '-h',
self.disk.lofile],
                                    stdout = subprocess.PIPE,
                                    stderr = dev_null).communicate()[0]
         finally:
@@ -273,7 +349,7 @@ class SparseExtLoopbackMount(SparseLoopbackMount):
         while top != (bot + 1):
             t = bot + ((top - bot) / 2)
 
-            if not resize2fs(self.lofile, t):
+            if not resize2fs(self.disk.lofile, t):
                 top = t
             else:
                 bot = t
@@ -284,8 +360,7 @@ class SparseExtLoopbackMount(SparseLoopbackMount):
         
         minsize = self.__resize_to_minimal()
 
-        self.truncate(minsize)
-        self.resize(size)
+        self.disk.truncate(minsize)
         return minsize
 
 class DeviceMapperSnapshot(object):
@@ -306,8 +381,8 @@ class DeviceMapperSnapshot(object):
         if self.__created:
             return
 
-        self.imgloop.loopsetup()
-        self.cowloop.loopsetup()
+        self.imgloop.create()
+        self.cowloop.create()
 
         self.__name = "imgcreate-%d-%d" % (os.getpid(),
                                            random.randint(0, 2**16))
@@ -315,8 +390,8 @@ class DeviceMapperSnapshot(object):
         size = os.stat(self.imgloop.lofile)[stat.ST_SIZE]
 
         table = "0 %d snapshot %s %s p 8" % (size / 512,
-                                             self.imgloop.loopdev,
-                                             self.cowloop.loopdev)
+                                             self.imgloop.device,
+                                             self.cowloop.device)
 
         args = ["/sbin/dmsetup", "create", self.__name,
"--table", table]
         if subprocess.call(args) != 0:
@@ -382,15 +457,14 @@ class DeviceMapperSnapshot(object):
 #   8) Create a squashfs of the COW
 #
 def create_image_minimizer(path, image, minimal_size):
-    imgloop = LoopbackMount(image, "None")
+    imgloop = LoopbackDisk(image, None) # Passing bogus size - doesn't matter
 
-    cowloop = SparseLoopbackMount(os.path.join(os.path.dirname(path),
"osmin"),
-                                  None, 64L * 1024L * 1024L)
+    cowloop = SparseLoopbackDisk(os.path.join(os.path.dirname(path), "osmin"),
+                                 64L * 1024L * 1024L)
 
     snapshot = DeviceMapperSnapshot(imgloop, cowloop)
 
     try:
-        cowloop.create()
         snapshot.create()
 
         resize2fs(snapshot.path, minimal_size)
diff --git a/imgcreate/live.py b/imgcreate/live.py
index 130f7fa..9ecd3cf 100644
--- a/imgcreate/live.py
+++ b/imgcreate/live.py
@@ -129,7 +129,7 @@ class LiveImageCreatorBase(LoopImageCreator):
     #
     def __base_on_iso(self, base_on):
         """helper function to extract ext3 file system from a live CD
ISO"""
-        isoloop = LoopbackMount(base_on, self._mkdtemp())
+        isoloop = Mount(LoopbackDisk(base_on), self._mkdtemp())
 
         try:
             isoloop.mount()
@@ -143,10 +143,10 @@ class LiveImageCreatorBase(LoopImageCreator):
         else:
             squashimg = isoloop.mountdir + "/LiveOS/squashfs.img"
             
-        squashloop = LoopbackMount(squashimg, self._mkdtemp(), "squashfs")
+        squashloop = Mount(LoopbackDisk(squashimg), self._mkdtemp(),
"squashfs")
 
         try:
-            if not os.path.exists(squashloop.lofile):
+            if not squashloop.disk.exists():
                 raise CreatorError("'%s' is not a valid live CD ISO :
"
                                    "squashfs.img doesn't exist" % base_on)
 
-- 
1.5.4.1

[PATCH] -fs.py-kickstart.py-chages-for-mulit-partitions

[David Huff]

---
 imgcreate/fs.py        |  223 +++++++++++++++++++++++++++++++++++++++++++++++-
 imgcreate/kickstart.py |    4 +
 2 files changed, 225 insertions(+), 2 deletions(-)

diff --git a/imgcreate/fs.py b/imgcreate/fs.py
index 314a776..aaff4c3 100644
--- a/imgcreate/fs.py
+++ b/imgcreate/fs.py
@@ -357,11 +357,229 @@ class ExtDiskMount(DiskMount):
 
     def resparse(self, size = None):
         self.cleanup()
-        
         minsize = self.__resize_to_minimal()
-
         self.disk.truncate(minsize)
         return minsize
+    
+class PartitionedMount(Mount):
+    def __init__(self, disks, mountdir):
+        Mount.__init__(self, mountdir)
+        self.disks = {}
+        for name in disks.keys():
+            self.disks[name] = { 'disk': disks[name],  # Disk object
+                                 'mapped': False, # True if kpartx mapping
exists
+                                 'numpart': 0, # Number of allocate partitions
+                                 'partitions': [], # indexes to self.partitions
+                                 'extended': 0, # Size of extended partition
+                                 'offset': 0 } # Offset of next partition
+
+        self.partitions = []
+        self.mapped = False
+        self.mountOrder = []
+        self.unmountOrder = []
+
+    def add_partition(self, size, disk, mountpoint, fstype = None):
+        self.partitions.append({'size': size,
+                                'mountpoint': mountpoint, # Mount relative to
chroot
+                                'fstype': fstype,
+                                'disk': disk,  # physical disk name holding
partition
+                                'device': None, # kpartx device node for
partition
+                                'mount': None, # Mount object
+                                'num': None}) # Partition number
+
+    def __format_disks(self):
+        logging.debug("Formatting disks")
+        for dev in self.disks.keys():
+            d = self.disks[dev]
+            logging.debug("Initializing partition table for %s" %
(d['disk'].device))
+            rc = subprocess.call(["/sbin/parted", "-s",
d['disk'].device, "mklabel", "msdos"])
+            if rc != 0:
+                raise MountError("Error writing partition table on %s" %
d.device)
+
+        logging.debug("Assigning partitions to disks")
+        for n in range(len(self.partitions)):
+            p = self.partitions[n]
+
+            if not self.disks.has_key(p['disk']):
+                raise MountError("No disk %s for partition %s" %
(p['disk'], p['mountpoint']))
+
+            d = self.disks[p['disk']]
+            d['numpart'] += 1
+            if d['numpart'] > 3:
+                # Increase allocation of extended partition to hold this partition
+                d['extended'] += p['size']
+                p['type'] = 'logical'
+                p['num'] = d['numpart'] + 1
+            else:
+                p['type'] = 'primary'
+                p['num'] = d['numpart']
+
+            p['start'] = d['offset']
+            d['offset'] += p['size']
+            d['partitions'].append(n)
+            logging.debug("Assigned %s to %s%d at %d at size %d" %
(p['mountpoint'], p['disk'], p['num'], p['start'],
p['size']))
+
+        # XXX we should probably work in cylinder units to keep fdisk happier..
+        start = 0
+        logging.debug("Creating partitions")
+        for p in self.partitions:
+            d = self.disks[p['disk']]
+            if p['num'] == 5:
+                logging.debug("Added extended part at %d of size %d" %
(p['start'], d['extended']))
+                rc = subprocess.call(["/sbin/parted", "-s",
d['disk'].device, "mkpart", "extended",
+                                      "%dM" % p['start'],
"%dM" % (p['start'] + d['extended'])])
+            
+            logging.debug("Add %s part at %d of size %d" % (p['type'],
p['start'], p['size']))
+            rc = subprocess.call(["/sbin/parted", "-s",
d['disk'].device, "mkpart",
+                                  p['type'], "%dM" %
p['start'], "%dM" % (p['start']+p['size'])])
+
+            # XXX disabled return code check because parted always fails to
+            # reload part table with loop devices. Annoying because we can't
+            # distinguish this failure from real partition failures :-(
+            if rc != 0 and 1 == 0: 
+                raise MountError("Error creating partition on %s" %
d['disk'].device)
+
+    def __map_partitions(self):
+        for dev in self.disks.keys():
+            d = self.disks[dev]
+            if d['mapped']:
+                continue
+
+            kpartx = subprocess.Popen(["/sbin/kpartx", "-l",
d['disk'].device],
+                                      stdout=subprocess.PIPE)
+
+            kpartxOutput = kpartx.communicate()[0].split("\n")
+            # Strip trailing blank
+            kpartxOutput = kpartxOutput[0:len(kpartxOutput)-1]
+
+            if kpartx.returncode:
+                raise MountError("Failed to query partition mapping for
'%s'" %
+                                 d.device)
+
+            # Quick sanity check that the number of partitions matches
+            # our expectation. If it doesn't, someone broke the code
+            # further up
+            if len(kpartxOutput) != d['numpart']:
+                raise MountError("Unexpected number of partitions from kpartx: %d !=
%d" %
+                                 (len(kpartxOutput), d['numpart']))
+
+            for i in range(len(kpartxOutput)):
+                line = kpartxOutput[i]
+                newdev = line.split()[0]
+                mapperdev = "/dev/mapper/" + newdev
+                loopdev = d['disk'].device + newdev[-1]
+
+                logging.debug("Dev %s: %s -> %s" % (newdev, loopdev,
mapperdev))
+                pnum = d['partitions'][i]
+                self.partitions[pnum]['device'] = loopdev
+
+                # grub's install wants partitions to be named
+                # to match their parent device + partition num
+                # kpartx doesn't work like this, so we add compat
+                # symlinks to point to /dev/mapper
+                os.symlink(mapperdev, loopdev)
+
+            logging.debug("Adding partx mapping for %s" %
d['disk'].device)
+            rc = subprocess.call(["/sbin/kpartx", "-a",
d['disk'].device])
+            if rc != 0:
+                raise MountError("Failed to map partitions for '%s'" %
+                                 d['disk'].device)
+            d['mapped'] = True
+
+
+    def __unmap_partitions(self):
+        for dev in self.disks.keys():
+            d = self.disks[dev]
+            if not d['mapped']:
+                continue
+
+            logging.debug("Removing compat symlinks")
+            for pnum in d['partitions']:
+                if self.partitions[pnum]['device'] != None:
+                    os.unlink(self.partitions[pnum]['device'])
+                    self.partitions[pnum]['device'] = None
+
+            logging.debug("Unmapping %s" % d['disk'].device)
+            rc = subprocess.call(["/sbin/kpartx", "-d",
d['disk'].device])
+            if rc != 0:
+                raise MountError("Failed to unmap partitions for '%s'"
%
+                                 d['disk'].device)
+
+            d['mapped'] = False
+
+
+    def __calculate_mountorder(self):
+        for p in self.partitions:
+            self.mountOrder.append(p['mountpoint'])
+            self.unmountOrder.append(p['mountpoint'])
+
+        self.mountOrder.sort()
+        self.unmountOrder.sort()
+        self.unmountOrder.reverse()
+        print str(self.mountOrder)
+
+    def cleanup(self):
+        Mount.cleanup(self)
+        self.__unmap_partitions()
+        for dev in self.disks.keys():
+            d = self.disks[dev]
+            try:
+                d['disk'].cleanup()
+            except:
+                pass
+
+    def unmount(self):
+        for mp in self.unmountOrder:
+            if mp == 'swap':
+                continue
+            p = None
+            for p1 in self.partitions:
+                if p1['mountpoint'] == mp:
+                    p = p1
+                    break
+
+            if p['mount'] != None:
+                try:
+                    p['mount'].cleanup()
+                except:
+                    pass
+                p['mount'] = None
+
+    def mount(self):
+        for dev in self.disks.keys():
+            d = self.disks[dev]
+            d['disk'].create()
+
+        self.__format_disks()
+        self.__map_partitions()
+        self.__calculate_mountorder()
+
+        for mp in self.mountOrder:
+            p = None
+            for p1 in self.partitions:
+                if p1['mountpoint'] == mp:
+                    p = p1
+                    break
+
+            if mp == 'swap':
+                subprocess.call(["/sbin/mkswap", p['device']])         
                        
+                continue
+
+            rmmountdir = False
+            if p['mountpoint'] == "/":
+                rmmountdir = True
+            pdisk = ExtDiskMount(RawDisk(p['size'] * 1024 * 1024,
p['device']),
+                                 self.mountdir + p['mountpoint'],
+                                 p['fstype'],
+                                 4096,
+                                 p['mountpoint'],
+                                 rmmountdir)
+            pdisk.mount()
+            p['mount'] = pdisk
+
+    def resparse(self, size = None):
+        # Can't re-sparse a disk image - too hard
+        pass
 
 class DeviceMapperSnapshot(object):
     def __init__(self, imgloop, cowloop):
@@ -478,3 +696,4 @@ def create_image_minimizer(path, image, minimal_size):
     mksquashfs(cowloop.lofile, path)
 
     os.unlink(cowloop.lofile)
+
diff --git a/imgcreate/kickstart.py b/imgcreate/kickstart.py
index ef7b9e4..eff28bb 100644
--- a/imgcreate/kickstart.py
+++ b/imgcreate/kickstart.py
@@ -474,6 +474,9 @@ def get_groups(ks, required = []):
 def get_excluded(ks, required = []):
     return ks.handler.packages.excludedList + required
 
+def get_partitions(ks, required = []):
+    return ks.handler.partition.partitions
+
 def ignore_missing(ks):
     return ks.handler.packages.handleMissing == ksconstants.KS_MISSING_IGNORE
 
@@ -490,3 +493,4 @@ def get_post_scripts(ks):
 
 def selinux_enabled(ks):
     return ks.handler.selinux.selinux == ksconstants.SELINUX_ENFORCING
+
-- 
1.5.4.1

[PATCH] -fs.py-kickstart.py-chages-for-mulit-partitions

[Jeremy Katz]

This I'm still not sure that I really want to be carrying in imgcreate. 
    There's already a lot of overlap functionality-wise with anaconda 
and I know that as soon as we start down this path, we'll end up having 
to add all the other crud for various more "advanced" partitioning 
things (especially LVM and dm-crypt).  Maintaining one copy of that kind 
of code is enough for me ;-)

I'd be okay with adding the kickstart hunk, though.  And then there's 
really not any big reason that I can see why you couldn't just have the 
PartitionedDisk bits just in your caller rather than having them a part 
of the main imgcreate API

Jeremy

[Fedora-livecd-list] [PATCH] -fs.py-kickstart.py-chages-for-mulit-partitions

[Jeremy Katz]

Jeremy Katz wrote:
>  This I'm still not sure that I really want to be carrying in
> imgcreate. 
>     There's already a lot of overlap functionality-wise with anaconda and 
>  I know that as soon as we start down this path, we'll end up having to 
>  add all the other crud for various more "advanced" partitioning things 
>  (especially LVM and dm-crypt).  Maintaining one copy of that kind of 
>  code is enough for me ;-)
>  
>  I'd be okay with adding the kickstart hunk, though.  And then there's 
>  really not any big reason that I can see why you couldn't just have the 
>  PartitionedDisk bits just in your caller rather than having them a part 
>  of the main imgcreate API 
... although I would still strongly ask the question of looking at what 
you're doing and seeing if it makes sense to build your images by 
actually going through an install (in a VM of some sort) rather than 
using imgcreate once you get to the point of needing partitions, etc

Jeremy

[Fedora-livecd-list] [PATCH] -fs.py-kickstart.py-chages-for-mulit-partitions

[David Huff]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Jeremy Katz wrote:
| Jeremy Katz wrote:
|> This I'm still not sure that I really want to be carrying in
|> imgcreate.    There's already a lot of overlap functionality-wise with
|> anaconda and I know that as soon as we start down this path, we'll end
|> up having to add all the other crud for various more "advanced"
|> partitioning things (especially LVM and dm-crypt).  Maintaining one
|> copy of that kind of code is enough for me ;-)
|>
|> I'd be okay with adding the kickstart hunk, though.  And then there's
|> really not any big reason that I can see why you couldn't just have
|> the PartitionedDisk bits just in your caller rather than having them a
|> part of the main imgcreate API
|
| ... although I would still strongly ask the question of looking at what
| you're doing and seeing if it makes sense to build your images by
| actually going through an install (in a VM of some sort) rather than
| using imgcreate once you get to the point of needing partitions, etc

Partitions are needed for mainly two reasons.  The most important reason
is that it allows the use of grub.  This makes any image created with
this tool bootable and it looks and feels like any bare metal system.
It also allows for the ability of creating multiple partitions so an
appliance can have a septate data partition(s).  This allows for
flexibility in your update/data model, where if you want to, you can
switch out your "appliance engine" and keep the data intact.

The ability for a tool to create these partitioned disk images via the
command like, livecd-creator, is very helpful in reproducing known state
appliances, epically with the rising use of visualization.  Going
through a actual install in a vm limits you to VM container, xen, kvm,
vmware and adds extra overhead of building on a machine with extra virt
resources.  We have also noticed that an image that was installed via
anaconda is larger that an image created with the appliance creator tools.



|
| Jeremy
|
| --
| Fedora-livecd-list mailing list
| Fedora-livecd-list(a)redhat.com
| https://www.redhat.com/mailman/listinfo/fedora-livecd-list




- --
David Huff
Red Hat, Raleigh, NC
Mobile: [phone]
Office: 919-754-4129

GPG Key ID: 6A20BBF7
GPG Fingerprint: FE13 8AF6 0E58 D92E A4E1 2D0A 71C1 CADF 6A20 BBF7
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.7 (GNU/Linux)
> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org
>
> iD8DBQFIPZfkccHK32ogu/cRAmclAKCI1upqzhaJWSGikyxlJ7LZMDfe4ACgj+F9
> JJAE6je2msv+vFjc7zO2/Bk=
> =QtyL
> -----END PGP SIGNATURE-----

[Fedora-livecd-list] [PATCH] -fs.py-kickstart.py-chages-for-mulit-partitions

[Jeremy Katz]

David Huff wrote:
>  Jeremy Katz wrote:
>  | Jeremy Katz wrote:
>  |> This I'm still not sure that I really want to be carrying in
>  |> imgcreate.    There's already a lot of overlap functionality-wise with
>  |> anaconda and I know that as soon as we start down this path, we'll end
>  |> up having to add all the other crud for various more "advanced"
>  |> partitioning things (especially LVM and dm-crypt).  Maintaining one
>  |> copy of that kind of code is enough for me ;-)
>  |>
>  |> I'd be okay with adding the kickstart hunk, though.  And then there's
>  |> really not any big reason that I can see why you couldn't just have
>  |> the PartitionedDisk bits just in your caller rather than having them a
>  |> part of the main imgcreate API
>  |
>  | ... although I would still strongly ask the question of looking at what
>  | you're doing and seeing if it makes sense to build your images by
>  | actually going through an install (in a VM of some sort) rather than
>  | using imgcreate once you get to the point of needing partitions, etc
>  
>  Partitions are needed for mainly two reasons.  The most important reason
>  is that it allows the use of grub.  This makes any image created with
>  this tool bootable and it looks and feels like any bare metal system.
>  It also allows for the ability of creating multiple partitions so an
>  appliance can have a septate data partition(s).  This allows for
>  flexibility in your update/data model, where if you want to, you can
>  switch out your "appliance engine" and keep the data intact. 
Oh, I can fully see why partitions are useful for a variety of cases...

>  The ability for a tool to create these partitioned disk images via
> the
>  command like, livecd-creator, is very helpful in reproducing known state
>  appliances, epically with the rising use of visualization.  Going
>  through a actual install in a vm limits you to VM container, xen, kvm,
>  vmware and adds extra overhead of building on a machine with extra virt
>  resources.   
Reproducing known state should work just as well with a kickstart config 
and doing an install via, eg, virt-install.  Yes, it adds overhead, but 
there are also advantages in functionality and it also means *not* 
maintaining this sort of code in multiple places.  And that's a pretty 
big win as partitioning code is not in the least bit trivial. 
Especially as you start trying to handle more and more of the cases.

>  We have also noticed that an image that was installed via
>  anaconda is larger that an image created with the appliance creator  
tools.

There shouldn't be any difference to speak of here.  Take a look at the 
package sets and see if there's anything, but I wouldn't expect this at 
all.  And if so, it's likely more due to bugs in imgcreate than anything 
else

Jeremy

[Fedora-livecd-list] [PATCH] -fs.py-kickstart.py-chages-for-mulit-partitions

[Mark McLoughlin]

On Wed, 2008-05-28 at 14:36 -0400, Jeremy Katz wrote:
>  > The ability for a tool to create these partitioned disk images
> via the
>  > command like, livecd-creator, is very helpful in reproducing known state
>  > appliances, epically with the rising use of visualization.  Going
>  > through a actual install in a vm limits you to VM container, xen, kvm,
>  > vmware and adds extra overhead of building on a machine with extra virt
>  > resources.  
>  
>  Reproducing known state should work just as well with a kickstart config 
>  and doing an install via, eg, virt-install.  Yes, it adds overhead, but 
>  there are also advantages in functionality and it also means *not* 
>  maintaining this sort of code in multiple places.  And that's a pretty 
>  big win as partitioning code is not in the least bit trivial. 
>  Especially as you start trying to handle more and more of the cases. 
That pretty lame - you're trying to curtail what use cases people make
imgcreate support purely because we haven't figured out how to do a
better job of sharing code with anaconda.

The very least we should be doing is allowing imgcreate to be extended
externally in this manner ... but that doesn't fix the "multiple copies
of this code" issue, it merely pushes it elsewhere and makes it less
likely it'll ever be fixed.

Clearly, one advantage of imgcreate tools over a full blown anaconda
install in a VM is that it's far less complex to debug when it all goes
wrong. That's a huge advantage to people repeatedly rebuilding images as
they develop a LiveCD or an appliance.

Cheers,
Mark.

[Fedora-livecd-list] [PATCH] -fs.py-kickstart.py-chages-for-mulit-partitions

[Jeremy Katz]

Mark McLoughlin wrote:
>  On Wed, 2008-05-28 at 14:36 -0400, Jeremy Katz wrote:
> >> The ability for a tool to create these partitioned disk images via the
> >> command like, livecd-creator, is very helpful in reproducing known state
> >> appliances, epically with the rising use of visualization.  Going
> >> through a actual install in a vm limits you to VM container, xen, kvm,
> >> vmware and adds extra overhead of building on a machine with extra virt
> >> resources.  
> > Reproducing known state should work just as well with a kickstart config 
> > and doing an install via, eg, virt-install.  Yes, it adds overhead, but 
> > there are also advantages in functionality and it also means *not* 
> > maintaining this sort of code in multiple places.  And that's a pretty 
> > big win as partitioning code is not in the least bit trivial. 
> > Especially as you start trying to handle more and more of the cases.
>  
>  That pretty lame - you're trying to curtail what use cases people make
>  imgcreate support purely because we haven't figured out how to do a
>  better job of sharing code with anaconda. 
If there were multiple people really clamoring for support to do 
partitioned images, it would be one thing.  But right now, it's one 
tool.  And to then carry the onus outside of that tool for keeping the 
non-trivial bits working?

>  The very least we should be doing is allowing imgcreate to be
> extended
>  externally in this manner ... but that doesn't fix the "multiple copies
>  of this code" issue, it merely pushes it elsewhere and makes it less
>  likely it'll ever be fixed. 
But we are doing the work to allow an external appliance-creator to 
carry the functionality if desired.  That's why the refactoring patch is 
reasonable (modulo fixing up the API break).

>  Clearly, one advantage of imgcreate tools over a full blown anaconda
>  install in a VM is that it's far less complex to debug when it all goes
>  wrong. That's a huge advantage to people repeatedly rebuilding images as
>  they develop a LiveCD or an appliance. 
The reason that it's less complex to debug is the fact that the code is 
more limited in what it supports than anaconda.  Adding more 
(complicated) pieces is directly in contradiction to keeping things 
simple to debug

Jeremy

[Fedora-livecd-list] [PATCH] -fs.py-kickstart.py-chages-for-mulit-partitions

[Bryan Kearney]

Jeremy Katz wrote:
>  Mark McLoughlin wrote:
> > On Wed, 2008-05-28 at 14:36 -0400, Jeremy Katz wrote:
> >>> The ability for a tool to create these partitioned disk images via the
> >>> command like, livecd-creator, is very helpful in reproducing known 
> >>> state
> >>> appliances, epically with the rising use of visualization.  Going
> >>> through a actual install in a vm limits you to VM container, xen, kvm,
> >>> vmware and adds extra overhead of building on a machine with extra virt
> >>> resources.  
> >> Reproducing known state should work just as well with a kickstart 
> >> config and doing an install via, eg, virt-install.  Yes, it adds 
> >> overhead, but there are also advantages in functionality and it also 
> >> means *not* maintaining this sort of code in multiple places.  And 
> >> that's a pretty big win as partitioning code is not in the least bit 
> >> trivial. Especially as you start trying to handle more and more of 
> >> the cases.
> >
> > That pretty lame - you're trying to curtail what use cases people make
> > imgcreate support purely because we haven't figured out how to do a
> > better job of sharing code with anaconda.
>  
>  If there were multiple people really clamoring for support to do 
>  partitioned images, it would be one thing.  But right now, it's one 
>  tool.  And to then carry the onus outside of that tool for keeping the 
>  non-trivial bits working?
>  
> > The very least we should be doing is allowing imgcreate to be extended
> > externally in this manner ... but that doesn't fix the "multiple copies
> > of this code" issue, it merely pushes it elsewhere and makes it less
> > likely it'll ever be fixed.
>  
>  But we are doing the work to allow an external appliance-creator to 
>  carry the functionality if desired.  That's why the refactoring patch is 
>  reasonable (modulo fixing up the API break).
>  
> > Clearly, one advantage of imgcreate tools over a full blown anaconda
> > install in a VM is that it's far less complex to debug when it all goes
> > wrong. That's a huge advantage to people repeatedly rebuilding images as
> > they develop a LiveCD or an appliance.
>  
>  The reason that it's less complex to debug is the fact that the code is 
>  more limited in what it supports than anaconda.  Adding more 
>  (complicated) pieces is directly in contradiction to keeping things 
>  simple to debug
>   
So.. to summarize.. as long as the partitioning code is in the appliance 
creator only.. we are fine. Is that correct?

-- bk

[Fedora-livecd-list] [PATCH] -fs.py-kickstart.py-chages-for-mulit-partitions

[Tim Wood]

I've been flat on my back for a while, so if I repeat anything 
previously sad, apologies.

I've got a kickstart that is used to create a two partition solution on 
a 1G flash device.  On the code side, I use a number of shell scripts.  
But, the main thing that makes is work is that I use a directory 
structure that looks like this

project
    - files/
    - partition_2/
    - kickstart file

files holds various odds & ends (some custom rpms, some specific files, 
etc).  partition_2 holds all the stuff I want on the second partion.  I 
then have a simple script that backbones the livecd creation process and 
a second script that bundles up everything in the partition 2 directory 
as another iso.  When I'm ready to burn the stuff to flash, I run a 
second script that handles the partitioning, writes the livecd to the 
first partition and the partition_2 iso's contents to the second partition.

Mark, if this sounds helpful, I'd be happy to tar/zip the scripts in 
question with some basic directions.  If the person who created the 
livecd creation tool (sorry ... I've forgotten your name and I seem to 
have accidently deleted the email) is interested, I could turn it into 
an rpm that could become part of that kickstart.

Tim


Mark McLoughlin wrote:
>  On Wed, 2008-05-28 at 14:36 -0400, Jeremy Katz wrote:
>    
> >> The ability for a tool to create these partitioned disk images via the
> >> command like, livecd-creator, is very helpful in reproducing known state
> >> appliances, epically with the rising use of visualization.  Going
> >> through a actual install in a vm limits you to VM container, xen, kvm,
> >> vmware and adds extra overhead of building on a machine with extra virt
> >> resources.  
> >>       
> > Reproducing known state should work just as well with a kickstart config 
> > and doing an install via, eg, virt-install.  Yes, it adds overhead, but 
> > there are also advantages in functionality and it also means *not* 
> > maintaining this sort of code in multiple places.  And that's a pretty 
> > big win as partitioning code is not in the least bit trivial. 
> > Especially as you start trying to handle more and more of the cases.
> >     
>
>  That pretty lame - you're trying to curtail what use cases people make
>  imgcreate support purely because we haven't figured out how to do a
>  better job of sharing code with anaconda.
>
>  The very least we should be doing is allowing imgcreate to be extended
>  externally in this manner ... but that doesn't fix the "multiple copies
>  of this code" issue, it merely pushes it elsewhere and makes it less
>  likely it'll ever be fixed.
>
>  Clearly, one advantage of imgcreate tools over a full blown anaconda
>  install in a VM is that it's far less complex to debug when it all goes
>  wrong. That's a huge advantage to people repeatedly rebuilding images as
>  they develop a LiveCD or an appliance.
>
>  Cheers,
>  Mark.
>
>  --
>  Fedora-livecd-list mailing list
>  Fedora-livecd-list(a)redhat.com
>  https://www.redhat.com/mailman/listinfo/fedora-livecd-list
>
>     

-- 
_________________________________
Tim Wood, CLP, RHCT
719.338.7484 (tel)

The Data Wranglers
Web, Database & more since    since 1994
www.datawranglers.com

[PATCH] -Refactor-disk-mount-classes-to-allow-multi-partitions

[Jeremy Katz]

So this patch ends up changing the API of imgcreate which then has the 
possibility of breaking any other callers.  While that would have been 
okay had the patch been finalized when it was first sent, we now can't 
do that.  Instead, there at least needs to be a compatibility layer for 
the old API to continue working.  This should be pretty trivial to wrap, 
though, so that both ways work.  And then it would make sense to have 
this patch split up a bit so that it's a series which is
1) Add new classes
2) Compat shims switching over the old class types to sit on top of the 
new classes
3) Switch internal code users to use the new classes

Jeremy

[Fedora-livecd-list] [PATCH] -Refactor-disk-mount-classes-to-allow-multi-partitions

[David Huff]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Jeremy Katz wrote:
| So this patch ends up changing the API of imgcreate which then has the
| possibility of breaking any other callers.  While that would have been
| okay had the patch been finalized when it was first sent, we now can't
| do that.

Why is this, just out of curiosity?

Instead, there at least needs to be a compatibility layer for
| the old API to continue working.  This should be pretty trivial to wrap,
| though, so that both ways work.  And then it would make sense to have
| this patch split up a bit so that it's a series which is
| 1) Add new classes
| 2) Compat shims switching over the old class types to sit on top of the
| new classes
| 3) Switch internal code users to use the new classes

I will work on moving this over to this type of compatibility
architecture.  Do you know of any examples I can look at.

|
| Jeremy
|
| --
| Fedora-livecd-list mailing list
| Fedora-livecd-list(a)redhat.com
| https://www.redhat.com/mailman/listinfo/fedora-livecd-list




- --
David Huff
Red Hat, Raleigh, NC
Mobile: [phone]
Office: 919-754-4129

GPG Key ID: 6A20BBF7
GPG Fingerprint: FE13 8AF6 0E58 D92E A4E1 2D0A 71C1 CADF 6A20 BBF7
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.7 (GNU/Linux)
> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org
>
> iD8DBQFIPZfBccHK32ogu/cRAubPAJ9c6imBocwAi0MrqRvsCebzwG1i+QCfbzGc
> 6JYgXEdgSZiFy27k5FjKzPs=
> =l38L
> -----END PGP SIGNATURE-----

[Fedora-livecd-list] [PATCH] -Refactor-disk-mount-classes-to-allow-multi-partitions

[Jeremy Katz]

David Huff wrote:
>  Jeremy Katz wrote:
>  | So this patch ends up changing the API of imgcreate which then has the
>  | possibility of breaking any other callers.  While that would have been
>  | okay had the patch been finalized when it was first sent, we now can't
>  | do that.
>  
>  Why is this, just out of curiosity? 
When the patch was originally posted, we hadn't had a release of 
livecd-tools with the API.  Now that the API is out there, we can't just 
change it out from under people.

>  Instead, there at least needs to be a compatibility layer for
>  | the old API to continue working.  This should be pretty trivial to wrap,
>  | though, so that both ways work.  And then it would make sense to have
>  | this patch split up a bit so that it's a series which is
>  | 1) Add new classes
>  | 2) Compat shims switching over the old class types to sit on top of the
>  | new classes
>  | 3) Switch internal code users to use the new classes
>  
>  I will work on moving this over to this type of compatibility
>  architecture.  Do you know of any examples I can look at. 
Nothing off-hand.  But it should be pretty straight-forward.  You'll 
just want to create a new, eg, LoopbackSpareMount class that sits on top 
of the new classes and internally creates the LoopbackDisk, etc objects

Jeremy

[PATCH] added get_partitions to kickstart.py

[David Huff]

---
 imgcreate/kickstart.py |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/imgcreate/kickstart.py b/imgcreate/kickstart.py
index 0f797b8..0fbefb7 100644
--- a/imgcreate/kickstart.py
+++ b/imgcreate/kickstart.py
@@ -495,6 +495,9 @@ def get_groups(ks, required = []):
 def get_excluded(ks, required = []):
     return ks.handler.packages.excludedList + required
 
+def get_partitions(ks, required = []):
+    return ks.handler.partition.partitions
+
 def ignore_missing(ks):
     return ks.handler.packages.handleMissing == ksconstants.KS_MISSING_IGNORE
 
-- 
1.5.4.1

[PATCH] added get_partitions to kickstart.py

[Jeremy Katz]

So, generally looking okay from the look I did.  Two big little things
left that I see

1) Commit messages should be more than one-liners.
http://www.tpope.net/node/106 actually has a really nice description of
what makes a good commit message
2) Instead of funky formatting of comments, just use the standard pydoc
""" -- it'll also mean that we can extract api information and use
help() if you're using the python interpreter on the command-line

Jeremy

re-factor the imgcreate/fs.py module edits

[David Huff]

Patch Edits included.  Patches still require the patch "added get_partitions to
kickstart.py."  Also the later patch that changes creator.py and live.py to use new
api, "switch internal code to use new api," is still applicable.

I have redone the "added compat shims for old api" patch to implement all public
methods in the old API.  I am pretty sure this is the correct functionality that you were
looking for however feedback is welcome.

-D

[PATCH] re-factor imgcreate/fs.py module

[David Huff]

From: Daniel P. Berrange <berrange redhat com>

Most of the change here involves re-factoring the imgcreate/fs.py module.
The SparseExtLoopbackMount, SparseLoopbackMount, LoopbackMount classes all
have the built-in limitation that the image being produced corresponds to
a single filesystem / loop mount.  With the disk creator, the image being
produced can be partitioned into multiple chunks, with many filesystems.
Furthermore, not all of them require loopback mounts, as the partitions
themselves are already visible via /dev/mapper/

So this patch separates the roles. There are now classes which deal with
accessing / creating disks:

  Disk               - generic base for disks
  RawDisk            - a disk backed by a block device
  LoopbackDisk       - a disk backed by a file
  SparseLoopbackDisk - a disk backed by a sparse file

The 'create' method must make the disk visible as a block device - eg
by calling losetup. For RawDisk, this is obviously a no-op. The 'cleanup'
method must undo the 'create' operation.

There are then classes which deal with mounting things:

   Mount        - generic base for mounts
   DiskMount    -  able to mount a Disk object
   ExtDiskMount - able to format/resize ext3 filesystems when mounting

Patch modified by David Huff <dhuff(a)redhat.com&gt;
Also fixed comments to use the standard pydoc """ format.
---
 imgcreate/fs.py |  346 +++++++++++++++++++++++++++++++++++++++++++++++++++----
 1 files changed, 322 insertions(+), 24 deletions(-)

diff --git a/imgcreate/fs.py b/imgcreate/fs.py
index 98c0db4..16cbbba 100644
--- a/imgcreate/fs.py
+++ b/imgcreate/fs.py
@@ -24,6 +24,7 @@ import stat
 import subprocess
 import random
 import string
+import logging
 
 from imgcreate.errors import *
 
@@ -288,6 +289,303 @@ class SparseExtLoopbackMount(SparseLoopbackMount):
         self.resize(size)
         return minsize
 
+class Disk:
+    """
+    With the new disk API the image being produced can be partitioned into 
+    multiple chunks, with many filesystems.  Furthermore, not all of them 
+    require loopback mounts, as the partitions themselves are already 
+    visible via /dev/mapper/
+    
+    There are now classes which deal with accessing / creating disks:
+    
+      Disk               - generic base for disks
+      RawDisk            - a disk backed by a block device
+      LoopbackDisk       - a disk backed by a file
+      SparseLoopbackDisk - a disk backed by a sparse file
+    
+    The 'create' method must make the disk visible as a block device - eg
+    by calling losetup. For RawDisk, this is obviously a no-op. The 'cleanup'
+    method must undo the 'create' operation.
+    
+    There are then classes which deal with mounting things:
+    
+       Mount        - generic base for mounts
+       DiskMount    -  able to mount a Disk object
+       ExtDiskMount - able to format/resize ext3 filesystems when mounting
+    """
+    def __init__(self, size, device = None):
+        self._device = device
+        self._size = size
+
+    def create(self):
+        pass
+
+    def cleanup(self):
+        pass
+
+    def get_device(self):
+        return self._device
+    def set_device(self, path):
+        self._device = path
+    device = property(get_device, set_device)
+
+    def get_size(self):
+        return self._size
+    size = property(get_size)
+
+
+class RawDisk(Disk):
+    def __init__(self, size, device):
+        Disk.__init__(self, size, device)
+
+    def fixed(self):
+        return True
+
+    def exists(self):
+        return True
+
+class LoopbackDisk(Disk):
+    def __init__(self, lofile, size):
+        Disk.__init__(self, size)
+        self.lofile = lofile
+
+    def fixed(self):
+        return False
+
+    def exists(self):
+        return os.path.exists(self.lofile)
+
+    def create(self):
+        if self.device is not None:
+            return
+
+        losetupProc = subprocess.Popen(["/sbin/losetup", "-f"],
+                                       stdout=subprocess.PIPE)
+        losetupOutput = losetupProc.communicate()[0]
+
+        if losetupProc.returncode:
+            raise MountError("Failed to allocate loop device for '%s'"
%
+                             self.lofile)
+
+        device = losetupOutput.split()[0]
+
+        logging.debug("Losetup add %s mapping to %s"  % (device, self.lofile))
+        rc = subprocess.call(["/sbin/losetup", device, self.lofile])
+        if rc != 0:
+            raise MountError("Failed to allocate loop device for '%s'"
%
+                             self.lofile)
+        self.device = device
+
+    def cleanup(self):
+        if self.device is None:
+            return
+        logging.debug("Losetup remove %s" % self.device)
+        rc = subprocess.call(["/sbin/losetup", "-d", self.device])
+        self.device = None
+
+
+
+class SparseLoopbackDisk(LoopbackDisk):
+    def __init__(self, lofile, size):
+        LoopbackDisk.__init__(self, lofile, size)
+
+    def expand(self, create = False, size = None):
+        flags = os.O_WRONLY
+        if create:
+            flags |= os.O_CREAT
+            makedirs(os.path.dirname(self.lofile))
+
+        if size is None:
+            size = self.size
+
+        logging.debug("Extending sparse file %s to %d" % (self.lofile, size))
+        fd = os.open(self.lofile, flags)
+
+        os.lseek(fd, size, 0)
+        os.write(fd, '\x00')
+        os.close(fd)
+
+    def truncate(self, size = None):
+        if size is None:
+            size = self.size
+
+        logging.debug("Truncating sparse file %s to %d" % (self.lofile, size))
+        fd = os.open(self.lofile, os.O_WRONLY)
+        os.ftruncate(fd, size)
+        os.close(fd)
+
+    def create(self):
+        self.expand(create = True)
+        LoopbackDisk.create(self)
+
+class Mount:
+    def __init__(self, mountdir):
+        self.mountdir = mountdir
+
+    def cleanup(self):
+        self.unmount()
+
+    def mount(self):
+        pass
+
+    def unmount(self):
+        pass
+
+class DiskMount(Mount):
+    def __init__(self, disk, mountdir, fstype = None, rmmountdir = True):
+        Mount.__init__(self, mountdir)
+
+        self.disk = disk
+        self.fstype = fstype
+        self.rmmountdir = rmmountdir
+
+        self.mounted = False
+        self.rmdir   = False
+
+    def cleanup(self):
+        Mount.cleanup(self)
+        self.disk.cleanup()
+
+    def unmount(self):
+        if self.mounted:
+            logging.debug("Unmounting directory %s" % self.mountdir)
+            rc = subprocess.call(["/bin/umount", self.mountdir])
+            if rc == 0:
+                self.mounted = False
+
+        if self.rmdir and not self.mounted:
+            try:
+                os.rmdir(self.mountdir)
+            except OSError, e:
+                pass
+            self.rmdir = False
+
+
+    def __create(self):
+        self.disk.create()
+
+
+    def mount(self):
+        if self.mounted:
+            return
+
+        if not os.path.isdir(self.mountdir):
+            logging.debug("Creating mount point %s" % self.mountdir)
+            os.makedirs(self.mountdir)
+            self.rmdir = self.rmmountdir
+
+        self.__create()
+
+        logging.debug("Mounting %s at %s" % (self.disk.device, self.mountdir))
+        args = [ "/bin/mount", self.disk.device, self.mountdir ]
+        if self.fstype:
+            args.extend(["-t", self.fstype])
+
+        rc = subprocess.call(args)
+        if rc != 0:
+            raise MountError("Failed to mount '%s' to '%s'" %
+                             (self.disk.device, self.mountdir))
+
+        self.mounted = True
+
+class ExtDiskMount(DiskMount):
+    def __init__(self, disk, mountdir, fstype, blocksize, fslabel, rmmountdir=True):
+        DiskMount.__init__(self, disk, mountdir, fstype, rmmountdir)
+        self.blocksize = blocksize
+        self.fslabel = fslabel
+
+    def __format_filesystem(self):
+        logging.debug("Formating %s filesystem on %s" % (self.fstype,
self.disk.device))
+        rc = subprocess.call(["/sbin/mkfs." + self.fstype,
+                              "-F", "-L", self.fslabel,
+                              "-m", "1", "-b",
str(self.blocksize),
+                              self.disk.device])
+        #                      str(self.disk.size / self.blocksize)])
+        if rc != 0:
+            raise MountError("Error creating %s filesystem" % (self.fstype,))
+        logging.debug("Tuning filesystem on %s" % self.disk.device)
+        subprocess.call(["/sbin/tune2fs", "-c0", "-i0",
"-Odir_index",
+                         "-ouser_xattr,acl", self.disk.device])
+
+    def __resize_filesystem(self, size = None):
+        current_size = os.stat(self.disk.lofile)[stat.ST_SIZE]
+
+        if size is None:
+            size = self.size
+
+        if size == current_size:
+            return
+
+        if size > current_size:
+            self.expand(size)
+
+        self.__fsck()
+
+        resize2fs(self.disk.lofile, size)
+        return size
+
+    def __create(self):
+        resize = False
+        if not self.disk.fixed() and self.disk.exists():
+            resize = True
+
+        self.disk.create()
+
+        if resize:
+            self.__resize_filesystem()
+        else:
+            self.__format_filesystem()
+
+    def mount(self):
+        self.__create()
+        DiskMount.mount(self)
+
+    def __fsck(self):
+        logging.debug("Checking filesystem %s" % self.disk.lofile)
+        subprocess.call(["/sbin/e2fsck", "-f", "-y",
self.disk.lofile])
+
+    def __get_size_from_filesystem(self):
+        def parse_field(output, field):
+            for line in output.split("\n"):
+                if line.startswith(field + ":"):
+                    return line[len(field) + 1:].strip()
+
+            raise KeyError("Failed to find field '%s' in output" %
field)
+
+        dev_null = os.open("/dev/null", os.O_WRONLY)
+        try:
+            out = subprocess.Popen(['/sbin/dumpe2fs', '-h',
self.disk.lofile],
+                                   stdout = subprocess.PIPE,
+                                   stderr = dev_null).communicate()[0]
+        finally:
+            os.close(dev_null)
+
+        return int(parse_field(out, "Block count")) * self.blocksize
+
+    def __resize_to_minimal(self):
+        self.__fsck()
+
+        #
+        # Use a binary search to find the minimal size
+        # we can resize the image to
+        #
+        bot = 0
+        top = self.__get_size_from_filesystem()
+        while top != (bot + 1):
+            t = bot + ((top - bot) / 2)
+
+            if not resize2fs(self.disk.lofile, t):
+                top = t
+            else:
+                bot = t
+        return top
+
+    def resparse(self, size = None):
+        self.cleanup()
+        minsize = self.__resize_to_minimal()
+        self.disk.truncate(minsize)
+        return minsize
+
 class DeviceMapperSnapshot(object):
     def __init__(self, imgloop, cowloop):
         self.imgloop = imgloop
@@ -306,8 +604,8 @@ class DeviceMapperSnapshot(object):
         if self.__created:
             return
 
-        self.imgloop.loopsetup()
-        self.cowloop.loopsetup()
+        self.imgloop.create()
+        self.cowloop.create()
 
         self.__name = "imgcreate-%d-%d" % (os.getpid(),
                                            random.randint(0, 2**16))
@@ -315,8 +613,8 @@ class DeviceMapperSnapshot(object):
         size = os.stat(self.imgloop.lofile)[stat.ST_SIZE]
 
         table = "0 %d snapshot %s %s p 8" % (size / 512,
-                                             self.imgloop.loopdev,
-                                             self.cowloop.loopdev)
+                                             self.imgloop.device,
+                                             self.cowloop.device)
 
         args = ["/sbin/dmsetup", "create", self.__name,
"--table", table]
         if subprocess.call(args) != 0:
@@ -365,32 +663,31 @@ class DeviceMapperSnapshot(object):
         except ValueError:
             raise SnapshotError("Failed to parse dmsetup status: " + out)
 
-#
-# Builds a copy-on-write image which can be used to
-# create a device-mapper snapshot of an image where
-# the image's filesystem is as small as possible
-#
-# The steps taken are:
-#   1) Create a sparse COW
-#   2) Loopback mount the image and the COW
-#   3) Create a device-mapper snapshot of the image
-#      using the COW
-#   4) Resize the filesystem to the minimal size
-#   5) Determine the amount of space used in the COW
-#   6) Restroy the device-mapper snapshot
-#   7) Truncate the COW, removing unused space
-#   8) Create a squashfs of the COW
-#
 def create_image_minimizer(path, image, minimal_size):
-    imgloop = LoopbackMount(image, "None")
+    """
+    Builds a copy-on-write image which can be used to
+    create a device-mapper snapshot of an image where
+    the image's filesystem is as small as possible
+    
+    The steps taken are:
+      1) Create a sparse COW
+      2) Loopback mount the image and the COW
+      3) Create a device-mapper snapshot of the image
+         using the COW
+      4) Resize the filesystem to the minimal size
+      5) Determine the amount of space used in the COW
+      6) Restroy the device-mapper snapshot
+      7) Truncate the COW, removing unused space
+      8) Create a squashfs of the COW
+    """
+    imgloop = LoopbackDisk(image, None) # Passing bogus size - doesn't matter
 
-    cowloop = SparseLoopbackMount(os.path.join(os.path.dirname(path),
"osmin"),
-                                  None, 64L * 1024L * 1024L)
+    cowloop = SparseLoopbackDisk(os.path.join(os.path.dirname(path), "osmin"),
+                                 64L * 1024L * 1024L)
 
     snapshot = DeviceMapperSnapshot(imgloop, cowloop)
 
     try:
-        cowloop.create()
         snapshot.create()
 
         resize2fs(snapshot.path, minimal_size)
@@ -404,3 +701,4 @@ def create_image_minimizer(path, image, minimal_size):
     mksquashfs(cowloop.lofile, path)
 
     os.unlink(cowloop.lofile)
+
-- 
1.5.4.1

[PATCH] added compat shims in fs.py for old api

[David Huff]

Compat shims switching over the old classes, LoopbackMount, SparseLoopbackMount,
SparseExtLoopbackMount, types to sit on top of the new Disk and Mount classes.
---
 imgcreate/fs.py |  166 ++++++++----------------------------------------------
 1 files changed, 25 insertions(+), 141 deletions(-)

diff --git a/imgcreate/fs.py b/imgcreate/fs.py
index 16cbbba..6de99fd 100644
--- a/imgcreate/fs.py
+++ b/imgcreate/fs.py
@@ -88,32 +88,16 @@ class BindChrootMount:
         self.mounted = False
 
 class LoopbackMount:
+    """LoopbackMount  compatibility layer for old API"""
     def __init__(self, lofile, mountdir, fstype = None):
-        self.lofile = lofile
-        self.mountdir = mountdir
-        self.fstype = fstype
-
-        self.mounted = False
+        DiskMount.__init__(LoopbackDisk(lofile,size = 0),mountdir,fstype,rmmountdir =
True)
         self.losetup = False
-        self.rmdir   = False
-        self.loopdev = None
-
+        
     def cleanup(self):
-        self.unmount()
-        self.lounsetup()
+        DiskMount.cleanup(self)
 
     def unmount(self):
-        if self.mounted:
-            rc = subprocess.call(["/bin/umount", self.mountdir])
-            if rc == 0:
-                self.mounted = False
-
-        if self.rmdir and not self.mounted:
-            try:
-                os.rmdir(self.mountdir)
-            except OSError, e:
-                pass
-            self.rmdir = False
+        DiskMount.unmount(self)
 
     def lounsetup(self):
         if self.losetup:
@@ -143,152 +127,52 @@ class LoopbackMount:
         self.losetup = True
 
     def mount(self):
-        if self.mounted:
-            return
-
-        self.loopsetup()
-
-        if not os.path.isdir(self.mountdir):
-            os.makedirs(self.mountdir)
-            self.rmdir = True
-
-        args = [ "/bin/mount", self.loopdev, self.mountdir ]
-        if self.fstype:
-            args.extend(["-t", self.fstype])
-
-        rc = subprocess.call(args)
-        if rc != 0:
-            raise MountError("Failed to mount '%s' to '%s'" %
-                             (self.loopdev, self.mountdir))
-
-        self.mounted = True
+        DiskMount.mount(self)
 
 class SparseLoopbackMount(LoopbackMount):
+    """LoopbackMount  compatibility layer for old API"""
     def __init__(self, lofile, mountdir, size, fstype = None):
-        LoopbackMount.__init__(self, lofile, mountdir, fstype)
-        self.size = size
+        DiskMount.__init__(SparseLoopbackDisk(lofile,size),mountdir,fstype,rmmountdir =
True)
 
     def expand(self, create = False, size = None):
-        flags = os.O_WRONLY
-        if create:
-            flags |= os.O_CREAT
-            makedirs(os.path.dirname(self.lofile))
-
-        if size is None:
-            size = self.size
-
-        fd = os.open(self.lofile, flags)
-
-        os.lseek(fd, size, 0)
-        os.write(fd, '\x00')
-        os.close(fd)
+        SparseLoopbackDisk.expand(self, create, size)
 
     def truncate(self, size = None):
-        if size is None:
-            size = self.size
-        fd = os.open(self.lofile, os.O_WRONLY)
-        os.ftruncate(fd, size)
-        os.close(fd)
+        SparseLoopbackDisk.truncate(self, size)
 
     def create(self):
-        self.expand(create = True)
+        SparseLoopbackDisk.create(self)
 
 class SparseExtLoopbackMount(SparseLoopbackMount):
+    """LoopbackMount  compatibility layer for old API"""
     def __init__(self, lofile, mountdir, size, fstype, blocksize, fslabel):
-        SparseLoopbackMount.__init__(self, lofile, mountdir, size, fstype)
-        self.blocksize = blocksize
-        self.fslabel = fslabel
+        ExtDiskMount.__init__(SparseLoopbackDisk(lofile,size), mountdir, fstype,
blocksize, fslabel, rmmountdir = True)
+
 
     def __format_filesystem(self):
-        rc = subprocess.call(["/sbin/mkfs." + self.fstype,
-                              "-F", "-L", self.fslabel,
-                              "-m", "1", "-b",
str(self.blocksize),
-                              self.lofile,
-                              str(self.size / self.blocksize)])
-        if rc != 0:
-            raise MountError("Error creating %s filesystem" % (self.fstype,))
-        subprocess.call(["/sbin/tune2fs", "-c0", "-i0",
"-Odir_index",
-                         "-ouser_xattr,acl", self.lofile])
+        ExtDiskMount.__format_filesystem(self)
 
     def create(self):
-        SparseLoopbackMount.create(self)
-        self.__format_filesystem()
+        SparseLoopbackDisk.create(self)
 
     def resize(self, size = None):
-        current_size = os.stat(self.lofile)[stat.ST_SIZE]
-
-        if size is None:
-            size = self.size
-
-        if size == current_size:
-            return
-
-        if size > current_size:
-            self.expand(size)
-
-        self.__fsck()
-
-        resize2fs(self.lofile, size)
-
-        if size < current_size:
-            self.truncate(size)
-        return size
+        ExtDiskMount.__resize_filesystem(self, size)
 
     def mount(self):
-        if not os.path.isfile(self.lofile):
-            self.create()
-        else:
-            self.resize()
-        return SparseLoopbackMount.mount(self)
-
+        ExtDiskMount.mount(self)
+        
     def __fsck(self):
-        subprocess.call(["/sbin/e2fsck", "-f", "-y",
self.lofile])
+        ExtDiskMount.__fsck(self)
 
     def __get_size_from_filesystem(self):
-        def parse_field(output, field):
-            for line in output.split("\n"):
-                if line.startswith(field + ":"):
-                    return line[len(field) + 1:].strip()
-
-            raise KeyError("Failed to find field '%s' in output" %
field)
-
-        dev_null = os.open("/dev/null", os.O_WRONLY)
-        try:
-            out = subprocess.Popen(['/sbin/dumpe2fs', '-h',
self.lofile],
-                                   stdout = subprocess.PIPE,
-                                   stderr = dev_null).communicate()[0]
-        finally:
-            os.close(dev_null)
-
-        return int(parse_field(out, "Block count")) * self.blocksize
-
+        ExtDiskMount.__get_size_from_filesystem(self)
+        
     def __resize_to_minimal(self):
-        self.__fsck()
-
-        #
-        # Use a binary search to find the minimal size
-        # we can resize the image to
-        #
-        bot = 0
-        top = self.__get_size_from_filesystem()
-        while top != (bot + 1):
-            t = bot + ((top - bot) / 2)
-
-            if not resize2fs(self.lofile, t):
-                top = t
-            else:
-                bot = t
-        return top
-
+        ExtDiskMount.__resize_to_minimal(self)
+        
     def resparse(self, size = None):
-        self.cleanup()
+        ExtDiskMount.resparse(self, size)
         
-        minsize = self.__resize_to_minimal()
-
-        self.truncate(minsize)
-        self.resize(size)
-        return minsize
-
 class Disk:
     """
     With the new disk API the image being produced can be partitioned into 
-- 
1.5.4.1

[PATCH] added compat shims in fs.py for old api

[David Huff]

fixed compatibility issues as well as image resizing issues.  I have tested this patch
along with the other patches against the latest livecd tools with no issues.

related patches needed:
[PATCH] added get_partitions to kickstart.py

[PATCH] re-factor imgcreate/fs.py module

this one

[PATCH] switch internal code to use new api

[PATCH] added compat shims in fs.py for old api

[David Huff]

Added a compat layer for backwards compatibility.  These compat classes internally create
the new Disk objects, switching the old api (LoopbackMount, SparseLoopbackMount,
SparseExtLoopbackMount) types to the new Disk and Mount classes
---
 imgcreate/fs.py |  166 ++++++++----------------------------------------------
 1 files changed, 25 insertions(+), 141 deletions(-)

diff --git a/imgcreate/fs.py b/imgcreate/fs.py
index 16cbbba..933cadc 100644
--- a/imgcreate/fs.py
+++ b/imgcreate/fs.py
@@ -88,32 +88,16 @@ class BindChrootMount:
         self.mounted = False
 
 class LoopbackMount:
+    """LoopbackMount  compatibility layer for old API"""
     def __init__(self, lofile, mountdir, fstype = None):
-        self.lofile = lofile
-        self.mountdir = mountdir
-        self.fstype = fstype
-
-        self.mounted = False
+        self.diskmount = DiskMount(LoopbackDisk(lofile,size =
0),mountdir,fstype,rmmountdir = True)
         self.losetup = False
-        self.rmdir   = False
-        self.loopdev = None
-
+        
     def cleanup(self):
-        self.unmount()
-        self.lounsetup()
+        self.diskmount.cleanup()
 
     def unmount(self):
-        if self.mounted:
-            rc = subprocess.call(["/bin/umount", self.mountdir])
-            if rc == 0:
-                self.mounted = False
-
-        if self.rmdir and not self.mounted:
-            try:
-                os.rmdir(self.mountdir)
-            except OSError, e:
-                pass
-            self.rmdir = False
+        self.diskmount.unmount()
 
     def lounsetup(self):
         if self.losetup:
@@ -143,152 +127,52 @@ class LoopbackMount:
         self.losetup = True
 
     def mount(self):
-        if self.mounted:
-            return
-
-        self.loopsetup()
-
-        if not os.path.isdir(self.mountdir):
-            os.makedirs(self.mountdir)
-            self.rmdir = True
-
-        args = [ "/bin/mount", self.loopdev, self.mountdir ]
-        if self.fstype:
-            args.extend(["-t", self.fstype])
-
-        rc = subprocess.call(args)
-        if rc != 0:
-            raise MountError("Failed to mount '%s' to '%s'" %
-                             (self.loopdev, self.mountdir))
-
-        self.mounted = True
+        self.diskmount.mount()
 
 class SparseLoopbackMount(LoopbackMount):
+    """SparseLoopbackMount  compatibility layer for old
API"""
     def __init__(self, lofile, mountdir, size, fstype = None):
-        LoopbackMount.__init__(self, lofile, mountdir, fstype)
-        self.size = size
+        self.diskmount =
DiskMount(SparseLoopbackDisk(lofile,size),mountdir,fstype,rmmountdir = True)
 
     def expand(self, create = False, size = None):
-        flags = os.O_WRONLY
-        if create:
-            flags |= os.O_CREAT
-            makedirs(os.path.dirname(self.lofile))
-
-        if size is None:
-            size = self.size
-
-        fd = os.open(self.lofile, flags)
-
-        os.lseek(fd, size, 0)
-        os.write(fd, '\x00')
-        os.close(fd)
+        self.diskmount.disk.expand(create, size)
 
     def truncate(self, size = None):
-        if size is None:
-            size = self.size
-        fd = os.open(self.lofile, os.O_WRONLY)
-        os.ftruncate(fd, size)
-        os.close(fd)
+        self.diskmount.disk.truncate(size)
 
     def create(self):
-        self.expand(create = True)
+        self.diskmount.disk.create()
 
 class SparseExtLoopbackMount(SparseLoopbackMount):
+    """SparseExtLoopbackMount  compatibility layer for old
API"""
     def __init__(self, lofile, mountdir, size, fstype, blocksize, fslabel):
-        SparseLoopbackMount.__init__(self, lofile, mountdir, size, fstype)
-        self.blocksize = blocksize
-        self.fslabel = fslabel
+        self.diskmount = ExtDiskMount(SparseLoopbackDisk(lofile,size), mountdir, fstype,
blocksize, fslabel, rmmountdir = True)
+
 
     def __format_filesystem(self):
-        rc = subprocess.call(["/sbin/mkfs." + self.fstype,
-                              "-F", "-L", self.fslabel,
-                              "-m", "1", "-b",
str(self.blocksize),
-                              self.lofile,
-                              str(self.size / self.blocksize)])
-        if rc != 0:
-            raise MountError("Error creating %s filesystem" % (self.fstype,))
-        subprocess.call(["/sbin/tune2fs", "-c0", "-i0",
"-Odir_index",
-                         "-ouser_xattr,acl", self.lofile])
+        self.diskmount.__format_filesystem()
 
     def create(self):
-        SparseLoopbackMount.create(self)
-        self.__format_filesystem()
+        self.diskmount.disk.create()
 
     def resize(self, size = None):
-        current_size = os.stat(self.lofile)[stat.ST_SIZE]
-
-        if size is None:
-            size = self.size
-
-        if size == current_size:
-            return
-
-        if size > current_size:
-            self.expand(size)
-
-        self.__fsck()
-
-        resize2fs(self.lofile, size)
-
-        if size < current_size:
-            self.truncate(size)
-        return size
+        self.diskmount.__resize_filesystem(size)
 
     def mount(self):
-        if not os.path.isfile(self.lofile):
-            self.create()
-        else:
-            self.resize()
-        return SparseLoopbackMount.mount(self)
-
+        self.diskmount.mount()
+        
     def __fsck(self):
-        subprocess.call(["/sbin/e2fsck", "-f", "-y",
self.lofile])
+        self.extdiskmount.__fsck()
 
     def __get_size_from_filesystem(self):
-        def parse_field(output, field):
-            for line in output.split("\n"):
-                if line.startswith(field + ":"):
-                    return line[len(field) + 1:].strip()
-
-            raise KeyError("Failed to find field '%s' in output" %
field)
-
-        dev_null = os.open("/dev/null", os.O_WRONLY)
-        try:
-            out = subprocess.Popen(['/sbin/dumpe2fs', '-h',
self.lofile],
-                                   stdout = subprocess.PIPE,
-                                   stderr = dev_null).communicate()[0]
-        finally:
-            os.close(dev_null)
-
-        return int(parse_field(out, "Block count")) * self.blocksize
-
+        self.diskmount.__get_size_from_filesystem()
+        
     def __resize_to_minimal(self):
-        self.__fsck()
-
-        #
-        # Use a binary search to find the minimal size
-        # we can resize the image to
-        #
-        bot = 0
-        top = self.__get_size_from_filesystem()
-        while top != (bot + 1):
-            t = bot + ((top - bot) / 2)
-
-            if not resize2fs(self.lofile, t):
-                top = t
-            else:
-                bot = t
-        return top
-
+        self.diskmount.__resize_to_minimal()
+        
     def resparse(self, size = None):
-        self.cleanup()
+        self.diskmount.resparse(size)
         
-        minsize = self.__resize_to_minimal()
-
-        self.truncate(minsize)
-        self.resize(size)
-        return minsize
-
 class Disk:
     """
     With the new disk API the image being produced can be partitioned into 
-- 
1.5.4.1

[PATCH] re-factor the imgcreate/fs.py module

[David Huff]

From: Daniel P. Berrange <berrange redhat com>

---
 imgcreate/fs.py |  328 +++++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 files changed, 320 insertions(+), 8 deletions(-)

diff --git a/imgcreate/fs.py b/imgcreate/fs.py
index 98c0db4..5d94018 100644
--- a/imgcreate/fs.py
+++ b/imgcreate/fs.py
@@ -24,6 +24,7 @@ import stat
 import subprocess
 import random
 import string
+import logging
 
 from imgcreate.errors import *
 
@@ -86,6 +87,9 @@ class BindChrootMount:
         subprocess.call(["/bin/umount", self.dest])
         self.mounted = False
 
+#===============================================================================
+# LoopbackMount  compatibility layer for old API
+#===============================================================================
 class LoopbackMount:
     def __init__(self, lofile, mountdir, fstype = None):
         self.lofile = lofile
@@ -162,6 +166,9 @@ class LoopbackMount:
 
         self.mounted = True
 
+#===============================================================================
+# SparseLoopbackMount  compatibility layer for old API
+#===============================================================================
 class SparseLoopbackMount(LoopbackMount):
     def __init__(self, lofile, mountdir, size, fstype = None):
         LoopbackMount.__init__(self, lofile, mountdir, fstype)
@@ -192,6 +199,9 @@ class SparseLoopbackMount(LoopbackMount):
     def create(self):
         self.expand(create = True)
 
+#===============================================================================
+# SparseExtLoopbackMount  compatibility layer for old API
+#===============================================================================
 class SparseExtLoopbackMount(SparseLoopbackMount):
     def __init__(self, lofile, mountdir, size, fstype, blocksize, fslabel):
         SparseLoopbackMount.__init__(self, lofile, mountdir, size, fstype)
@@ -288,6 +298,308 @@ class SparseExtLoopbackMount(SparseLoopbackMount):
         self.resize(size)
         return minsize
 
+#===============================================================================
+# Disk
+#Most of the change here involves re-factoring the imgcreate/fs.py module.
+#The SparseExtLoopbackMount, SparseLoopbackMount, LoopbackMount classes all
+#have the built-in limitation that the image being produced corresponds to
+#a single filesystem / loop mount.  With the disk creator, the image being
+#produced can be partitioned into multiple chunks, with many filesystems.
+#Furthermore, not all of them require loopback mounts, as the partitions
+#themselves are already visible via /dev/mapper/
+#
+#So this patch separates the roles. There are now classes which deal with
+#accessing / creating disks:
+#
+#  Disk               - generic base for disks
+#  RawDisk            - a disk backed by a block device
+#  LoopbackDisk       - a disk backed by a file
+#  SparseLoopbackDisk - a disk backed by a sparse file
+#
+#The 'create' method must make the disk visible as a block device - eg
+#by calling losetup. For RawDisk, this is obviously a no-op. The 'cleanup'
+#method must undo the 'create' operation.
+#
+#There are then classes which deal with mounting things:
+#
+#   Mount        - generic base for mounts
+#   DiskMount    -  able to mount a Disk object
+#   ExtDiskMount - able to format/resize ext3 filesystems when mounting
+#===============================================================================
+class Disk:
+    def __init__(self, size, device = None):
+        self._device = device
+        self._size = size
+
+    def create(self):
+        pass
+
+    def cleanup(self):
+        pass
+
+    def get_device(self):
+        return self._device
+    def set_device(self, path):
+        self._device = path
+    device = property(get_device, set_device)
+
+    def get_size(self):
+        return self._size
+    size = property(get_size)
+
+
+class RawDisk(Disk):
+    def __init__(self, size, device):
+        Disk.__init__(self, size, device)
+
+    def fixed(self):
+        return True
+
+    def exists(self):
+        return True
+
+class LoopbackDisk(Disk):
+    def __init__(self, lofile, size):
+        Disk.__init__(self, size)
+        self.lofile = lofile
+
+    def fixed(self):
+        return False
+
+    def exists(self):
+        return os.path.exists(self.lofile)
+
+    def create(self):
+        if self.device is not None:
+            return
+
+        losetupProc = subprocess.Popen(["/sbin/losetup", "-f"],
+                                       stdout=subprocess.PIPE)
+        losetupOutput = losetupProc.communicate()[0]
+
+        if losetupProc.returncode:
+            raise MountError("Failed to allocate loop device for '%s'"
%
+                             self.lofile)
+
+        device = losetupOutput.split()[0]
+
+        logging.debug("Losetup add %s mapping to %s"  % (device, self.lofile))
+        rc = subprocess.call(["/sbin/losetup", device, self.lofile])
+        if rc != 0:
+            raise MountError("Failed to allocate loop device for '%s'"
%
+                             self.lofile)
+        self.device = device
+
+    def cleanup(self):
+        if self.device is None:
+            return
+        logging.debug("Losetup remove %s" % self.device)
+        rc = subprocess.call(["/sbin/losetup", "-d", self.device])
+        self.device = None
+
+
+
+class SparseLoopbackDisk(LoopbackDisk):
+    def __init__(self, lofile, size):
+        LoopbackDisk.__init__(self, lofile, size)
+
+    def expand(self, create = False, size = None):
+        flags = os.O_WRONLY
+        if create:
+            flags |= os.O_CREAT
+            makedirs(os.path.dirname(self.lofile))
+
+        if size is None:
+            size = self.size
+
+        logging.debug("Extending sparse file %s to %d" % (self.lofile, size))
+        fd = os.open(self.lofile, flags)
+
+        os.lseek(fd, size, 0)
+        os.write(fd, '\x00')
+        os.close(fd)
+
+    def truncate(self, size = None):
+        if size is None:
+            size = self.size
+
+        logging.debug("Truncating sparse file %s to %d" % (self.lofile, size))
+        fd = os.open(self.lofile, os.O_WRONLY)
+        os.ftruncate(fd, size)
+        os.close(fd)
+
+    def create(self):
+        self.expand(create = True)
+        LoopbackDisk.create(self)
+
+class Mount:
+    def __init__(self, mountdir):
+        self.mountdir = mountdir
+
+    def cleanup(self):
+        self.unmount()
+
+    def mount(self):
+        pass
+
+    def unmount(self):
+        pass
+
+class DiskMount(Mount):
+    def __init__(self, disk, mountdir, fstype = None, rmmountdir = True):
+        Mount.__init__(self, mountdir)
+
+        self.disk = disk
+        self.fstype = fstype
+        self.rmmountdir = rmmountdir
+
+        self.mounted = False
+        self.rmdir   = False
+
+    def cleanup(self):
+        Mount.cleanup(self)
+        self.disk.cleanup()
+
+    def unmount(self):
+        if self.mounted:
+            logging.debug("Unmounting directory %s" % self.mountdir)
+            rc = subprocess.call(["/bin/umount", self.mountdir])
+            if rc == 0:
+                self.mounted = False
+
+        if self.rmdir and not self.mounted:
+            try:
+                os.rmdir(self.mountdir)
+            except OSError, e:
+                pass
+            self.rmdir = False
+
+
+    def __create(self):
+        self.disk.create()
+
+
+    def mount(self):
+        if self.mounted:
+            return
+
+        if not os.path.isdir(self.mountdir):
+            logging.debug("Creating mount point %s" % self.mountdir)
+            os.makedirs(self.mountdir)
+            self.rmdir = self.rmmountdir
+
+        self.__create()
+
+        logging.debug("Mounting %s at %s" % (self.disk.device, self.mountdir))
+        args = [ "/bin/mount", self.disk.device, self.mountdir ]
+        if self.fstype:
+            args.extend(["-t", self.fstype])
+
+        rc = subprocess.call(args)
+        if rc != 0:
+            raise MountError("Failed to mount '%s' to '%s'" %
+                             (self.disk.device, self.mountdir))
+
+        self.mounted = True
+
+class ExtDiskMount(DiskMount):
+    def __init__(self, disk, mountdir, fstype, blocksize, fslabel, rmmountdir=True):
+        DiskMount.__init__(self, disk, mountdir, fstype, rmmountdir)
+        self.blocksize = blocksize
+        self.fslabel = fslabel
+
+    def __format_filesystem(self):
+        logging.debug("Formating %s filesystem on %s" % (self.fstype,
self.disk.device))
+        rc = subprocess.call(["/sbin/mkfs." + self.fstype,
+                              "-F", "-L", self.fslabel,
+                              "-m", "1", "-b",
str(self.blocksize),
+                              self.disk.device])
+        #                      str(self.disk.size / self.blocksize)])
+        if rc != 0:
+            raise MountError("Error creating %s filesystem" % (self.fstype,))
+        logging.debug("Tuning filesystem on %s" % self.disk.device)
+        subprocess.call(["/sbin/tune2fs", "-c0", "-i0",
"-Odir_index",
+                         "-ouser_xattr,acl", self.disk.device])
+
+    def __resize_filesystem(self, size = None):
+        current_size = os.stat(self.disk.lofile)[stat.ST_SIZE]
+
+        if size is None:
+            size = self.size
+
+        if size == current_size:
+            return
+
+        if size > current_size:
+            self.expand(size)
+
+        self.__fsck()
+
+        resize2fs(self.disk.lofile, size)
+        return size
+
+    def __create(self):
+        resize = False
+        if not self.disk.fixed() and self.disk.exists():
+            resize = True
+
+        self.disk.create()
+
+        if resize:
+            self.__resize_filesystem()
+        else:
+            self.__format_filesystem()
+
+    def mount(self):
+        self.__create()
+        DiskMount.mount(self)
+
+    def __fsck(self):
+        logging.debug("Checking filesystem %s" % self.disk.lofile)
+        subprocess.call(["/sbin/e2fsck", "-f", "-y",
self.disk.lofile])
+
+    def __get_size_from_filesystem(self):
+        def parse_field(output, field):
+            for line in output.split("\n"):
+                if line.startswith(field + ":"):
+                    return line[len(field) + 1:].strip()
+
+            raise KeyError("Failed to find field '%s' in output" %
field)
+
+        dev_null = os.open("/dev/null", os.O_WRONLY)
+        try:
+            out = subprocess.Popen(['/sbin/dumpe2fs', '-h',
self.disk.lofile],
+                                   stdout = subprocess.PIPE,
+                                   stderr = dev_null).communicate()[0]
+        finally:
+            os.close(dev_null)
+
+        return int(parse_field(out, "Block count")) * self.blocksize
+
+    def __resize_to_minimal(self):
+        self.__fsck()
+
+        #
+        # Use a binary search to find the minimal size
+        # we can resize the image to
+        #
+        bot = 0
+        top = self.__get_size_from_filesystem()
+        while top != (bot + 1):
+            t = bot + ((top - bot) / 2)
+
+            if not resize2fs(self.disk.lofile, t):
+                top = t
+            else:
+                bot = t
+        return top
+
+    def resparse(self, size = None):
+        self.cleanup()
+        minsize = self.__resize_to_minimal()
+        self.disk.truncate(minsize)
+        return minsize
+
 class DeviceMapperSnapshot(object):
     def __init__(self, imgloop, cowloop):
         self.imgloop = imgloop
@@ -306,8 +618,8 @@ class DeviceMapperSnapshot(object):
         if self.__created:
             return
 
-        self.imgloop.loopsetup()
-        self.cowloop.loopsetup()
+        self.imgloop.create()
+        self.cowloop.create()
 
         self.__name = "imgcreate-%d-%d" % (os.getpid(),
                                            random.randint(0, 2**16))
@@ -315,8 +627,8 @@ class DeviceMapperSnapshot(object):
         size = os.stat(self.imgloop.lofile)[stat.ST_SIZE]
 
         table = "0 %d snapshot %s %s p 8" % (size / 512,
-                                             self.imgloop.loopdev,
-                                             self.cowloop.loopdev)
+                                             self.imgloop.device,
+                                             self.cowloop.device)
 
         args = ["/sbin/dmsetup", "create", self.__name,
"--table", table]
         if subprocess.call(args) != 0:
@@ -382,15 +694,14 @@ class DeviceMapperSnapshot(object):
 #   8) Create a squashfs of the COW
 #
 def create_image_minimizer(path, image, minimal_size):
-    imgloop = LoopbackMount(image, "None")
+    imgloop = LoopbackDisk(image, None) # Passing bogus size - doesn't matter
 
-    cowloop = SparseLoopbackMount(os.path.join(os.path.dirname(path),
"osmin"),
-                                  None, 64L * 1024L * 1024L)
+    cowloop = SparseLoopbackDisk(os.path.join(os.path.dirname(path), "osmin"),
+                                 64L * 1024L * 1024L)
 
     snapshot = DeviceMapperSnapshot(imgloop, cowloop)
 
     try:
-        cowloop.create()
         snapshot.create()
 
         resize2fs(snapshot.path, minimal_size)
@@ -404,3 +715,4 @@ def create_image_minimizer(path, image, minimal_size):
     mksquashfs(cowloop.lofile, path)
 
     os.unlink(cowloop.lofile)
+
-- 
1.5.4.1

[PATCH] added compat shims for old api

[David Huff]

---
 imgcreate/fs.py |  201 +-----------------------------------------------------
 1 files changed, 4 insertions(+), 197 deletions(-)

diff --git a/imgcreate/fs.py b/imgcreate/fs.py
index 5d94018..d8c6731 100644
--- a/imgcreate/fs.py
+++ b/imgcreate/fs.py
@@ -92,211 +92,21 @@ class BindChrootMount:
 #===============================================================================
 class LoopbackMount:
     def __init__(self, lofile, mountdir, fstype = None):
-        self.lofile = lofile
-        self.mountdir = mountdir
-        self.fstype = fstype
-
-        self.mounted = False
-        self.losetup = False
-        self.rmdir   = False
-        self.loopdev = None
-
-    def cleanup(self):
-        self.unmount()
-        self.lounsetup()
-
-    def unmount(self):
-        if self.mounted:
-            rc = subprocess.call(["/bin/umount", self.mountdir])
-            if rc == 0:
-                self.mounted = False
-
-        if self.rmdir and not self.mounted:
-            try:
-                os.rmdir(self.mountdir)
-            except OSError, e:
-                pass
-            self.rmdir = False
-
-    def lounsetup(self):
-        if self.losetup:
-            rc = subprocess.call(["/sbin/losetup", "-d",
self.loopdev])
-            self.losetup = False
-            self.loopdev = None
-
-    def loopsetup(self):
-        if self.losetup:
-            return
-
-        losetupProc = subprocess.Popen(["/sbin/losetup", "-f"],
-                                       stdout=subprocess.PIPE)
-        losetupOutput = losetupProc.communicate()[0]
-
-        if losetupProc.returncode:
-            raise MountError("Failed to allocate loop device for '%s'"
%
-                             self.lofile)
-
-        self.loopdev = losetupOutput.split()[0]
-
-        rc = subprocess.call(["/sbin/losetup", self.loopdev, self.lofile])
-        if rc != 0:
-            raise MountError("Failed to allocate loop device for '%s'"
%
-                             self.lofile)
-
-        self.losetup = True
-
-    def mount(self):
-        if self.mounted:
-            return
-
-        self.loopsetup()
-
-        if not os.path.isdir(self.mountdir):
-            os.makedirs(self.mountdir)
-            self.rmdir = True
-
-        args = [ "/bin/mount", self.loopdev, self.mountdir ]
-        if self.fstype:
-            args.extend(["-t", self.fstype])
-
-        rc = subprocess.call(args)
-        if rc != 0:
-            raise MountError("Failed to mount '%s' to '%s'" %
-                             (self.loopdev, self.mountdir))
-
-        self.mounted = True
+        DiskMount.__init__(LoopbackDisk(lofile,size = 0),mountdir,fstype,rmmountdir =
True)
 
 #===============================================================================
 # SparseLoopbackMount  compatibility layer for old API
 #===============================================================================
 class SparseLoopbackMount(LoopbackMount):
     def __init__(self, lofile, mountdir, size, fstype = None):
-        LoopbackMount.__init__(self, lofile, mountdir, fstype)
-        self.size = size
-
-    def expand(self, create = False, size = None):
-        flags = os.O_WRONLY
-        if create:
-            flags |= os.O_CREAT
-            makedirs(os.path.dirname(self.lofile))
-
-        if size is None:
-            size = self.size
-
-        fd = os.open(self.lofile, flags)
-
-        os.lseek(fd, size, 0)
-        os.write(fd, '\x00')
-        os.close(fd)
-
-    def truncate(self, size = None):
-        if size is None:
-            size = self.size
-        fd = os.open(self.lofile, os.O_WRONLY)
-        os.ftruncate(fd, size)
-        os.close(fd)
-
-    def create(self):
-        self.expand(create = True)
+        DiskMount.__init__(SparseLoopbackDisk(lofile,size),mountdir,fstype,rmmountdir =
True)
 
 #===============================================================================
 # SparseExtLoopbackMount  compatibility layer for old API
 #===============================================================================
 class SparseExtLoopbackMount(SparseLoopbackMount):
     def __init__(self, lofile, mountdir, size, fstype, blocksize, fslabel):
-        SparseLoopbackMount.__init__(self, lofile, mountdir, size, fstype)
-        self.blocksize = blocksize
-        self.fslabel = fslabel
-
-    def __format_filesystem(self):
-        rc = subprocess.call(["/sbin/mkfs." + self.fstype,
-                              "-F", "-L", self.fslabel,
-                              "-m", "1", "-b",
str(self.blocksize),
-                              self.lofile,
-                              str(self.size / self.blocksize)])
-        if rc != 0:
-            raise MountError("Error creating %s filesystem" % (self.fstype,))
-        subprocess.call(["/sbin/tune2fs", "-c0", "-i0",
"-Odir_index",
-                         "-ouser_xattr,acl", self.lofile])
-
-    def create(self):
-        SparseLoopbackMount.create(self)
-        self.__format_filesystem()
-
-    def resize(self, size = None):
-        current_size = os.stat(self.lofile)[stat.ST_SIZE]
-
-        if size is None:
-            size = self.size
-
-        if size == current_size:
-            return
-
-        if size > current_size:
-            self.expand(size)
-
-        self.__fsck()
-
-        resize2fs(self.lofile, size)
-
-        if size < current_size:
-            self.truncate(size)
-        return size
-
-    def mount(self):
-        if not os.path.isfile(self.lofile):
-            self.create()
-        else:
-            self.resize()
-        return SparseLoopbackMount.mount(self)
-
-    def __fsck(self):
-        subprocess.call(["/sbin/e2fsck", "-f", "-y",
self.lofile])
-
-    def __get_size_from_filesystem(self):
-        def parse_field(output, field):
-            for line in output.split("\n"):
-                if line.startswith(field + ":"):
-                    return line[len(field) + 1:].strip()
-
-            raise KeyError("Failed to find field '%s' in output" %
field)
-
-        dev_null = os.open("/dev/null", os.O_WRONLY)
-        try:
-            out = subprocess.Popen(['/sbin/dumpe2fs', '-h',
self.lofile],
-                                   stdout = subprocess.PIPE,
-                                   stderr = dev_null).communicate()[0]
-        finally:
-            os.close(dev_null)
-
-        return int(parse_field(out, "Block count")) * self.blocksize
-
-    def __resize_to_minimal(self):
-        self.__fsck()
-
-        #
-        # Use a binary search to find the minimal size
-        # we can resize the image to
-        #
-        bot = 0
-        top = self.__get_size_from_filesystem()
-        while top != (bot + 1):
-            t = bot + ((top - bot) / 2)
-
-            if not resize2fs(self.lofile, t):
-                top = t
-            else:
-                bot = t
-        return top
-
-    def resparse(self, size = None):
-        self.cleanup()
-        
-        minsize = self.__resize_to_minimal()
-
-        self.truncate(minsize)
-        self.resize(size)
-        return minsize
+        ExtDiskMount.__init__(SparseLoopbackDisk(lofile,size), mountdir, fstype,
blocksize, fslabel, rmmountdir = True)
 
 #===============================================================================
 # Disk
@@ -347,7 +157,6 @@ class Disk:
         return self._size
     size = property(get_size)
 
-
 class RawDisk(Disk):
     def __init__(self, size, device):
         Disk.__init__(self, size, device)
@@ -396,9 +205,7 @@ class LoopbackDisk(Disk):
         logging.debug("Losetup remove %s" % self.device)
         rc = subprocess.call(["/sbin/losetup", "-d", self.device])
         self.device = None
-
-
-
+              
 class SparseLoopbackDisk(LoopbackDisk):
     def __init__(self, lofile, size):
         LoopbackDisk.__init__(self, lofile, size)
-- 
1.5.4.1

[PATCH] switch internal code to use new api

[David Huff]

---
 imgcreate/creator.py |   17 ++++++++++-------
 imgcreate/live.py    |    6 +++---
 2 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/imgcreate/creator.py b/imgcreate/creator.py
index c9ed60c..e0f6288 100644
--- a/imgcreate/creator.py
+++ b/imgcreate/creator.py
@@ -209,7 +209,11 @@ class ImageCreator(object):
 
         """
         s =  "/dev/root  /         %s    defaults,noatime 0 0\n"
%(self._fstype)
-        s += "devpts     /dev/pts  devpts  gid=5,mode=620   0 0\n"
+        s += self._get_fstab_special()
+        return s
+
+    def _get_fstab_special(self):
+        s = "devpts     /dev/pts  devpts  gid=5,mode=620   0 0\n"
         s += "tmpfs      /dev/shm  tmpfs   defaults         0 0\n"
         s += "proc       /proc     proc    defaults         0 0\n"
         s += "sysfs      /sys      sysfs   defaults         0 0\n"
@@ -904,12 +908,11 @@ class LoopImageCreator(ImageCreator):
             self.__base_on(base_on)
             shutil.copyfile(base_on, self._image)
 
-        self.__instloop = SparseExtLoopbackMount(self._image,
-                                                 self._instroot,
-                                                 self.__image_size,
-                                                 self.__fstype,
-                                                 self.__blocksize,
-                                                 self.fslabel)
+        self.__instloop = ExtDiskMount(SparseLoopbackDisk(self._image,
self.__image_size),
+                                       self._instroot,
+                                       self.__fstype,
+                                       self.__blocksize,
+                                       self.fslabel)
 
         try:
             self.__instloop.mount()
diff --git a/imgcreate/live.py b/imgcreate/live.py
index 0bf50f2..1b486e2 100644
--- a/imgcreate/live.py
+++ b/imgcreate/live.py
@@ -131,7 +131,7 @@ class LiveImageCreatorBase(LoopImageCreator):
     #
     def __base_on(self, base_on):
         """helper function to extract ext3 file system from a live CD
ISO"""
-        isoloop = LoopbackMount(base_on, self._mkdtemp())
+        isoloop = Mount(LoopbackDisk(base_on), self._mkdtemp())
 
         try:
             isoloop.mount()
@@ -145,10 +145,10 @@ class LiveImageCreatorBase(LoopImageCreator):
         else:
             squashimg = isoloop.mountdir + "/LiveOS/squashfs.img"
             
-        squashloop = LoopbackMount(squashimg, self._mkdtemp(), "squashfs")
+        squashloop = Mount(LoopbackDisk(squashimg), self._mkdtemp(),
"squashfs")
 
         try:
-            if not os.path.exists(squashloop.lofile):
+            if not squashloop.disk.exists():
                 raise CreatorError("'%s' is not a valid live CD ISO :
"
                                    "squashfs.img doesn't exist" % base_on)
 
-- 
1.5.4.1