Sergey Gotliv has uploaded a new change for review. Change subject: [wip]engine: Resolve a race in HotPlugDiskToVmCommand ......................................................................
[wip]engine: Resolve a race in HotPlugDiskToVmCommand Change-Id: I1359a34a48a6261e22631ff1640d81d735e8c490 Bug-Url: https://bugzilla.redhat.com/1003649 Signed-off-by: Sergey Gotliv <sgot...@redhat.com> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/HotPlugDiskToVmCommand.java M packaging/dbscripts/vm_device_sp.sql 2 files changed, 57 insertions(+), 12 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/11/19311/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/HotPlugDiskToVmCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/HotPlugDiskToVmCommand.java index b6d9da5..136f793 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/HotPlugDiskToVmCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/HotPlugDiskToVmCommand.java @@ -1,6 +1,8 @@ package org.ovirt.engine.core.bll; +import java.util.Arrays; import java.util.Collections; +import java.util.HashMap; import java.util.List; import java.util.Map; @@ -100,31 +102,62 @@ performPlugCommand(getPlugAction(), disk, oldVmDevice); } - //Update boot order and isPlugged fields - final List<VmDevice> devices = VmDeviceUtils.updateBootOrderInVmDevice(getVm().getStaticData()); - for (VmDevice device:devices) { - if (device.getDeviceId().equals(oldVmDevice.getDeviceId())) { - device.setIsPlugged(!oldVmDevice.getIsPlugged()); - break; - } + updateVmDevicePluggedProperty(); + + if (disk.isBoot()) { + updateVmDevicesBootOrder(); } + setSucceeded(true); + } + + private void updateVmDevicePluggedProperty() { + final VmDevice device = getVmDeviceDao().get(oldVmDevice.getId()); + device.setIsPlugged(!oldVmDevice.getIsPlugged()); TransactionSupport.executeInNewTransaction(new TransactionMethod<Void>() { @Override public Void runInTransaction() { getVmStaticDAO().incrementDbGeneration(getVm().getId()); - getVmDeviceDao().updateAll("UpdateVmDeviceForHotPlugDisk", devices); + getVmDeviceDao().updateAll("UpdateVmDeviceForHotPlugDisk", Arrays.asList(device)); + return null; + } + }); + } + + private void updateVmDevicesBootOrder() { + final List<VmDevice> devices = VmDeviceUtils.updateBootOrderInVmDevice(getVm().getStaticData()); + TransactionSupport.executeInNewTransaction(new TransactionMethod<Void>() { + @Override + public Void runInTransaction() { + getVmStaticDAO().incrementDbGeneration(getVm().getId()); + getVmDeviceDao().updateAll("UpdateVmDeviceBootOrder", devices); VmHandler.updateDisksFromDb(getVm()); return null; } }); - setSucceeded(true); } @Override protected Map<String, Pair<String, String>> getSharedLocks() { return Collections.singletonMap(getVmId().toString(), LockMessagesMatchUtil.makeLockingPair(LockingGroup.VM, VdcBllMessages.ACTION_TYPE_FAILED_VM_IS_LOCKED)); + } + + @Override + protected Map<String, Pair<String, String>> getExclusiveLocks() { + Map<String, Pair<String, String>> exclusiveLock = new HashMap<>(); + + exclusiveLock.put(disk.getId().toString(), + LockMessagesMatchUtil.makeLockingPair(LockingGroup.DISK, + VdcBllMessages.ACTION_TYPE_FAILED_DISKS_LOCKED)); + + if (disk.isBoot()) { + exclusiveLock.put(getVmId().toString(), + LockMessagesMatchUtil.makeLockingPair(LockingGroup.VM_DISK_BOOT, + VdcBllMessages.ACTION_TYPE_FAILED_OBJECT_LOCKED)); + } + + return exclusiveLock; } @Override @@ -136,5 +169,4 @@ public String getDiskAlias() { return disk.getDiskAlias(); } - } diff --git a/packaging/dbscripts/vm_device_sp.sql b/packaging/dbscripts/vm_device_sp.sql index c6fd3bd..a5dbd1b 100644 --- a/packaging/dbscripts/vm_device_sp.sql +++ b/packaging/dbscripts/vm_device_sp.sql @@ -100,14 +100,12 @@ Create or replace FUNCTION UpdateVmDeviceForHotPlugDisk( v_device_id UUID, v_vm_id UUID, - v_boot_order int, v_is_plugged boolean) RETURNS VOID AS $procedure$ BEGIN UPDATE vm_device SET - boot_order = v_boot_order, is_plugged = v_is_plugged, _update_date = current_timestamp WHERE device_id = v_device_id and vm_id = v_vm_id; @@ -115,6 +113,21 @@ LANGUAGE plpgsql; +Create or replace FUNCTION UpdateVmDeviceBootOrder( + v_device_id UUID, + v_vm_id UUID, + v_boot_order int) +RETURNS VOID +AS $procedure$ +BEGIN + UPDATE vm_device + SET + boot_order = v_boot_order, + _update_date = current_timestamp + WHERE device_id = v_device_id and vm_id = v_vm_id; +END; $procedure$ +LANGUAGE plpgsql; + Create or replace FUNCTION DeleteVmDevice(v_device_id UUID, v_vm_id UUID) RETURNS VOID -- To view, visit http://gerrit.ovirt.org/19311 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I1359a34a48a6261e22631ff1640d81d735e8c490 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Sergey Gotliv <sgot...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
