Skip to content

Commit 97f21c1

Browse files
Spaceman1984Spaceman1984
authored
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
1 parent 7e50f4a commit 97f21c1

5 files changed

Lines changed: 53 additions & 55 deletions

File tree

‎engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1055,7 +1055,6 @@ public void orchestrateStart(final String vmUuid, final Map<VirtualMachineProfil
10551055
} catch (final AffinityConflictException e2) {
10561056
s_logger.warn("Unable to create deployment, affinity rules associted to the VM conflict", e2);
10571057
throw new CloudRuntimeException("Unable to create deployment, affinity rules associted to the VM conflict");
1058-
10591058
}
10601059

10611060
if (dest == null) {

‎plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java‎

Lines changed: 14 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,12 @@ public String toString() {
201201

202202
private final static int BASE_TO_CONVERT_BYTES_INTO_KILOBYTES = 1024;
203203

204+
private final static int USER_DEVICE_START_ID = 3;
205+
206+
private final static String VM_NAME_ISO_SUFFIX = "-ISO";
207+
208+
private final static String VM_FILE_ISO_SUFFIX = ".iso";
209+
204210
private static final XenServerConnectionPool ConnPool = XenServerConnectionPool.getInstance();
205211
// static min values for guests on xenserver
206212
private static final long mem_128m = 134217728L;
@@ -1120,7 +1126,7 @@ String createTemplateFromSnapshot(final Connection conn, final String templatePa
11201126
throw new CloudRuntimeException(errMsg);
11211127
}
11221128

1123-
public VBD createVbd(final Connection conn, final DiskTO volume, final String vmName, final VM vm, final BootloaderType bootLoaderType, VDI vdi) throws XmlRpcException, XenAPIException {
1129+
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 {
11241130
final Volume.Type type = volume.getType();
11251131

11261132
if (vdi == null) {
@@ -1156,7 +1162,7 @@ public VBD createVbd(final Connection conn, final DiskTO volume, final String vm
11561162
if (volume.getType() == Volume.Type.ISO) {
11571163
vbdr.mode = Types.VbdMode.RO;
11581164
vbdr.type = Types.VbdType.CD;
1159-
vbdr.userdevice = "3";
1165+
vbdr.userdevice = String.valueOf(USER_DEVICE_START_ID + isoCount);
11601166
} else {
11611167
vbdr.mode = Types.VbdMode.RW;
11621168
vbdr.type = Types.VbdType.DISK;
@@ -3868,7 +3874,7 @@ protected VDI mount(final Connection conn, final StoragePoolType poolType, final
38683874
return getVDIbyUuid(conn, volumePath);
38693875
}
38703876

3871-
protected VDI mount(final Connection conn, final String vmName, final DiskTO volume) throws XmlRpcException, XenAPIException {
3877+
public VDI mount(final Connection conn, final String vmName, final DiskTO volume) throws XmlRpcException, XenAPIException {
38723878
final DataTO data = volume.getData();
38733879
final Volume.Type type = volume.getType();
38743880
if (type == Volume.Type.ISO) {
@@ -3882,7 +3888,7 @@ protected VDI mount(final Connection conn, final String vmName, final DiskTO vol
38823888

38833889
// corer case, xenserver pv driver iso
38843890
final String templateName = iso.getName();
3885-
if (templateName.startsWith("xs-tools")) {
3891+
if (templateName != null && templateName.startsWith("xs-tools")) {
38863892
try {
38873893
final String actualTemplateName = getActualIsoTemplate(conn);
38883894
final Set<VDI> vdis = VDI.getByNameLabel(conn, actualTemplateName);
@@ -3911,7 +3917,7 @@ protected VDI mount(final Connection conn, final String vmName, final DiskTO vol
39113917
} catch (final URISyntaxException e) {
39123918
throw new CloudRuntimeException("Incorrect uri " + mountpoint, e);
39133919
}
3914-
final SR isoSr = createIsoSRbyURI(conn, uri, vmName, false);
3920+
final SR isoSr = createIsoSRbyURI(conn, uri, vmName, true);
39153921

39163922
final String isoname = isoPath.substring(index + 1);
39173923

@@ -4098,17 +4104,6 @@ public void prepareISO(final Connection conn, final String vmName, List<String[]
40984104
}
40994105
final VM vm = vms.iterator().next();
41004106

4101-
if (vmDataList != null) {
4102-
// create SR
4103-
SR sr = createLocalIsoSR(conn, _configDriveSRName + getHost().getIp());
4104-
4105-
// 1. create vm data files
4106-
createVmdataFiles(vmName, vmDataList, configDriveLabel);
4107-
4108-
// 2. copy config drive iso to host
4109-
copyConfigDriveIsoToHost(conn, sr, vmName);
4110-
}
4111-
41124107
final Set<VBD> vbds = vm.getVBDs(conn);
41134108
for (final VBD vbd : vbds) {
41144109
final VBD.Record vbdr = vbd.getRecord(conn);
@@ -5449,7 +5444,7 @@ public boolean attachConfigDriveIsoToVm(final Connection conn, final VM vm) thro
54495444
public SR createLocalIsoSR(final Connection conn, final String srName) throws XenAPIException, XmlRpcException {
54505445

54515446
// if config drive sr already exists then return
5452-
SR sr = getSRByNameLabelandHost(conn, _configDriveSRName + _host.getIp());
5447+
SR sr = getSRByNameLabelandHost(conn, srName);
54535448

54545449
if (sr != null) {
54555450
s_logger.debug("Config drive SR already exist, returing it");
@@ -5542,10 +5537,10 @@ public boolean attachConfigDriveToMigratedVm(Connection conn, String vmName, Str
55425537
s_logger.debug("Attaching config drive iso device for the VM " + vmName + " In host " + ipAddr);
55435538
Set<VM> vms = VM.getByNameLabel(conn, vmName);
55445539

5545-
SR sr = getSRByNameLabel(conn, _configDriveSRName + ipAddr);
5540+
SR sr = getSRByNameLabel(conn, vmName + VM_NAME_ISO_SUFFIX);
55465541
//Here you will find only two vdis with the <vmname>.iso.
55475542
//one is from source host and second from dest host
5548-
Set<VDI> vdis = VDI.getByNameLabel(conn, vmName + ".iso");
5543+
Set<VDI> vdis = VDI.getByNameLabel(conn, vmName + VM_FILE_ISO_SUFFIX);
55495544
if (vdis.isEmpty()) {
55505545
s_logger.debug("Could not find config drive ISO: " + vmName);
55515546
return false;

‎plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/wrapper/xenbase/CitrixMigrateCommandWrapper.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,8 @@ public Answer execute(final MigrateCommand command, final CitrixResourceBase cit
9191

9292
// The iso can be attached to vm only once the vm is (present in the host) migrated.
9393
// Attach the config drive iso device to VM
94-
if (!citrixResourceBase.attachConfigDriveToMigratedVm(conn, vmName, dstHostIpAddr)) {
94+
VM vm = vms.iterator().next();
95+
if (!citrixResourceBase.attachConfigDriveIsoToVm(conn, vm)) {
9596
s_logger.debug("Config drive ISO attach failed after migration for vm "+vmName);
9697
}
9798

‎plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/wrapper/xenbase/CitrixStartCommandWrapper.java‎

Lines changed: 32 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -98,34 +98,8 @@ public Answer execute(final StartCommand command, final CitrixResourceBase citri
9898
if (vmSpec.getType() != VirtualMachine.Type.User) {
9999
citrixResourceBase.createPatchVbd(conn, vmName, vm);
100100
}
101-
// put cdrom at the first place in the list
102-
List<DiskTO> disks = new ArrayList<DiskTO>(vmSpec.getDisks().length);
103-
int index = 0;
104-
for (final DiskTO disk : vmSpec.getDisks()) {
105-
if (Volume.Type.ISO.equals(disk.getType())) {
106-
disks.add(0, disk);
107-
} else {
108-
disks.add(index, disk);
109-
}
110-
index++;
111-
}
112-
113-
for (DiskTO disk : disks) {
114-
final VDI newVdi = citrixResourceBase.prepareManagedDisk(conn, disk, vmSpec.getId(), vmSpec.getName());
115-
116-
if (newVdi != null) {
117-
final Map<String, String> data = new HashMap<>();
118101

119-
final String path = newVdi.getUuid(conn);
120-
121-
data.put(StartAnswer.PATH, path);
122-
data.put(StartAnswer.IMAGE_FORMAT, Storage.ImageFormat.VHD.toString());
123-
124-
iqnToData.put(disk.getDetails().get(DiskTO.IQN), data);
125-
}
126-
127-
citrixResourceBase.createVbd(conn, disk, vmName, vm, vmSpec.getBootloader(), newVdi);
128-
}
102+
prepareDisks(vmSpec, citrixResourceBase, conn, iqnToData, vmName, vm);
129103

130104
for (final NicTO nic : vmSpec.getNics()) {
131105
citrixResourceBase.createVif(conn, vmName, vm, vmSpec, nic);
@@ -228,4 +202,35 @@ public Answer execute(final StartCommand command, final CitrixResourceBase citri
228202
}
229203
}
230204
}
205+
206+
private void prepareDisks(VirtualMachineTO vmSpec, CitrixResourceBase citrixResourceBase, Connection conn,
207+
Map<String, Map<String, String>> iqnToData, String vmName, VM vm) throws Exception {
208+
// put cdrom at the first place in the list
209+
List<DiskTO> disks = new ArrayList<DiskTO>(vmSpec.getDisks().length);
210+
int index = 0;
211+
for (final DiskTO disk : vmSpec.getDisks()) {
212+
if (Volume.Type.ISO.equals(disk.getType())) {
213+
disks.add(0, disk);
214+
} else {
215+
disks.add(index, disk);
216+
}
217+
index++;
218+
}
219+
int isoCount = 0;
220+
for (DiskTO disk : disks) {
221+
final VDI newVdi = citrixResourceBase.prepareManagedDisk(conn, disk, vmSpec.getId(), vmSpec.getName());
222+
if (newVdi != null) {
223+
final Map<String, String> data = new HashMap<>();
224+
final String path = newVdi.getUuid(conn);
225+
data.put(StartAnswer.PATH, path);
226+
data.put(StartAnswer.IMAGE_FORMAT, Storage.ImageFormat.VHD.toString());
227+
iqnToData.put(disk.getDetails().get(DiskTO.IQN), data);
228+
}
229+
citrixResourceBase.createVbd(conn, disk, vmName, vm, vmSpec.getBootloader(), newVdi, isoCount);
230+
231+
if (disk.getType() == Volume.Type.ISO) {
232+
isoCount++;
233+
}
234+
}
235+
}
231236
}

‎server/src/main/java/com/cloud/network/element/ConfigDriveNetworkElement.java‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -305,18 +305,16 @@ public boolean postStateTransitionEvent(StateMachine2.Transition<VirtualMachine.
305305

306306
@Override
307307
public boolean prepareMigration(NicProfile nic, Network network, VirtualMachineProfile vm, DeployDestination dest, ReservationContext context) {
308-
if (nic.isDefaultNic() && _networkModel.getUserDataUpdateProvider(network).getProvider().equals(Provider.ConfigDrive)) {
308+
if (_networkModel.getUserDataUpdateProvider(network).getProvider().equals(Provider.ConfigDrive)) {
309309
LOG.trace(String.format("[prepareMigration] for vm: %s", vm.getInstanceName()));
310-
final DataStore dataStore = findDataStore(vm, dest);
311-
312310
try {
313-
addConfigDriveDisk(vm, dataStore);
314-
} catch (ResourceUnavailableException e) {
311+
addPasswordAndUserdata(network, nic, vm, dest, context);
312+
} catch (InsufficientCapacityException | ResourceUnavailableException e) {
315313
LOG.error("Failed to add config disk drive due to: ", e);
314+
return false;
316315
}
317-
return false;
318316
}
319-
else return true;
317+
return true;
320318
}
321319

322320
@Override

0 commit comments

Comments
 (0)