Daniel Erez has uploaded a new change for review. Change subject: core: ensure unique unit for VirtIO_SCSI and SPAR_VSCSI disks ......................................................................
core: ensure unique unit for VirtIO_SCSI and SPAR_VSCSI disks Ensuring a unique address unit value for disks attached with VirtIO_SCSI or SPAR_VSCSI interfaces. The selected units may collide as the search process is depended on the VmDevice addresses which are updated asynchronously by the VmsMonitoring. E.g. when hot-plugging multiple disks using a script, identical unit values might be selected for different disks. Hence: * Updating VmDevice address in DB right after selecting an available unit value. * Taking an EngineLock on addAddress method to avoid races. Change-Id: I5e621618e8891fca52b48bff437cc4c2a1f695db Bug-Url: https://bugzilla.redhat.com/1206374 Signed-off-by: Daniel Erez <de...@redhat.com> --- M backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/locks/LockingGroup.java M backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/HotPlugDiskVDSCommand.java 2 files changed, 42 insertions(+), 5 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/71/39471/1 diff --git a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/locks/LockingGroup.java b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/locks/LockingGroup.java index 458424f..3aa005f 100644 --- a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/locks/LockingGroup.java +++ b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/locks/LockingGroup.java @@ -32,6 +32,7 @@ SYNC_LUNS, /** This group is used for indication that an operation is executed using the specified host */ VDS_EXECUTION, - VDS_POOL_AND_STORAGE_CONNECTIONS; + VDS_POOL_AND_STORAGE_CONNECTIONS, + VM_DISK_HOT_PLUG; } diff --git a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/HotPlugDiskVDSCommand.java b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/HotPlugDiskVDSCommand.java index 77b4c67..49c67de 100644 --- a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/HotPlugDiskVDSCommand.java +++ b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/HotPlugDiskVDSCommand.java @@ -1,30 +1,39 @@ package org.ovirt.engine.core.vdsbroker.vdsbroker; -import java.util.HashMap; -import java.util.Map; - import org.apache.commons.lang.StringUtils; import org.ovirt.engine.core.common.FeatureSupported; import org.ovirt.engine.core.common.businessentities.VM; import org.ovirt.engine.core.common.businessentities.VmDevice; import org.ovirt.engine.core.common.businessentities.storage.Disk; -import org.ovirt.engine.core.common.businessentities.storage.DiskStorageType; import org.ovirt.engine.core.common.businessentities.storage.DiskImage; import org.ovirt.engine.core.common.businessentities.storage.DiskInterface; +import org.ovirt.engine.core.common.businessentities.storage.DiskStorageType; import org.ovirt.engine.core.common.businessentities.storage.LunDisk; import org.ovirt.engine.core.common.businessentities.storage.PropagateErrors; import org.ovirt.engine.core.common.businessentities.storage.VolumeFormat; +import org.ovirt.engine.core.common.errors.VdcBllMessages; +import org.ovirt.engine.core.common.locks.LockingGroup; +import org.ovirt.engine.core.common.utils.Pair; import org.ovirt.engine.core.common.utils.VmDeviceType; import org.ovirt.engine.core.common.vdscommands.HotPlugDiskVDSParameters; import org.ovirt.engine.core.compat.Guid; import org.ovirt.engine.core.dal.dbbroker.DbFacade; import org.ovirt.engine.core.utils.archstrategy.ArchStrategyFactory; +import org.ovirt.engine.core.utils.lock.EngineLock; +import org.ovirt.engine.core.utils.lock.LockManagerFactory; +import org.ovirt.engine.core.utils.transaction.TransactionMethod; +import org.ovirt.engine.core.utils.transaction.TransactionSupport; import org.ovirt.engine.core.vdsbroker.architecture.GetControllerIndices; import org.ovirt.engine.core.vdsbroker.xmlrpc.XmlRpcStringUtils; + +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; public class HotPlugDiskVDSCommand<P extends HotPlugDiskVDSParameters> extends VdsBrokerCommand<P> { protected Map<String, Object> sendInfo = new HashMap<String, Object>(); + private EngineLock vmLock; public HotPlugDiskVDSCommand(P parameters) { super(parameters); @@ -103,6 +112,7 @@ } private void addAddress(Map<String, Object> map, String address) { + lockVmWithWait(); if (getParameters().getDisk().getDiskInterface() != DiskInterface.VirtIO_SCSI && getParameters().getDisk().getDiskInterface() != DiskInterface.SPAPR_VSCSI) { if (StringUtils.isNotBlank(address)) { @@ -127,6 +137,7 @@ addAddressForScsiDisk(map, address, vmDeviceUnitMap, sPaprVscsiIndex, true); } } + LockManagerFactory.getLockManager().releaseLock(vmLock); } private void addAddressForScsiDisk(Map<String, Object> map, @@ -151,5 +162,30 @@ else { map.put(VdsProperties.Address, VmInfoBuilder.createAddressForScsiDisk(controllerIndex, availableUnit)); } + + // Updating device's address immediately (instead of waiting to VmsMonitoring) + // to prevent a duplicate unit value (i.e. ensuring a unique unit value). + updateVmDeviceAddress(map.get(VdsProperties.Address).toString()); + } + + private void updateVmDeviceAddress(final String address) { + TransactionSupport.executeInNewTransaction(new TransactionMethod<Void>() { + @Override + public Void runInTransaction() { + VmDevice vmDevice = getParameters().getVmDevice(); + vmDevice.setAddress(address); + DbFacade.getInstance().getVmDeviceDao().update(vmDevice); + return null; + } + }); + } + + protected void lockVmWithWait() { + Map<String, Pair<String, String>> exsluciveLock = + Collections.singletonMap(getParameters().getVmId().toString(), + new Pair<>(LockingGroup.VM_DISK_HOT_PLUG.toString(), + VdcBllMessages.ACTION_TYPE_FAILED_OBJECT_LOCKED.toString())); + vmLock = new EngineLock(exsluciveLock, null); + LockManagerFactory.getLockManager().acquireLockWait(vmLock); } } -- To view, visit https://gerrit.ovirt.org/39471 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I5e621618e8891fca52b48bff437cc4c2a1f695db Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Daniel Erez <de...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
