From: "Denys Dmytriyenko" <denis@denix.org>
To: Joshua Watt <JPEWhacker@gmail.com>
Cc: openembedded-core@lists.openembedded.org
Subject: Re: [OE-core][PATCH v3] wic: Add --offset argument for partitions
Date: Mon, 8 Jun 2020 19:11:07 -0400 [thread overview]
Message-ID: <20200608231107.GT17660@denix.org> (raw)
In-Reply-To: <20200602134205.17838-1-JPEWhacker@gmail.com>
Not sure if it's related to this change, but I've been seeing this error
across the platforms in master past few days:
WARNING: bootloader config not specified, using defaults
Traceback (most recent call last):
File "/OE/poky-master-build/sources/poky/scripts/wic", line 540, in <module>
sys.exit(main(sys.argv[1:]))
File "/OE/poky-master-build/sources/poky/scripts/wic", line 535, in main
return hlp.invoke_subcommand(args, parser, hlp.wic_help_usage, subcommands)
File "/OE/poky-master-build/sources/poky/scripts/lib/wic/help.py", line 83, in invoke_subcommand
subcmd[0](args, usage)
File "/OE/poky-master-build/sources/poky/scripts/wic", line 219, in wic_create_subcommand
engine.wic_create(wks_file, rootfs_dir, bootimg_dir, kernel_dir,
File "/OE/poky-master-build/sources/poky/scripts/lib/wic/engine.py", line 190, in wic_create
plugin.do_create()
File "/OE/poky-master-build/sources/poky/scripts/lib/wic/plugins/imager/direct.py", line 86, in do_create
self.create()
File "/OE/poky-master-build/sources/poky/scripts/lib/wic/plugins/imager/direct.py", line 179, in create
self._image.prepare(self)
File "/OE/poky-master-build/sources/poky/scripts/lib/wic/plugins/imager/direct.py", line 352, in prepare
part.prepare(imager, imager.workdir, imager.oe_builddir,
File "/OE/poky-master-build/sources/poky/scripts/lib/wic/partition.py", line 175, in prepare
plugin.do_prepare_partition(self, srcparams_dict, creator,
File "/OE/poky-master-build/sources/poky/scripts/lib/wic/plugins/source/bootimg-partition.py", line 193, in do_prepare_partition
part.prepare_rootfs(cr_workdir, oe_builddir, hdddir,
File "/OE/poky-master-build/sources/poky/scripts/lib/wic/partition.py", line 238, in prepare_rootfs
method(rootfs, oe_builddir, rootfs_dir, native_sysroot, pseudo)
File "/OE/poky-master-build/sources/poky/scripts/lib/wic/partition.py", line 305, in prepare_rootfs_msdos
rootfs_size = self.get_rootfs_size(blocks)
File "/OE/poky-master-build/sources/poky/scripts/lib/wic/partition.py", line 100, in get_rootfs_size
if extra_blocks < self.extra_space:
TypeError: '<' not supported between instances of 'int' and 'function'
WARNING: /OE/poky-master-build/build-CORTEX_1/arago-tmp/work/am335x_evm-poky-linux-gnueabi/core-image-base/1.0-r0/temp/run.do_image_wic.3266:1 exit 1 from 'BUILDDIR="/OE/poky-master-build/build-CORTEX_1" PSEUDO_UNLOAD=1 wic create "$wks" --vars "/OE/poky-master-build/build-CORTEX_1/arago-tmp/sysroots/am335x-evm/imgdata/" -e "core-image-base" -o "$build_wic/"'
ERROR: Execution of '/OE/poky-master-build/build-CORTEX_1/arago-tmp/work/am335x_evm-poky-linux-gnueabi/core-image-base/1.0-r0/temp/run.do_image_wic.3266' failed with exit code 1:
Any clues? Has anyone seen anything like that?
--
Denys
On Tue, Jun 02, 2020 at 08:42:05AM -0500, Joshua Watt wrote:
> Add support for an --offset argument when defining a partition. Many
> SoCs require that boot partitions be located at specific offsets. Prior
> to this argument, most WKS files were using the --align attribute to
> specify the location of these fixed partitions but this is not ideal
> because in the event that the partition couldn't be placed in the
> specified location, wic would move it to the next sector with that
> alignment, often preventing the device from booting. Unlike the --align
> argument, wic will fail if a partition cannot be placed at the exact
> offset specified with --offset.
>
> Changes in V2:
> * Fixed a small typo that prevented test_fixed_size_error from passing
>
> Signed-off-by: Joshua Watt <JPEWhacker@gmail.com>
> ---
> meta/lib/oeqa/selftest/cases/wic.py | 118 ++++++++++++++++++-----
> scripts/lib/wic/ksparser.py | 46 +++++----
> scripts/lib/wic/partition.py | 1 +
> scripts/lib/wic/plugins/imager/direct.py | 15 +++
> 4 files changed, 135 insertions(+), 45 deletions(-)
>
> diff --git a/meta/lib/oeqa/selftest/cases/wic.py b/meta/lib/oeqa/selftest/cases/wic.py
> index c8765e5330..9e7be6168e 100644
> --- a/meta/lib/oeqa/selftest/cases/wic.py
> +++ b/meta/lib/oeqa/selftest/cases/wic.py
> @@ -790,41 +790,50 @@ class Wic2(WicTestCase):
> tempf.write("part " \
> "--source rootfs --ondisk hda --align 4 --fixed-size %d "
> "--fstype=ext4\n" % size)
> - wksname = os.path.splitext(os.path.basename(wkspath))[0]
>
> - return wkspath, wksname
> + return wkspath
>
> - def test_fixed_size(self):
> - """
> - Test creation of a simple image with partition size controlled through
> - --fixed-size flag
> - """
> - wkspath, wksname = Wic2._make_fixed_size_wks(200)
> + def _get_wic_partitions(self, wkspath, native_sysroot=None, ignore_status=False):
> + p = runCmd("wic create %s -e core-image-minimal -o %s" % (wkspath, self.resultdir),
> + ignore_status=ignore_status)
> +
> + if p.status:
> + return (p, None)
> +
> + wksname = os.path.splitext(os.path.basename(wkspath))[0]
>
> - runCmd("wic create %s -e core-image-minimal -o %s" \
> - % (wkspath, self.resultdir))
> - os.remove(wkspath)
> wicout = glob(self.resultdir + "%s-*direct" % wksname)
> - self.assertEqual(1, len(wicout))
> +
> + if not wicout:
> + return (p, None)
>
> wicimg = wicout[0]
>
> - native_sysroot = get_bb_var("RECIPE_SYSROOT_NATIVE", "wic-tools")
> + if not native_sysroot:
> + native_sysroot = get_bb_var("RECIPE_SYSROOT_NATIVE", "wic-tools")
>
> # verify partition size with wic
> - res = runCmd("parted -m %s unit mib p 2>/dev/null" % wicimg,
> + res = runCmd("parted -m %s unit kib p 2>/dev/null" % wicimg,
> native_sysroot=native_sysroot)
>
> # parse parted output which looks like this:
> # BYT;\n
> # /var/tmp/wic/build/tmpfwvjjkf_-201611101222-hda.direct:200MiB:file:512:512:msdos::;\n
> # 1:0.00MiB:200MiB:200MiB:ext4::;\n
> - partlns = res.output.splitlines()[2:]
> + return (p, res.output.splitlines()[2:])
>
> - self.assertEqual(1, len(partlns),
> - msg="Partition list '%s'" % res.output)
> - self.assertEqual("1:0.00MiB:200MiB:200MiB:ext4::;", partlns[0],
> - msg="Partition list '%s'" % res.output)
> + def test_fixed_size(self):
> + """
> + Test creation of a simple image with partition size controlled through
> + --fixed-size flag
> + """
> + wkspath = Wic2._make_fixed_size_wks(200)
> + _, partlns = self._get_wic_partitions(wkspath)
> + os.remove(wkspath)
> +
> + self.assertEqual(partlns, [
> + "1:4.00kiB:204804kiB:204800kiB:ext4::;",
> + ])
>
> def test_fixed_size_error(self):
> """
> @@ -832,13 +841,72 @@ class Wic2(WicTestCase):
> --fixed-size flag. The size of partition is intentionally set to 1MiB
> in order to trigger an error in wic.
> """
> - wkspath, wksname = Wic2._make_fixed_size_wks(1)
> -
> - self.assertEqual(1, runCmd("wic create %s -e core-image-minimal -o %s" \
> - % (wkspath, self.resultdir), ignore_status=True).status)
> + wkspath = Wic2._make_fixed_size_wks(1)
> + p, _ = self._get_wic_partitions(wkspath, ignore_status=True)
> os.remove(wkspath)
> - wicout = glob(self.resultdir + "%s-*direct" % wksname)
> - self.assertEqual(0, len(wicout))
> +
> + self.assertNotEqual(p.status, 0, "wic exited successfully when an error was expected:\n%s" % p.output)
> +
> + def test_offset(self):
> + native_sysroot = get_bb_var("RECIPE_SYSROOT_NATIVE", "wic-tools")
> +
> + with NamedTemporaryFile("w", suffix=".wks") as tempf:
> + # Test that partitions are placed at the correct offsets, default KB
> + tempf.write("bootloader --ptable gpt\n" \
> + "part / --source rootfs --ondisk hda --offset 32 --fixed-size 100M --fstype=ext4\n" \
> + "part /bar --ondisk hda --offset 102432 --fixed-size 100M --fstype=ext4\n")
> + tempf.flush()
> +
> + _, partlns = self._get_wic_partitions(tempf.name, native_sysroot)
> + self.assertEqual(partlns, [
> + "1:32.0kiB:102432kiB:102400kiB:ext4:primary:;",
> + "2:102432kiB:204832kiB:102400kiB:ext4:primary:;",
> + ])
> +
> + with NamedTemporaryFile("w", suffix=".wks") as tempf:
> + # Test that partitions are placed at the correct offsets, same with explicit KB
> + tempf.write("bootloader --ptable gpt\n" \
> + "part / --source rootfs --ondisk hda --offset 32K --fixed-size 100M --fstype=ext4\n" \
> + "part /bar --ondisk hda --offset 102432K --fixed-size 100M --fstype=ext4\n")
> + tempf.flush()
> +
> + _, partlns = self._get_wic_partitions(tempf.name, native_sysroot)
> + self.assertEqual(partlns, [
> + "1:32.0kiB:102432kiB:102400kiB:ext4:primary:;",
> + "2:102432kiB:204832kiB:102400kiB:ext4:primary:;",
> + ])
> +
> + with NamedTemporaryFile("w", suffix=".wks") as tempf:
> + # Test that partitions are placed at the correct offsets using MB
> + tempf.write("bootloader --ptable gpt\n" \
> + "part / --source rootfs --ondisk hda --offset 32K --fixed-size 100M --fstype=ext4\n" \
> + "part /bar --ondisk hda --offset 101M --fixed-size 100M --fstype=ext4\n")
> + tempf.flush()
> +
> + _, partlns = self._get_wic_partitions(tempf.name, native_sysroot)
> + self.assertEqual(partlns, [
> + "1:32.0kiB:102432kiB:102400kiB:ext4:primary:;",
> + "2:103424kiB:205824kiB:102400kiB:ext4:primary:;",
> + ])
> +
> + with NamedTemporaryFile("w", suffix=".wks") as tempf:
> + # Test that image creation fails if the partitions would overlap
> + tempf.write("bootloader --ptable gpt\n" \
> + "part / --source rootfs --ondisk hda --offset 32 --fixed-size 100M --fstype=ext4\n" \
> + "part /bar --ondisk hda --offset 102431 --fixed-size 100M --fstype=ext4\n")
> + tempf.flush()
> +
> + p, _ = self._get_wic_partitions(tempf.name, ignore_status=True)
> + self.assertNotEqual(p.status, 0, "wic exited successfully when an error was expected:\n%s" % p.output)
> +
> + with NamedTemporaryFile("w", suffix=".wks") as tempf:
> + # Test that partitions are not allowed to overlap with the booloader
> + tempf.write("bootloader --ptable gpt\n" \
> + "part / --source rootfs --ondisk hda --offset 8 --fixed-size 100M --fstype=ext4\n")
> + tempf.flush()
> +
> + p, _ = self._get_wic_partitions(tempf.name, ignore_status=True)
> + self.assertNotEqual(p.status, 0, "wic exited successfully when an error was expected:\n%s" % p.output)
>
> @only_for_arch(['i586', 'i686', 'x86_64'])
> def test_rawcopy_plugin_qemu(self):
> diff --git a/scripts/lib/wic/ksparser.py b/scripts/lib/wic/ksparser.py
> index b8befe78e3..05ae292ef5 100644
> --- a/scripts/lib/wic/ksparser.py
> +++ b/scripts/lib/wic/ksparser.py
> @@ -51,26 +51,31 @@ class KickStartParser(ArgumentParser):
> def error(self, message):
> raise ArgumentError(None, message)
>
> -def sizetype(arg):
> - """
> - Custom type for ArgumentParser
> - Converts size string in <num>[K|k|M|G] format into the integer value
> - """
> - if arg.isdigit():
> - return int(arg) * 1024
> +def sizetype(default):
> + def f(arg):
> + """
> + Custom type for ArgumentParser
> + Converts size string in <num>[K|k|M|G] format into the integer value
> + """
> + try:
> + suffix = default
> + size = int(arg)
> + except ValueError:
> + try:
> + suffix = arg[-1:]
> + size = int(arg[:-1])
> + except ValueError:
> + raise ArgumentTypeError("Invalid size: %r" % arg)
> +
> + if suffix == "k" or suffix == "K":
> + return size
> + if suffix == "M":
> + return size * 1024
> + if suffix == "G":
> + return size * 1024 * 1024
>
> - if not arg[:-1].isdigit():
> raise ArgumentTypeError("Invalid size: %r" % arg)
> -
> - size = int(arg[:-1])
> - if arg.endswith("k") or arg.endswith("K"):
> - return size
> - if arg.endswith("M"):
> - return size * 1024
> - if arg.endswith("G"):
> - return size * 1024 * 1024
> -
> - raise ArgumentTypeError("Invalid size: %r" % arg)
> + return f
>
> def overheadtype(arg):
> """
> @@ -136,6 +141,7 @@ class KickStart():
> part.add_argument('mountpoint', nargs='?')
> part.add_argument('--active', action='store_true')
> part.add_argument('--align', type=int)
> + part.add_argument('--offset', type=sizetype("K"))
> part.add_argument('--exclude-path', nargs='+')
> part.add_argument('--include-path', nargs='+', action='append')
> part.add_argument('--change-directory')
> @@ -161,8 +167,8 @@ class KickStart():
> # --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)
> + sizeexcl.add_argument('--size', type=sizetype("M"), default=0)
> + sizeexcl.add_argument('--fixed-size', type=sizetype("M"), default=0)
>
> part.add_argument('--source')
> part.add_argument('--sourceparams')
> diff --git a/scripts/lib/wic/partition.py b/scripts/lib/wic/partition.py
> index 7d9dd616a6..85eb15c8b8 100644
> --- a/scripts/lib/wic/partition.py
> +++ b/scripts/lib/wic/partition.py
> @@ -40,6 +40,7 @@ class Partition():
> self.mountpoint = args.mountpoint
> self.no_table = args.no_table
> self.num = None
> + self.offset = args.offset
> self.overhead_factor = args.overhead_factor
> self.part_name = args.part_name
> self.part_type = args.part_type
> diff --git a/scripts/lib/wic/plugins/imager/direct.py b/scripts/lib/wic/plugins/imager/direct.py
> index 2d06c242b6..1f65a7afe5 100644
> --- a/scripts/lib/wic/plugins/imager/direct.py
> +++ b/scripts/lib/wic/plugins/imager/direct.py
> @@ -428,6 +428,21 @@ class PartitionedImage():
> # increase the offset so we actually start the partition on right alignment
> self.offset += align_sectors
>
> + if part.offset is not None:
> + offset = (part.offset * 1024) // self.sector_size
> +
> + if offset * self.sector_size != part.offset * 1024:
> + raise WicError("Could not place %s%s at offset %dK with sector size %d" % (part.disk, self.numpart, part.offset, self.sector_size))
> +
> + delta = offset - self.offset
> + if delta < 0:
> + raise WicError("Could not place %s%s at offset %dK: next free sector is %d (delta: %d)" % (part.disk, self.numpart, part.offset, offset, delta))
> +
> + logger.debug("Skipping %d sectors to place %s%s at offset %dK",
> + delta, part.disk, self.numpart, part.offset)
> +
> + self.offset = offset
> +
> part.start = self.offset
> self.offset += part.size_sec
>
> --
> 2.26.2
>
>
next prev parent reply other threads:[~2020-06-08 23:11 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-05-11 19:26 [OE-core][PATCH] wic: Add --offset argument for partitions Joshua Watt
2020-05-11 19:48 ` [OE-core][PATCH v2] " Joshua Watt
2020-05-13 16:22 ` Richard Purdie
2020-06-02 13:42 ` [OE-core][PATCH v3] " Joshua Watt
2020-06-08 23:11 ` Denys Dmytriyenko [this message]
2020-06-09 13:26 ` Joshua Watt
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200608231107.GT17660@denix.org \
--to=denis@denix.org \
--cc=JPEWhacker@gmail.com \
--cc=openembedded-core@lists.openembedded.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox