stephankruggg commented on code in PR #6508:
URL: https://github.com/apache/cloudstack/pull/6508#discussion_r1032637581
##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java:
##########
@@ -1206,15 +1219,115 @@ protected synchronized String
attachOrDetachDevice(final Connect conn, final boo
}
}
}
+ }
- return null;
+ /**
+ * Waits {@link #waitDelayForVirshCommands} milliseconds before checking
again if the device has been removed.
+ * @return The configured value in wait.detach.device reduced by {@link
#waitDelayForVirshCommands}
+ * @throws LibvirtException
+ */
+ private long getWaitAfterSleep(Domain dm, String diskPath, long wait)
throws LibvirtException {
+ try {
+ wait -= waitDelayForVirshCommands;
+ Thread.sleep(waitDelayForVirshCommands);
+ s_logger.trace(String.format("Trying to detach device [%s] from VM
instance with UUID [%s]. " +
+ "Waiting [%s] milliseconds before assuming the VM was
unable to detach the volume.", diskPath, dm.getUUIDString(), wait));
+ } catch (InterruptedException e) {
+ throw new CloudRuntimeException(e);
+ }
+ return wait;
+ }
+
+ /**
+ * Checks if the device has been removed from the instance
+ * @param diskPath Path to the device that was removed
+ * @param dm instance to be checked if the device was properly removed
+ * @throws LibvirtException
+ */
+ protected boolean checkDetachSuccess(String diskPath, Domain dm) throws
LibvirtException {
+ LibvirtDomainXMLParser parser = new LibvirtDomainXMLParser();
+ parser.parseDomainXML(dm.getXMLDesc(0));
+ List<DiskDef> disks = parser.getDisks();
+ for (DiskDef diskDef : disks) {
+ if (StringUtils.equals(diskPath, diskDef.getDiskPath())) {
+ s_logger.debug(String.format("The hypervisor sent the detach
command, but it is still possible to identify the device [%s] in the instance
with UUID [%s].",
+ diskPath, dm.getUUIDString()));
+ return false;
+ }
+ }
+ return true;
}
- protected synchronized String attachOrDetachDisk(final Connect conn, final
boolean attach, final String vmName, final KVMPhysicalDisk attachingDisk, final
int devId, final String serial,
- final Long bytesReadRate, final Long bytesReadRateMax, final Long
bytesReadRateMaxLength,
- final Long bytesWriteRate, final Long bytesWriteRateMax, final
Long bytesWriteRateMaxLength,
- final Long iopsReadRate, final Long iopsReadRateMax, final Long
iopsReadRateMaxLength,
- final Long iopsWriteRate, final Long iopsWriteRateMax, final Long
iopsWriteRateMaxLength, final String cacheMode, final
DiskDef.LibvirtDiskEncryptDetails encryptDetails) throws LibvirtException,
InternalErrorException {
+ /**
+ * attach or detach a disk to an instance
Review Comment:
```suggestion
* Attaches or detaches a disk to an instance.
```
##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java:
##########
@@ -1169,27 +1162,47 @@ private String getDataStoreUrlFromStore(DataStoreTO
store) {
}
return store.getUrl();
}
+ protected synchronized void attachOrDetachDevice(final Connect conn, final
boolean attach, final String vmName, final DiskDef xml)
+ throws LibvirtException, InternalErrorException {
+ attachOrDetachDevice(conn, attach, vmName, xml, 0l);
+ }
- protected synchronized String attachOrDetachDevice(final Connect conn,
final boolean attach, final String vmName, final String xml) throws
LibvirtException, InternalErrorException {
+ /**
+ * attach or detach a device (ISO or disk) to an instance
Review Comment:
```suggestion
* Attaches or detaches a device (ISO or disk) to an instance.
```
##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java:
##########
@@ -1206,15 +1219,115 @@ protected synchronized String
attachOrDetachDevice(final Connect conn, final boo
}
}
}
+ }
- return null;
+ /**
+ * Waits {@link #waitDelayForVirshCommands} milliseconds before checking
again if the device has been removed.
+ * @return The configured value in wait.detach.device reduced by {@link
#waitDelayForVirshCommands}
+ * @throws LibvirtException
+ */
+ private long getWaitAfterSleep(Domain dm, String diskPath, long wait)
throws LibvirtException {
+ try {
+ wait -= waitDelayForVirshCommands;
+ Thread.sleep(waitDelayForVirshCommands);
+ s_logger.trace(String.format("Trying to detach device [%s] from VM
instance with UUID [%s]. " +
+ "Waiting [%s] milliseconds before assuming the VM was
unable to detach the volume.", diskPath, dm.getUUIDString(), wait));
+ } catch (InterruptedException e) {
+ throw new CloudRuntimeException(e);
+ }
+ return wait;
+ }
+
+ /**
+ * Checks if the device has been removed from the instance
+ * @param diskPath Path to the device that was removed
+ * @param dm instance to be checked if the device was properly removed
+ * @throws LibvirtException
+ */
+ protected boolean checkDetachSuccess(String diskPath, Domain dm) throws
LibvirtException {
+ LibvirtDomainXMLParser parser = new LibvirtDomainXMLParser();
+ parser.parseDomainXML(dm.getXMLDesc(0));
+ List<DiskDef> disks = parser.getDisks();
+ for (DiskDef diskDef : disks) {
+ if (StringUtils.equals(diskPath, diskDef.getDiskPath())) {
+ s_logger.debug(String.format("The hypervisor sent the detach
command, but it is still possible to identify the device [%s] in the instance
with UUID [%s].",
+ diskPath, dm.getUUIDString()));
+ return false;
+ }
+ }
+ return true;
}
- protected synchronized String attachOrDetachDisk(final Connect conn, final
boolean attach, final String vmName, final KVMPhysicalDisk attachingDisk, final
int devId, final String serial,
- final Long bytesReadRate, final Long bytesReadRateMax, final Long
bytesReadRateMaxLength,
- final Long bytesWriteRate, final Long bytesWriteRateMax, final
Long bytesWriteRateMaxLength,
- final Long iopsReadRate, final Long iopsReadRateMax, final Long
iopsReadRateMaxLength,
- final Long iopsWriteRate, final Long iopsWriteRateMax, final Long
iopsWriteRateMaxLength, final String cacheMode, final
DiskDef.LibvirtDiskEncryptDetails encryptDetails) throws LibvirtException,
InternalErrorException {
+ /**
+ * attach or detach a disk to an instance
+ * @param conn libvirt connection
+ * @param attach boolean that determines whether the device will be
attached or detached
+ * @param vmName instance name
+ * @param attachingDisk kvm physical disk
+ * @param devId device id in instance
+ * @param serial
+ * @param bytesReadRate bytes read rate
+ * @param bytesReadRateMax bytes read rate max
+ * @param bytesReadRateMaxLength bytes read rate max length
+ * @param bytesWriteRate bytes write rate
+ * @param bytesWriteRateMax bytes write rate amx
+ * @param bytesWriteRateMaxLength bytes write rate max length
+ * @param iopsReadRate iops read rate
+ * @param iopsReadRateMax iops read rate max
+ * @param iopsReadRateMaxLength iops read rate max length
+ * @param iopsWriteRate iops write rate
+ * @param iopsWriteRateMax iops write rate max
+ * @param iopsWriteRateMaxLength iops write rate max length
+ * @param cacheMode cache mode
+ * @param encryptDetails encrypt details
+ * @throws LibvirtException
+ * @throws InternalErrorException
+ */
+ protected synchronized void attachOrDetachDisk(final Connect conn, final
boolean attach, final String vmName, final KVMPhysicalDisk attachingDisk, final
int devId,
+ final String serial, final
Long bytesReadRate, final Long bytesReadRateMax, final Long
bytesReadRateMaxLength,
+ final Long bytesWriteRate,
final Long bytesWriteRateMax, final Long bytesWriteRateMaxLength, final Long
iopsReadRate,
+ final Long iopsReadRateMax,
final Long iopsReadRateMaxLength, final Long iopsWriteRate, final Long
iopsWriteRateMax,
+ final Long
iopsWriteRateMaxLength, final String cacheMode, final
DiskDef.LibvirtDiskEncryptDetails encryptDetails)
+ throws LibvirtException, InternalErrorException {
+ attachOrDetachDisk(conn, attach, vmName, attachingDisk, devId, serial,
bytesReadRate, bytesReadRateMax, bytesReadRateMaxLength,
+ bytesWriteRate, bytesWriteRateMax, bytesWriteRateMaxLength,
iopsReadRate, iopsReadRateMax, iopsReadRateMaxLength, iopsWriteRate,
+ iopsWriteRateMax, iopsWriteRateMaxLength, cacheMode,
encryptDetails, 0l);
+ }
+
+ /**
+ *
+ * attach or detach a disk to an instance
Review Comment:
```suggestion
* Attaches or detaches a disk to an instance.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
