Skip to content

stages/disks: allow nesting of partitions and raid #434

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
ajeddeloh wants to merge 1 commit into from
Closed

stages/disks: allow nesting of partitions and raid #434

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
171 changes: 93 additions & 78 deletions internal/exec/stages/disks/disks.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"fmt"
"os/exec"
"strings"
"sync"

"github.com/coreos/ignition/config/types"
"github.com/coreos/ignition/internal/exec/stages"
Expand Down Expand Up @@ -81,13 +82,8 @@ func (s stage) Run(config types.Config) bool {
return true
}

if err := s.createPartitions(config); err != nil {
s.Logger.Crit("create partitions failed: %v", err)
return false
}

if err := s.createRaids(config); err != nil {
s.Logger.Crit("failed to create raids: %v", err)
if err := s.createPartitionsAndRaids(config); err != nil {
s.Logger.Crit("failed to create partitions and raids: %v", err)
return false
}

Expand Down Expand Up @@ -170,103 +166,122 @@ func (s stage) waitOnDevicesAndCreateAliases(devs []string, ctxt string) error {
return nil
}

// createPartitions creates the partitions described in config.Storage.Disks.
func (s stage) createPartitions(config types.Config) error {
if len(config.Storage.Disks) == 0 {
// createPartitionsAndRaids creates the partitions and raid devices described in config.Storage.Partitions
// and config.Storage.Raid.
func (s stage) createPartitionsAndRaids(config types.Config) error {
if len(config.Storage.Disks) == 0 && len(config.Storage.Raid) == 0 {
return nil
}
s.Logger.PushPrefix("createPartitions")
s.Logger.PushPrefix("createPartitionsAndRaids")
defer s.Logger.PopPrefix()

devs := []string{}
for _, disk := range config.Storage.Disks {
devs = append(devs, string(disk.Device))
}

if err := s.waitOnDevicesAndCreateAliases(devs, "disks"); err != nil {
return err
}
errChan := make(chan error)
// Mutex for preventing udev races by only doing one thing at a time. Also makes logs easier to read.
mutex := sync.Mutex{}

for _, dev := range config.Storage.Disks {
devAlias := util.DeviceAlias(string(dev.Device))

err := s.Logger.LogOp(func() error {
op := sgdisk.Begin(s.Logger, devAlias)
if dev.WipeTable {
s.Logger.Info("wiping partition table requested on %q", devAlias)
op.WipeTable(true)
}

for _, part := range dev.Partitions {
op.CreatePartition(sgdisk.Partition{
Number: part.Number,
Length: uint64(part.Size),
Offset: uint64(part.Start),
Label: string(part.Label),
TypeGUID: string(part.TypeGUID),
GUID: string(part.GUID),
})
}
go s.createPartitions(errChan, &mutex, dev)
}
for _, dev := range config.Storage.Raid {
go s.createRaid(errChan, &mutex, dev)
}

if err := op.Commit(); err != nil {
return fmt.Errorf("commit failure: %v", err)
}
return nil
}, "partitioning %q", devAlias)
if err != nil {
// wait for all the goroutines to finish
for i := 0; i < len(config.Storage.Disks)+len(config.Storage.Raid); i++ {
if err := <-errChan; err != nil {
return err
Copy link
Contributor Author

@ajeddeloh ajeddeloh Aug 30, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can leak goroutines that are waiting for devices or the mutex. It could be handled but Ignition will be exiting shortly then anyway. Is it worth handling that or just let them be destroyed with the process when Ignition dies?

Copy link
Contributor

@cgonyeo cgonyeo Aug 30, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's no easy way to kill goroutines, so I say let them be leaked here and then destroyed when the program exits. Plus some of them could be endlessly stuck waiting on devices if one of them fails.

}
}

return nil
}

// createRaids creates the raid arrays described in config.Storage.Raid.
func (s stage) createRaids(config types.Config) error {
if len(config.Storage.Raid) == 0 {
// createPartitions creates all the partitions for the disk specified by the types.Disk struct.
func (s stage) createPartitions(done chan error, mutex *sync.Mutex, disk types.Disk) {
devs := []string{disk.Device}
if err := s.waitOnDevicesAndCreateAliases(devs, "disks"); err != nil {
done <- err
return
}
// This should only be unlocked if we were successful to prevent ignition from continuing
// to create partitions or raid devices if it failed
mutex.Lock()

devAlias := util.DeviceAlias(string(disk.Device))

err := s.Logger.LogOp(func() error {
op := sgdisk.Begin(s.Logger, devAlias)
if disk.WipeTable {
s.Logger.Info("wiping partition table requested on %q", devAlias)
op.WipeTable(true)
}

for _, part := range disk.Partitions {
op.CreatePartition(sgdisk.Partition{
Number: part.Number,
Length: uint64(part.Size),
Offset: uint64(part.Start),
Label: string(part.Label),
TypeGUID: string(part.TypeGUID),
GUID: string(part.GUID),
})
}

if err := op.Commit(); err != nil {
return fmt.Errorf("commit failure: %v", err)
}
return nil
}, "partitioning %q", devAlias)
if err != nil {
done <- err
return
}
s.Logger.PushPrefix("createRaids")
defer s.Logger.PopPrefix()
done <- nil
mutex.Unlock()
}

// createRaid creates the raid array specified by the types.Raid struct.
func (s stage) createRaid(done chan error, mutex *sync.Mutex, array types.Raid) {
// go--
devs := []string{}
for _, array := range config.Storage.Raid {
for _, dev := range array.Devices {
devs = append(devs, string(dev))
}
for _, dev := range array.Devices {
devs = append(devs, string(dev))
}

if err := s.waitOnDevicesAndCreateAliases(devs, "raids"); err != nil {
return err
done <- err
return
}
// This should only be unlocked if we were successful to prevent ignition from continuing
// to create partitions or raid devices if it failed
mutex.Lock()

args := []string{
"--create", array.Name,
"--force",
"--run",
"--homehost", "any",
"--level", array.Level,
"--raid-devices", fmt.Sprintf("%d", len(array.Devices)-array.Spares),
}

for _, md := range config.Storage.Raid {
args := []string{
"--create", md.Name,
"--force",
"--run",
"--homehost", "any",
"--level", md.Level,
"--raid-devices", fmt.Sprintf("%d", len(md.Devices)-md.Spares),
}

if md.Spares > 0 {
args = append(args, "--spare-devices", fmt.Sprintf("%d", md.Spares))
}

for _, dev := range md.Devices {
args = append(args, util.DeviceAlias(string(dev)))
}
if array.Spares > 0 {
args = append(args, "--spare-devices", fmt.Sprintf("%d", array.Spares))
}

if _, err := s.Logger.LogCmd(
exec.Command("/sbin/mdadm", args...),
"creating %q", md.Name,
); err != nil {
return fmt.Errorf("mdadm failed: %v", err)
}
for _, dev := range array.Devices {
args = append(args, util.DeviceAlias(string(dev)))
}

return nil
if _, err := s.Logger.LogCmd(
exec.Command("/sbin/mdadm", args...),
"creating %q", array.Name,
); err != nil {
done <- fmt.Errorf("mdadm failed: %v", err)
return
}
done <- nil
mutex.Unlock()
}

// createFilesystems creates the filesystems described in config.Storage.Filesystems.
Expand Down