Skip to content

Commit

Permalink
many: order volume structures in offset order
Browse files Browse the repository at this point in the history
This is a preparation for min-size. Also, move YamlIndex to
gadget.VolumeStructure so we can keep track of the original order in
the yaml (in the end it is implicit data of the gadget, so it make
more sense there instead of in gadget.LaidOutStructure).
  • Loading branch information
alfonsosanchezbeato committed Apr 5, 2023
1 parent 20dc020 commit bbbd513
Show file tree
Hide file tree
Showing 14 changed files with 221 additions and 46 deletions.
6 changes: 6 additions & 0 deletions daemon/api_systems_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -978,6 +978,7 @@ func (s *systemsSuite) TestSystemsGetSpecificLabelIntegration(c *check.C) {
Image: "pc-boot.img",
},
},
YamlIndex: 0,
},
{
Name: "BIOS Boot",
Expand All @@ -994,6 +995,7 @@ func (s *systemsSuite) TestSystemsGetSpecificLabelIntegration(c *check.C) {
Image: "pc-core.img",
},
},
YamlIndex: 1,
},
{
Name: "ubuntu-seed",
Expand All @@ -1014,6 +1016,7 @@ func (s *systemsSuite) TestSystemsGetSpecificLabelIntegration(c *check.C) {
Target: "EFI/boot/bootx64.efi",
},
},
YamlIndex: 2,
},
{
Name: "ubuntu-boot",
Expand All @@ -1024,6 +1027,7 @@ func (s *systemsSuite) TestSystemsGetSpecificLabelIntegration(c *check.C) {
Offset: asOffsetPtr(1202 * quantity.OffsetMiB),
Size: 750 * quantity.SizeMiB,
Filesystem: "ext4",
YamlIndex: 3,
},
{
Name: "ubuntu-save",
Expand All @@ -1034,6 +1038,7 @@ func (s *systemsSuite) TestSystemsGetSpecificLabelIntegration(c *check.C) {
Offset: asOffsetPtr(1952 * quantity.OffsetMiB),
Size: 16 * quantity.SizeMiB,
Filesystem: "ext4",
YamlIndex: 4,
},
{
Name: "ubuntu-data",
Expand All @@ -1044,6 +1049,7 @@ func (s *systemsSuite) TestSystemsGetSpecificLabelIntegration(c *check.C) {
Offset: asOffsetPtr(1968 * quantity.OffsetMiB),
Size: 1 * quantity.SizeGiB,
Filesystem: "ext4",
YamlIndex: 5,
},
},
},
Expand Down
1 change: 1 addition & 0 deletions gadget/export_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ var (
OnDiskStructureIsLikelyImplicitSystemDataRole = onDiskStructureIsLikelyImplicitSystemDataRole

SearchForVolumeWithTraits = searchForVolumeWithTraits
OrderStructuresByOffset = orderStructuresByOffset
)

func MockEvalSymlinks(mock func(path string) (string, error)) (restore func()) {
Expand Down
69 changes: 69 additions & 0 deletions gadget/gadget.go
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,9 @@ type VolumeStructure struct {
// and just used as part of the POST /systems/<label> API that
// is used by an installer.
Device string `yaml:"-" json:"device,omitempty"`

// Index of the structure definition in gadget YAML, note this starts at 0.
YamlIndex int `yaml:"-" json:"-"`
}

// IsRoleMBR tells us if v has MBR role or not.
Expand Down Expand Up @@ -579,6 +582,68 @@ func compatWithPibootOrIndeterminate(m Model) bool {
return m == nil || m.Grade() != asserts.ModelGradeUnset
}

// Ancillary structs to sort volume structures. We split volumes in
// contiguousStructs slices, with each of these beginning with a structure with
// a known fixed offset, followed by structures for which the offset is unknown
// so we can know for sure that they appear after the first structure in
// contiguousStruct. The unknown offsets appear because of min-size use. The
// contiguousStructs are the things that we need to order, as all have a known
// starting offset, which is not true for all the volume structures.
type contiguousStructs struct {
// vss contains contiguous structures with the first one
// containing a valid Offset
vss []VolumeStructure
}

type contiguousStructsSet []*contiguousStructs

func (scss contiguousStructsSet) Len() int {
return len(scss)
}

func (scss contiguousStructsSet) Less(i, j int) bool {
return *scss[i].vss[0].Offset < *scss[j].vss[0].Offset
}

func (scss contiguousStructsSet) Swap(i, j int) {
scss[i], scss[j] = scss[j], scss[i]
}

func orderStructuresByOffset(vss []VolumeStructure) []VolumeStructure {
if vss == nil {
return nil
}

// Build contiguous structures
scss := contiguousStructsSet{}
var currentCont *contiguousStructs
for _, s := range vss {
// If offset is set we can start a new "block", otherwise the
// structure goes right after the latest structure of the
// current block. Note that currentCont will never be accessed
// as nil as necessarily the first structure in gadget.yaml will
// have offset explicitly or implicitly (the only way for a
// structure to have nil offset is when the current structure
// does not have explicit offset and the previous one either
// does not have itself offset or has min-size set).
if s.Offset != nil {
currentCont = &contiguousStructs{}
scss = append(scss, currentCont)
}

currentCont.vss = append(currentCont.vss, s)
}

sort.Sort(scss)

// Build plain list of structures now
ordVss := []VolumeStructure{}
for _, cs := range scss {
ordVss = append(ordVss, cs.vss...)
}
return ordVss
}

// InfoFromGadgetYaml parses the provided gadget metadata.
// If model is nil only self-consistency checks are performed.
// If model is not nil implied values for filesystem labels will be set
Expand Down Expand Up @@ -638,6 +703,8 @@ func InfoFromGadgetYaml(gadgetYaml []byte, model Model) (*Info, error) {
return nil, fmt.Errorf("invalid volume %q: %v", name, err)
}

v.Structure = orderStructuresByOffset(v.Structure)

switch v.Bootloader {
case "":
// pass
Expand Down Expand Up @@ -702,6 +769,8 @@ func setImplicitForVolume(vol *Volume, model Model) error {
for i := range vol.Structure {
// set the VolumeName for the structure from the volume itself
vol.Structure[i].VolumeName = vol.Name
// Store index as we will reorder later
vol.Structure[i].YamlIndex = i

// set other implicit data for the structure
if err := setImplicitForVolumeStructure(&vol.Structure[i], rs, knownFsLabels); err != nil {
Expand Down
120 changes: 120 additions & 0 deletions gadget/gadget_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -826,6 +826,7 @@ func (s *gadgetYamlTestSuite) TestReadMultiVolumeGadgetYamlValid(c *C) {
Target: ".",
},
},
YamlIndex: 0,
},
{
VolumeName: "frobinator-image",
Expand All @@ -836,6 +837,7 @@ func (s *gadgetYamlTestSuite) TestReadMultiVolumeGadgetYamlValid(c *C) {
Type: "83",
Filesystem: "ext4",
Size: mustParseGadgetSize(c, "380M"),
YamlIndex: 1,
},
},
},
Expand Down Expand Up @@ -3936,3 +3938,121 @@ func (s *gadgetYamlTestSuite) TestVolumeMinSize(c *C) {
s.testVolumeMinSize(c, tc.gadgetYaml, tc.volsSizes)
}
}

func (s *gadgetYamlTestSuite) TestOrderStructuresByOffset(c *C) {
for _, tc := range []struct {
unordered []gadget.VolumeStructure
ordered []gadget.VolumeStructure
description string
}{
{
unordered: []gadget.VolumeStructure{
{Offset: asOffsetPtr(100)},
{Offset: asOffsetPtr(0)},
{Offset: nil},
{Offset: asOffsetPtr(50)},
},
ordered: []gadget.VolumeStructure{
{Offset: asOffsetPtr(0)},
{Offset: nil},
{Offset: asOffsetPtr(50)},
{Offset: asOffsetPtr(100)},
},
description: "test one",
},
{
unordered: []gadget.VolumeStructure{},
ordered: []gadget.VolumeStructure{},
description: "test two",
},
{
unordered: []gadget.VolumeStructure{
{Offset: asOffsetPtr(300)},
{Offset: nil, Name: "nil1"},
{Offset: asOffsetPtr(1)},
{Offset: asOffsetPtr(100)},
{Offset: nil, Name: "nil2"},
{Offset: nil, Name: "nil3"},
},
ordered: []gadget.VolumeStructure{
{Offset: asOffsetPtr(1)},
{Offset: asOffsetPtr(100)},
{Offset: nil, Name: "nil2"},
{Offset: nil, Name: "nil3"},
{Offset: asOffsetPtr(300)},
{Offset: nil, Name: "nil1"},
},
description: "test three",
},
} {
c.Logf("testing order structures: %s", tc.description)
ordered := gadget.OrderStructuresByOffset(tc.unordered)
c.Check(ordered, DeepEquals, tc.ordered)
}
}

func (s *gadgetYamlTestSuite) TestGadgetUnorderedStructures(c *C) {
var unorderedYaml = []byte(`
volumes:
unordered:
bootloader: u-boot
schema: gpt
structure:
- name: ubuntu-seed
filesystem: ext4
size: 499M
type: 83,0FC63DAF-8483-4772-8E79-3D69D8477DE4
role: system-seed
- name: ubuntu-save
size: 100M
offset: 700M
filesystem: ext4
type: 83,0FC63DAF-8483-4772-8E79-3D69D8477DE4
role: system-save
- name: ubuntu-boot
filesystem: ext4
size: 100M
offset: 500M
type: 83,0FC63DAF-8483-4772-8E79-3D69D8477DE4
role: system-boot
- name: other1
size: 100M
type: bare
- name: ubuntu-data
filesystem: ext4
offset: 800M
size: 1G
type: 83,0FC63DAF-8483-4772-8E79-3D69D8477DE4
role: system-data
`)

// TODO add more tests when min-size is introduced
tests := []struct {
yaml []byte
orderedNames []string
info string
}{
{
yaml: unorderedYaml,
orderedNames: []string{"ubuntu-seed", "ubuntu-boot",
"other1", "ubuntu-save", "ubuntu-data"},
info: "test one",
},
}
for _, tc := range tests {
c.Logf("tc: %s", tc.info)
giMeta, err := gadget.InfoFromGadgetYaml(tc.yaml, nil)
c.Assert(err, IsNil)
c.Assert(len(giMeta.Volumes), Equals, 1)

var vol *gadget.Volume
for vn := range giMeta.Volumes {
vol = giMeta.Volumes[vn]
}
names := []string{}
for _, s := range vol.Structure {
names = append(names, s.Name)
}
c.Check(names, DeepEquals, tc.orderedNames)
}
}
2 changes: 1 addition & 1 deletion gadget/install/content_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,8 @@ func mockOnDiskStructureSystemSeed(gadgetRoot string) *gadget.LaidOutStructure {
Target: "EFI/boot/grubx64.efi",
},
},
YamlIndex: 1000, // to demonstrate we do not use the laid out index
},
YamlIndex: 1000, // to demonstrate we do not use the laid out index
ResolvedContent: []gadget.ResolvedContent{
{
VolumeContent: &gadget.VolumeContent{
Expand Down
8 changes: 4 additions & 4 deletions gadget/install/partition_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -185,9 +185,9 @@ var mockLaidoutStructureWritable = gadget.LaidOutStructure{
// Note the DiskIndex appears to be the same as the YamlIndex, but this is
// because YamlIndex starts at 0 and DiskIndex starts at 1, and there is a
// yaml structure (the MBR) that does not appear on disk
Offset: asOffsetPtr(1260388352),
Offset: asOffsetPtr(1260388352),
YamlIndex: 3,
},
YamlIndex: 3,
}

var mockLaidoutStructureSave = gadget.LaidOutStructure{
Expand All @@ -213,8 +213,8 @@ var mockLaidoutStructureSave = gadget.LaidOutStructure{
Role: "system-save",
Filesystem: "ext4",
Offset: asOffsetPtr(1260388352),
YamlIndex: 3,
},
YamlIndex: 3,
}

var mockLaidoutStructureWritableAfterSave = gadget.LaidOutStructure{
Expand All @@ -241,8 +241,8 @@ var mockLaidoutStructureWritableAfterSave = gadget.LaidOutStructure{
Label: "ubuntu-data",
Filesystem: "ext4",
Offset: asOffsetPtr(1394606080),
YamlIndex: 4,
},
YamlIndex: 4,
}

type uc20Model struct{}
Expand Down
5 changes: 1 addition & 4 deletions gadget/layout.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,6 @@ type LaidOutStructure struct {
// AbsoluteOffsetWrite is the resolved absolute position of offset-write
// for this structure element within the enclosing volume
AbsoluteOffsetWrite *quantity.Offset
// Index of the structure definition in gadget YAML, note this starts at 0.
YamlIndex int
// LaidOutContent is a list of raw content inside the structure
LaidOutContent []LaidOutContent
// ResolvedContent is a list of filesystem content that has all
Expand Down Expand Up @@ -139,7 +137,7 @@ func (l *LaidOutStructure) IsPartition() bool {
}

func (p LaidOutStructure) String() string {
return fmtIndexAndName(p.YamlIndex, p.Name())
return fmtIndexAndName(p.VolumeStructure.YamlIndex, p.Name())
}

type byStartOffset []LaidOutStructure
Expand Down Expand Up @@ -193,7 +191,6 @@ func layoutVolumeStructures(volume *Volume) (structures []LaidOutStructure, byNa
for idx := range volume.Structure {
ps := LaidOutStructure{
VolumeStructure: &volume.Structure[idx],
YamlIndex: idx,
}

if ps.Name() != "" {
Expand Down
Loading

0 comments on commit bbbd513

Please sign in to comment.