Skip to content

Commit

Permalink
Removed unneeded return of domain spec
Browse files Browse the repository at this point in the history 
Signed-off-by: Marc Sluiter <[email protected]>
  • Loading branch information
@slintes
slintes committed Nov 5, 2019
1 parent 49b19c5 commit 311131c
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 80 deletions.
4 changes: 2 additions & 2 deletions pkg/virt-launcher/virtwrap/cmd-server/server.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ func (l *Launcher) PauseVirtualMachine(ctx context.Context, request *cmdv1.VMIRe
return response, nil
}

if _, err := l.domainManager.PauseVMI(vmi); err != nil {
if err := l.domainManager.PauseVMI(vmi); err != nil {
log.Log.Object(vmi).Reason(err).Errorf("Failed to pause vmi")
response.Success = false
response.Message = getErrorMessage(err)
Expand All @@ -191,7 +191,7 @@ func (l *Launcher) UnpauseVirtualMachine(ctx context.Context, request *cmdv1.VMI
return response, nil
}

if _, err := l.domainManager.UnpauseVMI(vmi); err != nil {
if err := l.domainManager.UnpauseVMI(vmi); err != nil {
log.Log.Object(vmi).Reason(err).Errorf("Failed to unpause vmi")
response.Success = false
response.Message = getErrorMessage(err)
Expand Down
14 changes: 6 additions & 8 deletions pkg/virt-launcher/virtwrap/generated_mock_manager.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,22 +45,20 @@ func (_mr *_MockDomainManagerRecorder) SyncVMI(arg0, arg1, arg2 interface{}) *go
return _mr.mock.ctrl.RecordCall(_mr.mock, "SyncVMI", arg0, arg1, arg2)
}

func (_m *MockDomainManager) PauseVMI(_param0 *v1.VirtualMachineInstance) (*api.DomainSpec, error) {
func (_m *MockDomainManager) PauseVMI(_param0 *v1.VirtualMachineInstance) error {
ret := _m.ctrl.Call(_m, "PauseVMI", _param0)
ret0, _ := ret[0].(*api.DomainSpec)
ret1, _ := ret[1].(error)
return ret0, ret1
ret0, _ := ret[0].(error)
return ret0
}

func (_mr *_MockDomainManagerRecorder) PauseVMI(arg0 interface{}) *gomock.Call {
return _mr.mock.ctrl.RecordCall(_mr.mock, "PauseVMI", arg0)
}

func (_m *MockDomainManager) UnpauseVMI(_param0 *v1.VirtualMachineInstance) (*api.DomainSpec, error) {
func (_m *MockDomainManager) UnpauseVMI(_param0 *v1.VirtualMachineInstance) error {
ret := _m.ctrl.Call(_m, "UnpauseVMI", _param0)
ret0, _ := ret[0].(*api.DomainSpec)
ret1, _ := ret[1].(error)
return ret0, ret1
ret0, _ := ret[0].(error)
return ret0
}

func (_mr *_MockDomainManagerRecorder) UnpauseVMI(arg0 interface{}) *gomock.Call {
Expand Down
68 changes: 24 additions & 44 deletions pkg/virt-launcher/virtwrap/manager.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,8 @@ const vgpuEnvPrefix = "VGPU_PASSTHROUGH_DEVICES"

type DomainManager interface {
SyncVMI(*v1.VirtualMachineInstance, bool, *cmdv1.VirtualMachineOptions) (*api.DomainSpec, error)
PauseVMI(*v1.VirtualMachineInstance) (*api.DomainSpec, error)
UnpauseVMI(*v1.VirtualMachineInstance) (*api.DomainSpec, error)
PauseVMI(*v1.VirtualMachineInstance) error
UnpauseVMI(*v1.VirtualMachineInstance) error
KillVMI(*v1.VirtualMachineInstance) error
DeleteVMI(*v1.VirtualMachineInstance) error
SignalShutdownVMI(*v1.VirtualMachineInstance) error
Expand Down Expand Up @@ -1115,7 +1115,7 @@ func (l *LibvirtDomainManager) getDomainSpec(dom cli.VirDomain) (*api.DomainSpec
return util.GetDomainSpec(state, dom)
}

func (l *LibvirtDomainManager) PauseVMI(vmi *v1.VirtualMachineInstance) (*api.DomainSpec, error) {
func (l *LibvirtDomainManager) PauseVMI(vmi *v1.VirtualMachineInstance) error {
l.domainModifyLock.Lock()
defer l.domainModifyLock.Unlock()

Expand All @@ -1126,46 +1126,36 @@ func (l *LibvirtDomainManager) PauseVMI(vmi *v1.VirtualMachineInstance) (*api.Do
if err != nil {
// If the VirtualMachineInstance does not exist, we are done
if domainerrors.IsNotFound(err) {
return nil, fmt.Errorf("Domain not found.")
return fmt.Errorf("Domain not found.")
} else {
log.Log.Object(vmi).Reason(err).Error("Getting the domain failed during pause.")
return nil, err
logger.Reason(err).Error("Getting the domain failed during pause.")
return err
}
}
defer dom.Free()

domState, _, err := dom.GetState()
if err != nil {
log.Log.Object(vmi).Reason(err).Error("Getting the domain state failed.")
return nil, err
logger.Reason(err).Error("Getting the domain state failed.")
return err
}

if domState == libvirt.DOMAIN_RUNNING {
err = dom.Suspend()
if err != nil {
log.Log.Object(vmi).Reason(err).Error("Signalling suspension failed.")
return nil, err
logger.Reason(err).Error("Signalling suspension failed.")
return err
}
log.Log.Object(vmi).Infof("Signaled pause for %s", vmi.GetObjectMeta().GetName())
logger.Infof("Signaled pause for %s", vmi.GetObjectMeta().GetName())
l.paused.add(vmi.UID)
} else {
log.Log.Object(vmi).Infof("Domain is not running for %s", vmi.GetObjectMeta().GetName())
logger.Infof("Domain is not running for %s", vmi.GetObjectMeta().GetName())
}

xmlstr, err := dom.GetXMLDesc(0)
if err != nil {
return nil, err
}
var newSpec api.DomainSpec
err = xml.Unmarshal([]byte(xmlstr), &newSpec)
if err != nil {
logger.Reason(err).Error("Parsing domain XML failed.")
return nil, err
}
return &newSpec, nil
return nil
}

func (l *LibvirtDomainManager) UnpauseVMI(vmi *v1.VirtualMachineInstance) (*api.DomainSpec, error) {
func (l *LibvirtDomainManager) UnpauseVMI(vmi *v1.VirtualMachineInstance) error {
l.domainModifyLock.Lock()
defer l.domainModifyLock.Unlock()

Expand All @@ -1176,44 +1166,34 @@ func (l *LibvirtDomainManager) UnpauseVMI(vmi *v1.VirtualMachineInstance) (*api.
if err != nil {
// If the VirtualMachineInstance does not exist, we are done
if domainerrors.IsNotFound(err) {
return nil, fmt.Errorf("Domain not found.")
return fmt.Errorf("Domain not found.")
} else {
log.Log.Object(vmi).Reason(err).Error("Getting the domain failed during unpause.")
return nil, err
logger.Reason(err).Error("Getting the domain failed during unpause.")
return err
}
}
defer dom.Free()

domState, _, err := dom.GetState()
if err != nil {
log.Log.Object(vmi).Reason(err).Error("Getting the domain state failed.")
return nil, err
logger.Reason(err).Error("Getting the domain state failed.")
return err
}

if domState == libvirt.DOMAIN_PAUSED {
err = dom.Resume()
if err != nil {
log.Log.Object(vmi).Reason(err).Error("Signalling unpause failed.")
return nil, err
logger.Reason(err).Error("Signalling unpause failed.")
return err
}
log.Log.Object(vmi).Infof("Signaled unpause for %s", vmi.GetObjectMeta().GetName())
logger.Infof("Signaled unpause for %s", vmi.GetObjectMeta().GetName())
l.paused.remove(vmi.UID)

} else {
log.Log.Object(vmi).Infof("Domain is not paused for %s", vmi.GetObjectMeta().GetName())
logger.Infof("Domain is not paused for %s", vmi.GetObjectMeta().GetName())
}

xmlstr, err := dom.GetXMLDesc(0)
if err != nil {
return nil, err
}
var newSpec api.DomainSpec
err = xml.Unmarshal([]byte(xmlstr), &newSpec)
if err != nil {
logger.Reason(err).Error("Parsing domain XML failed.")
return nil, err
}
return &newSpec, nil
return nil
}

func (l *LibvirtDomainManager) SignalShutdownVMI(vmi *v1.VirtualMachineInstance) error {
Expand Down
34 changes: 8 additions & 26 deletions pkg/virt-launcher/virtwrap/manager_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ var _ = Describe("Manager", func() {
table.Entry("shutoff", libvirt.DOMAIN_SHUTOFF),
table.Entry("unknown", libvirt.DOMAIN_NOSTATE),
)
It("should unpause a paused VirtualMachineInstance", func() {
It("should unpause a paused VirtualMachineInstance on SyncVMI, which was not paused by user", func() {
// Make sure that we always free the domain after use
mockDomain.EXPECT().Free()
vmi := newVMI(testNamespace, testVmName)
Expand All @@ -214,7 +214,7 @@ var _ = Describe("Manager", func() {
Expect(err).To(BeNil())
Expect(newspec).ToNot(BeNil())
})
It("should not unpause a paused VirtualMachineInstance", func() {
It("should not unpause a paused VirtualMachineInstance on SyncVMI, which was paused by user", func() {
// Make sure that we always free the domain after use
mockDomain.EXPECT().Free()
vmi := newVMI(testNamespace, testVmName)
Expand All @@ -224,90 +224,72 @@ var _ = Describe("Manager", func() {
mockConn.EXPECT().LookupDomainByName(testDomainName).Return(mockDomain, nil)
mockDomain.EXPECT().GetState().Return(libvirt.DOMAIN_RUNNING, 1, nil)
mockDomain.EXPECT().Suspend().Return(nil)
mockDomain.EXPECT().GetXMLDesc(libvirt.DomainXMLFlags(0)).Return(string(xml), nil)
manager, _ := NewLibvirtDomainManager(mockConn, "fake", nil, 0)

newspec, err := manager.PauseVMI(vmi)
err = manager.PauseVMI(vmi)
Expect(err).To(BeNil())
Expect(newspec).ToNot(BeNil())

mockDomain.EXPECT().Free()
mockConn.EXPECT().LookupDomainByName(testDomainName).Return(mockDomain, nil)
mockDomain.EXPECT().GetState().Return(libvirt.DOMAIN_PAUSED, 1, nil)
mockDomain.EXPECT().GetXMLDesc(libvirt.DomainXMLFlags(0)).Return(string(xml), nil)
// no expected call to unpause

newspec, err = manager.SyncVMI(vmi, true, &cmdv1.VirtualMachineOptions{VirtualMachineSMBios: &cmdv1.SMBios{}})
newspec, err := manager.SyncVMI(vmi, true, &cmdv1.VirtualMachineOptions{VirtualMachineSMBios: &cmdv1.SMBios{}})
Expect(err).To(BeNil())
Expect(newspec).ToNot(BeNil())
})
It("should pause a VirtualMachineInstance", func() {
// Make sure that we always free the domain after use
mockDomain.EXPECT().Free()
vmi := newVMI(testNamespace, testVmName)
domainSpec := expectIsolationDetectionForVMI(vmi)
xml, err := xml.Marshal(domainSpec)

mockConn.EXPECT().LookupDomainByName(testDomainName).Return(mockDomain, nil)
mockDomain.EXPECT().GetState().Return(libvirt.DOMAIN_RUNNING, 1, nil)
mockDomain.EXPECT().Suspend().Return(nil)
mockDomain.EXPECT().GetXMLDesc(libvirt.DomainXMLFlags(0)).Return(string(xml), nil)
manager, _ := NewLibvirtDomainManager(mockConn, "fake", nil, 0)

newspec, err := manager.PauseVMI(vmi)
err = manager.PauseVMI(vmi)
Expect(err).To(BeNil())
Expect(newspec).ToNot(BeNil())
})
It("should not try to pause a paused VirtualMachineInstance", func() {
// Make sure that we always free the domain after use
mockDomain.EXPECT().Free()
vmi := newVMI(testNamespace, testVmName)
domainSpec := expectIsolationDetectionForVMI(vmi)
xml, err := xml.Marshal(domainSpec)

mockConn.EXPECT().LookupDomainByName(testDomainName).Return(mockDomain, nil)
mockDomain.EXPECT().GetState().Return(libvirt.DOMAIN_PAUSED, 1, nil)
mockDomain.EXPECT().GetXMLDesc(libvirt.DomainXMLFlags(0)).Return(string(xml), nil)
manager, _ := NewLibvirtDomainManager(mockConn, "fake", nil, 0)
// no call to suspend

newspec, err := manager.PauseVMI(vmi)
err = manager.PauseVMI(vmi)
Expect(err).To(BeNil())
Expect(newspec).ToNot(BeNil())
})
It("should unpause a VirtualMachineInstance", func() {
// Make sure that we always free the domain after use
mockDomain.EXPECT().Free()
vmi := newVMI(testNamespace, testVmName)
domainSpec := expectIsolationDetectionForVMI(vmi)
xml, err := xml.Marshal(domainSpec)

mockConn.EXPECT().LookupDomainByName(testDomainName).Return(mockDomain, nil)
mockDomain.EXPECT().GetState().Return(libvirt.DOMAIN_PAUSED, 1, nil)
mockDomain.EXPECT().Resume().Return(nil)
mockDomain.EXPECT().GetXMLDesc(libvirt.DomainXMLFlags(0)).Return(string(xml), nil)
manager, _ := NewLibvirtDomainManager(mockConn, "fake", nil, 0)

newspec, err := manager.UnpauseVMI(vmi)
err = manager.UnpauseVMI(vmi)
Expect(err).To(BeNil())
Expect(newspec).ToNot(BeNil())
})
It("should not try to unpause a running VirtualMachineInstance", func() {
// Make sure that we always free the domain after use
mockDomain.EXPECT().Free()
vmi := newVMI(testNamespace, testVmName)
domainSpec := expectIsolationDetectionForVMI(vmi)
xml, err := xml.Marshal(domainSpec)

mockConn.EXPECT().LookupDomainByName(testDomainName).Return(mockDomain, nil)
mockDomain.EXPECT().GetState().Return(libvirt.DOMAIN_RUNNING, 1, nil)
mockDomain.EXPECT().GetXMLDesc(libvirt.DomainXMLFlags(0)).Return(string(xml), nil)
manager, _ := NewLibvirtDomainManager(mockConn, "fake", nil, 0)
// no call to unpause

newspec, err := manager.UnpauseVMI(vmi)
err = manager.UnpauseVMI(vmi)
Expect(err).To(BeNil())
Expect(newspec).ToNot(BeNil())
})
})
Context("test migration monitor", func() {
Expand Down

0 comments on commit 311131c

Please sign in to comment.