vm: make Instance implement io.Closer

It's better to follow standard interfaces.
xairy · Jul 11, 2024 · bdca406 · bdca406
1 parent a893841
commit bdca406
Show file tree

Hide file tree

Showing 14 changed files with 36 additions and 19 deletions.
diff --git a/vm/adb/adb.go b/vm/adb/adb.go
@@ -487,8 +487,9 @@ func (inst *instance) getBatteryLevel(numRetry int) (int, error) {
 	return val, nil
 }
 
-func (inst *instance) Close() {
+func (inst *instance) Close() error {
 	close(inst.closed)
+	return nil
 }
 
 func (inst *instance) Copy(hostSrc string) (string, error) {

diff --git a/vm/bhyve/bhyve.go b/vm/bhyve/bhyve.go
@@ -276,7 +276,7 @@ func (inst *instance) Boot() error {
 	return nil
 }
 
-func (inst *instance) Close() {
+func (inst *instance) Close() error {
 	if inst.consolew != nil {
 		inst.consolew.Close()
 	}
@@ -294,6 +294,7 @@ func (inst *instance) Close() {
 		osutil.RunCmd(time.Minute, "", "ifconfig", inst.tapdev, "destroy")
 		inst.tapdev = ""
 	}
+	return nil
 }
 
 func (inst *instance) Forward(port int) (string, error) {

diff --git a/vm/cuttlefish/cuttlefish.go b/vm/cuttlefish/cuttlefish.go
@@ -160,8 +160,8 @@ func (inst *instance) Forward(port int) (string, error) {
 	return "", fmt.Errorf("unable to forward port on device: %w", err)
 }
 
-func (inst *instance) Close() {
-	inst.gceInst.Close()
+func (inst *instance) Close() error {
+	return inst.gceInst.Close()
 }
 
 func (inst *instance) Run(timeout time.Duration, stop <-chan bool, command string) (

diff --git a/vm/gce/gce.go b/vm/gce/gce.go
@@ -241,12 +241,16 @@ func (pool *Pool) Create(workdir string, index int) (vmimpl.Instance, error) {
 	return inst, nil
 }
 
-func (inst *instance) Close() {
+func (inst *instance) Close() error {
 	close(inst.closed)
-	inst.GCE.DeleteInstance(inst.name, false)
+	err := inst.GCE.DeleteInstance(inst.name, false)
 	if inst.consolew != nil {
-		inst.consolew.Close()
+		err2 := inst.consolew.Close()
+		if err == nil {
+			err = err2
+		}
 	}
+	return err
 }
 
 func (inst *instance) Forward(port int) (string, error) {

diff --git a/vm/gvisor/gvisor.go b/vm/gvisor/gvisor.go
@@ -257,14 +257,15 @@ func (inst *instance) runscCmd(add ...string) *exec.Cmd {
 	return cmd
 }
 
-func (inst *instance) Close() {
+func (inst *instance) Close() error {
 	time.Sleep(3 * time.Second)
 	osutil.Run(time.Minute, inst.runscCmd("delete", "-force", inst.name))
 	inst.cmd.Process.Kill()
 	inst.merger.Wait()
 	inst.cmd.Wait()
 	osutil.Run(time.Minute, inst.runscCmd("delete", "-force", inst.name))
 	time.Sleep(3 * time.Second)
+	return nil
 }
 
 func (inst *instance) Forward(port int) (string, error) {

diff --git a/vm/isolated/isolated.go b/vm/isolated/isolated.go
@@ -282,8 +282,9 @@ func (inst *instance) waitForReboot(timeout int) error {
 	return fmt.Errorf("isolated: the machine did not reboot on repair")
 }
 
-func (inst *instance) Close() {
+func (inst *instance) Close() error {
 	close(inst.closed)
+	return nil
 }
 
 func (inst *instance) Copy(hostSrc string) (string, error) {

diff --git a/vm/proxyapp/proxyappclient.go b/vm/proxyapp/proxyappclient.go
@@ -578,7 +578,7 @@ func (inst *instance) Diagnose(r *report.Report) (diagnosis []byte, wait bool) {
 	return []byte(reply.Diagnosis), false
 }
 
-func (inst *instance) Close() {
+func (inst *instance) Close() error {
 	var reply proxyrpc.CloseReply
 	err := inst.ProxyApp.Call(
 		"ProxyVM.Close",
@@ -589,6 +589,7 @@ func (inst *instance) Close() {
 	if err != nil {
 		log.Logf(0, "error closing instance %v: %v", inst.ID, err)
 	}
+	return err
 }
 
 type stdInOutCloser struct {

diff --git a/vm/qemu/qemu.go b/vm/qemu/qemu.go
@@ -395,7 +395,7 @@ func (pool *Pool) ctor(workdir, sshkey, sshuser string, index int) (vmimpl.Insta
 	return inst, nil
 }
 
-func (inst *instance) Close() {
+func (inst *instance) Close() error {
 	if inst.qemu != nil {
 		inst.qemu.Process.Kill()
 		inst.qemu.Wait()
@@ -412,6 +412,7 @@ func (inst *instance) Close() {
 	if inst.mon != nil {
 		inst.mon.Close()
 	}
+	return nil
 }
 
 func (inst *instance) boot() error {

diff --git a/vm/starnix/starnix.go b/vm/starnix/starnix.go
@@ -165,7 +165,7 @@ func (inst *instance) boot() error {
 	return nil
 }
 
-func (inst *instance) Close() {
+func (inst *instance) Close() error {
 	inst.ffx("emu", "stop", inst.name)
 	if inst.fuchsiaLogs != nil {
 		inst.fuchsiaLogs.Process.Kill()
@@ -180,6 +180,7 @@ func (inst *instance) Close() {
 	if inst.wpipe != nil {
 		inst.wpipe.Close()
 	}
+	return nil
 }
 
 func (inst *instance) startFuchsiaVM() error {

diff --git a/vm/vm.go b/vm/vm.go
@@ -264,10 +264,13 @@ func (inst *Instance) Index() int {
 	return inst.index
 }
 
-func (inst *Instance) Close() {
-	inst.impl.Close()
-	os.RemoveAll(inst.workdir)
+func (inst *Instance) Close() error {
+	err := inst.impl.Close()
+	if retErr := os.RemoveAll(inst.workdir); err == nil {
+		err = retErr
+	}
 	inst.onClose()
+	return err
 }
 
 type monitor struct {

diff --git a/vm/vm_test.go b/vm/vm_test.go
@@ -69,7 +69,8 @@ func (inst *testInstance) Diagnose(rep *report.Report) ([]byte, bool) {
 	return nil, true
 }
 
-func (inst *testInstance) Close() {
+func (inst *testInstance) Close() error {
+	return nil
 }
 
 func init() {

diff --git a/vm/vmimpl/vmimpl.go b/vm/vmimpl/vmimpl.go
@@ -58,7 +58,7 @@ type Instance interface {
 	Diagnose(rep *report.Report) (diagnosis []byte, wait bool)
 
 	// Close stops and destroys the VM.
-	Close()
+	io.Closer
 }
 
 // Infoer is an optional interface that can be implemented by Instance.

diff --git a/vm/vmm/vmm.go b/vm/vmm/vmm.go
@@ -216,7 +216,7 @@ func (inst *instance) lookupSSHAddress() (string, error) {
 	return fmt.Sprintf("100.64.%s.3", matches[1]), nil
 }
 
-func (inst *instance) Close() {
+func (inst *instance) Close() error {
 	inst.vmctl("stop", "-f", inst.vmName)
 	if inst.consolew != nil {
 		inst.consolew.Close()
@@ -226,6 +226,7 @@ func (inst *instance) Close() {
 		inst.vmm.Wait()
 	}
 	inst.merger.Wait()
+	return nil
 }
 
 func (inst *instance) Forward(port int) (string, error) {

diff --git a/vm/vmware/vmware.go b/vm/vmware/vmware.go
@@ -144,7 +144,7 @@ func (inst *instance) Forward(port int) (string, error) {
 	return fmt.Sprintf("127.0.0.1:%v", port), nil
 }
 
-func (inst *instance) Close() {
+func (inst *instance) Close() error {
 	if inst.debug {
 		log.Logf(0, "stopping %v", inst.vmx)
 	}
@@ -154,6 +154,7 @@ func (inst *instance) Close() {
 	}
 	osutil.RunCmd(2*time.Minute, "", "vmrun", "deleteVM", inst.vmx)
 	close(inst.closed)
+	return nil
 }
 
 func (inst *instance) Copy(hostSrc string) (string, error) {