Skip to content

server: Avoid device id collision between config drive ISO and data volumes on KVM #14073

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
weizhouapache wants to merge 1 commit into
base: 4.22
Choose a base branch
from
+19 −5
Open

server: Avoid device id collision between config drive ISO and data volumes on KVM #14073

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 11 additions & 3 deletions server/src/main/java/com/cloud/hypervisor/KVMGuru.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import com.cloud.hypervisor.Hypervisor.HypervisorType;
import com.cloud.hypervisor.dao.HypervisorCapabilitiesDao;
import com.cloud.hypervisor.kvm.dpdk.DpdkHelper;
import com.cloud.network.element.ConfigDriveNetworkElement;
import com.cloud.service.ServiceOfferingVO;
import com.cloud.storage.DataStoreRole;
import com.cloud.storage.GuestOSHypervisorVO;
Expand All @@ -42,7 +43,9 @@
import com.cloud.vm.VMInstanceVO;
import com.cloud.vm.VirtualMachine;
import com.cloud.vm.VirtualMachineProfile;
import com.cloud.vm.VmDetailConstants;
import com.cloud.vm.dao.VMInstanceDao;
import com.cloud.vm.dao.VMInstanceDetailsDao;
import org.apache.cloudstack.backup.Backup;
import org.apache.cloudstack.storage.command.CopyCommand;
import org.apache.cloudstack.storage.command.StorageSubSystemCommand;
Expand Down Expand Up @@ -71,6 +74,8 @@ public class KVMGuru extends HypervisorGuruBase implements HypervisorGuru {
VolumeDao _volumeDao;
@Inject
HypervisorCapabilitiesDao _hypervisorCapabilitiesDao;
@Inject
VMInstanceDetailsDao _vmInstanceDetailsDao;


@Override
Expand All @@ -86,7 +91,7 @@ protected KVMGuru() {
* Get next free DeviceId for a KVM Guest
*/

protected Long getNextAvailableDeviceId(List<VolumeVO> vmVolumes) {
protected Long getNextAvailableDeviceId(long vmId, List<VolumeVO> vmVolumes) {

int maxDataVolumesSupported;
int maxDeviceId;
Expand All @@ -103,6 +108,9 @@ protected Long getNextAvailableDeviceId(List<VolumeVO> vmVolumes) {
devIds.add(String.valueOf(i));
}
devIds.remove("3");
if (_vmInstanceDetailsDao.findDetail(vmId, VmDetailConstants.CONFIG_DRIVE_LOCATION) != null) {
devIds.remove(ConfigDriveNetworkElement.CONFIGDRIVEDISKSEQ.toString());
}
Comment on lines 110 to +113
for (VolumeVO vmVolume : vmVolumes) {
devIds.remove(vmVolume.getDeviceId().toString().trim());
}
Expand Down Expand Up @@ -371,7 +379,7 @@ public VirtualMachine importVirtualMachineFromBackup(long zoneId, long domainId,
} else if (VMVolToRestore.getType() == Volume.Type.DATADISK) {
List<VolumeVO> vmVolumes = _volumeDao.findByInstance(vm.getId());
_volumeDao.update(volume.getId(), volume);
_volumeDao.attachVolume(volume.getId(), vm.getId(), getNextAvailableDeviceId(vmVolumes));
_volumeDao.attachVolume(volume.getId(), vm.getId(), getNextAvailableDeviceId(vm.getId(), vmVolumes));
}
UsageEventUtils.publishUsageEvent(EventTypes.EVENT_VOLUME_ATTACH, volume.getAccountId(), volume.getDataCenterId(), volume.getId(), volume.getName(),
volume.getDiskOfferingId(), volume.getTemplateId(), volume.getSize(), Volume.class.getName(), volume.getUuid(), vm.getId(), volume.isDisplay());
Expand All @@ -389,7 +397,7 @@ public VirtualMachine importVirtualMachineFromBackup(long zoneId, long domainId,
VolumeVO restoredVolume = _volumeDao.findByUuid(location);
if (restoredVolume != null) {
try {
_volumeDao.attachVolume(restoredVolume.getId(), vm.getId(), getNextAvailableDeviceId(vmVolumes));
_volumeDao.attachVolume(restoredVolume.getId(), vm.getId(), getNextAvailableDeviceId(vm.getId(), vmVolumes));
restoredVolume.setState(Volume.State.Ready);
_volumeDao.update(restoredVolume.getId(), restoredVolume);
UsageEventUtils.publishUsageEvent(EventTypes.EVENT_VOLUME_ATTACH, restoredVolume.getAccountId(), restoredVolume.getDataCenterId(), restoredVolume.getId(), restoredVolume.getName(),
Expand Down
2 changes: 1 addition & 1 deletion server/src/main/java/com/cloud/network/element/ConfigDriveNetworkElement.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ public class ConfigDriveNetworkElement extends AdapterBase implements NetworkEle
@Inject
private HypervisorGuruManager _hvGuruMgr;

private final static Integer CONFIGDRIVEDISKSEQ = 4;
public final static Integer CONFIGDRIVEDISKSEQ = 4;

private boolean canHandle(TrafficType trafficType) {
return trafficType.equals(TrafficType.Guest);
Expand Down
8 changes: 7 additions & 1 deletion server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@
import com.cloud.hypervisor.Hypervisor.HypervisorType;
import com.cloud.hypervisor.HypervisorCapabilitiesVO;
import com.cloud.hypervisor.dao.HypervisorCapabilitiesDao;
import com.cloud.network.element.ConfigDriveNetworkElement;
import com.cloud.offering.DiskOffering;
import com.cloud.org.Cluster;
import com.cloud.org.Grouping;
Expand Down Expand Up @@ -5042,8 +5043,10 @@ private Long getDeviceId(UserVmVO vm, Long deviceId) {
int maxDevices = getMaxDataVolumesSupported(vm) + 2; // add 2 to consider devices root volume and cdrom
int maxDeviceId = maxDevices - 1;
List<VolumeVO> vols = _volsDao.findByInstance(vm.getId());
boolean vmHasConfigDrive = vmInstanceDetailsDao.findDetail(vm.getId(), VmDetailConstants.CONFIG_DRIVE_LOCATION) != null;
if (deviceId != null) {
Comment on lines 5043 to 5047
if (deviceId.longValue() < 0 || deviceId.longValue() > maxDeviceId || deviceId.longValue() == 3) {
if (deviceId.longValue() < 0 || deviceId.longValue() > maxDeviceId || deviceId.longValue() == 3
|| (vmHasConfigDrive && deviceId.longValue() == ConfigDriveNetworkElement.CONFIGDRIVEDISKSEQ)) {
throw new RuntimeException("deviceId should be 0,1,2,4-" + maxDeviceId);
}
for (VolumeVO vol : vols) {
Expand All @@ -5058,6 +5061,9 @@ private Long getDeviceId(UserVmVO vm, Long deviceId) {
devIds.add(String.valueOf(i));
}
devIds.remove("3");
if (vmHasConfigDrive) {
devIds.remove(ConfigDriveNetworkElement.CONFIGDRIVEDISKSEQ.toString());
}
for (VolumeVO vol : vols) {
devIds.remove(vol.getDeviceId().toString().trim());
}
Expand Down
Loading