fix(hostman): avoid panic when disk is nil

pkg/apis/compute/snapshot_const.go

@@ -19,12 +19,13 @@ const (
 	SNAPSHOT_MANUAL = "manual"
 	SNAPSHOT_AUTO   = "auto"
 
-	SNAPSHOT_CREATING    = "creating"
-	SNAPSHOT_ROLLBACKING = "rollbacking"
-	SNAPSHOT_FAILED      = "create_failed"
-	SNAPSHOT_READY       = "ready"
-	SNAPSHOT_DELETING    = "deleting"
-	SNAPSHOT_UNKNOWN     = "unknown"
+	SNAPSHOT_CREATING      = "creating"
+	SNAPSHOT_ROLLBACKING   = "rollbacking"
+	SNAPSHOT_FAILED        = "create_failed"
+	SNAPSHOT_READY         = "ready"
+	SNAPSHOT_DELETE_FAILED = "delete_failed"
+	SNAPSHOT_DELETING      = "deleting"
+	SNAPSHOT_UNKNOWN       = "unknown"
 
 	SNAPSHOT_POLICY_CREATING = "creating"
 

pkg/compute/storagedrivers/rbd.go

@@ -220,7 +220,7 @@ func (self *SRbdStorageDriver) RequestDeleteSnapshot(ctx context.Context, snapsh
 	params.Set("snapshot_id", jsonutils.NewString(snapshot.Id))
 	_, _, err := httputils.JSONRequest(httputils.GetDefaultClient(), ctx, "POST", url, header, params, false)
 	if err != nil {
-		return errors.Wrap(err, "request create snapshot")
+		return errors.Wrap(err, "request delete snapshot")
 	}
 	return nil
 }

pkg/compute/tasks/snapshot_delete_task.go

@@ -19,10 +19,12 @@ import (
 
 	"yunion.io/x/jsonutils"
 	"yunion.io/x/log"
+	"yunion.io/x/pkg/errors"
 
 	api "yunion.io/x/onecloud/pkg/apis/compute"
 	"yunion.io/x/onecloud/pkg/cloudcommon/db"
 	"yunion.io/x/onecloud/pkg/cloudcommon/db/taskman"
+	"yunion.io/x/onecloud/pkg/cloudprovider"
 	"yunion.io/x/onecloud/pkg/compute/models"
 	"yunion.io/x/onecloud/pkg/util/logclient"
 )
@@ -72,7 +74,12 @@ func (self *SnapshotDeleteTask) OnInit(ctx context.Context, obj db.IStandaloneMo
 	regionDriver := snapshot.GetRegionDriver()
 
 	self.SetStage("OnRequestSnapshot", nil)
-	if err := regionDriver.RequestDeleteSnapshot(ctx, snapshot, self); err != nil {
+	err := regionDriver.RequestDeleteSnapshot(ctx, snapshot, self)
+	if err != nil {
+		if errors.Cause(err) == cloudprovider.ErrNotFound {
+			self.ScheduleRun(jsonutils.Marshal(map[string]bool{"deleted": true}))
+			return
+		}
 		self.TaskFailed(ctx, snapshot, jsonutils.NewString(err.Error()))
 	}
 }
@@ -146,9 +153,7 @@ func (self *SnapshotDeleteTask) TaskComplete(ctx context.Context, snapshot *mode
 }
 
 func (self *SnapshotDeleteTask) TaskFailed(ctx context.Context, snapshot *models.SSnapshot, reason jsonutils.JSONObject) {
-	if snapshot.Status == api.SNAPSHOT_DELETING {
-		snapshot.SetStatus(self.UserCred, api.SNAPSHOT_READY, "On SnapshotDeleteTask TaskFailed")
-	}
+	snapshot.SetStatus(self.UserCred, api.SNAPSHOT_DELETE_FAILED, reason.String())
 	db.OpsLog.LogEvent(snapshot, db.ACT_SNAPSHOT_DELETE_FAIL, reason, self.UserCred)
 	logclient.AddActionLogWithStartable(self, snapshot, logclient.ACT_DELOCATE, reason, self.UserCred, false)
 	self.SetStageFailed(ctx, reason)

pkg/esxi/handler/handlers.go

@@ -21,6 +21,7 @@ import (
 
 	"yunion.io/x/jsonutils"
 	"yunion.io/x/log"
+	"yunion.io/x/pkg/errors"
 
 	"yunion.io/x/onecloud/pkg/appctx"
 	"yunion.io/x/onecloud/pkg/appsrv"
@@ -94,7 +95,11 @@ func deployHandler(ctx context.Context, w http.ResponseWriter, r *http.Request)
 func deleteHandler(ctx context.Context, w http.ResponseWriter, r *http.Request) {
 	params, _, _ := appsrv.FetchEnv(ctx, w, r)
 	diskId := params["<disk_id>"]
-	disk := esxi.EsxiAgent.AgentStorage.GetDiskById(diskId)
+	disk, err := esxi.EsxiAgent.AgentStorage.GetDiskById(diskId)
+	if err != nil {
+		httperrors.GeneralServerError(ctx, w, errors.Wrapf(err, "GetDiskById(%s)", diskId))
+		return
+	}
 	if taskId := ctx.Value(appctx.APP_CONTEXT_KEY_TASK_ID); taskId == nil {
 		if disk != nil {
 			_, err := disk.Delete(ctx, nil)
@@ -171,9 +176,9 @@ func fetchHandler(ctx context.Context, w http.ResponseWriter, r *http.Request) {
 func diskAndDiskInfo(ctx context.Context, w http.ResponseWriter, r *http.Request) (storageman.IDisk, jsonutils.JSONObject, error) {
 	params, _, body := appsrv.FetchEnv(ctx, w, r)
 	diskId := params["<disk_id>"]
-	disk := esxi.EsxiAgent.AgentStorage.GetDiskById(diskId)
-	if disk == nil {
-		return nil, nil, httperrors.NewNotFoundError("disk '%s'", diskId)
+	disk, err := esxi.EsxiAgent.AgentStorage.GetDiskById(diskId)
+	if err != nil {
+		return nil, nil, httperrors.NewGeneralError(errors.Wrapf(err, "GetDiskById(%s)", diskId))
 	}
 	diskInfo, err := body.Get("disk")
 	if err != nil {

pkg/hostman/downloader/downloadhandler.go

@@ -19,6 +19,8 @@ import (
 	"fmt"
 	"net/http"
 
+	"yunion.io/x/pkg/errors"
+
 	"yunion.io/x/onecloud/pkg/appsrv"
 	"yunion.io/x/onecloud/pkg/hostman/hostutils"
 	"yunion.io/x/onecloud/pkg/hostman/options"
@@ -117,9 +119,9 @@ func diskPrecheck(
 	if storage == nil {
 		return nil, httperrors.NewNotFoundError("Storage %s not found", storageId)
 	}
-	disk := storage.GetDiskById(diskId)
-	if disk == nil {
-		return nil, httperrors.NewNotFoundError("Disk %s not found", diskId)
+	disk, err := storage.GetDiskById(diskId)
+	if err != nil {
+		return nil, errors.Wrapf(err, "GetDiskById(%s)", diskId)
 	}
 	return disk, nil
 }

pkg/hostman/storageman/core.go

@@ -188,7 +188,8 @@ func (s *SStorageManager) GetStorage(storageId string) IStorage {
 
 func (s *SStorageManager) GetStorageDisk(storageId, diskId string) IDisk {
 	if storage := s.GetStorage(storageId); storage != nil {
-		return storage.GetDiskById(diskId)
+		disk, _ := storage.GetDiskById(diskId)
+		return disk
 	}
 	return nil
 }
@@ -212,7 +213,8 @@ func (s *SStorageManager) GetDiskByPath(diskPath string) IDisk {
 	}
 	storage := s.GetStorageByPath(sPath)
 	if storage != nil {
-		return storage.GetDiskById(diskId)
+		disk, _ := storage.GetDiskById(diskId)
+		return disk
 	}
 	return nil
 }

pkg/hostman/storageman/diskhandlers/diskhandler.go

@@ -21,11 +21,13 @@ import (
 
 	"yunion.io/x/jsonutils"
 	"yunion.io/x/log"
+	"yunion.io/x/pkg/errors"
 	"yunion.io/x/pkg/util/regutils"
 
 	"yunion.io/x/onecloud/pkg/apis/compute"
 	"yunion.io/x/onecloud/pkg/appsrv"
 	"yunion.io/x/onecloud/pkg/cloudcommon/workmanager"
+	"yunion.io/x/onecloud/pkg/cloudprovider"
 	"yunion.io/x/onecloud/pkg/hostman/guestman"
 	"yunion.io/x/onecloud/pkg/hostman/hostutils"
 	"yunion.io/x/onecloud/pkg/hostman/storageman"
@@ -140,8 +142,12 @@ func getDiskStatus(ctx context.Context, w http.ResponseWriter, r *http.Request)
 		return
 	}
 	ret := jsonutils.NewDict()
-	disk := storage.GetDiskById(diskId)
-	if disk == nil {
+	_, err := storage.GetDiskById(diskId)
+	if err != nil {
+		if errors.Cause(err) != cloudprovider.ErrNotFound {
+			hostutils.Response(ctx, w, httperrors.NewGeneralError(errors.Wrapf(err, "GetDiskById(%s)", diskId)))
+			return
+		}
 		ret.Set("status", jsonutils.NewString(compute.DISK_NOT_EXIST))
 	} else {
 		// Note: the statuses of disk on host are either exist or not exist
@@ -220,20 +226,28 @@ func perfomrDiskActions(ctx context.Context, w http.ResponseWriter, r *http.Requ
 		hostutils.Response(ctx, w, httperrors.NewNotFoundError("Storage %s not found", storageId))
 		return
 	}
-	disk := storage.GetDiskById(diskId)
 
-	if f, ok := actionFuncs[action]; !ok {
-		hostutils.Response(ctx, w, httperrors.NewNotFoundError("Not found"))
-	} else {
-		res, err := f(ctx, storage, diskId, disk, body)
-		if err != nil {
-			hostutils.Response(ctx, w, err)
-		} else if res != nil {
-			hostutils.Response(ctx, w, res)
-		} else {
-			hostutils.ResponseOk(ctx, w)
-		}
+	disk, err := storage.GetDiskById(diskId)
+	if err != nil {
+		hostutils.Response(ctx, w, httperrors.NewGeneralError(errors.Wrapf(err, "GetDiskById(%s)", diskId)))
+		return
 	}
+
+	f, ok := actionFuncs[action]
+	if !ok {
+		hostutils.Response(ctx, w, httperrors.NewNotFoundError("Action %s Not found", action))
+		return
+	}
+	res, err := f(ctx, storage, diskId, disk, body)
+	if err != nil {
+		hostutils.Response(ctx, w, err)
+		return
+	}
+	if res != nil {
+		hostutils.Response(ctx, w, res)
+		return
+	}
+	hostutils.ResponseOk(ctx, w)
 }
 
 func diskCreate(ctx context.Context, storage storageman.IStorage, diskId string, disk storageman.IDisk, body jsonutils.JSONObject) (interface{}, error) {

pkg/hostman/storageman/storage_agent.go

@@ -54,8 +54,8 @@ func NewAgentStorage(manager *SStorageManager, agent iagent.IAgent, path string)
 	return s
 }
 
-func (as *SAgentStorage) GetDiskById(diskId string) IDisk {
-	return NewAgentDisk(as, diskId)
+func (as *SAgentStorage) GetDiskById(diskId string) (IDisk, error) {
+	return NewAgentDisk(as, diskId), nil
 }
 
 func (as *SAgentStorage) CreateDiskByDiskInfo(ctx context.Context, params interface{}) (jsonutils.JSONObject, error) {
@@ -74,7 +74,10 @@ func (as *SAgentStorage) CreateDiskByDiskInfo(ctx context.Context, params interf
 	if err != nil {
 		return nil, errors.Wrap(err, "as.SLocalStorage.CreateDiskByDiskinfo")
 	}
-	disk := as.GetDiskById(createParams.DiskId)
+	disk, err := as.GetDiskById(createParams.DiskId)
+	if err != nil {
+		return nil, errors.Wrapf(err, "GetDiskById(%s)", createParams.DiskId)
+	}
 
 	_, ds, err := as.getHostAndDatastore(ctx, hd)
 	if err != nil {

pkg/hostman/storageman/storage_base.go

@@ -104,7 +104,7 @@ type IStorage interface {
 	GetCapacity() int
 
 	// Find owner disks first, if not found, call create disk
-	GetDiskById(diskId string) IDisk
+	GetDiskById(diskId string) (IDisk, error)
 	CreateDisk(diskId string) IDisk
 	RemoveDisk(IDisk)
 

pkg/hostman/storageman/storage_local.go

@@ -28,6 +28,7 @@ import (
 	"yunion.io/x/pkg/util/timeutils"
 
 	api "yunion.io/x/onecloud/pkg/apis/compute"
+	"yunion.io/x/onecloud/pkg/cloudprovider"
 	deployapi "yunion.io/x/onecloud/pkg/hostman/hostdeployer/apis"
 	"yunion.io/x/onecloud/pkg/hostman/hostdeployer/deployclient"
 	"yunion.io/x/onecloud/pkg/hostman/hostutils"
@@ -127,25 +128,20 @@ func (s *SLocalStorage) SyncStorageInfo() (jsonutils.JSONObject, error) {
 	return res, err
 }
 
-func (s *SLocalStorage) GetDiskById(diskId string) IDisk {
+func (s *SLocalStorage) GetDiskById(diskId string) (IDisk, error) {
 	s.DiskLock.Lock()
 	defer s.DiskLock.Unlock()
 	for i := 0; i < len(s.Disks); i++ {
 		if s.Disks[i].GetId() == diskId {
-			if s.Disks[i].Probe() == nil {
-				return s.Disks[i]
-			} else {
-				return nil
-			}
+			return s.Disks[i], s.Disks[i].Probe()
 		}
 	}
 	var disk = NewLocalDisk(s, diskId)
 	if disk.Probe() == nil {
 		s.Disks = append(s.Disks, disk)
-		return disk
-	} else {
-		return nil
+		return disk, nil
 	}
+	return nil, cloudprovider.ErrNotFound
 }
 
 func (s *SLocalStorage) CreateDisk(diskId string) IDisk {

pkg/hostman/storageman/storage_nas.go

@@ -22,6 +22,7 @@ import (
 	"yunion.io/x/jsonutils"
 	"yunion.io/x/log"
 
+	"yunion.io/x/onecloud/pkg/cloudprovider"
 	"yunion.io/x/onecloud/pkg/hostman/hostutils"
 	"yunion.io/x/onecloud/pkg/mcclient/modules"
 )
@@ -58,25 +59,20 @@ func (s *SNasStorage) CreateDisk(diskId string) IDisk {
 	return disk
 }
 
-func (s *SNasStorage) GetDiskById(diskId string) IDisk {
+func (s *SNasStorage) GetDiskById(diskId string) (IDisk, error) {
 	s.DiskLock.Lock()
 	defer s.DiskLock.Unlock()
 	for i := 0; i < len(s.Disks); i++ {
 		if s.Disks[i].GetId() == diskId {
-			if s.Disks[i].Probe() == nil {
-				return s.Disks[i]
-			} else {
-				return nil
-			}
+			return s.Disks[i], s.Disks[i].Probe()
 		}
 	}
 	var disk = s.ins.newDisk(diskId)
 	if disk.Probe() == nil {
 		s.Disks = append(s.Disks, disk)
-		return disk
-	} else {
-		return nil
+		return disk, nil
 	}
+	return nil, cloudprovider.ErrNotFound
 }
 
 func (s *SNasStorage) SyncStorageInfo() (jsonutils.JSONObject, error) {

pkg/hostman/storageman/storage_rbd.go

@@ -33,6 +33,7 @@ import (
 
 	api "yunion.io/x/onecloud/pkg/apis/compute"
 	"yunion.io/x/onecloud/pkg/cloudcommon/db/lockman"
+	"yunion.io/x/onecloud/pkg/cloudprovider"
 	deployapi "yunion.io/x/onecloud/pkg/hostman/hostdeployer/apis"
 	"yunion.io/x/onecloud/pkg/hostman/hostdeployer/deployclient"
 	"yunion.io/x/onecloud/pkg/hostman/hostutils"
@@ -595,23 +596,27 @@ func (s *SRbdStorage) SyncStorageInfo() (jsonutils.JSONObject, error) {
 	return modules.Storages.Get(hostutils.GetComputeSession(context.Background()), s.StorageName, jsonutils.Marshal(content))
 }
 
-func (s *SRbdStorage) GetDiskById(diskId string) IDisk {
+func (s *SRbdStorage) GetDiskById(diskId string) (IDisk, error) {
 	s.DiskLock.Lock()
 	defer s.DiskLock.Unlock()
 	for i := 0; i < len(s.Disks); i++ {
 		if s.Disks[i].GetId() == diskId {
-			if s.Disks[i].Probe() == nil {
-				return s.Disks[i]
+			err := s.Disks[i].Probe()
+			if err != nil {
+				if errors.Cause(err) == rbd.RbdErrorNotFound {
+					return nil, cloudprovider.ErrNotFound
+				}
+				return nil, errors.Wrapf(err, "disk.Prob")
 			}
+			return s.Disks[i], nil
 		}
 	}
 	var disk = NewRBDDisk(s, diskId)
 	if disk.Probe() == nil {
 		s.Disks = append(s.Disks, disk)
-		return disk
-	} else {
-		return nil
+		return disk, nil
 	}
+	return nil, cloudprovider.ErrNotFound
 }
 
 func (s *SRbdStorage) CreateDisk(diskId string) IDisk {