DaanHoogland commented on code in PR #6508:
URL: https://github.com/apache/cloudstack/pull/6508#discussion_r907459468


##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java:
##########
@@ -1169,26 +1177,37 @@ private String getDataStoreUrlFromStore(DataStoreTO 
store) {
         return store.getUrl();
     }
 
-    protected synchronized String attachOrDetachDevice(final Connect conn, 
final boolean attach, final String vmName, final String xml) throws 
LibvirtException, InternalErrorException {
+    protected synchronized String attachOrDetachDevice(final Connect conn, 
final boolean attach, final String vmName, final String xml)
+            throws LibvirtException, InternalErrorException {
         Domain dm = null;
         try {
             dm = conn.domainLookupByName(vmName);
 
             if (attach) {
                 s_logger.debug("Attaching device: " + xml);
                 dm.attachDevice(xml);
-            } else {
-                s_logger.debug("Detaching device: " + xml);
-                dm.detachDevice(xml);
-                LibvirtDomainXMLParser parser = new LibvirtDomainXMLParser();
-                parser.parseDomainXML(dm.getXMLDesc(0));
-                List<DiskDef> disks = parser.getDisks();
-                for (DiskDef diskDef : disks) {
-                    if (StringUtils.contains(xml, diskDef.getDiskPath())) {
-                        throw new InternalErrorException("Could not detach 
volume. Probably the VM is in boot state at the moment");
-                    }
+                return null;

Review Comment:
   better throw an exception or a specific error value. this is a bad pattern.



-- 
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]

Reply via email to