Qemu: use nbd sockets for multipath disks#1315
Qemu: use nbd sockets for multipath disks#1315openshift-merge-robot merged 1 commit intocoreos:masterfrom darkmuggle:feature/nbd-disks
Conversation
| if err != nil { | ||
| return err | ||
| } | ||
| defer inst.Destroy() |
There was a problem hiding this comment.
It turns out we were leaving a bunch of swtpm defunct processes laying around. I had ~200 on my system.
There was a problem hiding this comment.
This is probably right, but we use prctl(PR_SET_PDEATHSIG) though which should have prevented that anyways.
| if err != nil { | ||
| return err | ||
| } | ||
| defer inst.Destroy() |
There was a problem hiding this comment.
This is probably right, but we use prctl(PR_SET_PDEATHSIG) though which should have prevented that anyways.
| } | ||
| if inst.qemu != nil { | ||
| // check if qemu is dead before trying to kill it | ||
| _, notFound := os.FindProcess(int(inst.Pid())) |
There was a problem hiding this comment.
But wait, we're the only thing that should be calling waitpid() on it, so it can't have gone away - it should be safe to call os.Kill() even on a zombie right?
(really what we want are pidfds but we can't rely on those yet)
There was a problem hiding this comment.
The reason for checking that was otherwise there's an exit error for Wait() having already been called.
There was a problem hiding this comment.
Oh right of course, because the main process blocks in Wait(). So I think the right way to fix this though is to nilify the pointers after calling Wait(). We already guard against nil above:
if inst.qemu != nil {
inst.qemu.Wait()
inst.qemu = nil
}
And the same for the inst.nbdServers, clear out the array via inst.nbdServers = nil afterwards.
There was a problem hiding this comment.
In Rust, this would be Option<Process> and we'd do something like:
if let Some(proc) = inst.qemu.take() {
proc.wait()
}
And the "move semantics" here is a lot clearer.
There was a problem hiding this comment.
Updated that. exec.cmd.Kill() already calls Wait() anyway. https://github.com/coreos/mantle/blob/cl/system/exec/exec.go#L71
darkmuggle
added
WIP
|
Triple checked this. I've ensured:
|
This changes up the multipath support by using a NBD server to open the the qcow2 disks.
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cgwalters, darkmuggle The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
4 participants
This changes up the multipath support by using a NBD server to open the
the qcow2 disks.
This was explored due to #1296 (comment)