From 97f21c1835765c42e237aa3a8c61e296a8f82dd0 Mon Sep 17 00:00:00 2001 From: Spaceman1984 <49917670+Spaceman1984@users.noreply.github.com> Date: Tue, 23 Jun 2020 08:51:50 +0200 Subject: [PATCH] xenserver: Fixed null pointer and deployment issue on Xenserver with L2 Guest network with configDrive (#4004) This PR fixes an issue where an instance fails to deploy due to a null pointer when using an L2 Guest Network with DefaultL2NetworkOfferingConfigDrive on Xenserver. It also fixes migrating an instance to another host. This has been tested by: - Creating an L2 Guest network, using DefaultL2NetworkOfferingConfigDrive as the network offering. - Deploying an instance using the L2 Guest network created. - Migrating the instance away from the host and back --- .../cloud/vm/VirtualMachineManagerImpl.java | 1 - .../resource/CitrixResourceBase.java | 33 +++++------ .../xenbase/CitrixMigrateCommandWrapper.java | 3 +- .../xenbase/CitrixStartCommandWrapper.java | 59 ++++++++++--------- .../element/ConfigDriveNetworkElement.java | 12 ++-- 5 files changed, 53 insertions(+), 55 deletions(-) diff --git a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java index b7c7ad338fd..13c9019816c 100755 --- a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java +++ b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java @@ -1055,7 +1055,6 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac } catch (final AffinityConflictException e2) { s_logger.warn("Unable to create deployment, affinity rules associted to the VM conflict", e2); throw new CloudRuntimeException("Unable to create deployment, affinity rules associted to the VM conflict"); - } if (dest == null) { diff --git a/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java b/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java index 79a9fb22972..f9ec05a99cc 100644 --- a/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java +++ b/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java @@ -201,6 +201,12 @@ public abstract class CitrixResourceBase implements ServerResource, HypervisorRe private final static int BASE_TO_CONVERT_BYTES_INTO_KILOBYTES = 1024; + private final static int USER_DEVICE_START_ID = 3; + + private final static String VM_NAME_ISO_SUFFIX = "-ISO"; + + private final static String VM_FILE_ISO_SUFFIX = ".iso"; + private static final XenServerConnectionPool ConnPool = XenServerConnectionPool.getInstance(); // static min values for guests on xenserver private static final long mem_128m = 134217728L; @@ -1120,7 +1126,7 @@ public abstract class CitrixResourceBase implements ServerResource, HypervisorRe throw new CloudRuntimeException(errMsg); } - public VBD createVbd(final Connection conn, final DiskTO volume, final String vmName, final VM vm, final BootloaderType bootLoaderType, VDI vdi) throws XmlRpcException, XenAPIException { + public VBD createVbd(final Connection conn, final DiskTO volume, final String vmName, final VM vm, final BootloaderType bootLoaderType, VDI vdi, int isoCount) throws XmlRpcException, XenAPIException { final Volume.Type type = volume.getType(); if (vdi == null) { @@ -1156,7 +1162,7 @@ public abstract class CitrixResourceBase implements ServerResource, HypervisorRe if (volume.getType() == Volume.Type.ISO) { vbdr.mode = Types.VbdMode.RO; vbdr.type = Types.VbdType.CD; - vbdr.userdevice = "3"; + vbdr.userdevice = String.valueOf(USER_DEVICE_START_ID + isoCount); } else { vbdr.mode = Types.VbdMode.RW; vbdr.type = Types.VbdType.DISK; @@ -3868,7 +3874,7 @@ public abstract class CitrixResourceBase implements ServerResource, HypervisorRe return getVDIbyUuid(conn, volumePath); } - protected VDI mount(final Connection conn, final String vmName, final DiskTO volume) throws XmlRpcException, XenAPIException { + public VDI mount(final Connection conn, final String vmName, final DiskTO volume) throws XmlRpcException, XenAPIException { final DataTO data = volume.getData(); final Volume.Type type = volume.getType(); if (type == Volume.Type.ISO) { @@ -3882,7 +3888,7 @@ public abstract class CitrixResourceBase implements ServerResource, HypervisorRe // corer case, xenserver pv driver iso final String templateName = iso.getName(); - if (templateName.startsWith("xs-tools")) { + if (templateName != null && templateName.startsWith("xs-tools")) { try { final String actualTemplateName = getActualIsoTemplate(conn); final Set vdis = VDI.getByNameLabel(conn, actualTemplateName); @@ -3911,7 +3917,7 @@ public abstract class CitrixResourceBase implements ServerResource, HypervisorRe } catch (final URISyntaxException e) { throw new CloudRuntimeException("Incorrect uri " + mountpoint, e); } - final SR isoSr = createIsoSRbyURI(conn, uri, vmName, false); + final SR isoSr = createIsoSRbyURI(conn, uri, vmName, true); final String isoname = isoPath.substring(index + 1); @@ -4098,17 +4104,6 @@ public abstract class CitrixResourceBase implements ServerResource, HypervisorRe } final VM vm = vms.iterator().next(); - if (vmDataList != null) { - // create SR - SR sr = createLocalIsoSR(conn, _configDriveSRName + getHost().getIp()); - - // 1. create vm data files - createVmdataFiles(vmName, vmDataList, configDriveLabel); - - // 2. copy config drive iso to host - copyConfigDriveIsoToHost(conn, sr, vmName); - } - final Set vbds = vm.getVBDs(conn); for (final VBD vbd : vbds) { final VBD.Record vbdr = vbd.getRecord(conn); @@ -5449,7 +5444,7 @@ public abstract class CitrixResourceBase implements ServerResource, HypervisorRe public SR createLocalIsoSR(final Connection conn, final String srName) throws XenAPIException, XmlRpcException { // if config drive sr already exists then return - SR sr = getSRByNameLabelandHost(conn, _configDriveSRName + _host.getIp()); + SR sr = getSRByNameLabelandHost(conn, srName); if (sr != null) { s_logger.debug("Config drive SR already exist, returing it"); @@ -5542,10 +5537,10 @@ public abstract class CitrixResourceBase implements ServerResource, HypervisorRe s_logger.debug("Attaching config drive iso device for the VM " + vmName + " In host " + ipAddr); Set vms = VM.getByNameLabel(conn, vmName); - SR sr = getSRByNameLabel(conn, _configDriveSRName + ipAddr); + SR sr = getSRByNameLabel(conn, vmName + VM_NAME_ISO_SUFFIX); //Here you will find only two vdis with the .iso. //one is from source host and second from dest host - Set vdis = VDI.getByNameLabel(conn, vmName + ".iso"); + Set vdis = VDI.getByNameLabel(conn, vmName + VM_FILE_ISO_SUFFIX); if (vdis.isEmpty()) { s_logger.debug("Could not find config drive ISO: " + vmName); return false; diff --git a/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/wrapper/xenbase/CitrixMigrateCommandWrapper.java b/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/wrapper/xenbase/CitrixMigrateCommandWrapper.java index 12d19c84fa1..9115559d860 100644 --- a/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/wrapper/xenbase/CitrixMigrateCommandWrapper.java +++ b/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/wrapper/xenbase/CitrixMigrateCommandWrapper.java @@ -91,7 +91,8 @@ public class CitrixMigrateCommandWrapper extends CommandWrapper disks = new ArrayList(vmSpec.getDisks().length); - int index = 0; - for (final DiskTO disk : vmSpec.getDisks()) { - if (Volume.Type.ISO.equals(disk.getType())) { - disks.add(0, disk); - } else { - disks.add(index, disk); - } - index++; - } - for (DiskTO disk : disks) { - final VDI newVdi = citrixResourceBase.prepareManagedDisk(conn, disk, vmSpec.getId(), vmSpec.getName()); - - if (newVdi != null) { - final Map data = new HashMap<>(); - - final String path = newVdi.getUuid(conn); - - data.put(StartAnswer.PATH, path); - data.put(StartAnswer.IMAGE_FORMAT, Storage.ImageFormat.VHD.toString()); - - iqnToData.put(disk.getDetails().get(DiskTO.IQN), data); - } - - citrixResourceBase.createVbd(conn, disk, vmName, vm, vmSpec.getBootloader(), newVdi); - } + prepareDisks(vmSpec, citrixResourceBase, conn, iqnToData, vmName, vm); for (final NicTO nic : vmSpec.getNics()) { citrixResourceBase.createVif(conn, vmName, vm, vmSpec, nic); @@ -228,4 +202,35 @@ public final class CitrixStartCommandWrapper extends CommandWrapper> iqnToData, String vmName, VM vm) throws Exception { + // put cdrom at the first place in the list + List disks = new ArrayList(vmSpec.getDisks().length); + int index = 0; + for (final DiskTO disk : vmSpec.getDisks()) { + if (Volume.Type.ISO.equals(disk.getType())) { + disks.add(0, disk); + } else { + disks.add(index, disk); + } + index++; + } + int isoCount = 0; + for (DiskTO disk : disks) { + final VDI newVdi = citrixResourceBase.prepareManagedDisk(conn, disk, vmSpec.getId(), vmSpec.getName()); + if (newVdi != null) { + final Map data = new HashMap<>(); + final String path = newVdi.getUuid(conn); + data.put(StartAnswer.PATH, path); + data.put(StartAnswer.IMAGE_FORMAT, Storage.ImageFormat.VHD.toString()); + iqnToData.put(disk.getDetails().get(DiskTO.IQN), data); + } + citrixResourceBase.createVbd(conn, disk, vmName, vm, vmSpec.getBootloader(), newVdi, isoCount); + + if (disk.getType() == Volume.Type.ISO) { + isoCount++; + } + } + } } diff --git a/server/src/main/java/com/cloud/network/element/ConfigDriveNetworkElement.java b/server/src/main/java/com/cloud/network/element/ConfigDriveNetworkElement.java index e2c3ca7b597..53312221c97 100644 --- a/server/src/main/java/com/cloud/network/element/ConfigDriveNetworkElement.java +++ b/server/src/main/java/com/cloud/network/element/ConfigDriveNetworkElement.java @@ -305,18 +305,16 @@ public class ConfigDriveNetworkElement extends AdapterBase implements NetworkEle @Override public boolean prepareMigration(NicProfile nic, Network network, VirtualMachineProfile vm, DeployDestination dest, ReservationContext context) { - if (nic.isDefaultNic() && _networkModel.getUserDataUpdateProvider(network).getProvider().equals(Provider.ConfigDrive)) { + if (_networkModel.getUserDataUpdateProvider(network).getProvider().equals(Provider.ConfigDrive)) { LOG.trace(String.format("[prepareMigration] for vm: %s", vm.getInstanceName())); - final DataStore dataStore = findDataStore(vm, dest); - try { - addConfigDriveDisk(vm, dataStore); - } catch (ResourceUnavailableException e) { + addPasswordAndUserdata(network, nic, vm, dest, context); + } catch (InsufficientCapacityException | ResourceUnavailableException e) { LOG.error("Failed to add config disk drive due to: ", e); + return false; } - return false; } - else return true; + return true; } @Override