Daniel Erez has uploaded a new change for review. Change subject: core: set address explicitly for VirtIO-SCSI devices ......................................................................
core: set address explicitly for VirtIO-SCSI devices Upon VM start, libvirt uses automatic address allocation for defining addresses for each device (when not defined addresses explicitly). As part of the automatic allocation, libvirt limits maximum units (disks) for each controller [6 (narrow bus) / 15 (wide bus)]. In order to bypass this limitation, engine should set each device address explicitly while keeping unit value (disk's index in VirtIO-SCSI controller) unique and consecutive in the VM devices. More details and full discussion: https://www.redhat.com/archives/libvir-list/2013-November/msg01113.html Change-Id: Iee79ff3f72b8018d0c26b37503f480643a845765 Bug-Url: https://bugzilla.redhat.com/1035453 Signed-off-by: Daniel Erez <de...@redhat.com> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/HotPlugDiskToVmCommand.java M backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/HotPlugDiskVDSCommand.java M backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VdsProperties.java M backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VmInfoBuilder.java M backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VmInfoBuilderBase.java 5 files changed, 95 insertions(+), 6 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/85/22385/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 b9173b3..0d1746b 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 @@ -124,6 +124,7 @@ @Override protected void executeVmCommand() { if (getVm().getStatus().isUpOrPaused()) { + updateDisksFromDb(); performPlugCommand(getPlugAction(), getDisk(), oldVmDevice); } 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 3fe74de..b515fb8 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 @@ -3,10 +3,12 @@ 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.Disk; import org.ovirt.engine.core.common.businessentities.Disk.DiskStorageType; import org.ovirt.engine.core.common.businessentities.DiskImage; +import org.ovirt.engine.core.common.businessentities.DiskInterface; import org.ovirt.engine.core.common.businessentities.LunDisk; import org.ovirt.engine.core.common.businessentities.PropagateErrors; import org.ovirt.engine.core.common.businessentities.VmDevice; @@ -73,8 +75,30 @@ } private void addAddress(Map<String, Object> map, String address) { - if (org.apache.commons.lang.StringUtils.isNotBlank(address)) { - map.put("address", XmlRpcStringUtils.string2Map(getParameters().getVmDevice().getAddress())); + if (getParameters().getDisk().getDiskInterface() != DiskInterface.VirtIO_SCSI) { + if (StringUtils.isNotBlank(address)) { + map.put("address", XmlRpcStringUtils.string2Map(address)); + } + } + else { + Map<VmDevice, Integer> vmDeviceUnitMap = VmInfoBuilder.getVmDeviceUnitMapForVirtioScsiDisks(getParameters().getVm()); + int availableUnit = VmInfoBuilder.getAvailableUnitForVirtioScsiDisk(vmDeviceUnitMap); + + // If address has been already set before, verify its uniqueness; + // Otherwise, set address according to the next available unit. + if (StringUtils.isNotBlank(address)) { + Map<String, String> addressMap = XmlRpcStringUtils.string2Map(address); + int unit = Integer.valueOf(addressMap.get(VdsProperties.Unit)); + if (!vmDeviceUnitMap.containsValue(unit)) { + map.put(VdsProperties.Address, addressMap); + } + else { + map.put(VdsProperties.Address, VmInfoBuilder.createAddressForVirtioScsiDisk(availableUnit)); + } + } + else { + map.put(VdsProperties.Address, VmInfoBuilder.createAddressForVirtioScsiDisk(availableUnit)); + } } } } diff --git a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VdsProperties.java b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VdsProperties.java index 9e3bd78..edbc71e 100644 --- a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VdsProperties.java +++ b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VdsProperties.java @@ -256,6 +256,7 @@ public static final String VirtioScsi = "virtio-scsi"; public static final String Scsi = "scsi"; public static final String Sgio = "sgio"; + public static final String Unit = "unit"; public static final String Path = "path"; public static final String Ide = "ide"; public static final String Fdc = "fdc"; diff --git a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VmInfoBuilder.java b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VmInfoBuilder.java index 1163f51..73770f0 100644 --- a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VmInfoBuilder.java +++ b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VmInfoBuilder.java @@ -14,6 +14,7 @@ import org.ovirt.engine.core.common.businessentities.Disk; import org.ovirt.engine.core.common.businessentities.Disk.DiskStorageType; import org.ovirt.engine.core.common.businessentities.DiskImage; +import org.ovirt.engine.core.common.businessentities.DiskInterface; import org.ovirt.engine.core.common.businessentities.DisplayType; import org.ovirt.engine.core.common.businessentities.Entities; import org.ovirt.engine.core.common.businessentities.LunDisk; @@ -242,13 +243,11 @@ protected void buildVmDrives() { boolean bootDiskFound = false; List<Disk> disks = getSortedDisks(); + Map<VmDevice, Integer> vmDeviceUnitMap = getVmDeviceUnitMapForVirtioScsiDisks(vm); for (Disk disk : disks) { Map<String, Object> struct = new HashMap<String, Object>(); // get vm device for this disk from DB - VmDevice vmDevice = - DbFacade.getInstance() - .getVmDeviceDao() - .get(new VmDeviceId(disk.getId(), vm.getId())); + VmDevice vmDevice = getVmDeviceByDiskId(disk.getId(), vm.getId()); // skip unamanged devices (handled separtely) if (!vmDevice.getIsManaged()) { continue; @@ -268,6 +267,11 @@ if (disk.getDiskStorageType() == DiskStorageType.LUN) { struct.put(VdsProperties.Device, VmDeviceType.LUN.getName()); struct.put(VdsProperties.Sgio, disk.getSgio().toString().toLowerCase()); + } + if (StringUtils.isEmpty(vmDevice.getAddress())) { + // Explicitly define device's address if missing + int unit = vmDeviceUnitMap.get(vmDevice); + vmDevice.setAddress(createAddressForVirtioScsiDisk(unit).toString()); } break; default: @@ -913,4 +917,55 @@ addDevice(struct, vmDevice, null); } } + + /** + * @return a map containing an appropriate unit (disk's index in VirtIO-SCSI controller) for each vm device. + */ + public static Map<VmDevice, Integer> getVmDeviceUnitMapForVirtioScsiDisks(VM vm) { + List<Disk> disks = new ArrayList<Disk>(vm.getDiskMap().values()); + Map<VmDevice, Integer> vmDeviceUnitMap = new HashMap<>(); + Map<VmDevice, Disk> vmDeviceDiskMap = new HashMap<>(); + + for (Disk disk : disks) { + if (disk.getDiskInterface() == DiskInterface.VirtIO_SCSI) { + VmDevice vmDevice = getVmDeviceByDiskId(disk.getId(), vm.getId()); + Map<String, String> address = XmlRpcStringUtils.string2Map(vmDevice.getAddress()); + String unitStr = address.get(VdsProperties.Unit); + + // If unit property is available adding to 'vmDeviceUnitMap'; + // Otherwise, adding to 'vmDeviceDiskMap' for setting the unit property later. + if (StringUtils.isNotEmpty(unitStr)) { + vmDeviceUnitMap.put(vmDevice, Integer.valueOf(unitStr)); + } + else { + vmDeviceDiskMap.put(vmDevice, disk); + } + } + } + + // Find available unit (disk's index in VirtIO-SCSI controller) for disks with empty address + for (Entry<VmDevice, Disk> entry : vmDeviceDiskMap.entrySet()) { + int unit = getAvailableUnitForVirtioScsiDisk(vmDeviceUnitMap); + vmDeviceUnitMap.put(entry.getKey(), unit); + } + + return vmDeviceUnitMap; + } + + public static int getAvailableUnitForVirtioScsiDisk(Map<VmDevice, Integer> vmDeviceUnitMap) { + int unit = 0; + while (vmDeviceUnitMap.containsValue(unit)) { + unit++; + } + return unit; + } + + public static Map<String, String> createAddressForVirtioScsiDisk(int unit) { + Map<String, String> addressMap = new HashMap<>(); + addressMap.put(VdsProperties.Type, "drive"); + addressMap.put(VdsProperties.Bus, "0"); + addressMap.put(VdsProperties.target, "0"); + addressMap.put(VdsProperties.Unit, String.valueOf(unit)); + return addressMap; + } } diff --git a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VmInfoBuilderBase.java b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VmInfoBuilderBase.java index dc83c98..ae0a4cc 100644 --- a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VmInfoBuilderBase.java +++ b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VmInfoBuilderBase.java @@ -15,6 +15,8 @@ import org.ovirt.engine.core.common.businessentities.Disk; import org.ovirt.engine.core.common.businessentities.DisplayType; import org.ovirt.engine.core.common.businessentities.VM; +import org.ovirt.engine.core.common.businessentities.VmDevice; +import org.ovirt.engine.core.common.businessentities.VmDeviceId; import org.ovirt.engine.core.common.businessentities.comparators.DiskImageByDiskAliasComparator; import org.ovirt.engine.core.common.businessentities.network.Network; import org.ovirt.engine.core.common.businessentities.network.NetworkCluster; @@ -24,6 +26,7 @@ import org.ovirt.engine.core.common.config.ConfigValues; import org.ovirt.engine.core.common.osinfo.OsRepository; import org.ovirt.engine.core.common.utils.SimpleDependecyInjector; +import org.ovirt.engine.core.compat.Guid; import org.ovirt.engine.core.compat.WindowsJavaTimezoneMapping; import org.ovirt.engine.core.dal.dbbroker.DbFacade; import org.ovirt.engine.core.dal.dbbroker.auditloghandling.AuditLogDirector; @@ -262,6 +265,11 @@ AuditLogDirector.log(event, AuditLogType.VNIC_PROFILE_UNSUPPORTED_FEATURES); } + protected static VmDevice getVmDeviceByDiskId(Guid diskId, Guid vmId) { + // get vm device for this disk from DB + return DbFacade.getInstance().getVmDeviceDao().get(new VmDeviceId(diskId, vmId)); + } + protected abstract void buildVmVideoCards(); protected abstract void buildVmCD(); -- To view, visit http://gerrit.ovirt.org/22385 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Iee79ff3f72b8018d0c26b37503f480643a845765 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: ovirt-engine-3.3 Gerrit-Owner: Daniel Erez <de...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
