On Wed, Feb 20, 2019 at 11:02:34AM -0300, Antonio Terceiro wrote:
> On Wed, Feb 20, 2019 at 10:46:35AM +0200, Lars Wirzenius wrote:
> > On Tue, Feb 19, 2019 at 03:34:35PM -0300, Antonio Terceiro wrote:
> > > Only vmdb2 can know exactly what device is mounted as the image rootfs.
> > > Therefore, the only reliable way of creating /etc/fstab to have a
> > > working image, is to do that from withing vmdb2 itself.
> > > 
> > > Patch attached.
> > 
> > Thank you. Creating an fstab seems like a good idea. However, it looks
> > to me like it only handles one mount point, and vmdb2 allows any
> > number, e.g., /boot may be separate.
> > 
> > Would it make sense to change this so that it creates an fstab that
> > has all mount points without them needing to be listed by the user? I
> > think this should be doable by iterating over the tags structure. What
> > do you think?
> 
> Yes, makes sense. I will come up with an updated patch.

It turns out I needed to make quite a few changes. I needed to add two
new attributes to tags: one to record the filesystem type of each
partition, and another to record the target mount point, because the
existing mount point attribute records the mount point in the host.

You will find the final patches attached.

I tested with the following specification files:

1) simple vm, / only

----------------8<----------------8<----------------8<-----------------
steps:
  - mkimg: "{{ image }}"
    size: 4G

  - mklabel: msdos
    device: "{{ image }}"

  - mkpart: primary
    device: "{{ image }}"
    start: 0%
    end: 100%
    tag: root

  - kpartx: "{{ image }}"

  - mkfs: ext4
    partition: root

  - mount: root

  - unpack-rootfs: root

  - debootstrap: stretch
    mirror: http://deb.debian.org/debian
    target: root
    unless: rootfs_unpacked

  - apt: install
    packages:
      - linux-image-amd64
    tag: root
    unless: rootfs_unpacked

  - cache-rootfs: root
    unless: rootfs_unpacked

  - chroot: root
    shell: |
      passwd --delete root
      echo testvm > /etc/hostname

  - fstab: root

  - grub: bios
    tag: root
    console: serial
----------------8<----------------8<----------------8<-----------------

2) simple vm with separate /boot:

----------------8<----------------8<----------------8<-----------------
steps:
  - mkimg: "{{ image }}"
    size: 4G

  - mklabel: msdos
    device: "{{ image }}"

  - mkpart: primary
    device: "{{ image }}"
    start: 0%
    end: 75%
    tag: root

  - mkpart: primary
    device: "{{ image }}"
    start: 75%
    end: 100%
    tag: boot

  - kpartx: "{{ image }}"

  - mkfs: ext4
    partition: root

  - mkfs: ext2
    partition: boot

  - mount: root

  - mount: boot
    dirname: /boot
    mount-on: root

  - unpack-rootfs: root

  - debootstrap: stretch
    mirror: http://deb.debian.org/debian
    target: root
    unless: rootfs_unpacked

  - apt: install
    packages:
      - linux-image-amd64
    tag: root
    unless: rootfs_unpacked

  - cache-rootfs: root
    unless: rootfs_unpacked

  - chroot: root
    shell: |
      passwd --delete root
      echo testvm > /etc/hostname

  - fstab: root

  - grub: bios
    tag: root
    console: serial
----------------8<----------------8<----------------8<-----------------
From 96488ffce6693550abf7e4413c673f898bfaf9e7 Mon Sep 17 00:00:00 2001
From: Antonio Terceiro <terce...@debian.org>
Date: Wed, 20 Feb 2019 10:41:06 -0300
Subject: [PATCH 1/7] Change: remove misplaced comment

That was probably an artifact of a copy & paste.
---
 vmdb/tags.py | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/vmdb/tags.py b/vmdb/tags.py
index e9d73b3..e5a190d 100644
--- a/vmdb/tags.py
+++ b/vmdb/tags.py
@@ -16,12 +16,6 @@
 # =*= License: GPL-3+ =*=
 
 
-# Unmount a directory, including any mount points under that
-# directory. If /mnt/foo is given, and /mnt/foo/bar is also mounted,
-# unmount /mnt/foo/bar first, and /mnt/foo then. Look for sub-mounts
-# in /proc/mounts.
-
-
 class Tags:
 
     def __init__(self):
-- 
2.20.1

From d728b4e0a31b230d790d7bb9620d458cb6a4e760 Mon Sep 17 00:00:00 2001
From: Antonio Terceiro <terce...@debian.org>
Date: Wed, 20 Feb 2019 10:44:29 -0300
Subject: [PATCH 2/7] Change: fix typos in tags_tests

---
 vmdb/tags_tests.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/vmdb/tags_tests.py b/vmdb/tags_tests.py
index 928b875..6a78987 100644
--- a/vmdb/tags_tests.py
+++ b/vmdb/tags_tests.py
@@ -24,7 +24,7 @@ import vmdb
 
 class TagsTests(unittest.TestCase):
 
-    def test_lists_not_tags_initally(self):
+    def test_lists_no_tags_initially(self):
         tags = vmdb.Tags()
         self.assertEqual(tags.get_tags(), [])
 
-- 
2.20.1

From a806aa63ea34711e153bdb3b9838bade48b8af7e Mon Sep 17 00:00:00 2001
From: Antonio Terceiro <terce...@debian.org>
Date: Wed, 20 Feb 2019 20:08:57 -0300
Subject: [PATCH 3/7] Add: a fstype attribute for tags

This will be used to record which filesystem is used for each partition,
and can be queried for example to create a fstab, or systemd mount
units.
---
 vmdb/__init__.py   |  2 +-
 vmdb/tags.py       | 17 +++++++++++++++++
 vmdb/tags_tests.py | 13 +++++++++++++
 3 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/vmdb/__init__.py b/vmdb/__init__.py
index 91aa015..eb049f1 100644
--- a/vmdb/__init__.py
+++ b/vmdb/__init__.py
@@ -31,7 +31,7 @@ from .runcmd import (
     progress,
     error,
 )
-from .tags import Tags, UnknownTag, TagInUse, AlreadyHasDev, AlreadyMounted
+from .tags import Tags, UnknownTag, TagInUse, AlreadyHasDev, AlreadyHasFsType, AlreadyMounted
 from .unmount import unmount, NotMounted
 from .spec import (
     Spec,
diff --git a/vmdb/tags.py b/vmdb/tags.py
index e5a190d..c3034b7 100644
--- a/vmdb/tags.py
+++ b/vmdb/tags.py
@@ -36,6 +36,10 @@ class Tags:
         item = self._get(tag)
         return item['mount_point']
 
+    def get_fstype(self, tag):
+        item = self._get(tag)
+        return item['fstype']
+
     def is_cached(self, tag):
         item = self._get(tag)
         return item.get('cached', False)
@@ -47,6 +51,7 @@ class Tags:
         self._tags[tag] = {
             'dev': None,
             'mount_point': None,
+            'fstype': None,
         }
 
     def set_dev(self, tag, dev):
@@ -62,6 +67,12 @@ class Tags:
         item['mount_point'] = mount_point
         item['cached'] = cached
 
+    def set_fstype(self, tag, fstype):
+        item = self._get(tag)
+        if item['fstype'] is not None:
+            raise AlreadyHasFsType(tag)
+        item['fstype'] = fstype
+
     def _get(self, tag):
         item = self._tags.get(tag)
         if item is None:
@@ -91,3 +102,9 @@ class AlreadyMounted(Exception):
 
     def __init__(self, tag):
         super().__init__('Already mounted tag: {}'.format(tag))
+
+
+class AlreadyHasFsType(Exception):
+
+    def __init__(self, tag):
+        super().__init__('Already has filesytem type: {}'.format(tag))
diff --git a/vmdb/tags_tests.py b/vmdb/tags_tests.py
index 6a78987..1fc1f27 100644
--- a/vmdb/tags_tests.py
+++ b/vmdb/tags_tests.py
@@ -108,3 +108,16 @@ class TagsTests(unittest.TestCase):
         tags.set_dev('first', '/dev/foo')
         with self.assertRaises(vmdb.AlreadyHasDev):
             tags.set_dev('first', '/dev/foo')
+
+    def test_set_fstype(self):
+        tags = vmdb.Tags()
+        tags.append('first')
+        tags.set_fstype('first', 'ext4')
+        self.assertEqual(tags.get_fstype('first'), 'ext4')
+
+    def test_set_fstype_raises_error_for_double_fstype(self):
+        tags = vmdb.Tags()
+        tags.append('first')
+        tags.set_fstype('first', 'ext3')
+        with self.assertRaises(vmdb.AlreadyHasFsType):
+            tags.set_fstype('first', 'ext4')
-- 
2.20.1

From 0ac2e12669a0dff03c4954a5b5f91d8f5067f29e Mon Sep 17 00:00:00 2001
From: Antonio Terceiro <terce...@debian.org>
Date: Wed, 20 Feb 2019 20:11:24 -0300
Subject: [PATCH 4/7] Change: make mkfs record the filesystem type

---
 vmdb/plugins/mkfs_plugin.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/vmdb/plugins/mkfs_plugin.py b/vmdb/plugins/mkfs_plugin.py
index da24130..b9d45d0 100644
--- a/vmdb/plugins/mkfs_plugin.py
+++ b/vmdb/plugins/mkfs_plugin.py
@@ -49,3 +49,5 @@ class MkfsStepRunner(vmdb.StepRunnerInterface):
             cmd.append(step['label'])
         cmd.append(device)
         vmdb.runcmd(cmd)
+
+        state.tags.set_fstype(tag, fstype)
-- 
2.20.1

From 0728a98a0635cdcd19f161a3213b1b0c47eec540 Mon Sep 17 00:00:00 2001
From: Antonio Terceiro <terce...@debian.org>
Date: Thu, 21 Feb 2019 09:07:02 -0300
Subject: [PATCH 5/7] Add: target mount point attribute to tags

---
 vmdb/__init__.py   |  2 +-
 vmdb/tags.py       | 17 +++++++++++++++++
 vmdb/tags_tests.py | 13 +++++++++++++
 3 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/vmdb/__init__.py b/vmdb/__init__.py
index eb049f1..b8f618f 100644
--- a/vmdb/__init__.py
+++ b/vmdb/__init__.py
@@ -31,7 +31,7 @@ from .runcmd import (
     progress,
     error,
 )
-from .tags import Tags, UnknownTag, TagInUse, AlreadyHasDev, AlreadyHasFsType, AlreadyMounted
+from .tags import Tags, UnknownTag, TagInUse, AlreadyHasDev, AlreadyHasFsType, AlreadyHasTargetMountPoint, AlreadyMounted
 from .unmount import unmount, NotMounted
 from .spec import (
     Spec,
diff --git a/vmdb/tags.py b/vmdb/tags.py
index c3034b7..5ab8233 100644
--- a/vmdb/tags.py
+++ b/vmdb/tags.py
@@ -40,6 +40,10 @@ class Tags:
         item = self._get(tag)
         return item['fstype']
 
+    def get_target_mount_point(self, tag):
+        item = self._get(tag)
+        return item['target_mount_point']
+
     def is_cached(self, tag):
         item = self._get(tag)
         return item.get('cached', False)
@@ -52,6 +56,7 @@ class Tags:
             'dev': None,
             'mount_point': None,
             'fstype': None,
+            'target_mount_point': None,
         }
 
     def set_dev(self, tag, dev):
@@ -73,6 +78,12 @@ class Tags:
             raise AlreadyHasFsType(tag)
         item['fstype'] = fstype
 
+    def set_target_mount_point(self, tag, target_mount_point):
+        item = self._get(tag)
+        if item['target_mount_point'] is not None:
+            raise AlreadyHasTargetMountPoint(tag)
+        item['target_mount_point'] = target_mount_point
+
     def _get(self, tag):
         item = self._tags.get(tag)
         if item is None:
@@ -108,3 +119,9 @@ class AlreadyHasFsType(Exception):
 
     def __init__(self, tag):
         super().__init__('Already has filesytem type: {}'.format(tag))
+
+
+class AlreadyHasTargetMountPoint(Exception):
+
+    def __init__(self, tag):
+        super().__init__('Already has target mount point: {}'.format(tag))
diff --git a/vmdb/tags_tests.py b/vmdb/tags_tests.py
index 1fc1f27..383ac14 100644
--- a/vmdb/tags_tests.py
+++ b/vmdb/tags_tests.py
@@ -121,3 +121,16 @@ class TagsTests(unittest.TestCase):
         tags.set_fstype('first', 'ext3')
         with self.assertRaises(vmdb.AlreadyHasFsType):
             tags.set_fstype('first', 'ext4')
+
+    def test_set_target_mount_point(self):
+        tags = vmdb.Tags()
+        tags.append('first')
+        tags.set_target_mount_point('first', '/boot')
+        self.assertEqual(tags.get_target_mount_point('first'), '/boot')
+
+    def test_set_target_mount_point_raises_error_for_double_target_mount_point(self):
+        tags = vmdb.Tags()
+        tags.append('first')
+        tags.set_target_mount_point('first', '/boot')
+        with self.assertRaises(vmdb.AlreadyHasTargetMountPoint):
+            tags.set_target_mount_point('first', '/')
-- 
2.20.1

From 427af9db0e6f183189ef97502ea3cee44544b9ec Mon Sep 17 00:00:00 2001
From: Antonio Terceiro <terce...@debian.org>
Date: Thu, 21 Feb 2019 09:09:01 -0300
Subject: [PATCH 6/7] Change: mount plugin to record target mount point

---
 vmdb/plugins/mount_plugin.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/vmdb/plugins/mount_plugin.py b/vmdb/plugins/mount_plugin.py
index 2ef3ef1..0fbfb8d 100644
--- a/vmdb/plugins/mount_plugin.py
+++ b/vmdb/plugins/mount_plugin.py
@@ -63,10 +63,12 @@ class MountStepRunner(vmdb.StepRunnerInterface):
             if not os.path.exists(mount_point):
                 os.makedirs(mount_point)
         else:
+            dirname = '/'
             mount_point = tempfile.mkdtemp()
 
         vmdb.runcmd(['mount', device, mount_point])
         state.tags.set_mount_point(tag, mount_point, cached=True)
+        state.tags.set_target_mount_point(tag, dirname)
 
         return mount_point
 
-- 
2.20.1

From c615ed4afa061b7a8e428f88fda67db45aa01124 Mon Sep 17 00:00:00 2001
From: Antonio Terceiro <terce...@debian.org>
Date: Tue, 19 Feb 2019 14:59:49 -0300
Subject: [PATCH 7/7] Add: step to create /etc/fstab

---
 vmdb/plugins/fstab.mdwn      | 12 +++++++
 vmdb/plugins/fstab_plugin.py | 61 ++++++++++++++++++++++++++++++++++++
 without-tests                |  1 +
 3 files changed, 74 insertions(+)
 create mode 100644 vmdb/plugins/fstab.mdwn
 create mode 100644 vmdb/plugins/fstab_plugin.py

diff --git a/vmdb/plugins/fstab.mdwn b/vmdb/plugins/fstab.mdwn
new file mode 100644
index 0000000..a2c8eef
--- /dev/null
+++ b/vmdb/plugins/fstab.mdwn
@@ -0,0 +1,12 @@
+Step: `fstab`
+-----------------------------------------------------------------------------
+
+Create `/etc/fstab` inside the the image.
+
+Step keys:
+
+* `fstab` &mdash; REQUIRED; value is the tag for the root filesystem.
+
+Example (in the .vmdb file):
+
+    - fstab: root
diff --git a/vmdb/plugins/fstab_plugin.py b/vmdb/plugins/fstab_plugin.py
new file mode 100644
index 0000000..da985bc
--- /dev/null
+++ b/vmdb/plugins/fstab_plugin.py
@@ -0,0 +1,61 @@
+# Copyright 2019 Antonio Terceiro
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+# =*= License: GPL-3+ =*=
+
+import os
+
+import cliapp
+
+import vmdb
+
+
+class FstabPlugin(cliapp.Plugin):
+
+    def enable(self):
+        self.app.step_runners.add(FstabStepRunner())
+
+
+class FstabStepRunner(vmdb.StepRunnerInterface):
+
+    def get_required_keys(self):
+        return ['fstab']
+
+    def run(self, step, setting, state):
+        tag = step['fstab']
+        chroot = state.tags.get_mount_point(tag)
+
+        filesystems = []
+
+        for tag in state.tags.get_tags():
+            device = state.tags.get_dev(tag)
+            mount_point = state.tags.get_target_mount_point(tag)
+            fstype = state.tags.get_fstype(tag)
+            output = vmdb.runcmd(['blkid', '-c', '/dev/null', '-o', 'value', '-s', 'UUID', device])
+            if output:
+                uuid = output.decode().strip()
+                filesystems.append({
+                    'uuid': uuid,
+                    'mount_point': mount_point,
+                    'fstype': fstype,
+                })
+            else:
+                raise Exception('Unknown UUID for device {} (to be mounted on {})'.format(device, mount_point))
+
+        fstab_path = os.path.join(chroot, 'etc/fstab')
+        line = "UUID={uuid} {mount_point} {fstype} errors=remount-ro 0 1\n"
+        with open(fstab_path, 'w') as fstab:
+            for entry in filesystems:
+                fstab.write(line.format(**entry))
diff --git a/without-tests b/without-tests
index 5745e27..7b479e0 100644
--- a/without-tests
+++ b/without-tests
@@ -11,6 +11,7 @@ vmdb/plugins/chroot_plugin.py
 vmdb/plugins/debootstrap_plugin.py
 vmdb/plugins/echo_plugin.py
 vmdb/plugins/error_plugin.py
+vmdb/plugins/fstab_plugin.py
 vmdb/plugins/grub_plugin.py
 vmdb/plugins/kernel_plugin.py
 vmdb/plugins/lvm2_plugin.py
-- 
2.20.1

Attachment: signature.asc
Description: PGP signature

Reply via email to