From baf02101bb1ff81a7dac92cd7ff3b5abe872079a Mon Sep 17 00:00:00 2001 From: Harikrishna Patnala Date: Wed, 31 Jan 2024 10:41:59 +0530 Subject: [PATCH] Address review comments --- .../storage/volume/VolumeServiceImpl.java | 3 ++- ...virtCheckAndRepairVolumeCommandWrapper.java | 4 ++-- .../apache/cloudstack/utils/qemu/QemuImg.java | 18 +++++++++--------- .../cloud/storage/VolumeApiServiceImpl.java | 18 +++++++----------- 4 files changed, 20 insertions(+), 23 deletions(-) diff --git a/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java b/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java index 580e7207404..ed2f72ffd3e 100644 --- a/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java +++ b/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java @@ -128,6 +128,7 @@ import com.cloud.storage.VMTemplateStorageResourceAssoc.Status; import com.cloud.storage.VMTemplateVO; import com.cloud.storage.Volume; import com.cloud.storage.Volume.State; +import com.cloud.storage.VolumeApiServiceImpl; import com.cloud.storage.VolumeDetailVO; import com.cloud.storage.VolumeVO; import com.cloud.storage.dao.VMTemplateDao; @@ -2768,7 +2769,7 @@ public class VolumeServiceImpl implements VolumeService { @Override public void checkAndRepairVolumeBasedOnConfig(DataObject dataObject, Host host) { if (HypervisorType.KVM.equals(host.getHypervisorType()) && DataObjectType.VOLUME.equals(dataObject.getType())) { - if (com.cloud.storage.VolumeApiServiceImpl.AllowCheckAndRepairVolume.value()) { + if (VolumeApiServiceImpl.AllowCheckAndRepairVolume.value()) { s_logger.info(String.format("Trying to check and repair the volume %d", dataObject.getId())); String repair = CheckAndRepairVolumeCmd.RepairValues.LEAKS.name().toLowerCase(); CheckAndRepairVolumePayload payload = new CheckAndRepairVolumePayload(repair); diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCheckAndRepairVolumeCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCheckAndRepairVolumeCommandWrapper.java index 38eb3187516..a9d9cd948e6 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCheckAndRepairVolumeCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCheckAndRepairVolumeCommandWrapper.java @@ -66,7 +66,7 @@ public class LibvirtCheckAndRepairVolumeCommandWrapper extends CommandWrapper qemuObjects, final String repair) throws QemuImgException { - final Script s = new Script(_qemuImgPath); - s.add("check"); + final Script script = new Script(_qemuImgPath); + script.add("check"); if (imageOptions == null) { - s.add(file.getFileName()); + script.add(file.getFileName()); } for (QemuObject o : qemuObjects) { - s.add(o.toCommandFlag()); + script.add(o.toCommandFlag()); } if (imageOptions != null) { - s.add(imageOptions.toCommandFlag()); + script.add(imageOptions.toCommandFlag()); } - s.add("--output=json"); + script.add("--output=json"); if (StringUtils.isNotEmpty(repair)) { - s.add("-r"); - s.add(repair); + script.add("-r"); + script.add(repair); } OutputInterpreter.AllLinesParser parser = new OutputInterpreter.AllLinesParser(); - final String result = s.execute(parser); + final String result = script.execute(parser); if (result != null) { throw new QemuImgException(result); } diff --git a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java index dcf9fb3f80d..36b0aee98c5 100644 --- a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java +++ b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java @@ -1340,7 +1340,7 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic outcome.get(); } catch (InterruptedException e) { throw new RuntimeException("Operation was interrupted", e); - } catch (java.util.concurrent.ExecutionException e) { + } catch (ExecutionException e) { throw new RuntimeException("Execution exception", e); } @@ -1853,7 +1853,7 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic outcome.get(); } catch (InterruptedException e) { throw new RuntimeException("Operation is interrupted", e); - } catch (java.util.concurrent.ExecutionException e) { + } catch (ExecutionException e) { throw new RuntimeException("Execution exception--", e); } @@ -1922,10 +1922,6 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic throw new InvalidParameterValueException("Checking volume and repairing failed due to volume:" + volumeId + " doesn't exist"); } - if (volume.getState() != Volume.State.Ready) { - throw new InvalidParameterValueException("VolumeId: " + volumeId + " is not in " + Volume.State.Ready + " state but " + volume.getState() + ". Cannot check and repair the volume."); - } - CheckAndRepairVolumePayload payload = new CheckAndRepairVolumePayload(repair); volume.addPayload(payload); @@ -2132,7 +2128,7 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic outcome.get(); } catch (InterruptedException e) { throw new RuntimeException("Operation was interrupted", e); - } catch (java.util.concurrent.ExecutionException e) { + } catch (ExecutionException e) { throw new RuntimeException("Execution exception", e); } @@ -2918,7 +2914,7 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic outcome.get(); } catch (InterruptedException e) { throw new RuntimeException("Operation is interrupted", e); - } catch (java.util.concurrent.ExecutionException e) { + } catch (ExecutionException e) { throw new RuntimeException("Execution excetion", e); } @@ -3326,7 +3322,7 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic outcome.get(); } catch (InterruptedException e) { throw new RuntimeException("Operation is interrupted", e); - } catch (java.util.concurrent.ExecutionException e) { + } catch (ExecutionException e) { throw new RuntimeException("Execution excetion", e); } @@ -3655,7 +3651,7 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic outcome.get(); } catch (InterruptedException e) { throw new RuntimeException("Operation is interrupted", e); - } catch (java.util.concurrent.ExecutionException e) { + } catch (ExecutionException e) { throw new RuntimeException("Execution excetion", e); } @@ -3972,7 +3968,7 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic outcome.get(); } catch (InterruptedException e) { throw new RuntimeException("Operation is interrupted", e); - } catch (java.util.concurrent.ExecutionException e) { + } catch (ExecutionException e) { throw new RuntimeException("Execution excetion", e); }