From 0075bf85ba0ecb74ab8cb8ab5c9e04e109db1278 Mon Sep 17 00:00:00 2001 From: Peng Tao Date: Wed, 21 Aug 2019 11:13:29 +0800 Subject: [PATCH] hypervisor: allow to return a slice of pids so that for qemu, we can save and export virtiofsd pid, and put it to the same cgroup as the qemu process. Fixes: #1972 Signed-off-by: Peng Tao --- virtcontainers/acrn.go | 8 ++--- virtcontainers/cgroups.go | 17 ++++++---- virtcontainers/fc.go | 8 ++--- virtcontainers/hypervisor.go | 12 ++++++- virtcontainers/kata_agent.go | 2 +- virtcontainers/mock_hypervisor.go | 4 +-- virtcontainers/persist/api/hypervisor.go | 1 + virtcontainers/persist/api/version.go | 2 +- virtcontainers/qemu.go | 41 +++++++++++++++--------- virtcontainers/qemu_test.go | 37 +++++++++++++++++++++ virtcontainers/vm.go | 2 +- 11 files changed, 99 insertions(+), 35 deletions(-) diff --git a/virtcontainers/acrn.go b/virtcontainers/acrn.go index 877ff166aa..a4a431c0d0 100644 --- a/virtcontainers/acrn.go +++ b/virtcontainers/acrn.go @@ -617,8 +617,8 @@ func (a *acrn) cleanup() error { return nil } -func (a *acrn) pid() int { - return a.info.PID +func (a *acrn) getPids() []int { + return []int{a.info.PID} } func (a *acrn) fromGrpc(ctx context.Context, hypervisorConfig *HypervisorConfig, store *store.VCStore, j []byte) error { @@ -639,7 +639,7 @@ func (a *acrn) storeInfo() error { } func (a *acrn) save() (s persistapi.HypervisorState) { - s.Pid = a.pid() + s.Pid = a.info.PID s.Type = string(AcrnHypervisor) s.UUID = a.state.UUID return @@ -651,7 +651,7 @@ func (a *acrn) load(s persistapi.HypervisorState) { } func (a *acrn) check() error { - if err := syscall.Kill(a.pid(), syscall.Signal(0)); err != nil { + if err := syscall.Kill(a.info.PID, syscall.Signal(0)); err != nil { return errors.Wrapf(err, "failed to ping acrn process") } diff --git a/virtcontainers/cgroups.go b/virtcontainers/cgroups.go index 8cfd9db005..89f91b70ea 100644 --- a/virtcontainers/cgroups.go +++ b/virtcontainers/cgroups.go @@ -209,9 +209,9 @@ func (s *Sandbox) deleteCgroups() error { } func (s *Sandbox) constrainHypervisor(cgroup cgroups.Cgroup) error { - pid := s.hypervisor.pid() - if pid <= 0 { - return fmt.Errorf("Invalid hypervisor PID: %d", pid) + pids := s.hypervisor.getPids() + if len(pids) == 0 || pids[0] == 0 { + return fmt.Errorf("Invalid hypervisor PID: %+v", pids) } // Move hypervisor into cgroups without constraints, @@ -222,9 +222,14 @@ func (s *Sandbox) constrainHypervisor(cgroup cgroups.Cgroup) error { if err != nil { return fmt.Errorf("Could not create cgroup %v: %v", path, err) } - - if err := noConstraintsCgroup.Add(cgroups.Process{Pid: pid}); err != nil { - return fmt.Errorf("Could not add hypervisor PID %d to cgroup %v: %v", pid, path, err) + for _, pid := range pids { + if pid <= 0 { + s.Logger().Warnf("Invalid hypervisor pid: %d", pid) + continue + } + if err := noConstraintsCgroup.Add(cgroups.Process{Pid: pid}); err != nil { + return fmt.Errorf("Could not add hypervisor PID %d to cgroup %v: %v", pid, path, err) + } } // when new container joins, new CPU could be hotplugged, so we diff --git a/virtcontainers/fc.go b/virtcontainers/fc.go index f8bc56285d..c498d6e932 100644 --- a/virtcontainers/fc.go +++ b/virtcontainers/fc.go @@ -985,8 +985,8 @@ func (fc *firecracker) cleanup() error { return nil } -func (fc *firecracker) pid() int { - return fc.info.PID +func (fc *firecracker) getPids() []int { + return []int{fc.info.PID} } func (fc *firecracker) fromGrpc(ctx context.Context, hypervisorConfig *HypervisorConfig, store *store.VCStore, j []byte) error { @@ -998,7 +998,7 @@ func (fc *firecracker) toGrpc() ([]byte, error) { } func (fc *firecracker) save() (s persistapi.HypervisorState) { - s.Pid = fc.pid() + s.Pid = fc.info.PID s.Type = string(FirecrackerHypervisor) return } @@ -1008,7 +1008,7 @@ func (fc *firecracker) load(s persistapi.HypervisorState) { } func (fc *firecracker) check() error { - if err := syscall.Kill(fc.pid(), syscall.Signal(0)); err != nil { + if err := syscall.Kill(fc.info.PID, syscall.Signal(0)); err != nil { return errors.Wrapf(err, "failed to ping fc process") } diff --git a/virtcontainers/hypervisor.go b/virtcontainers/hypervisor.go index 4a795f7bac..bd81056cf5 100644 --- a/virtcontainers/hypervisor.go +++ b/virtcontainers/hypervisor.go @@ -648,6 +648,14 @@ func RunningOnVMM(cpuInfoPath string) (bool, error) { return false, fmt.Errorf("Couldn't find %q from %q output", flagsField, cpuInfoPath) } +func getHypervisorPid(h hypervisor) int { + pids := h.getPids() + if len(pids) == 0 { + return 0 + } + return pids[0] +} + // hypervisor is the virtcontainers hypervisor interface. // The default hypervisor implementation is Qemu. type hypervisor interface { @@ -668,7 +676,9 @@ type hypervisor interface { hypervisorConfig() HypervisorConfig getThreadIDs() (vcpuThreadIDs, error) cleanup() error - pid() int + // getPids returns a slice of hypervisor related process ids. + // The hypervisor pid must be put at index 0. + getPids() []int fromGrpc(ctx context.Context, hypervisorConfig *HypervisorConfig, store *store.VCStore, j []byte) error toGrpc() ([]byte, error) check() error diff --git a/virtcontainers/kata_agent.go b/virtcontainers/kata_agent.go index 254b8211ae..c933971106 100644 --- a/virtcontainers/kata_agent.go +++ b/virtcontainers/kata_agent.go @@ -657,7 +657,7 @@ func (k *kataAgent) startProxy(sandbox *Sandbox) error { proxyParams := proxyParams{ id: sandbox.id, - hid: sandbox.hypervisor.pid(), + hid: getHypervisorPid(sandbox.hypervisor), path: sandbox.config.ProxyConfig.Path, agentURL: agentURL, consoleURL: consoleURL, diff --git a/virtcontainers/mock_hypervisor.go b/virtcontainers/mock_hypervisor.go index 90611aeb75..86ed118258 100644 --- a/virtcontainers/mock_hypervisor.go +++ b/virtcontainers/mock_hypervisor.go @@ -104,8 +104,8 @@ func (m *mockHypervisor) cleanup() error { return nil } -func (m *mockHypervisor) pid() int { - return m.mockPid +func (m *mockHypervisor) getPids() []int { + return []int{m.mockPid} } func (m *mockHypervisor) fromGrpc(ctx context.Context, hypervisorConfig *HypervisorConfig, store *store.VCStore, j []byte) error { diff --git a/virtcontainers/persist/api/hypervisor.go b/virtcontainers/persist/api/hypervisor.go index 75e7c315c5..b64c32e0cb 100644 --- a/virtcontainers/persist/api/hypervisor.go +++ b/virtcontainers/persist/api/hypervisor.go @@ -39,5 +39,6 @@ type HypervisorState struct { // HotpluggedCPUs is the list of CPUs that were hot-added HotpluggedVCPUs []CPUDevice HotpluggedMemory int + VirtiofsdPid int HotplugVFIOOnRootBus bool } diff --git a/virtcontainers/persist/api/version.go b/virtcontainers/persist/api/version.go index 9575af7a57..a30ceef0c6 100644 --- a/virtcontainers/persist/api/version.go +++ b/virtcontainers/persist/api/version.go @@ -15,5 +15,5 @@ const ( // If you can't be sure if the change in persistapi package // requires a bump of CurPersistVersion or not, do it for peace! // --@WeiZhang555 - CurPersistVersion uint = 1 + CurPersistVersion uint = 2 ) diff --git a/virtcontainers/qemu.go b/virtcontainers/qemu.go index 45e8b19275..727ab70fb7 100644 --- a/virtcontainers/qemu.go +++ b/virtcontainers/qemu.go @@ -67,6 +67,7 @@ type QemuState struct { HotpluggedMemory int UUID string HotplugVFIOOnRootBus bool + VirtiofsdPid int } // qemu is an Hypervisor interface implementation for the Linux qemu hypervisor. @@ -459,14 +460,14 @@ func (q *qemu) setupFileBackedMem(knobs *govmmQemu.Knobs, memory *govmmQemu.Memo } // createSandbox is the Hypervisor sandbox creation implementation for govmmQemu. -func (q *qemu) createSandbox(ctx context.Context, id string, networkNS NetworkNamespace, hypervisorConfig *HypervisorConfig, store *store.VCStore) error { +func (q *qemu) createSandbox(ctx context.Context, id string, networkNS NetworkNamespace, hypervisorConfig *HypervisorConfig, vcStore *store.VCStore) error { // Save the tracing context q.ctx = ctx span, _ := q.trace("createSandbox") defer span.Finish() - if err := q.setup(id, hypervisorConfig, store); err != nil { + if err := q.setup(id, hypervisorConfig, vcStore); err != nil { return err } @@ -558,8 +559,6 @@ func (q *qemu) createSandbox(ctx context.Context, id string, networkNS NetworkNa return err } - pidFile := q.pidFile() - qemuConfig := govmmQemu.Config{ Name: fmt.Sprintf("sandbox-%s", q.id), UUID: q.state.UUID, @@ -578,7 +577,7 @@ func (q *qemu) createSandbox(ctx context.Context, id string, networkNS NetworkNa VGA: "none", GlobalParam: "kvm-pit.lost_tick_policy=discard", Bios: firmwarePath, - PidFile: pidFile, + PidFile: filepath.Join(store.RunVMStoragePath, q.id, "pid"), } if ioThread != nil { @@ -636,6 +635,8 @@ func (q *qemu) setupVirtiofsd(timeout int) (remain int, err error) { defer func() { if err != nil { cmd.Process.Kill() + } else { + q.state.VirtiofsdPid = cmd.Process.Pid } }() @@ -743,6 +744,9 @@ func (q *qemu) startSandbox(timeout int) error { if err != nil { return err } + if err = q.storeState(); err != nil { + return err + } } var strErr string @@ -1910,24 +1914,26 @@ func (q *qemu) cleanup() error { return nil } -func (q *qemu) pidFile() string { - return filepath.Join(store.RunVMStoragePath, q.id, "pid") -} - -func (q *qemu) pid() int { - data, err := ioutil.ReadFile(q.pidFile()) +func (q *qemu) getPids() []int { + data, err := ioutil.ReadFile(q.qemuConfig.PidFile) if err != nil { q.Logger().WithError(err).Error("Could not read qemu pid file") - return 0 + return []int{0} } pid, err := strconv.Atoi(strings.Trim(string(data), "\n\t ")) if err != nil { q.Logger().WithError(err).Error("Could not convert string to int") - return 0 + return []int{0} } - return pid + var pids []int + pids = append(pids, pid) + if q.state.VirtiofsdPid != 0 { + pids = append(pids, q.state.VirtiofsdPid) + } + + return pids } type qemuGrpc struct { @@ -1992,7 +1998,11 @@ func (q *qemu) storeState() error { } func (q *qemu) save() (s persistapi.HypervisorState) { - s.Pid = q.pid() + pids := q.getPids() + if len(pids) != 0 { + s.Pid = pids[0] + } + s.VirtiofsdPid = q.state.VirtiofsdPid s.Type = string(QemuHypervisor) s.UUID = q.state.UUID s.HotpluggedMemory = q.state.HotpluggedMemory @@ -2019,6 +2029,7 @@ func (q *qemu) load(s persistapi.HypervisorState) { q.state.UUID = s.UUID q.state.HotpluggedMemory = s.HotpluggedMemory q.state.HotplugVFIOOnRootBus = s.HotplugVFIOOnRootBus + q.state.VirtiofsdPid = s.VirtiofsdPid for _, bridge := range s.Bridges { q.state.Bridges = append(q.state.Bridges, types.PCIBridge{ diff --git a/virtcontainers/qemu_test.go b/virtcontainers/qemu_test.go index 18680af718..e95f5d044a 100644 --- a/virtcontainers/qemu_test.go +++ b/virtcontainers/qemu_test.go @@ -568,3 +568,40 @@ func TestQemuWaitVirtiofsd(t *testing.T) { assert.NotNil(err) assert.True(remain == 0) } + +func TestQemuGetpids(t *testing.T) { + assert := assert.New(t) + + qemuConfig := newQemuConfig() + q := &qemu{} + pids := q.getPids() + assert.NotNil(pids) + assert.True(len(pids) == 1) + assert.True(pids[0] == 0) + + q = &qemu{ + config: qemuConfig, + } + f, err := ioutil.TempFile("", "qemu-test-") + assert.Nil(err) + tmpfile := f.Name() + f.Close() + defer os.Remove(tmpfile) + + q.qemuConfig.PidFile = tmpfile + pids = q.getPids() + assert.True(len(pids) == 1) + assert.True(pids[0] == 0) + + err = ioutil.WriteFile(tmpfile, []byte("100"), 0) + assert.Nil(err) + pids = q.getPids() + assert.True(len(pids) == 1) + assert.True(pids[0] == 100) + + q.state.VirtiofsdPid = 200 + pids = q.getPids() + assert.True(len(pids) == 2) + assert.True(pids[0] == 100) + assert.True(pids[1] == 200) +} diff --git a/virtcontainers/vm.go b/virtcontainers/vm.go index ad7e3791ab..af24cca72d 100644 --- a/virtcontainers/vm.go +++ b/virtcontainers/vm.go @@ -475,7 +475,7 @@ func (v *VM) ToGrpc(config VMConfig) (*pb.GrpcVM, error) { func (v *VM) GetVMStatus() *pb.GrpcVMStatus { return &pb.GrpcVMStatus{ - Pid: int64(v.hypervisor.pid()), + Pid: int64(getHypervisorPid(v.hypervisor)), Cpu: v.cpu, Memory: v.memory, }