Merge pull request #7918 from swordqiu/hotfix/qj-lock-unlock-cleanup-20200915

fix: lock/unlock cleanup

pkg/compute/models/disks.go

@@ -2566,13 +2566,18 @@ func (self *SDisk) PerformChangeOwner(ctx context.Context, userCred mcclient.Tok
 	}
 	for i := range snapshots {
 		snapshot := snapshots[i]
-		lockman.LockObject(ctx, &snapshot)
-		_, err := snapshot.PerformChangeOwner(ctx, userCred, query, input)
+		err := func() error {
+			lockman.LockObject(ctx, &snapshot)
+			defer lockman.ReleaseObject(ctx, &snapshot)
+			_, err := snapshot.PerformChangeOwner(ctx, userCred, query, input)
+			if err != nil {
+				return err
+			}
+			return nil
+		}()
 		if err != nil {
-			lockman.ReleaseObject(ctx, &snapshot)
 			return nil, errors.Wrapf(err, "fail to change owner of this disk(%s)'s snapshot %s", self.Id, snapshot.Id)
 		}
-		lockman.ReleaseObject(ctx, &snapshot)
 	}
 	return nil, nil
 }

pkg/compute/models/guests.go

@@ -2984,8 +2984,8 @@ func (self *SGuest) attach2Disk(ctx context.Context, disk *SDisk, userCred mccli
 	guestdisk.DiskId = disk.Id
 	guestdisk.GuestId = self.Id
 
-	defer lockman.ReleaseObject(ctx, self)
 	lockman.LockObject(ctx, self)
+	defer lockman.ReleaseObject(ctx, self)
 
 	guestdisk.Index = self.getDiskIndex()
 	err = guestdisk.DoSave(ctx, driver, cache, mountpoint)

pkg/compute/models/hosts.go

@@ -4853,10 +4853,12 @@ func (manager *SHostManager) PingDetectionTask(ctx context.Context, userCred mcc
 		var host = new(SHost)
 		q.Row2Struct(rows, host)
 		host.SetModelManager(manager, host)
-		lockman.LockObject(ctx, host)
-		host.PerformOffline(ctx, userCred, nil, data)
-		host.MarkGuestUnknown(userCred)
-		lockman.ReleaseObject(ctx, host)
+		func() {
+			lockman.LockObject(ctx, host)
+			defer lockman.ReleaseObject(ctx, host)
+			host.PerformOffline(ctx, userCred, nil, data)
+			host.MarkGuestUnknown(userCred)
+		}()
 	}
 }
 

pkg/compute/models/purge.go

@@ -652,7 +652,7 @@ func (manager *SSnapshotPolicyCacheManager) purgeAll(ctx context.Context, userCr
 
 func (spc *SSnapshotPolicyCache) purge(ctx context.Context, userCred mcclient.TokenCredential) error {
 	lockman.LockObject(ctx, spc)
-	defer lockman.LockObject(ctx, spc)
+	defer lockman.ReleaseObject(ctx, spc)
 	err := spc.ValidateDeleteCondition(ctx)
 	if err != nil {
 		return err

pkg/compute/models/vpcs_mapped_address.go

@@ -39,7 +39,7 @@ func (man *SGuestnetworkManager) lockAllocMappedAddr(ctx context.Context) {
 }
 
 func (man *SGuestnetworkManager) unlockAllocMappedAddr(ctx context.Context) {
-	defer lockman.ReleaseRawObject(ctx, LOCK_CLASS_guestnetworks_mapped_addr, LOCK_OBJ_guestnetworks_mapped_addr)
+	lockman.ReleaseRawObject(ctx, LOCK_CLASS_guestnetworks_mapped_addr, LOCK_OBJ_guestnetworks_mapped_addr)
 }
 
 func (man *SGuestnetworkManager) allocMappedIpAddr(ctx context.Context) (string, error) {

pkg/compute/tasks/scaling_group_delete_task.go

@@ -59,14 +59,16 @@ func (self *ScalingGroupDeleteTask) OnInit(ctx context.Context, obj db.IStandalo
 	spids := make([]string, len(sps))
 	for i := range sps {
 		spids[i] = sps[i].GetId()
-		lockman.LockObject(ctx, &sps[i])
-		err := sps[i].SetStatus(self.UserCred, api.SP_STATUS_DELETING, "delete scaling group")
+		err := func() error {
+			lockman.LockObject(ctx, &sps[i])
+			defer lockman.ReleaseObject(ctx, &sps[i])
+			return sps[i].SetStatus(self.UserCred, api.SP_STATUS_DELETING, "delete scaling group")
+		}()
 		if err != nil {
 			self.taskFailed(ctx, sg, jsonutils.NewString(fmt.Sprintf("set scaling policy %s as deleting status failed: %s",
 				sps[i].GetId(), err)))
 			return
 		}
-		lockman.ReleaseObject(ctx, &sps[i])
 	}
 
 	log.Debugf("finish to mark all scaling policies deleted")

pkg/keystone/models/identity_provider.go

@@ -1008,7 +1008,7 @@ func (self *SIdentityProvider) SyncOrCreateDomain(ctx context.Context, extId str
 	}
 
 	lockman.LockClass(ctx, DomainManager, "")
-	lockman.ReleaseClass(ctx, DomainManager, "")
+	defer lockman.ReleaseClass(ctx, DomainManager, "")
 
 	domain = &SDomain{}
 	domain.SetModelManager(DomainManager, domain)

pkg/monitor/models/alert.go

@@ -557,8 +557,9 @@ func (alert *SAlert) AttachNotification(
 		return nil, httperrors.NewNotAcceptableError("alert already attached to notification")
 	}
 
-	defer lockman.ReleaseObject(ctx, alert)
 	lockman.LockObject(ctx, alert)
+	defer lockman.ReleaseObject(ctx, alert)
+
 	alertNoti := new(SAlertnotification)
 	alertNoti.AlertId = alert.GetId()
 	alertNoti.Index, err = alert.getNotificationIndex()

pkg/multicloud/esxi/virtualmachine.go

@@ -40,6 +40,7 @@ import (
 	"yunion.io/x/onecloud/pkg/cloudcommon/db/lockman"
 	cloudtypes "yunion.io/x/onecloud/pkg/cloudcommon/types"
 	"yunion.io/x/onecloud/pkg/cloudprovider"
+	"yunion.io/x/onecloud/pkg/httperrors"
 	"yunion.io/x/onecloud/pkg/multicloud"
 	"yunion.io/x/onecloud/pkg/util/billing"
 	"yunion.io/x/onecloud/pkg/util/netutils2"
@@ -387,19 +388,14 @@ func (self *SVirtualMachine) StartVM(ctx context.Context) error {
 	return self.startVM(ctx)
 }
 
-func (self *SVirtualMachine) lockHost(ctx context.Context) {
-	ihost := self.GetIHost()
-	lockman.LockRawObject(ctx, "host", ihost.GetGlobalId())
-}
-
-func (self *SVirtualMachine) releaseHost(ctx context.Context) {
-	ihost := self.GetIHost()
-	lockman.ReleaseRawObject(ctx, "host", ihost.GetGlobalId())
-}
-
 func (self *SVirtualMachine) startVM(ctx context.Context) error {
-	self.lockHost(ctx)
-	defer self.releaseHost(ctx)
+	ihost := self.GetIHost()
+	if ihost == nil {
+		return errors.Wrap(httperrors.ErrInvalidStatus, "no valid host")
+	}
+
+	lockman.LockRawObject(ctx, "host", ihost.GetGlobalId())
+	defer lockman.ReleaseRawObject(ctx, "host", ihost.GetGlobalId())
 
 	err := self.makeNicsStartConnected(ctx)
 	if err != nil {