Skip to content
Open
5 changes: 4 additions & 1 deletion engine/schema/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@
</dependencies>
<executions>
<execution>
<?m2e execute onConfiguration,onIncremental?>
<id>setproperty</id>
<phase>validate</phase>
<goals>
Expand All @@ -90,6 +91,7 @@
</configuration>
</execution>
<execution>
<?m2e execute onConfiguration,onIncremental?>
<id>set-properties</id>
<phase>generate-sources</phase>
<goals>
Expand All @@ -106,7 +108,7 @@
templateList.add("systemvmtemplate-${csVersion}.${patch}-x86_64-xen")
templateList.add("systemvmtemplate-${csVersion}.${patch}-x86_64-ovm")
templateList.add("systemvmtemplate-${csVersion}.${patch}-x86_64-hyperv")
File file = new File("./engine/schema/dist/systemvm-templates/md5sum.txt")
File file = new File("${basedir}/../../engine/schema/dist/systemvm-templates/md5sum.txt")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't look right.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, it technically will work, although it could have been expressed:

File file = new File("${basedir}/dist/systemvm-templates/md5sum.txt")

The primary motivation was to eliminate the relative File path that seems to cause problems in Visual Studio Code during compilation.

def lines = file.readLines()
for (template in templateList) {
def data = lines.findAll { it.contains(template) }
Expand All @@ -128,6 +130,7 @@
<artifactId>download-maven-plugin</artifactId>
<version>1.6.3</version>
<executions>
<?m2e execute onConfiguration,onIncremental?>
<execution>
<id>download-checksums</id>
<phase>validate</phase>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
// Licensed to the Apache Software Foundation (ASF) under one
// or more contributor license agreements. See the NOTICE file
// distributed with this work for additional information
// regarding copyright ownership. The ASF licenses this file
// to you under the Apache License, Version 2.0 (the
// "License"); you may not use this file except in compliance
// with the License. You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing,
// software distributed under the License is distributed on an
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
// KIND, either express or implied. See the License for the
// specific language governing permissions and limitations
// under the License.
package com.cloud.hypervisor.kvm.storage;

import java.util.Map;

/**
* Decorator for StorageAdapters that implement asynchronous physical disk connections to improve
* performance on VM starts with large numbers of disks.
*/
public interface AsyncPhysicalDiskConnectorDecorator {
/**
* Initiates a connection attempt (may or may not complete it depending on implementation)
* @param path
* @param pool
* @param details
* @return
*/
public boolean startConnectPhysicalDisk(String path, KVMStoragePool pool, Map<String,String> details);

/**
* Tests if the physical disk is connected
* @param path
* @param pool
* @param details
* @return
*/
public boolean isConnected(String path, KVMStoragePool pool, Map<String,String> details);

/**
* Completes a connection attempt after isConnected returns true;
* @param path
* @param pool
* @param details
* @return
* @throws Exception
*/
public boolean finishConnectPhysicalDisk(String path, KVMStoragePool pool, Map<String,String> details) throws Exception;
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import java.lang.reflect.Modifier;
import java.net.URI;
import java.net.URISyntaxException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
Expand All @@ -42,9 +43,11 @@
import com.cloud.hypervisor.kvm.resource.KVMHABase.PoolType;
import com.cloud.hypervisor.kvm.resource.KVMHAMonitor;
import com.cloud.storage.Storage;
import com.cloud.storage.StorageManager;
import com.cloud.storage.Storage.StoragePoolType;
import com.cloud.storage.StorageLayer;
import com.cloud.storage.Volume;
import com.cloud.utils.StringUtils;
import com.cloud.utils.Pair;
import com.cloud.utils.Ternary;
import com.cloud.utils.exception.CloudRuntimeException;
Expand Down Expand Up @@ -164,13 +167,30 @@ public boolean connectPhysicalDisk(StoragePoolType type, String poolUuid, String
return adaptor.connectPhysicalDisk(volPath, pool, details, false);
}

private static class ConnectingDiskInfo {
ConnectingDiskInfo(VolumeObjectTO volume, StorageAdaptor adaptor, KVMStoragePool pool, Map<String, String> details) {
this.volume = volume;
this.adapter = adaptor;
this.pool = pool;
this.details = details;
}
VolumeObjectTO volume;
KVMStoragePool pool = null;
StorageAdaptor adapter = null;
Map<String,String> details = null;
}

public boolean connectPhysicalDisksViaVmSpec(VirtualMachineTO vmSpec, boolean isVMMigrate) {
boolean result = false;

final String vmName = vmSpec.getName();

List<DiskTO> disks = Arrays.asList(vmSpec.getDisks());


// disks that connect in background
List<ConnectingDiskInfo> connectingDisks = new ArrayList<>();

for (DiskTO disk : disks) {
if (disk.getType() == Volume.Type.ISO) {
result = true;
Expand All @@ -187,17 +207,79 @@ public boolean connectPhysicalDisksViaVmSpec(VirtualMachineTO vmSpec, boolean is
KVMStoragePool pool = getStoragePool(store.getPoolType(), store.getUuid());
StorageAdaptor adaptor = getStorageAdaptor(pool.getType());

result = adaptor.connectPhysicalDisk(vol.getPath(), pool, disk.getDetails(), isVMMigrate);
if (adaptor instanceof AsyncPhysicalDiskConnectorDecorator) {
// If the adaptor supports async disk connection, we can start the connection
// and return immediately, allowing the connection to complete in the background.
result = ((AsyncPhysicalDiskConnectorDecorator) adaptor).startConnectPhysicalDisk(vol.getPath(), pool, disk.getDetails());
if (!result) {
logger.error("Failed to start connecting disks via vm spec for vm: " + vmName + " volume:" + vol.toString());
return false;
}

if (!result) {
logger.error("Failed to connect disks via vm spec for vm: " + vmName + " volume:" + vol.toString());
return result;
// add disk to list of disks to check later
connectingDisks.add(new ConnectingDiskInfo(vol, adaptor, pool, disk.getDetails()));
} else {
result = adaptor.connectPhysicalDisk(vol.getPath(), pool, disk.getDetails(), isVMMigrate);

if (!result) {
logger.error("Failed to connect disks via vm spec for vm: " + vmName + " volume:" + vol.toString());
return result;
}
}
}

// if we have any connecting disks to check, wait for them to connect or timeout
if (!connectingDisks.isEmpty()) {
for (ConnectingDiskInfo connectingDisk : connectingDisks) {
StorageAdaptor adaptor = connectingDisk.adapter;
KVMStoragePool pool = connectingDisk.pool;
VolumeObjectTO volume = connectingDisk.volume;
Map<String, String> details = connectingDisk.details;
long diskWaitTimeMillis = getDiskWaitTimeMillis(details);

// wait for the disk to connect
long startTime = System.currentTimeMillis();
while (System.currentTimeMillis() - startTime < diskWaitTimeMillis) {
if (((AsyncPhysicalDiskConnectorDecorator) adaptor).isConnected(volume.getPath(), pool, details)) {
logger.debug(String.format("Disk %s connected successfully for VM %s", volume.getPath(), vmName));
break;
}

sleep(1000); // wait for 1 second before checking again
}
}
}

return result;
}

private long getDiskWaitTimeMillis(Map<String,String> details) {
int waitTimeInSec = 60; // default wait time in seconds
if (details != null && details.containsKey(StorageManager.STORAGE_POOL_DISK_WAIT.toString())) {
String waitTime = details.get(StorageManager.STORAGE_POOL_DISK_WAIT.toString());
if (StringUtils.isNotEmpty(waitTime)) {
waitTimeInSec = Integer.valueOf(waitTime).intValue();
logger.debug(String.format("%s set to %s", waitTimeInSec, StorageManager.STORAGE_POOL_DISK_WAIT.toString()));
}
} else {
// wait at least 60 seconds even if input was lower
if (waitTimeInSec < 60) {
logger.debug(String.format("%s was less than 60s. Increasing to 60s default.", StorageManager.STORAGE_POOL_DISK_WAIT.toString()));
waitTimeInSec = 60;
}
}
return waitTimeInSec * 1000; // convert to milliseconds
}

private boolean sleep(long millis) {
try {
Thread.sleep(millis);
return true;
} catch (InterruptedException e) {
return false;
}
}

public boolean disconnectPhysicalDisk(Map<String, String> volumeToDisconnect) {
logger.debug(String.format("Disconnect physical disks using volume map: %s", volumeToDisconnect.toString()));
if (MapUtils.isEmpty(volumeToDisconnect)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1607,8 +1607,20 @@ public Answer dettachVolume(final DettachCommand cmd) {
storagePoolMgr.disconnectPhysicalDisk(primaryStore.getPoolType(), primaryStore.getUuid(), vol.getPath());

return new DettachAnswer(disk);
} catch (final LibvirtException | InternalErrorException | CloudRuntimeException e) {
logger.debug(String.format("Failed to detach volume [id: %d, uuid: %s, name: %s, path: %s], due to ", vol.getId(), vol.getUuid(), vol.getName(), vol.getPath()), e);
} catch (final LibvirtException e) {
// check if the error was related to an already unplugged event - we can safely ignore
if (e.getMessage() != null && e.getMessage().contains("is already in the process of unplug")) {
logger.debug("Volume: " + vol.getPath() + " is already unplugged, ignoring the error");
return new DettachAnswer(disk);
} else {
logger.debug("Failed to detach volume: " + vol.getPath() + ", due to ", e);
return new DettachAnswer(e.toString());
}
} catch (final InternalErrorException e) {
logger.debug("Failed to detach volume: " + vol.getPath() + ", due to ", e);
return new DettachAnswer(e.toString());
} catch (final CloudRuntimeException e) {
logger.debug("Failed to detach volume: " + vol.getPath() + ", due to ", e);
return new DettachAnswer(e.toString());
} finally {
vol.clearPassphrase();
Expand Down
Loading
Loading