From bdca406b2cf89f029207aa8213460ec7a3267ccc Mon Sep 17 00:00:00 2001
From: Aleksandr Nogikh <nogikh@google.com>
Date: Mon, 8 Jul 2024 12:45:31 +0200
Subject: vm: make Instance implement io.Closer

It's better to follow standard interfaces.
---
 vm/adb/adb.go                 |  3 ++-
 vm/bhyve/bhyve.go             |  3 ++-
 vm/cuttlefish/cuttlefish.go   |  4 ++--
 vm/gce/gce.go                 | 10 +++++++---
 vm/gvisor/gvisor.go           |  3 ++-
 vm/isolated/isolated.go       |  3 ++-
 vm/proxyapp/proxyappclient.go |  3 ++-
 vm/qemu/qemu.go               |  3 ++-
 vm/starnix/starnix.go         |  3 ++-
 vm/vm.go                      |  9 ++++++---
 vm/vm_test.go                 |  3 ++-
 vm/vmimpl/vmimpl.go           |  2 +-
 vm/vmm/vmm.go                 |  3 ++-
 vm/vmware/vmware.go           |  3 ++-
 14 files changed, 36 insertions(+), 19 deletions(-)

diff --git a/vm/adb/adb.go b/vm/adb/adb.go
index ad0347342..2fd401084 100644
--- 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
index e645ca191..57515ba22 100644
--- 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
index 94610ed4e..73014cf4e 100644
--- 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
index 12f49e3d7..8de0b56db 100644
--- 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
index 44e3db575..ca3a8472c 100644
--- a/vm/gvisor/gvisor.go
+++ b/vm/gvisor/gvisor.go
@@ -257,7 +257,7 @@ 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()
@@ -265,6 +265,7 @@ func (inst *instance) Close() {
 	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
index 1201a6ac8..99e7ad78a 100755
--- 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
index 31a19fdec..72faf5655 100644
--- 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
index e12d8d7c6..4d9859c0c 100644
--- 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
index 7d52c6c61..65c921ee3 100644
--- 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
index e1dd75ab8..efc306e5e 100644
--- 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
index 357f037d5..278a4b63d 100644
--- 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
index e29412d75..fa2719f5f 100644
--- 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
index 71dfd39b4..6a6242882 100644
--- 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
index f2b3431c0..210a4957a 100644
--- 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) {
-- 
cgit mrf-deployment