stages/disks: allow nesting of partitions and raid #434
Conversation
| 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 |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
internal/exec/stages/disks/disks.go
Outdated
| done <- err | ||
| return | ||
| } | ||
| mutex.Lock() |
There was a problem hiding this comment.
I typically do a defer mutex.Unlock() here. Having different goroutines lock and unlock the mutex makes me nervous.
There was a problem hiding this comment.
Currently it's unlocked only on success, so if something fails a blocked goroutine doesn't start partitioning something or creating a raid array while ignition is failing (which, at the very least, could create very confusing logs). It might make sense to move it right before the successful return though. I'll add a comment in the code regardless.
internal/exec/stages/disks/disks.go
Outdated
| done <- err | ||
| return | ||
| } | ||
| mutex.Lock() |
There was a problem hiding this comment.
Same defer comment as above
|
kola test: coreos/mantle#699 |
ajeddeloh
force-pushed
the
nestedraid
branch
from
9b9800a
to
e7883c1
Compare
Spawn one goroutine per disk/array that blocks until the devices it needs are available. This eliminates the requirement that all partitions be created before all raids, and allows raids to be nested.
ajeddeloh
force-pushed
the
nestedraid
branch
from
e7883c1
to
899ce32
Compare
|
Closing since I no longer want to implement this in this way and since master is significatnly changed. |
Spawn one goroutine per disk/array that blocks until the devices it needs are available. This eliminates the requirement that all partitions be created before all raids, and allows raids to be nested.
Fixes: coreos/bugs#2126