Fix structure sorting and partition numbering #97

warsaw · 2016-11-18T19:55:31Z

Revert previous change which sorted structure volumes by their offset. Instead, we preserve gadget.yaml order for purposes of partition numbering, but we still provide implicit offsets when they are missing, and we still sanity check for partition overlap.

vorlonofportland · 2016-11-18T22:45:13Z

ubuntu_image/tests/test_builder.py

+                )
+            part2 = SimpleNamespace(
+                name='gamma',
+                type='mbr',


type='mbr', not role=mbr?

I haven't yet done a wholesale change of the test suite to use role yet. This is mostly a copy-n-paste of an existing test. LP: #1643086

vorlonofportland · 2016-11-18T22:53:56Z

ubuntu_image/tests/test_builder.py

+                # Part 1's image lives at MiB(4).
+                fp.seek(MiB(4))
+                self.assertEqual(fp.read(15), b'\2' * 12 + b'\0' * 3)
+                # Part 2's image is an MBR so it must live at offset 0.


Do we have any tests that verify the correctness of behavior of writing out partitions without an offset defined? That would probably be a good idea to land as part of this change, to ensure we aren't regressing other cases.

Yeah, good idea. We have lots of tests for implicit offsets, but not one specifically for testing the written partitions. I'll add that in this branch.

I just remembered that implicit offsets are handled in the parser, so effectively we do have tests for this use case. If an offset is not defined for a structure, it's assigned the "last offset", i.e. the offset of the previous structure + its size. We definitely have parser tests for this use case.

By the time the builder gets to making the disk, all structures will have offsets defined, so I think the tests added in this branch are sufficient.

walimis · 2016-11-19T00:01:12Z

Hi barry,

My original gadget.yaml is:

volumes:
  artik10:
    schema: mbr
    bootloader: u-boot
    structure:
      - name: bl1
        size: 4M
        content:
          - image: bl1.bin
            offset: 512
          - image: bl2.bin
            offset: 15872
          - image: u-boot.bin
            offset: 32256
          - image: tzsw.bin
            offset: 1080832
          - image: params_sdboot.bin
            offset: 2129408
        type: mbr
      - type: 0C
        offset: 4M
        filesystem: vfat
        filesystem-label: system-boot
        size: 128M
        content:
          - source: u-boot.bin
            target: u-boot.bin

After modifying , the new gadget.yaml is :

volumes:
  artik10:
    schema: mbr
    bootloader: u-boot
    structure:
      - name: bl1
        size: 4M
        content:
          - image: bl1.bin
            offset: 512
          - image: bl2.bin
            offset: 15872
          - image: u-boot.bin
            offset: 32256
          - image: tzsw.bin
            offset: 1080832
          - image: params_sdboot.bin
            offset: 2129408
        type: 0C
      - type: 0C
        filesystem: vfat
        filesystem-label: system-boot
        size: 128M
        content:
          - source: u-boot.bin
            target: u-boot.bin

Using the commit with new gadget.yaml, I got following error:

$ sudo ubuntu-image  -c beta -o artik5-newtools.img   --extra-snaps artik5-kernel_3.10.0_armhf.snap --extra-snaps artik5_16.04-0.1_armhf.snap  --extra-snaps bluez --extra-snaps artik-brcm-bt_0.1_armhf.snap artik5.model.new
Fetching core
Copying "artik5-kernel_3.10.0_armhf.snap" (artik5-kernel)
Copying "artik5_16.04-0.1_armhf.snap" (artik5)
artik5-kernel already prepared, skipping
artik5 already prepared, skipping
Fetching bluez
Copying "artik-brcm-bt_0.1_armhf.snap" (artik-brcm-bt)
Crash in state machine
Traceback (most recent call last):
  File "/usr/local/lib/python3.5/dist-packages/ubuntu_image-0.12+17.04ubuntu1-py3.5.egg/ubuntu_image/__main__.py", line 158, in main
    list(state_machine)
  File "/usr/local/lib/python3.5/dist-packages/ubuntu_image-0.12+17.04ubuntu1-py3.5.egg/ubuntu_image/state.py", line 73, in __next__
    step()
  File "/usr/local/lib/python3.5/dist-packages/ubuntu_image-0.12+17.04ubuntu1-py3.5.egg/ubuntu_image/builder.py", line 416, in make_disk
    image.partition(part_id, **partition_args)
  File "/usr/local/lib/python3.5/dist-packages/ubuntu_image-0.12+17.04ubuntu1-py3.5.egg/ubuntu_image/image.py", line 190, in partition
    .format(key))
ValueError: change_name option not supported for MBR partitions

warsaw added 2 commits November 18, 2016 12:59

Typo

ebe2fe2

LP: #1642999

96587bc

Revert previous change which sorted structure volumes by their offset. Instead, we preserve gadget.yaml order for purposes of partition numbering, but we still provide implicit offsets when they are missing, and we still sanity check for partition overlap.

vorlonofportland reviewed Nov 18, 2016

View reviewed changes

warsaw merged commit ccc5d4a into master Nov 21, 2016

warsaw deleted the lp1642999 branch November 21, 2016 15:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix structure sorting and partition numbering #97

Fix structure sorting and partition numbering #97

warsaw commented Nov 18, 2016

vorlonofportland Nov 18, 2016

warsaw Nov 18, 2016

vorlonofportland Nov 18, 2016

warsaw Nov 18, 2016

warsaw Nov 21, 2016

walimis commented Nov 19, 2016 •

edited by warsaw

Fix structure sorting and partition numbering #97

Fix structure sorting and partition numbering #97

Conversation

warsaw commented Nov 18, 2016

vorlonofportland Nov 18, 2016

Choose a reason for hiding this comment

warsaw Nov 18, 2016

Choose a reason for hiding this comment

vorlonofportland Nov 18, 2016

Choose a reason for hiding this comment

warsaw Nov 18, 2016

Choose a reason for hiding this comment

warsaw Nov 21, 2016

Choose a reason for hiding this comment

walimis commented Nov 19, 2016 • edited by warsaw

walimis commented Nov 19, 2016 •

edited by warsaw