public inbox for openembedded-core@lists.openembedded.org
 help / color / mirror / Atom feed
* [PATCH 0/5] wic: bugfixes & --fixed-size support
@ 2016-11-08 15:56 Maciej Borzecki
  2016-11-08 15:56 ` [PATCH 1/5] wic: make sure that partition size is always an integer in internal processing Maciej Borzecki
                   ` (6 more replies)
  0 siblings, 7 replies; 14+ messages in thread
From: Maciej Borzecki @ 2016-11-08 15:56 UTC (permalink / raw)
  To: openembedded-core; +Cc: Maciej Borzecki

The patch series is a follow-up after a previous attempt of adding
--reserved-size option to wic[1].

The series introduces a number of fixes in patches 1 - 4.

The last patch in the series introduces --fixed-size option as discussed in [1].
The patch also introduces minor refactoring to code responsible for computing
partition size.

Aside from new option, another user-visible change is how the size rootfs
partitions with vfat is calculated. In previous code, vfat rootfs partition size
did not account for --extra-space nor --overhead-factor. Now, all rootfs
partitions (except for squashfs) are subject to the same rules of partition
sizing.

http://lists.openembedded.org/pipermail/openembedded-core/2016-October/127634.html

Maciej Borzecki (5):
  wic: make sure that partition size is always an integer in internal
    processing
  wic: use partition size when creating empty partition files
  wic: check that filesystem is specified for a rootfs partition
  wic: fix function comment typos
  wic: add --fixed-size wks option

 scripts/lib/wic/help.py                | 14 +++--
 scripts/lib/wic/imager/direct.py       |  2 +-
 scripts/lib/wic/ksparser.py            | 41 +++++++++++++--
 scripts/lib/wic/partition.py           | 93 +++++++++++++++++++++-------------
 scripts/lib/wic/utils/partitionedfs.py |  6 +--
 5 files changed, 109 insertions(+), 47 deletions(-)

-- 
2.5.0



^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 1/5] wic: make sure that partition size is always an integer in internal processing
  2016-11-08 15:56 [PATCH 0/5] wic: bugfixes & --fixed-size support Maciej Borzecki
@ 2016-11-08 15:56 ` Maciej Borzecki
  2016-11-08 15:56 ` [PATCH 2/5] wic: use partition size when creating empty partition files Maciej Borzecki
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 14+ messages in thread
From: Maciej Borzecki @ 2016-11-08 15:56 UTC (permalink / raw)
  To: openembedded-core; +Cc: Maciej Borzecki

Signed-off-by: Maciej Borzecki <maciej.borzecki@rndity.com>
---
 scripts/lib/wic/partition.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/scripts/lib/wic/partition.py b/scripts/lib/wic/partition.py
index 89c33ab8b7d54bb14678b2e07e706e3feb6ae57a..4b8d769437120adadb5dba2f3919d4eb96141292 100644
--- a/scripts/lib/wic/partition.py
+++ b/scripts/lib/wic/partition.py
@@ -157,7 +157,7 @@ class Partition():
         out = exec_cmd(du_cmd)
         rootfs_size = out.split()[0]
 
-        self.size = rootfs_size
+        self.size = int(rootfs_size)
         self.source_file = rootfs
 
     def prepare_rootfs(self, cr_workdir, oe_builddir, rootfs_dir,
@@ -194,7 +194,7 @@ class Partition():
                 # get the rootfs size in the right units for kickstart (kB)
                 du_cmd = "du -Lbks %s" % rootfs
                 out = exec_cmd(du_cmd)
-                self.size = out.split()[0]
+                self.size = int(out.split()[0])
 
                 break
 
@@ -379,7 +379,7 @@ class Partition():
         out = exec_cmd(du_cmd)
         fs_size = out.split()[0]
 
-        self.size = fs_size
+        self.size = int(fs_size)
 
     def prepare_swap_partition(self, cr_workdir, oe_builddir, native_sysroot):
         """
-- 
2.5.0



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 2/5] wic: use partition size when creating empty partition files
  2016-11-08 15:56 [PATCH 0/5] wic: bugfixes & --fixed-size support Maciej Borzecki
  2016-11-08 15:56 ` [PATCH 1/5] wic: make sure that partition size is always an integer in internal processing Maciej Borzecki
@ 2016-11-08 15:56 ` Maciej Borzecki
  2016-11-09  9:23   ` Ed Bartosh
  2016-11-08 15:56 ` [PATCH 3/5] wic: check that filesystem is specified for a rootfs partition Maciej Borzecki
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 14+ messages in thread
From: Maciej Borzecki @ 2016-11-08 15:56 UTC (permalink / raw)
  To: openembedded-core; +Cc: Maciej Borzecki

It seems that prepare_empty_partition_ext() and
prepare_empty_partition_btrfs() got broken in commit
c8669749e37fe865c197c98d5671d9de176ff4dd, thus one could observe the
following backtrace:

Backtrace:
  File "<snip>/poky/scripts/lib/wic/plugins/imager/direct_plugin.py", line 93, in do_create
    creator.create()
  File "<snip>/poky/scripts/lib/wic/imager/baseimager.py", line 159, in create
    self._create()
  File "<snip>/poky/scripts/lib/wic/imager/direct.py", line 290, in _create
    self.bootimg_dir, self.kernel_dir, self.native_sysroot)
  File "<snip>/poky/scripts/lib/wic/partition.py", line 146, in prepare
    method(rootfs, oe_builddir, native_sysroot)
  File "<snip>/poky/scripts/lib/wic/partition.py", line 325, in prepare_empty_partition_ext
    os.ftruncate(sparse.fileno(), rootfs_size * 1024)
NameError: name 'rootfs_size' is not defined

Signed-off-by: Maciej Borzecki <maciej.borzecki@rndity.com>
---
 scripts/lib/wic/partition.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/lib/wic/partition.py b/scripts/lib/wic/partition.py
index 4b8d769437120adadb5dba2f3919d4eb96141292..8adc698240c8e3bd9f4118663a5d7a167e0bb4a4 100644
--- a/scripts/lib/wic/partition.py
+++ b/scripts/lib/wic/partition.py
@@ -308,7 +308,7 @@ class Partition():
         Prepare an empty ext2/3/4 partition.
         """
         with open(rootfs, 'w') as sparse:
-            os.ftruncate(sparse.fileno(), rootfs_size * 1024)
+            os.ftruncate(sparse.fileno(), self.size * 1024)
 
         extra_imagecmd = "-i 8192"
 
@@ -326,7 +326,7 @@ class Partition():
         Prepare an empty btrfs partition.
         """
         with open(rootfs, 'w') as sparse:
-            os.ftruncate(sparse.fileno(), rootfs_size * 1024)
+            os.ftruncate(sparse.fileno(), self.size * 1024)
 
         label_str = ""
         if self.label:
-- 
2.5.0



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 3/5] wic: check that filesystem is specified for a rootfs partition
  2016-11-08 15:56 [PATCH 0/5] wic: bugfixes & --fixed-size support Maciej Borzecki
  2016-11-08 15:56 ` [PATCH 1/5] wic: make sure that partition size is always an integer in internal processing Maciej Borzecki
  2016-11-08 15:56 ` [PATCH 2/5] wic: use partition size when creating empty partition files Maciej Borzecki
@ 2016-11-08 15:56 ` Maciej Borzecki
  2016-11-09  9:44   ` Ed Bartosh
  2016-11-08 15:56 ` [PATCH 4/5] wic: fix function comment typos Maciej Borzecki
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 14+ messages in thread
From: Maciej Borzecki @ 2016-11-08 15:56 UTC (permalink / raw)
  To: openembedded-core; +Cc: Maciej Borzecki

Signed-off-by: Maciej Borzecki <maciej.borzecki@rndity.com>
---
 scripts/lib/wic/partition.py | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/scripts/lib/wic/partition.py b/scripts/lib/wic/partition.py
index 8adc698240c8e3bd9f4118663a5d7a167e0bb4a4..24e657592738dc7c5cdff78e3740d7c373021e9d 100644
--- a/scripts/lib/wic/partition.py
+++ b/scripts/lib/wic/partition.py
@@ -184,6 +184,10 @@ class Partition():
         if os.path.isfile(rootfs):
             os.remove(rootfs)
 
+        if not self.fstype:
+            msger.error("File system for partition %s not specified in kickstart, " \
+                        "use --fstype option" % (self.mountpoint))
+
         for prefix in ("ext", "btrfs", "vfat", "squashfs"):
             if self.fstype.startswith(prefix):
                 method = getattr(self, "prepare_rootfs_" + prefix)
-- 
2.5.0



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 4/5] wic: fix function comment typos
  2016-11-08 15:56 [PATCH 0/5] wic: bugfixes & --fixed-size support Maciej Borzecki
                   ` (2 preceding siblings ...)
  2016-11-08 15:56 ` [PATCH 3/5] wic: check that filesystem is specified for a rootfs partition Maciej Borzecki
@ 2016-11-08 15:56 ` Maciej Borzecki
  2016-11-08 15:56 ` [PATCH 5/5] wic: add --fixed-size wks option Maciej Borzecki
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 14+ messages in thread
From: Maciej Borzecki @ 2016-11-08 15:56 UTC (permalink / raw)
  To: openembedded-core; +Cc: Maciej Borzecki

Fix typos in documentation of Image.add_partition() and
Image.__format_disks().

Signed-off-by: Maciej Borzecki <maciej.borzecki@rndity.com>
---
 scripts/lib/wic/utils/partitionedfs.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/lib/wic/utils/partitionedfs.py b/scripts/lib/wic/utils/partitionedfs.py
index cb03009fc7e3c97305079629ded7d2ff01eba4c4..9e76487844eebfffc7227d053a65dc9fdab3678b 100644
--- a/scripts/lib/wic/utils/partitionedfs.py
+++ b/scripts/lib/wic/utils/partitionedfs.py
@@ -92,7 +92,7 @@ class Image():
     def add_partition(self, size, disk_name, mountpoint, source_file=None, fstype=None,
                       label=None, fsopts=None, boot=False, align=None, no_table=False,
                       part_type=None, uuid=None, system_id=None):
-        """ Add the next partition. Prtitions have to be added in the
+        """ Add the next partition. Partitions have to be added in the
         first-to-last order. """
 
         ks_pnum = len(self.partitions)
@@ -292,7 +292,7 @@ class Image():
             # even number of sectors.
             if part['mountpoint'] == "/boot" and part['fstype'] in ["vfat", "msdos"] \
                and part['size'] % 2:
-                msger.debug("Substracting one sector from '%s' partition to " \
+                msger.debug("Subtracting one sector from '%s' partition to " \
                             "get even number of sectors for the partition" % \
                             part['mountpoint'])
                 part['size'] -= 1
-- 
2.5.0



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 5/5] wic: add --fixed-size wks option
  2016-11-08 15:56 [PATCH 0/5] wic: bugfixes & --fixed-size support Maciej Borzecki
                   ` (3 preceding siblings ...)
  2016-11-08 15:56 ` [PATCH 4/5] wic: fix function comment typos Maciej Borzecki
@ 2016-11-08 15:56 ` Maciej Borzecki
  2016-11-09  9:36   ` Ed Bartosh
  2016-11-09  9:39 ` [PATCH 0/5] wic: bugfixes & --fixed-size support Ed Bartosh
  2016-11-09 13:17 ` Burton, Ross
  6 siblings, 1 reply; 14+ messages in thread
From: Maciej Borzecki @ 2016-11-08 15:56 UTC (permalink / raw)
  To: openembedded-core; +Cc: Maciej Borzecki

Added new option --fixed-size to wks. The option can be used to indicate
the exact size of a partition. The option cannot be added together with
--size, in which case an error will be raised. Other options that
influence automatic partition size (--extra-space, --overhead-factor),
if specifiec along with --fixed-size, will raise an error.

If it partition data is larger than the amount of space specified with
--fixed-size option wic will raise an error.

Signed-off-by: Maciej Borzecki <maciej.borzecki@rndity.com>
---
 scripts/lib/wic/help.py                | 14 ++++--
 scripts/lib/wic/imager/direct.py       |  2 +-
 scripts/lib/wic/ksparser.py            | 41 +++++++++++++++--
 scripts/lib/wic/partition.py           | 83 ++++++++++++++++++++--------------
 scripts/lib/wic/utils/partitionedfs.py |  2 +-
 5 files changed, 100 insertions(+), 42 deletions(-)

diff --git a/scripts/lib/wic/help.py b/scripts/lib/wic/help.py
index e5347ec4b7c900c68fc64351a5293e75de0672b3..daa11bf489c135627ddfe4cef968e48f8e3ad1d8 100644
--- a/scripts/lib/wic/help.py
+++ b/scripts/lib/wic/help.py
@@ -646,6 +646,12 @@ DESCRIPTION
                  not specified, the size is in MB.
                  You do not need this option if you use --source.
 
+         --fixed-size: Exact partition size. Value format is the same
+                       as for --size option. This option cannot be
+                       specified along with --size. If partition data
+                       is larger than --fixed-size and error will be
+                       raised when assembling disk image.
+
          --source: This option is a wic-specific option that names the
                    source of the data that will populate the
                    partition.  The most common value for this option
@@ -719,13 +725,15 @@ DESCRIPTION
                         space after the space filled by the content
                         of the partition. The final size can go
                         beyond the size specified by --size.
-                        By default, 10MB.
+                        By default, 10MB. This option cannot be used
+                        with --fixed-size option.
 
          --overhead-factor: This option is specific to wic. The
                             size of the partition is multiplied by
                             this factor. It has to be greater than or
-                            equal to 1.
-                            The default value is 1.3.
+                            equal to 1. The default value is 1.3.
+                            This option cannot be used with --fixed-size
+                            option.
 
          --part-type: This option is specific to wic. It specifies partition
                       type GUID for GPT partitions.
diff --git a/scripts/lib/wic/imager/direct.py b/scripts/lib/wic/imager/direct.py
index 2bedef08d6450096c786def6f75a9ee53fcd4b3b..c01a1ea538428e36a75ac5b31a822e01901bea6a 100644
--- a/scripts/lib/wic/imager/direct.py
+++ b/scripts/lib/wic/imager/direct.py
@@ -290,7 +290,7 @@ class DirectImageCreator(BaseImageCreator):
                          self.bootimg_dir, self.kernel_dir, self.native_sysroot)
 
 
-            self.__image.add_partition(int(part.size),
+            self.__image.add_partition(part.get_size(),
                                        part.disk,
                                        part.mountpoint,
                                        part.source_file,
diff --git a/scripts/lib/wic/ksparser.py b/scripts/lib/wic/ksparser.py
index 0894e2b199a299fbbed272f2e1c95e9d692e3ab1..62c490274aa92bf82aac304d9323250e3b728d0c 100644
--- a/scripts/lib/wic/ksparser.py
+++ b/scripts/lib/wic/ksparser.py
@@ -113,6 +113,9 @@ def systemidtype(arg):
 class KickStart():
     """"Kickstart parser implementation."""
 
+    DEFAULT_EXTRA_SPACE = 10*1024
+    DEFAULT_OVERHEAD_FACTOR = 1.3
+
     def __init__(self, confpath):
 
         self.partitions = []
@@ -127,16 +130,24 @@ class KickStart():
         part.add_argument('mountpoint', nargs='?')
         part.add_argument('--active', action='store_true')
         part.add_argument('--align', type=int)
-        part.add_argument("--extra-space", type=sizetype, default=10*1024)
+        part.add_argument("--extra-space", type=sizetype)
         part.add_argument('--fsoptions', dest='fsopts')
         part.add_argument('--fstype')
         part.add_argument('--label')
         part.add_argument('--no-table', action='store_true')
         part.add_argument('--ondisk', '--ondrive', dest='disk')
-        part.add_argument("--overhead-factor", type=overheadtype, default=1.3)
+        part.add_argument("--overhead-factor", type=overheadtype)
         part.add_argument('--part-type')
         part.add_argument('--rootfs-dir')
-        part.add_argument('--size', type=sizetype, default=0)
+
+        # --size and --fixed-size cannot be specified together; options
+        # ----extra-space and --overhead-factor should also raise a parser
+        # --error, but since nesting mutually exclusive groups does not work,
+        # ----extra-space/--overhead-factor are handled later
+        sizeexcl = part.add_mutually_exclusive_group()
+        sizeexcl.add_argument('--size', type=sizetype, default=0)
+        sizeexcl.add_argument('--fixed-size', type=sizetype, default=0)
+
         part.add_argument('--source')
         part.add_argument('--sourceparams')
         part.add_argument('--system-id', type=systemidtype)
@@ -170,11 +181,33 @@ class KickStart():
                 lineno += 1
                 if line and line[0] != '#':
                     try:
-                        parsed = parser.parse_args(shlex.split(line))
+                        line_args = shlex.split(line)
+                        parsed = parser.parse_args(line_args)
                     except ArgumentError as err:
                         raise KickStartError('%s:%d: %s' % \
                                              (confpath, lineno, err))
                     if line.startswith('part'):
+                        # using ArgumentParser one cannot easily tell if option
+                        # was passed as argument, if said option has a default
+                        # value; --overhead-factor/--extra-space cannot be used
+                        # with --fixed-size, so at least detect when these were
+                        # passed with non-0 values ...
+                        if parsed.fixed_size:
+                            if parsed.overhead_factor or parsed.extra_space:
+                                err = "%s:%d: arguments --overhead-factor and --extra-space not "\
+                                      "allowed with argument --fixed-size" \
+                                      % (confpath, lineno)
+                                raise KickStartError(err)
+                        else:
+                            # ... and provide defaults if not using
+                            # --fixed-size iff given option was not used
+                            # (again, one cannot tell if option was passed but
+                            # with value equal to 0)
+                            if '--overhead-factor' not in line_args:
+                                parsed.overhead_factor = self.DEFAULT_OVERHEAD_FACTOR
+                            if '--extra-space' not in line_args:
+                                parsed.extra_space = self.DEFAULT_EXTRA_SPACE
+
                         self.partnum += 1
                         self.partitions.append(Partition(parsed, self.partnum))
                     elif line.startswith('include'):
diff --git a/scripts/lib/wic/partition.py b/scripts/lib/wic/partition.py
index 24e657592738dc7c5cdff78e3740d7c373021e9d..354d4b44c50c77baa54331e95ce0876c32d09339 100644
--- a/scripts/lib/wic/partition.py
+++ b/scripts/lib/wic/partition.py
@@ -54,6 +54,7 @@ class Partition():
         self.part_type = args.part_type
         self.rootfs_dir = args.rootfs_dir
         self.size = args.size
+        self.fixed_size = args.fixed_size
         self.source = args.source
         self.sourceparams = args.sourceparams
         self.system_id = args.system_id
@@ -87,6 +88,39 @@ class Partition():
         else:
             return 0
 
+    def get_rootfs_size(self, actual_rootfs_size=0):
+        """
+        Calculate the required size of rootfs taking into consideration
+        --size/--fixed-size flags as well as overhead and extra space, as
+        specified in kickstart file. Raises an error if the
+        `actual_rootfs_size` is larger than fixed-size rootfs.
+
+        """
+        if self.fixed_size:
+            rootfs_size = self.fixed_size
+            if actual_rootfs_size > rootfs_size:
+                msger.error("Actual rootfs size (%d kB) is larger than allowed size %d kB" \
+                            %(actual_rootfs_size, rootfs_size))
+        else:
+            extra_blocks = self.get_extra_block_count(actual_rootfs_size)
+            if extra_blocks < self.extra_space:
+                extra_blocks = self.extra_space
+
+            rootfs_size = actual_rootfs_size + extra_blocks
+            rootfs_size *= self.overhead_factor
+
+            msger.debug("Added %d extra blocks to %s to get to %d total blocks" % \
+                        (extra_blocks, self.mountpoint, rootfs_size))
+
+        return rootfs_size
+
+    def get_size(self):
+        """
+        Obtain partition size taking into consideration --size/--fixed-size
+        options.
+        """
+        return self.fixed_size if self.fixed_size else self.size
+
     def prepare(self, creator, cr_workdir, oe_builddir, rootfs_dir,
                 bootimg_dir, kernel_dir, native_sysroot):
         """
@@ -97,9 +131,9 @@ class Partition():
             self.sourceparams_dict = parse_sourceparams(self.sourceparams)
 
         if not self.source:
-            if not self.size:
-                msger.error("The %s partition has a size of zero.  Please "
-                            "specify a non-zero --size for that partition." % \
+            if not self.size and not self.fixed_size:
+                msger.error("The %s partition has a size of zero. Please "
+                            "specify a non-zero --size/--fixed-size for that partition." % \
                             self.mountpoint)
             if self.fstype and self.fstype == "swap":
                 self.prepare_swap_partition(cr_workdir, oe_builddir,
@@ -146,6 +180,10 @@ class Partition():
                                                      oe_builddir,
                                                      bootimg_dir, kernel_dir, rootfs_dir,
                                                      native_sysroot)
+        if self.fixed_size and self.size > self.fixed_size:
+            msger.error("File system image of partition %s is larger (%d kB) than its"\
+                        "allowed size %d kB" % (self.mountpoint,
+                                                self.size, self.fixed_size))
 
     def prepare_rootfs_from_fs_image(self, cr_workdir, oe_builddir,
                                      rootfs_dir):
@@ -211,15 +249,7 @@ class Partition():
         out = exec_cmd(du_cmd)
         actual_rootfs_size = int(out.split()[0])
 
-        extra_blocks = self.get_extra_block_count(actual_rootfs_size)
-        if extra_blocks < self.extra_space:
-            extra_blocks = self.extra_space
-
-        rootfs_size = actual_rootfs_size + extra_blocks
-        rootfs_size *= self.overhead_factor
-
-        msger.debug("Added %d extra blocks to %s to get to %d total blocks" % \
-                    (extra_blocks, self.mountpoint, rootfs_size))
+        rootfs_size = self.get_rootfs_size(actual_rootfs_size)
 
         with open(rootfs, 'w') as sparse:
             os.ftruncate(sparse.fileno(), rootfs_size * 1024)
@@ -245,15 +275,7 @@ class Partition():
         out = exec_cmd(du_cmd)
         actual_rootfs_size = int(out.split()[0])
 
-        extra_blocks = self.get_extra_block_count(actual_rootfs_size)
-        if extra_blocks < self.extra_space:
-            extra_blocks = self.extra_space
-
-        rootfs_size = actual_rootfs_size + extra_blocks
-        rootfs_size *= self.overhead_factor
-
-        msger.debug("Added %d extra blocks to %s to get to %d total blocks" % \
-                    (extra_blocks, self.mountpoint, rootfs_size))
+        rootfs_size = self.get_rootfs_size(actual_rootfs_size)
 
         with open(rootfs, 'w') as sparse:
             os.ftruncate(sparse.fileno(), rootfs_size * 1024)
@@ -275,20 +297,13 @@ class Partition():
         out = exec_cmd(du_cmd)
         blocks = int(out.split()[0])
 
-        extra_blocks = self.get_extra_block_count(blocks)
-        if extra_blocks < self.extra_space:
-            extra_blocks = self.extra_space
-
-        blocks += extra_blocks
-
-        msger.debug("Added %d extra blocks to %s to get to %d total blocks" % \
-                    (extra_blocks, self.mountpoint, blocks))
+        rootfs_size = self.get_rootfs_size(blocks)
 
         label_str = "-n boot"
         if self.label:
             label_str = "-n %s" % self.label
 
-        dosfs_cmd = "mkdosfs %s -S 512 -C %s %d" % (label_str, rootfs, blocks)
+        dosfs_cmd = "mkdosfs %s -S 512 -C %s %d" % (label_str, rootfs, rootfs_size)
         exec_native_cmd(dosfs_cmd, native_sysroot)
 
         mcopy_cmd = "mcopy -i %s -s %s/* ::/" % (rootfs, rootfs_dir)
@@ -311,8 +326,9 @@ class Partition():
         """
         Prepare an empty ext2/3/4 partition.
         """
+        size = self.get_size()
         with open(rootfs, 'w') as sparse:
-            os.ftruncate(sparse.fileno(), self.size * 1024)
+            os.ftruncate(sparse.fileno(), size * 1024)
 
         extra_imagecmd = "-i 8192"
 
@@ -329,8 +345,9 @@ class Partition():
         """
         Prepare an empty btrfs partition.
         """
+        size = self.get_size()
         with open(rootfs, 'w') as sparse:
-            os.ftruncate(sparse.fileno(), self.size * 1024)
+            os.ftruncate(sparse.fileno(), size * 1024)
 
         label_str = ""
         if self.label:
@@ -345,7 +362,7 @@ class Partition():
         """
         Prepare an empty vfat partition.
         """
-        blocks = self.size
+        blocks = self.get_size()
 
         label_str = "-n boot"
         if self.label:
diff --git a/scripts/lib/wic/utils/partitionedfs.py b/scripts/lib/wic/utils/partitionedfs.py
index 9e76487844eebfffc7227d053a65dc9fdab3678b..cfa5f5ce09b764c1c2a9b7a3f7bf7d677a6811c4 100644
--- a/scripts/lib/wic/utils/partitionedfs.py
+++ b/scripts/lib/wic/utils/partitionedfs.py
@@ -209,7 +209,7 @@ class Image():
             msger.debug("Assigned %s to %s%d, sectors range %d-%d size %d "
                         "sectors (%d bytes)." \
                             % (part['mountpoint'], part['disk_name'], part['num'],
-                               part['start'], part['start'] + part['size'] - 1,
+                               part['start'], disk['offset'] - 1,
                                part['size'], part['size'] * self.sector_size))
 
         # Once all the partitions have been layed out, we can calculate the
-- 
2.5.0



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/5] wic: use partition size when creating empty partition files
  2016-11-08 15:56 ` [PATCH 2/5] wic: use partition size when creating empty partition files Maciej Borzecki
@ 2016-11-09  9:23   ` Ed Bartosh
  0 siblings, 0 replies; 14+ messages in thread
From: Ed Bartosh @ 2016-11-09  9:23 UTC (permalink / raw)
  To: Maciej Borzecki; +Cc: Maciej Borzecki, openembedded-core

On Tue, Nov 08, 2016 at 04:56:08PM +0100, Maciej Borzecki wrote:
> It seems that prepare_empty_partition_ext() and
> prepare_empty_partition_btrfs() got broken in commit
> c8669749e37fe865c197c98d5671d9de176ff4dd, thus one could observe the
> following backtrace:
> 
> Backtrace:
>   File "<snip>/poky/scripts/lib/wic/plugins/imager/direct_plugin.py", line 93, in do_create
>     creator.create()
>   File "<snip>/poky/scripts/lib/wic/imager/baseimager.py", line 159, in create
>     self._create()
>   File "<snip>/poky/scripts/lib/wic/imager/direct.py", line 290, in _create
>     self.bootimg_dir, self.kernel_dir, self.native_sysroot)
>   File "<snip>/poky/scripts/lib/wic/partition.py", line 146, in prepare
>     method(rootfs, oe_builddir, native_sysroot)
>   File "<snip>/poky/scripts/lib/wic/partition.py", line 325, in prepare_empty_partition_ext
>     os.ftruncate(sparse.fileno(), rootfs_size * 1024)
> NameError: name 'rootfs_size' is not defined
> 
> Signed-off-by: Maciej Borzecki <maciej.borzecki@rndity.com>
> ---
>  scripts/lib/wic/partition.py | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/scripts/lib/wic/partition.py b/scripts/lib/wic/partition.py
> index 4b8d769437120adadb5dba2f3919d4eb96141292..8adc698240c8e3bd9f4118663a5d7a167e0bb4a4 100644
> --- a/scripts/lib/wic/partition.py
> +++ b/scripts/lib/wic/partition.py
> @@ -308,7 +308,7 @@ class Partition():
>          Prepare an empty ext2/3/4 partition.
>          """
>          with open(rootfs, 'w') as sparse:
> -            os.ftruncate(sparse.fileno(), rootfs_size * 1024)
> +            os.ftruncate(sparse.fileno(), self.size * 1024)
>  
>          extra_imagecmd = "-i 8192"
>  
> @@ -326,7 +326,7 @@ class Partition():
>          Prepare an empty btrfs partition.
>          """
>          with open(rootfs, 'w') as sparse:
> -            os.ftruncate(sparse.fileno(), rootfs_size * 1024)
> +            os.ftruncate(sparse.fileno(), self.size * 1024)
>  
>          label_str = ""
>          if self.label:

Thank you for the fix. Sorry for breaking this. I thought I
double-checked and all tests were passing :(


--
Regards,
Ed


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 5/5] wic: add --fixed-size wks option
  2016-11-08 15:56 ` [PATCH 5/5] wic: add --fixed-size wks option Maciej Borzecki
@ 2016-11-09  9:36   ` Ed Bartosh
  2016-11-09 12:08     ` Maciej Borzęcki
  0 siblings, 1 reply; 14+ messages in thread
From: Ed Bartosh @ 2016-11-09  9:36 UTC (permalink / raw)
  To: Maciej Borzecki; +Cc: Maciej Borzecki, openembedded-core

On Tue, Nov 08, 2016 at 04:56:11PM +0100, Maciej Borzecki wrote:
> Added new option --fixed-size to wks. The option can be used to indicate
> the exact size of a partition. The option cannot be added together with
> --size, in which case an error will be raised. Other options that
> influence automatic partition size (--extra-space, --overhead-factor),
> if specifiec along with --fixed-size, will raise an error.
> 
> If it partition data is larger than the amount of space specified with
> --fixed-size option wic will raise an error.
> 
> Signed-off-by: Maciej Borzecki <maciej.borzecki@rndity.com>
> ---
>  scripts/lib/wic/help.py                | 14 ++++--
>  scripts/lib/wic/imager/direct.py       |  2 +-
>  scripts/lib/wic/ksparser.py            | 41 +++++++++++++++--
>  scripts/lib/wic/partition.py           | 83 ++++++++++++++++++++--------------
>  scripts/lib/wic/utils/partitionedfs.py |  2 +-
>  5 files changed, 100 insertions(+), 42 deletions(-)
> 
> diff --git a/scripts/lib/wic/help.py b/scripts/lib/wic/help.py
> index e5347ec4b7c900c68fc64351a5293e75de0672b3..daa11bf489c135627ddfe4cef968e48f8e3ad1d8 100644
> --- a/scripts/lib/wic/help.py
> +++ b/scripts/lib/wic/help.py
> @@ -646,6 +646,12 @@ DESCRIPTION
>                   not specified, the size is in MB.
>                   You do not need this option if you use --source.
>  
> +         --fixed-size: Exact partition size. Value format is the same
> +                       as for --size option. This option cannot be
> +                       specified along with --size. If partition data
> +                       is larger than --fixed-size and error will be
> +                       raised when assembling disk image.
> +
>           --source: This option is a wic-specific option that names the
>                     source of the data that will populate the
>                     partition.  The most common value for this option
> @@ -719,13 +725,15 @@ DESCRIPTION
>                          space after the space filled by the content
>                          of the partition. The final size can go
>                          beyond the size specified by --size.
> -                        By default, 10MB.
> +                        By default, 10MB. This option cannot be used
> +                        with --fixed-size option.
>  
>           --overhead-factor: This option is specific to wic. The
>                              size of the partition is multiplied by
>                              this factor. It has to be greater than or
> -                            equal to 1.
> -                            The default value is 1.3.
> +                            equal to 1. The default value is 1.3.
> +                            This option cannot be used with --fixed-size
> +                            option.
>  
>           --part-type: This option is specific to wic. It specifies partition
>                        type GUID for GPT partitions.
> diff --git a/scripts/lib/wic/imager/direct.py b/scripts/lib/wic/imager/direct.py
> index 2bedef08d6450096c786def6f75a9ee53fcd4b3b..c01a1ea538428e36a75ac5b31a822e01901bea6a 100644
> --- a/scripts/lib/wic/imager/direct.py
> +++ b/scripts/lib/wic/imager/direct.py
> @@ -290,7 +290,7 @@ class DirectImageCreator(BaseImageCreator):
>                           self.bootimg_dir, self.kernel_dir, self.native_sysroot)
>  
>  
> -            self.__image.add_partition(int(part.size),
> +            self.__image.add_partition(part.get_size(),
>                                         part.disk,
>                                         part.mountpoint,
>                                         part.source_file,
> diff --git a/scripts/lib/wic/ksparser.py b/scripts/lib/wic/ksparser.py
> index 0894e2b199a299fbbed272f2e1c95e9d692e3ab1..62c490274aa92bf82aac304d9323250e3b728d0c 100644
> --- a/scripts/lib/wic/ksparser.py
> +++ b/scripts/lib/wic/ksparser.py
> @@ -113,6 +113,9 @@ def systemidtype(arg):
>  class KickStart():
>      """"Kickstart parser implementation."""
>  
> +    DEFAULT_EXTRA_SPACE = 10*1024
> +    DEFAULT_OVERHEAD_FACTOR = 1.3
> +
>      def __init__(self, confpath):
>  
>          self.partitions = []
> @@ -127,16 +130,24 @@ class KickStart():
>          part.add_argument('mountpoint', nargs='?')
>          part.add_argument('--active', action='store_true')
>          part.add_argument('--align', type=int)
> -        part.add_argument("--extra-space", type=sizetype, default=10*1024)
> +        part.add_argument("--extra-space", type=sizetype)
>          part.add_argument('--fsoptions', dest='fsopts')
>          part.add_argument('--fstype')
>          part.add_argument('--label')
>          part.add_argument('--no-table', action='store_true')
>          part.add_argument('--ondisk', '--ondrive', dest='disk')
> -        part.add_argument("--overhead-factor", type=overheadtype, default=1.3)
> +        part.add_argument("--overhead-factor", type=overheadtype)
>          part.add_argument('--part-type')
>          part.add_argument('--rootfs-dir')
> -        part.add_argument('--size', type=sizetype, default=0)
> +
> +        # --size and --fixed-size cannot be specified together; options
> +        # ----extra-space and --overhead-factor should also raise a parser
> +        # --error, but since nesting mutually exclusive groups does not work,
> +        # ----extra-space/--overhead-factor are handled later
> +        sizeexcl = part.add_mutually_exclusive_group()
> +        sizeexcl.add_argument('--size', type=sizetype, default=0)
> +        sizeexcl.add_argument('--fixed-size', type=sizetype, default=0)
> +
>          part.add_argument('--source')
>          part.add_argument('--sourceparams')
>          part.add_argument('--system-id', type=systemidtype)
> @@ -170,11 +181,33 @@ class KickStart():
>                  lineno += 1
>                  if line and line[0] != '#':
>                      try:
> -                        parsed = parser.parse_args(shlex.split(line))
> +                        line_args = shlex.split(line)
> +                        parsed = parser.parse_args(line_args)
>                      except ArgumentError as err:
>                          raise KickStartError('%s:%d: %s' % \
>                                               (confpath, lineno, err))
>                      if line.startswith('part'):
> +                        # using ArgumentParser one cannot easily tell if option
> +                        # was passed as argument, if said option has a default
> +                        # value; --overhead-factor/--extra-space cannot be used
> +                        # with --fixed-size, so at least detect when these were
> +                        # passed with non-0 values ...
I'd suggest to handle this using argparse mutual exclusion:
https://docs.python.org/2/library/argparse.html#mutual-exclusion

> +                        if parsed.fixed_size:
> +                            if parsed.overhead_factor or parsed.extra_space:
> +                                err = "%s:%d: arguments --overhead-factor and --extra-space not "\
> +                                      "allowed with argument --fixed-size" \
> +                                      % (confpath, lineno)
> +                                raise KickStartError(err)
> +                        else:
> +                            # ... and provide defaults if not using
> +                            # --fixed-size iff given option was not used
> +                            # (again, one cannot tell if option was passed but
> +                            # with value equal to 0)
> +                            if '--overhead-factor' not in line_args:
> +                                parsed.overhead_factor = self.DEFAULT_OVERHEAD_FACTOR
> +                            if '--extra-space' not in line_args:
> +                                parsed.extra_space = self.DEFAULT_EXTRA_SPACE
> +
>                          self.partnum += 1
>                          self.partitions.append(Partition(parsed, self.partnum))
>                      elif line.startswith('include'):
> diff --git a/scripts/lib/wic/partition.py b/scripts/lib/wic/partition.py
> index 24e657592738dc7c5cdff78e3740d7c373021e9d..354d4b44c50c77baa54331e95ce0876c32d09339 100644
> --- a/scripts/lib/wic/partition.py
> +++ b/scripts/lib/wic/partition.py
> @@ -54,6 +54,7 @@ class Partition():
>          self.part_type = args.part_type
>          self.rootfs_dir = args.rootfs_dir
>          self.size = args.size
> +        self.fixed_size = args.fixed_size
>          self.source = args.source
>          self.sourceparams = args.sourceparams
>          self.system_id = args.system_id
> @@ -87,6 +88,39 @@ class Partition():
>          else:
>              return 0
>  
> +    def get_rootfs_size(self, actual_rootfs_size=0):
> +        """
> +        Calculate the required size of rootfs taking into consideration
> +        --size/--fixed-size flags as well as overhead and extra space, as
> +        specified in kickstart file. Raises an error if the
> +        `actual_rootfs_size` is larger than fixed-size rootfs.
> +
> +        """
> +        if self.fixed_size:
> +            rootfs_size = self.fixed_size
> +            if actual_rootfs_size > rootfs_size:
> +                msger.error("Actual rootfs size (%d kB) is larger than allowed size %d kB" \
> +                            %(actual_rootfs_size, rootfs_size))
> +        else:
> +            extra_blocks = self.get_extra_block_count(actual_rootfs_size)
> +            if extra_blocks < self.extra_space:
> +                extra_blocks = self.extra_space
> +
> +            rootfs_size = actual_rootfs_size + extra_blocks
> +            rootfs_size *= self.overhead_factor
> +
> +            msger.debug("Added %d extra blocks to %s to get to %d total blocks" % \
> +                        (extra_blocks, self.mountpoint, rootfs_size))
> +
> +        return rootfs_size
> +
> +    def get_size(self):
> +        """
> +        Obtain partition size taking into consideration --size/--fixed-size
> +        options.
> +        """
> +        return self.fixed_size if self.fixed_size else self.size
> +
>      def prepare(self, creator, cr_workdir, oe_builddir, rootfs_dir,
>                  bootimg_dir, kernel_dir, native_sysroot):
Can you make self.size and self.fixed_size getters as properties(use @property decorator)?
It should make code look more pythonic.

>          """
> @@ -97,9 +131,9 @@ class Partition():
>              self.sourceparams_dict = parse_sourceparams(self.sourceparams)
>  
>          if not self.source:
> -            if not self.size:
> -                msger.error("The %s partition has a size of zero.  Please "
> -                            "specify a non-zero --size for that partition." % \
> +            if not self.size and not self.fixed_size:
> +                msger.error("The %s partition has a size of zero. Please "
> +                            "specify a non-zero --size/--fixed-size for that partition." % \
>                              self.mountpoint)
>              if self.fstype and self.fstype == "swap":
>                  self.prepare_swap_partition(cr_workdir, oe_builddir,
> @@ -146,6 +180,10 @@ class Partition():
>                                                       oe_builddir,
>                                                       bootimg_dir, kernel_dir, rootfs_dir,
>                                                       native_sysroot)
> +        if self.fixed_size and self.size > self.fixed_size:
> +            msger.error("File system image of partition %s is larger (%d kB) than its"\
> +                        "allowed size %d kB" % (self.mountpoint,
> +                                                self.size, self.fixed_size))
>  
>      def prepare_rootfs_from_fs_image(self, cr_workdir, oe_builddir,
>                                       rootfs_dir):
> @@ -211,15 +249,7 @@ class Partition():
>          out = exec_cmd(du_cmd)
>          actual_rootfs_size = int(out.split()[0])
>  
> -        extra_blocks = self.get_extra_block_count(actual_rootfs_size)
> -        if extra_blocks < self.extra_space:
> -            extra_blocks = self.extra_space
> -
> -        rootfs_size = actual_rootfs_size + extra_blocks
> -        rootfs_size *= self.overhead_factor
> -
> -        msger.debug("Added %d extra blocks to %s to get to %d total blocks" % \
> -                    (extra_blocks, self.mountpoint, rootfs_size))
> +        rootfs_size = self.get_rootfs_size(actual_rootfs_size)
>  
>          with open(rootfs, 'w') as sparse:
>              os.ftruncate(sparse.fileno(), rootfs_size * 1024)
> @@ -245,15 +275,7 @@ class Partition():
>          out = exec_cmd(du_cmd)
>          actual_rootfs_size = int(out.split()[0])
>  
> -        extra_blocks = self.get_extra_block_count(actual_rootfs_size)
> -        if extra_blocks < self.extra_space:
> -            extra_blocks = self.extra_space
> -
> -        rootfs_size = actual_rootfs_size + extra_blocks
> -        rootfs_size *= self.overhead_factor
> -
> -        msger.debug("Added %d extra blocks to %s to get to %d total blocks" % \
> -                    (extra_blocks, self.mountpoint, rootfs_size))
> +        rootfs_size = self.get_rootfs_size(actual_rootfs_size)
>  
>          with open(rootfs, 'w') as sparse:
>              os.ftruncate(sparse.fileno(), rootfs_size * 1024)
> @@ -275,20 +297,13 @@ class Partition():
>          out = exec_cmd(du_cmd)
>          blocks = int(out.split()[0])
>  
> -        extra_blocks = self.get_extra_block_count(blocks)
> -        if extra_blocks < self.extra_space:
> -            extra_blocks = self.extra_space
> -
> -        blocks += extra_blocks
> -
> -        msger.debug("Added %d extra blocks to %s to get to %d total blocks" % \
> -                    (extra_blocks, self.mountpoint, blocks))
> +        rootfs_size = self.get_rootfs_size(blocks)
>  
>          label_str = "-n boot"
>          if self.label:
>              label_str = "-n %s" % self.label
>  
> -        dosfs_cmd = "mkdosfs %s -S 512 -C %s %d" % (label_str, rootfs, blocks)
> +        dosfs_cmd = "mkdosfs %s -S 512 -C %s %d" % (label_str, rootfs, rootfs_size)
>          exec_native_cmd(dosfs_cmd, native_sysroot)
>  
>          mcopy_cmd = "mcopy -i %s -s %s/* ::/" % (rootfs, rootfs_dir)
> @@ -311,8 +326,9 @@ class Partition():
>          """
>          Prepare an empty ext2/3/4 partition.
>          """
> +        size = self.get_size()
>          with open(rootfs, 'w') as sparse:
> -            os.ftruncate(sparse.fileno(), self.size * 1024)
> +            os.ftruncate(sparse.fileno(), size * 1024)
>  
>          extra_imagecmd = "-i 8192"
>  
> @@ -329,8 +345,9 @@ class Partition():
>          """
>          Prepare an empty btrfs partition.
>          """
> +        size = self.get_size()
>          with open(rootfs, 'w') as sparse:
> -            os.ftruncate(sparse.fileno(), self.size * 1024)
> +            os.ftruncate(sparse.fileno(), size * 1024)
>  
>          label_str = ""
>          if self.label:
> @@ -345,7 +362,7 @@ class Partition():
>          """
>          Prepare an empty vfat partition.
>          """
> -        blocks = self.size
> +        blocks = self.get_size()
>  
>          label_str = "-n boot"
>          if self.label:
> diff --git a/scripts/lib/wic/utils/partitionedfs.py b/scripts/lib/wic/utils/partitionedfs.py
> index 9e76487844eebfffc7227d053a65dc9fdab3678b..cfa5f5ce09b764c1c2a9b7a3f7bf7d677a6811c4 100644
> --- a/scripts/lib/wic/utils/partitionedfs.py
> +++ b/scripts/lib/wic/utils/partitionedfs.py
> @@ -209,7 +209,7 @@ class Image():
>              msger.debug("Assigned %s to %s%d, sectors range %d-%d size %d "
>                          "sectors (%d bytes)." \
>                              % (part['mountpoint'], part['disk_name'], part['num'],
> -                               part['start'], part['start'] + part['size'] - 1,
> +                               part['start'], disk['offset'] - 1,
>                                 part['size'], part['size'] * self.sector_size))
>  
>          # Once all the partitions have been layed out, we can calculate the
> -- 
> 2.5.0
> 

-- 
--
Regards,
Ed


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 0/5] wic: bugfixes & --fixed-size support
  2016-11-08 15:56 [PATCH 0/5] wic: bugfixes & --fixed-size support Maciej Borzecki
                   ` (4 preceding siblings ...)
  2016-11-08 15:56 ` [PATCH 5/5] wic: add --fixed-size wks option Maciej Borzecki
@ 2016-11-09  9:39 ` Ed Bartosh
  2016-11-09 13:17 ` Burton, Ross
  6 siblings, 0 replies; 14+ messages in thread
From: Ed Bartosh @ 2016-11-09  9:39 UTC (permalink / raw)
  To: Maciej Borzecki; +Cc: Maciej Borzecki, openembedded-core

On Tue, Nov 08, 2016 at 04:56:06PM +0100, Maciej Borzecki wrote:
> The patch series is a follow-up after a previous attempt of adding
> --reserved-size option to wic[1].
> 
> The series introduces a number of fixes in patches 1 - 4.
> 
> The last patch in the series introduces --fixed-size option as discussed in [1].
> The patch also introduces minor refactoring to code responsible for computing
> partition size.
> 
> Aside from new option, another user-visible change is how the size rootfs
> partitions with vfat is calculated. In previous code, vfat rootfs partition size
> did not account for --extra-space nor --overhead-factor. Now, all rootfs
> partitions (except for squashfs) are subject to the same rules of partition
> sizing.
> 
Thank you for the patchset!

+1 (if my comments are taken into account)

It would be great if you add tests for this functionality to oe-selftest
wic suite.

> http://lists.openembedded.org/pipermail/openembedded-core/2016-October/127634.html
> 
> Maciej Borzecki (5):
>   wic: make sure that partition size is always an integer in internal
>     processing
>   wic: use partition size when creating empty partition files
>   wic: check that filesystem is specified for a rootfs partition
>   wic: fix function comment typos
>   wic: add --fixed-size wks option
> 
>  scripts/lib/wic/help.py                | 14 +++--
>  scripts/lib/wic/imager/direct.py       |  2 +-
>  scripts/lib/wic/ksparser.py            | 41 +++++++++++++--
>  scripts/lib/wic/partition.py           | 93 +++++++++++++++++++++-------------
>  scripts/lib/wic/utils/partitionedfs.py |  6 +--
>  5 files changed, 109 insertions(+), 47 deletions(-)
> 
> -- 
> 2.5.0
> 

-- 
--
Regards,
Ed


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 3/5] wic: check that filesystem is specified for a rootfs partition
  2016-11-08 15:56 ` [PATCH 3/5] wic: check that filesystem is specified for a rootfs partition Maciej Borzecki
@ 2016-11-09  9:44   ` Ed Bartosh
  2016-11-09 10:42     ` Maciej Borzęcki
  0 siblings, 1 reply; 14+ messages in thread
From: Ed Bartosh @ 2016-11-09  9:44 UTC (permalink / raw)
  To: Maciej Borzecki; +Cc: Maciej Borzecki, openembedded-core

On Tue, Nov 08, 2016 at 04:56:09PM +0100, Maciej Borzecki wrote:
> Signed-off-by: Maciej Borzecki <maciej.borzecki@rndity.com>
> ---
>  scripts/lib/wic/partition.py | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/scripts/lib/wic/partition.py b/scripts/lib/wic/partition.py
> index 8adc698240c8e3bd9f4118663a5d7a167e0bb4a4..24e657592738dc7c5cdff78e3740d7c373021e9d 100644
> --- a/scripts/lib/wic/partition.py
> +++ b/scripts/lib/wic/partition.py
> @@ -184,6 +184,10 @@ class Partition():
>          if os.path.isfile(rootfs):
>              os.remove(rootfs)
>  
> +        if not self.fstype:
> +            msger.error("File system for partition %s not specified in kickstart, " \
> +                        "use --fstype option" % (self.mountpoint))
> +
Would it make sense to make --fstype mandatory in ksparser?

>          for prefix in ("ext", "btrfs", "vfat", "squashfs"):
>              if self.fstype.startswith(prefix):
>                  method = getattr(self, "prepare_rootfs_" + prefix)
> -- 
> 2.5.0
> 

-- 
--
Regards,
Ed


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 3/5] wic: check that filesystem is specified for a rootfs partition
  2016-11-09  9:44   ` Ed Bartosh
@ 2016-11-09 10:42     ` Maciej Borzęcki
  0 siblings, 0 replies; 14+ messages in thread
From: Maciej Borzęcki @ 2016-11-09 10:42 UTC (permalink / raw)
  To: Ed Bartosh
  Cc: Maciej Borzecki, Patches and discussions about the oe-core layer

On Wed, Nov 9, 2016 at 10:44 AM, Ed Bartosh <ed.bartosh@linux.intel.com> wrote:
> On Tue, Nov 08, 2016 at 04:56:09PM +0100, Maciej Borzecki wrote:
>> Signed-off-by: Maciej Borzecki <maciej.borzecki@rndity.com>
>> ---
>>  scripts/lib/wic/partition.py | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/scripts/lib/wic/partition.py b/scripts/lib/wic/partition.py
>> index 8adc698240c8e3bd9f4118663a5d7a167e0bb4a4..24e657592738dc7c5cdff78e3740d7c373021e9d 100644
>> --- a/scripts/lib/wic/partition.py
>> +++ b/scripts/lib/wic/partition.py
>> @@ -184,6 +184,10 @@ class Partition():
>>          if os.path.isfile(rootfs):
>>              os.remove(rootfs)
>>
>> +        if not self.fstype:
>> +            msger.error("File system for partition %s not specified in kickstart, " \
>> +                        "use --fstype option" % (self.mountpoint))
>> +
> Would it make sense to make --fstype mandatory in ksparser?

I'm afraid that would make fstype mandatory in all cases, while we only
need it for rootfs source plugin.


-- 
Maciej Borzecki
RnDity


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 5/5] wic: add --fixed-size wks option
  2016-11-09  9:36   ` Ed Bartosh
@ 2016-11-09 12:08     ` Maciej Borzęcki
  0 siblings, 0 replies; 14+ messages in thread
From: Maciej Borzęcki @ 2016-11-09 12:08 UTC (permalink / raw)
  To: Ed Bartosh
  Cc: Maciej Borzecki, Patches and discussions about the oe-core layer

On Wed, Nov 9, 2016 at 10:36 AM, Ed Bartosh <ed.bartosh@linux.intel.com> wrote:
> On Tue, Nov 08, 2016 at 04:56:11PM +0100, Maciej Borzecki wrote:
>> Added new option --fixed-size to wks. The option can be used to indicate
>> the exact size of a partition. The option cannot be added together with
>> --size, in which case an error will be raised. Other options that
>> influence automatic partition size (--extra-space, --overhead-factor),
>> if specifiec along with --fixed-size, will raise an error.
>>
>> If it partition data is larger than the amount of space specified with
>> --fixed-size option wic will raise an error.
>>
>> Signed-off-by: Maciej Borzecki <maciej.borzecki@rndity.com>
>> ---
>>  scripts/lib/wic/help.py                | 14 ++++--
>>  scripts/lib/wic/imager/direct.py       |  2 +-
>>  scripts/lib/wic/ksparser.py            | 41 +++++++++++++++--
>>  scripts/lib/wic/partition.py           | 83 ++++++++++++++++++++--------------
>>  scripts/lib/wic/utils/partitionedfs.py |  2 +-
>>  5 files changed, 100 insertions(+), 42 deletions(-)
>>
>> diff --git a/scripts/lib/wic/help.py b/scripts/lib/wic/help.py
>> index e5347ec4b7c900c68fc64351a5293e75de0672b3..daa11bf489c135627ddfe4cef968e48f8e3ad1d8 100644
>> --- a/scripts/lib/wic/help.py
>> +++ b/scripts/lib/wic/help.py
>> @@ -646,6 +646,12 @@ DESCRIPTION
>>                   not specified, the size is in MB.
>>                   You do not need this option if you use --source.
>>
>> +         --fixed-size: Exact partition size. Value format is the same
>> +                       as for --size option. This option cannot be
>> +                       specified along with --size. If partition data
>> +                       is larger than --fixed-size and error will be
>> +                       raised when assembling disk image.
>> +
>>           --source: This option is a wic-specific option that names the
>>                     source of the data that will populate the
>>                     partition.  The most common value for this option
>> @@ -719,13 +725,15 @@ DESCRIPTION
>>                          space after the space filled by the content
>>                          of the partition. The final size can go
>>                          beyond the size specified by --size.
>> -                        By default, 10MB.
>> +                        By default, 10MB. This option cannot be used
>> +                        with --fixed-size option.
>>
>>           --overhead-factor: This option is specific to wic. The
>>                              size of the partition is multiplied by
>>                              this factor. It has to be greater than or
>> -                            equal to 1.
>> -                            The default value is 1.3.
>> +                            equal to 1. The default value is 1.3.
>> +                            This option cannot be used with --fixed-size
>> +                            option.
>>
>>           --part-type: This option is specific to wic. It specifies partition
>>                        type GUID for GPT partitions.
>> diff --git a/scripts/lib/wic/imager/direct.py b/scripts/lib/wic/imager/direct.py
>> index 2bedef08d6450096c786def6f75a9ee53fcd4b3b..c01a1ea538428e36a75ac5b31a822e01901bea6a 100644
>> --- a/scripts/lib/wic/imager/direct.py
>> +++ b/scripts/lib/wic/imager/direct.py
>> @@ -290,7 +290,7 @@ class DirectImageCreator(BaseImageCreator):
>>                           self.bootimg_dir, self.kernel_dir, self.native_sysroot)
>>
>>
>> -            self.__image.add_partition(int(part.size),
>> +            self.__image.add_partition(part.get_size(),
>>                                         part.disk,
>>                                         part.mountpoint,
>>                                         part.source_file,
>> diff --git a/scripts/lib/wic/ksparser.py b/scripts/lib/wic/ksparser.py
>> index 0894e2b199a299fbbed272f2e1c95e9d692e3ab1..62c490274aa92bf82aac304d9323250e3b728d0c 100644
>> --- a/scripts/lib/wic/ksparser.py
>> +++ b/scripts/lib/wic/ksparser.py
>> @@ -113,6 +113,9 @@ def systemidtype(arg):
>>  class KickStart():
>>      """"Kickstart parser implementation."""
>>
>> +    DEFAULT_EXTRA_SPACE = 10*1024
>> +    DEFAULT_OVERHEAD_FACTOR = 1.3
>> +
>>      def __init__(self, confpath):
>>
>>          self.partitions = []
>> @@ -127,16 +130,24 @@ class KickStart():
>>          part.add_argument('mountpoint', nargs='?')
>>          part.add_argument('--active', action='store_true')
>>          part.add_argument('--align', type=int)
>> -        part.add_argument("--extra-space", type=sizetype, default=10*1024)
>> +        part.add_argument("--extra-space", type=sizetype)
>>          part.add_argument('--fsoptions', dest='fsopts')
>>          part.add_argument('--fstype')
>>          part.add_argument('--label')
>>          part.add_argument('--no-table', action='store_true')
>>          part.add_argument('--ondisk', '--ondrive', dest='disk')
>> -        part.add_argument("--overhead-factor", type=overheadtype, default=1.3)
>> +        part.add_argument("--overhead-factor", type=overheadtype)
>>          part.add_argument('--part-type')
>>          part.add_argument('--rootfs-dir')
>> -        part.add_argument('--size', type=sizetype, default=0)
>> +
>> +        # --size and --fixed-size cannot be specified together; options
>> +        # ----extra-space and --overhead-factor should also raise a parser
>> +        # --error, but since nesting mutually exclusive groups does not work,
>> +        # ----extra-space/--overhead-factor are handled later
>> +        sizeexcl = part.add_mutually_exclusive_group()
>> +        sizeexcl.add_argument('--size', type=sizetype, default=0)
>> +        sizeexcl.add_argument('--fixed-size', type=sizetype, default=0)
>> +
>>          part.add_argument('--source')
>>          part.add_argument('--sourceparams')
>>          part.add_argument('--system-id', type=systemidtype)
>> @@ -170,11 +181,33 @@ class KickStart():
>>                  lineno += 1
>>                  if line and line[0] != '#':
>>                      try:
>> -                        parsed = parser.parse_args(shlex.split(line))
>> +                        line_args = shlex.split(line)
>> +                        parsed = parser.parse_args(line_args)
>>                      except ArgumentError as err:
>>                          raise KickStartError('%s:%d: %s' % \
>>                                               (confpath, lineno, err))
>>                      if line.startswith('part'):
>> +                        # using ArgumentParser one cannot easily tell if option
>> +                        # was passed as argument, if said option has a default
>> +                        # value; --overhead-factor/--extra-space cannot be used
>> +                        # with --fixed-size, so at least detect when these were
>> +                        # passed with non-0 values ...
> I'd suggest to handle this using argparse mutual exclusion:
> https://docs.python.org/2/library/argparse.html#mutual-exclusion

I don't think argpare.ArgumentParser() can handle nested groups. Right
now --fixed-size and --size are in exclusive group, there's this part in
the patch:

>> +        sizeexcl = part.add_mutually_exclusive_group()
>> +        sizeexcl.add_argument('--size', type=sizetype, default=0)
>> +        sizeexcl.add_argument('--fixed-size', type=sizetype, default=0)

The most logical way to express a conflict between options would be
using a snippet like this:

   sizeexcl = part.add_mutually_exclusive_group()
   sizeexcl.add_argument('--fixed-size', type=sizetype, default=0)
   szg = sizeexcl.add_argument_group()
   szg.add_argument('--size', type=sizetype, default=0)
   szg.add_argument('--overhead-factor', type=sizetype, default=0)
   szg.add_argument('--extra-space', type=sizetype, default=0)

Unfortunately, this does not work as expected with vanilla
ArgumentParser. That's why I've added this workaround, along with a
lengthy comment.

>
>> +                        if parsed.fixed_size:
>> +                            if parsed.overhead_factor or parsed.extra_space:
>> +                                err = "%s:%d: arguments --overhead-factor and --extra-space not "\
>> +                                      "allowed with argument --fixed-size" \
>> +                                      % (confpath, lineno)
>> +                                raise KickStartError(err)
>> +                        else:
>> +                            # ... and provide defaults if not using
>> +                            # --fixed-size iff given option was not used
>> +                            # (again, one cannot tell if option was passed but
>> +                            # with value equal to 0)
>> +                            if '--overhead-factor' not in line_args:
>> +                                parsed.overhead_factor = self.DEFAULT_OVERHEAD_FACTOR
>> +                            if '--extra-space' not in line_args:
>> +                                parsed.extra_space = self.DEFAULT_EXTRA_SPACE
>> +
>>                          self.partnum += 1
>>                          self.partitions.append(Partition(parsed, self.partnum))
>>                      elif line.startswith('include'):
>> diff --git a/scripts/lib/wic/partition.py b/scripts/lib/wic/partition.py
>> index 24e657592738dc7c5cdff78e3740d7c373021e9d..354d4b44c50c77baa54331e95ce0876c32d09339 100644
>> --- a/scripts/lib/wic/partition.py
>> +++ b/scripts/lib/wic/partition.py
>> @@ -54,6 +54,7 @@ class Partition():
>>          self.part_type = args.part_type
>>          self.rootfs_dir = args.rootfs_dir
>>          self.size = args.size
>> +        self.fixed_size = args.fixed_size
>>          self.source = args.source
>>          self.sourceparams = args.sourceparams
>>          self.system_id = args.system_id
>> @@ -87,6 +88,39 @@ class Partition():
>>          else:
>>              return 0
>>
>> +    def get_rootfs_size(self, actual_rootfs_size=0):
>> +        """
>> +        Calculate the required size of rootfs taking into consideration
>> +        --size/--fixed-size flags as well as overhead and extra space, as
>> +        specified in kickstart file. Raises an error if the
>> +        `actual_rootfs_size` is larger than fixed-size rootfs.
>> +
>> +        """
>> +        if self.fixed_size:
>> +            rootfs_size = self.fixed_size
>> +            if actual_rootfs_size > rootfs_size:
>> +                msger.error("Actual rootfs size (%d kB) is larger than allowed size %d kB" \
>> +                            %(actual_rootfs_size, rootfs_size))
>> +        else:
>> +            extra_blocks = self.get_extra_block_count(actual_rootfs_size)
>> +            if extra_blocks < self.extra_space:
>> +                extra_blocks = self.extra_space
>> +
>> +            rootfs_size = actual_rootfs_size + extra_blocks
>> +            rootfs_size *= self.overhead_factor
>> +
>> +            msger.debug("Added %d extra blocks to %s to get to %d total blocks" % \
>> +                        (extra_blocks, self.mountpoint, rootfs_size))
>> +
>> +        return rootfs_size
>> +
>> +    def get_size(self):
>> +        """
>> +        Obtain partition size taking into consideration --size/--fixed-size
>> +        options.
>> +        """
>> +        return self.fixed_size if self.fixed_size else self.size
>> +
>>      def prepare(self, creator, cr_workdir, oe_builddir, rootfs_dir,
>>                  bootimg_dir, kernel_dir, native_sysroot):
> Can you make self.size and self.fixed_size getters as properties(use @property decorator)?
> It should make code look more pythonic.
>

Good point. I'll incorporate this in v2.

>>          """
>> @@ -97,9 +131,9 @@ class Partition():
>>              self.sourceparams_dict = parse_sourceparams(self.sourceparams)
>>
>>          if not self.source:
>> -            if not self.size:
>> -                msger.error("The %s partition has a size of zero.  Please "
>> -                            "specify a non-zero --size for that partition." % \
>> +            if not self.size and not self.fixed_size:
>> +                msger.error("The %s partition has a size of zero. Please "
>> +                            "specify a non-zero --size/--fixed-size for that partition." % \
>>                              self.mountpoint)
>>              if self.fstype and self.fstype == "swap":
>>                  self.prepare_swap_partition(cr_workdir, oe_builddir,
>> @@ -146,6 +180,10 @@ class Partition():
>>                                                       oe_builddir,
>>                                                       bootimg_dir, kernel_dir, rootfs_dir,
>>                                                       native_sysroot)
>> +        if self.fixed_size and self.size > self.fixed_size:
>> +            msger.error("File system image of partition %s is larger (%d kB) than its"\
>> +                        "allowed size %d kB" % (self.mountpoint,
>> +                                                self.size, self.fixed_size))
>>
>>      def prepare_rootfs_from_fs_image(self, cr_workdir, oe_builddir,
>>                                       rootfs_dir):
>> @@ -211,15 +249,7 @@ class Partition():
>>          out = exec_cmd(du_cmd)
>>          actual_rootfs_size = int(out.split()[0])
>>
>> -        extra_blocks = self.get_extra_block_count(actual_rootfs_size)
>> -        if extra_blocks < self.extra_space:
>> -            extra_blocks = self.extra_space
>> -
>> -        rootfs_size = actual_rootfs_size + extra_blocks
>> -        rootfs_size *= self.overhead_factor
>> -
>> -        msger.debug("Added %d extra blocks to %s to get to %d total blocks" % \
>> -                    (extra_blocks, self.mountpoint, rootfs_size))
>> +        rootfs_size = self.get_rootfs_size(actual_rootfs_size)
>>
>>          with open(rootfs, 'w') as sparse:
>>              os.ftruncate(sparse.fileno(), rootfs_size * 1024)
>> @@ -245,15 +275,7 @@ class Partition():
>>          out = exec_cmd(du_cmd)
>>          actual_rootfs_size = int(out.split()[0])
>>
>> -        extra_blocks = self.get_extra_block_count(actual_rootfs_size)
>> -        if extra_blocks < self.extra_space:
>> -            extra_blocks = self.extra_space
>> -
>> -        rootfs_size = actual_rootfs_size + extra_blocks
>> -        rootfs_size *= self.overhead_factor
>> -
>> -        msger.debug("Added %d extra blocks to %s to get to %d total blocks" % \
>> -                    (extra_blocks, self.mountpoint, rootfs_size))
>> +        rootfs_size = self.get_rootfs_size(actual_rootfs_size)
>>
>>          with open(rootfs, 'w') as sparse:
>>              os.ftruncate(sparse.fileno(), rootfs_size * 1024)
>> @@ -275,20 +297,13 @@ class Partition():
>>          out = exec_cmd(du_cmd)
>>          blocks = int(out.split()[0])
>>
>> -        extra_blocks = self.get_extra_block_count(blocks)
>> -        if extra_blocks < self.extra_space:
>> -            extra_blocks = self.extra_space
>> -
>> -        blocks += extra_blocks
>> -
>> -        msger.debug("Added %d extra blocks to %s to get to %d total blocks" % \
>> -                    (extra_blocks, self.mountpoint, blocks))
>> +        rootfs_size = self.get_rootfs_size(blocks)
>>
>>          label_str = "-n boot"
>>          if self.label:
>>              label_str = "-n %s" % self.label
>>
>> -        dosfs_cmd = "mkdosfs %s -S 512 -C %s %d" % (label_str, rootfs, blocks)
>> +        dosfs_cmd = "mkdosfs %s -S 512 -C %s %d" % (label_str, rootfs, rootfs_size)
>>          exec_native_cmd(dosfs_cmd, native_sysroot)
>>
>>          mcopy_cmd = "mcopy -i %s -s %s/* ::/" % (rootfs, rootfs_dir)
>> @@ -311,8 +326,9 @@ class Partition():
>>          """
>>          Prepare an empty ext2/3/4 partition.
>>          """
>> +        size = self.get_size()
>>          with open(rootfs, 'w') as sparse:
>> -            os.ftruncate(sparse.fileno(), self.size * 1024)
>> +            os.ftruncate(sparse.fileno(), size * 1024)
>>
>>          extra_imagecmd = "-i 8192"
>>
>> @@ -329,8 +345,9 @@ class Partition():
>>          """
>>          Prepare an empty btrfs partition.
>>          """
>> +        size = self.get_size()
>>          with open(rootfs, 'w') as sparse:
>> -            os.ftruncate(sparse.fileno(), self.size * 1024)
>> +            os.ftruncate(sparse.fileno(), size * 1024)
>>
>>          label_str = ""
>>          if self.label:
>> @@ -345,7 +362,7 @@ class Partition():
>>          """
>>          Prepare an empty vfat partition.
>>          """
>> -        blocks = self.size
>> +        blocks = self.get_size()
>>
>>          label_str = "-n boot"
>>          if self.label:
>> diff --git a/scripts/lib/wic/utils/partitionedfs.py b/scripts/lib/wic/utils/partitionedfs.py
>> index 9e76487844eebfffc7227d053a65dc9fdab3678b..cfa5f5ce09b764c1c2a9b7a3f7bf7d677a6811c4 100644
>> --- a/scripts/lib/wic/utils/partitionedfs.py
>> +++ b/scripts/lib/wic/utils/partitionedfs.py
>> @@ -209,7 +209,7 @@ class Image():
>>              msger.debug("Assigned %s to %s%d, sectors range %d-%d size %d "
>>                          "sectors (%d bytes)." \
>>                              % (part['mountpoint'], part['disk_name'], part['num'],
>> -                               part['start'], part['start'] + part['size'] - 1,
>> +                               part['start'], disk['offset'] - 1,
>>                                 part['size'], part['size'] * self.sector_size))
>>
>>          # Once all the partitions have been layed out, we can calculate the
>> --
>> 2.5.0
>>
>
> --
> --
> Regards,
> Ed



-- 
Maciej Borzecki
RnDity


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 0/5] wic: bugfixes & --fixed-size support
  2016-11-08 15:56 [PATCH 0/5] wic: bugfixes & --fixed-size support Maciej Borzecki
                   ` (5 preceding siblings ...)
  2016-11-09  9:39 ` [PATCH 0/5] wic: bugfixes & --fixed-size support Ed Bartosh
@ 2016-11-09 13:17 ` Burton, Ross
  2016-11-09 13:49   ` Maciej Borzęcki
  6 siblings, 1 reply; 14+ messages in thread
From: Burton, Ross @ 2016-11-09 13:17 UTC (permalink / raw)
  To: Maciej Borzecki; +Cc: Maciej Borzecki, OE-core

[-- Attachment #1: Type: text/plain, Size: 4207 bytes --]

I pulled this series into a test run on the autobuilder and it is throwing
quite a lot of exceptions, such as:

Traceback (most recent call last):
  File
"/home/pokybuild/yocto-autobuilder/yocto-worker/nightly-x86-64-lsb/build/scripts/wic",
line 319, in <module>
    sys.exit(main(sys.argv[1:]))
  File
"/home/pokybuild/yocto-autobuilder/yocto-worker/nightly-x86-64-lsb/build/scripts/wic",
line 314, in main
    return hlp.invoke_subcommand(args, parser, hlp.wic_help_usage,
subcommands)
  File
"/home/pokybuild/yocto-autobuilder/yocto-worker/nightly-x86-64-lsb/build/scripts/lib/wic/help.py",
line 95, in invoke_subcommand
    subcommands.get(args[0], subcommand_error)[0](args[1:], usage)
  File
"/home/pokybuild/yocto-autobuilder/yocto-worker/nightly-x86-64-lsb/build/scripts/wic",
line 247, in wic_create_subcommand
    options.compressor, options.bmap, options.debug)
  File
"/home/pokybuild/yocto-autobuilder/yocto-worker/nightly-x86-64-lsb/build/scripts/lib/wic/engine.py",
line 195, in wic_create
    crobj.main(cmdline)
  File
"/home/pokybuild/yocto-autobuilder/yocto-worker/nightly-x86-64-lsb/build/scripts/lib/wic/creator.py",
line 125, in main
    return self._subcmds[pname](options, *args[1:])
  File
"/home/pokybuild/yocto-autobuilder/yocto-worker/nightly-x86-64-lsb/build/scripts/lib/wic/plugins/imager/direct_plugin.py",
line 93, in do_create
    creator.create()
  File
"/home/pokybuild/yocto-autobuilder/yocto-worker/nightly-x86-64-lsb/build/scripts/lib/wic/imager/baseimager.py",
line 159, in create
    self._create()
  File
"/home/pokybuild/yocto-autobuilder/yocto-worker/nightly-x86-64-lsb/build/scripts/lib/wic/imager/direct.py",
line 305, in _create
    system_id=part.system_id)
  File
"/home/pokybuild/yocto-autobuilder/yocto-worker/nightly-x86-64-lsb/build/scripts/lib/wic/utils/partitionedfs.py",
line 101, in add_partition
    size = size * 1024 // self.sector_size
TypeError: unsupported operand type(s) for //: 'str' and 'int'
WARNING:
TOPDIR/tmp/work/genericx86_64-poky-linux/core-image-lsb/1.0-r0/temp/run.do_image_wic.15783:1
exit 1 from 'BUILDDIR="TOPDIR" wic create "$wks" --vars
"TOPDIR/tmp/sysroots/genericx86-64/imgdata/" -e "core-image-lsb" -o "$out/"'

See
http://errors.yoctoproject.org/Errors/Latest/Autobuilder/?filter=a44a6a635c9ed02edb2a3dee0a13b0067becc425&type=commit
for more examples and context.

Ross

On 8 November 2016 at 15:56, Maciej Borzecki <maciej.borzecki@rndity.com>
wrote:

> The patch series is a follow-up after a previous attempt of adding
> --reserved-size option to wic[1].
>
> The series introduces a number of fixes in patches 1 - 4.
>
> The last patch in the series introduces --fixed-size option as discussed
> in [1].
> The patch also introduces minor refactoring to code responsible for
> computing
> partition size.
>
> Aside from new option, another user-visible change is how the size rootfs
> partitions with vfat is calculated. In previous code, vfat rootfs
> partition size
> did not account for --extra-space nor --overhead-factor. Now, all rootfs
> partitions (except for squashfs) are subject to the same rules of partition
> sizing.
>
> http://lists.openembedded.org/pipermail/openembedded-core/
> 2016-October/127634.html
>
> Maciej Borzecki (5):
>   wic: make sure that partition size is always an integer in internal
>     processing
>   wic: use partition size when creating empty partition files
>   wic: check that filesystem is specified for a rootfs partition
>   wic: fix function comment typos
>   wic: add --fixed-size wks option
>
>  scripts/lib/wic/help.py                | 14 +++--
>  scripts/lib/wic/imager/direct.py       |  2 +-
>  scripts/lib/wic/ksparser.py            | 41 +++++++++++++--
>  scripts/lib/wic/partition.py           | 93 +++++++++++++++++++++---------
> ----
>  scripts/lib/wic/utils/partitionedfs.py |  6 +--
>  5 files changed, 109 insertions(+), 47 deletions(-)
>
> --
> 2.5.0
>
> --
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core
>

[-- Attachment #2: Type: text/html, Size: 5439 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 0/5] wic: bugfixes & --fixed-size support
  2016-11-09 13:17 ` Burton, Ross
@ 2016-11-09 13:49   ` Maciej Borzęcki
  0 siblings, 0 replies; 14+ messages in thread
From: Maciej Borzęcki @ 2016-11-09 13:49 UTC (permalink / raw)
  To: Burton, Ross; +Cc: Maciej Borzecki, OE-core

On Wed, Nov 9, 2016 at 2:17 PM, Burton, Ross <ross.burton@intel.com> wrote:
> I pulled this series into a test run on the autobuilder and it is throwing
> quite a lot of exceptions, such as:
>
> Traceback (most recent call last):
>   File
> "/home/pokybuild/yocto-autobuilder/yocto-worker/nightly-x86-64-lsb/build/scripts/wic",
> line 319, in <module>
>     sys.exit(main(sys.argv[1:]))
>   File
> "/home/pokybuild/yocto-autobuilder/yocto-worker/nightly-x86-64-lsb/build/scripts/wic",
> line 314, in main
>     return hlp.invoke_subcommand(args, parser, hlp.wic_help_usage,
> subcommands)
>   File
> "/home/pokybuild/yocto-autobuilder/yocto-worker/nightly-x86-64-lsb/build/scripts/lib/wic/help.py",
> line 95, in invoke_subcommand
>     subcommands.get(args[0], subcommand_error)[0](args[1:], usage)
>   File
> "/home/pokybuild/yocto-autobuilder/yocto-worker/nightly-x86-64-lsb/build/scripts/wic",
> line 247, in wic_create_subcommand
>     options.compressor, options.bmap, options.debug)
>   File
> "/home/pokybuild/yocto-autobuilder/yocto-worker/nightly-x86-64-lsb/build/scripts/lib/wic/engine.py",
> line 195, in wic_create
>     crobj.main(cmdline)
>   File
> "/home/pokybuild/yocto-autobuilder/yocto-worker/nightly-x86-64-lsb/build/scripts/lib/wic/creator.py",
> line 125, in main
>     return self._subcmds[pname](options, *args[1:])
>   File
> "/home/pokybuild/yocto-autobuilder/yocto-worker/nightly-x86-64-lsb/build/scripts/lib/wic/plugins/imager/direct_plugin.py",
> line 93, in do_create
>     creator.create()
>   File
> "/home/pokybuild/yocto-autobuilder/yocto-worker/nightly-x86-64-lsb/build/scripts/lib/wic/imager/baseimager.py",
> line 159, in create
>     self._create()
>   File
> "/home/pokybuild/yocto-autobuilder/yocto-worker/nightly-x86-64-lsb/build/scripts/lib/wic/imager/direct.py",
> line 305, in _create
>     system_id=part.system_id)
>   File
> "/home/pokybuild/yocto-autobuilder/yocto-worker/nightly-x86-64-lsb/build/scripts/lib/wic/utils/partitionedfs.py",
> line 101, in add_partition
>     size = size * 1024 // self.sector_size
> TypeError: unsupported operand type(s) for //: 'str' and 'int'
> WARNING:
> TOPDIR/tmp/work/genericx86_64-poky-linux/core-image-lsb/1.0-r0/temp/run.do_image_wic.15783:1
> exit 1 from 'BUILDDIR="TOPDIR" wic create "$wks" --vars
> "TOPDIR/tmp/sysroots/genericx86-64/imgdata/" -e "core-image-lsb" -o "$out/"'
>
> See
> http://errors.yoctoproject.org/Errors/Latest/Autobuilder/?filter=a44a6a635c9ed02edb2a3dee0a13b0067becc425&type=commit
> for more examples and context.

Thanks. I see now that bootimg-efi and rawcopy plugins do not convert partition
size to int as they should be.


>
> Ross
>
> On 8 November 2016 at 15:56, Maciej Borzecki <maciej.borzecki@rndity.com>
> wrote:
>>
>> The patch series is a follow-up after a previous attempt of adding
>> --reserved-size option to wic[1].
>>
>> The series introduces a number of fixes in patches 1 - 4.
>>
>> The last patch in the series introduces --fixed-size option as discussed
>> in [1].
>> The patch also introduces minor refactoring to code responsible for
>> computing
>> partition size.
>>
>> Aside from new option, another user-visible change is how the size rootfs
>> partitions with vfat is calculated. In previous code, vfat rootfs
>> partition size
>> did not account for --extra-space nor --overhead-factor. Now, all rootfs
>> partitions (except for squashfs) are subject to the same rules of
>> partition
>> sizing.
>>
>>
>> http://lists.openembedded.org/pipermail/openembedded-core/2016-October/127634.html
>>
>> Maciej Borzecki (5):
>>   wic: make sure that partition size is always an integer in internal
>>     processing
>>   wic: use partition size when creating empty partition files
>>   wic: check that filesystem is specified for a rootfs partition
>>   wic: fix function comment typos
>>   wic: add --fixed-size wks option
>>
>>  scripts/lib/wic/help.py                | 14 +++--
>>  scripts/lib/wic/imager/direct.py       |  2 +-
>>  scripts/lib/wic/ksparser.py            | 41 +++++++++++++--
>>  scripts/lib/wic/partition.py           | 93
>> +++++++++++++++++++++-------------
>>  scripts/lib/wic/utils/partitionedfs.py |  6 +--
>>  5 files changed, 109 insertions(+), 47 deletions(-)
>>
>> --
>> 2.5.0
>>
>> --
>> _______________________________________________
>> Openembedded-core mailing list
>> Openembedded-core@lists.openembedded.org
>> http://lists.openembedded.org/mailman/listinfo/openembedded-core
>
>



-- 
Maciej Borzecki
RnDity


^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2016-11-09 13:49 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-11-08 15:56 [PATCH 0/5] wic: bugfixes & --fixed-size support Maciej Borzecki
2016-11-08 15:56 ` [PATCH 1/5] wic: make sure that partition size is always an integer in internal processing Maciej Borzecki
2016-11-08 15:56 ` [PATCH 2/5] wic: use partition size when creating empty partition files Maciej Borzecki
2016-11-09  9:23   ` Ed Bartosh
2016-11-08 15:56 ` [PATCH 3/5] wic: check that filesystem is specified for a rootfs partition Maciej Borzecki
2016-11-09  9:44   ` Ed Bartosh
2016-11-09 10:42     ` Maciej Borzęcki
2016-11-08 15:56 ` [PATCH 4/5] wic: fix function comment typos Maciej Borzecki
2016-11-08 15:56 ` [PATCH 5/5] wic: add --fixed-size wks option Maciej Borzecki
2016-11-09  9:36   ` Ed Bartosh
2016-11-09 12:08     ` Maciej Borzęcki
2016-11-09  9:39 ` [PATCH 0/5] wic: bugfixes & --fixed-size support Ed Bartosh
2016-11-09 13:17 ` Burton, Ross
2016-11-09 13:49   ` Maciej Borzęcki

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox