addressed review comments

This commit is contained in:
Harikrishna Patnala 2024-01-09 16:28:19 +05:30
parent 702aa801c9
commit b3d4f56f69
8 changed files with 28 additions and 51 deletions

View File

@ -52,7 +52,7 @@ public class CheckVolumeAndRepairCmd extends BaseCmd {
@Parameter(name = ApiConstants.ID, type = CommandType.UUID, entityType = VolumeResponse.class, required = true, description = "The ID of the volume")
private Long id;
@Parameter(name = ApiConstants.REPAIR, type = CommandType.BOOLEAN, required = false, description = "true if the volume has leaks and repair the volume")
@Parameter(name = ApiConstants.REPAIR, type = CommandType.BOOLEAN, required = false, description = "true to repair the volume, repairs if it has any leaks")
private Boolean repair;
/////////////////////////////////////////////////////

View File

@ -22,55 +22,36 @@ package com.cloud.agent.api.storage;
import com.cloud.agent.api.Answer;
public class CheckVolumeAndRepairAnswer extends Answer {
private long leaks;
private boolean repaired;
private long leaksFixed;
private String volumeCheckExecutionResult;
private String volumeRepairedExecutionResult;
private String volumeRepairExecutionResult;
protected CheckVolumeAndRepairAnswer() {
super();
}
public CheckVolumeAndRepairAnswer(CheckVolumeAndRepairCommand cmd, boolean result, String details, long leaks,
boolean repaired, long leaksFixed, String volumeCheckExecutionResult, String volumeRepairedExecutionResult) {
public CheckVolumeAndRepairAnswer(CheckVolumeAndRepairCommand cmd, boolean result, String details, String volumeCheckExecutionResult, String volumeRepairedExecutionResult) {
super(cmd, result, details);
this.leaks = leaks;
this.repaired = repaired;
this.leaksFixed = leaksFixed;
this.volumeCheckExecutionResult = volumeCheckExecutionResult;
this.volumeRepairedExecutionResult = volumeRepairedExecutionResult;
this.volumeRepairExecutionResult = volumeRepairedExecutionResult;
}
public CheckVolumeAndRepairAnswer(CheckVolumeAndRepairCommand cmd, boolean result, String details) {
super(cmd, result, details);
}
public long getLeaks() {
return leaks;
}
public boolean isRepaired() {
return repaired;
}
public long getLeaksFixed() {
return leaksFixed;
}
public String getVolumeCheckExecutionResult() {
return volumeCheckExecutionResult;
}
public String getVolumeRepairedExecutionResult() {
return volumeRepairedExecutionResult;
public String getVolumeRepairExecutionResult() {
return volumeRepairExecutionResult;
}
public void setVolumeCheckExecutionResult(String volumeCheckExecutionResult) {
this.volumeCheckExecutionResult = volumeCheckExecutionResult;
}
public void setVolumeRepairedExecutionResult(String volumeRepairedExecutionResult) {
this.volumeRepairedExecutionResult = volumeRepairedExecutionResult;
public void setVolumeRepairExecutionResult(String volumeRepairExecutionResult) {
this.volumeRepairExecutionResult = volumeRepairExecutionResult;
}
}

View File

@ -2777,9 +2777,9 @@ public class VolumeServiceImpl implements VolumeService {
s_logger.debug("Check volume response result: " + answer.getDetails());
payload.setVolumeCheckExecutionResult(answer.getVolumeCheckExecutionResult());
if (payload.isRepair()) {
payload.setVolumeRepairedExecutionResult(answer.getVolumeRepairedExecutionResult());
payload.setVolumeRepairedExecutionResult(answer.getVolumeRepairExecutionResult());
}
return new Pair<>(answer.getVolumeCheckExecutionResult(), answer.getVolumeRepairedExecutionResult());
return new Pair<>(answer.getVolumeCheckExecutionResult(), answer.getVolumeRepairExecutionResult());
} else {
s_logger.debug("Failed to check and repair the volume with error " + answer.getDetails());
}

View File

@ -78,7 +78,7 @@ public class LibvirtCheckVolumeAndRepairCommandWrapper extends CommandWrapper<Ch
s_logger.info(String.format("Repair Volume result for the volume %s is %s", vol.getName(), repairVolumeResult));
answer = new CheckVolumeAndRepairAnswer(command, true, finalResult);
answer.setVolumeRepairedExecutionResult(repairVolumeResult);
answer.setVolumeRepairExecutionResult(repairVolumeResult);
answer.setVolumeCheckExecutionResult(checkVolumeResult);
}
return answer;

View File

@ -826,7 +826,9 @@ public class QemuImg {
public String checkAndRepair(final QemuImgFile file, final QemuImageOptions imageOptions, final List<QemuObject> qemuObjects, final boolean repair) throws QemuImgException {
final Script s = new Script(_qemuImgPath);
s.add("check");
s.add(file.getFileName());
if (imageOptions == null) {
s.add(file.getFileName());
}
for (QemuObject o : qemuObjects) {
s.add(o.toCommandFlag());

View File

@ -1825,23 +1825,18 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
long volumeId = cmd.getId();
boolean repair = cmd.getRepair();
validationsForCheckVolumeOperation(volumeId);
final VolumeVO volume = _volsDao.findById(volumeId);
Long vmId = volume.getInstanceId();
UserVmVO vm = null;
if (vmId != null) {
vm = _userVmDao.findById(vmId);
}
validationsForCheckVolumeOperation(volume);
if (vm != null) {
Long vmId = volume.getInstanceId();
if (vmId != null) {
// serialize VM operation
AsyncJobExecutionContext jobContext = AsyncJobExecutionContext.getCurrentExecutionContext();
if (jobContext.isJobDispatchedBy(VmWorkConstants.VM_WORK_JOB_DISPATCHER)) {
// avoid re-entrance
VmWorkJobVO placeHolder = null;
placeHolder = createPlaceHolderWork(vm.getId());
placeHolder = createPlaceHolderWork(vmId);
try {
Pair<String, String> result = orchestrateCheckVolumeAndRepair(volumeId, repair);
return result;
@ -1849,7 +1844,7 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
_workJobDao.expunge(placeHolder.getId());
}
} else {
Outcome<Pair> outcome = checkVolumeAndRepairThroughJobQueue(vm.getId(), volumeId, repair);
Outcome<Pair> outcome = checkVolumeAndRepairThroughJobQueue(vmId, volumeId, repair);
try {
outcome.get();
@ -1887,15 +1882,14 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
}
}
protected void validationsForCheckVolumeOperation(long volumeId) {
final VolumeVO volume = _volsDao.findById(volumeId);
protected void validationsForCheckVolumeOperation(VolumeVO volume) {
Account caller = CallContext.current().getCallingAccount();
_accountMgr.checkAccess(caller, null, true, volume);
Long volumeId = volume.getId();
Long vmId = volume.getInstanceId();
UserVmVO vm = null;
if (vmId != null) {
vm = _userVmDao.findById(vmId);
UserVmVO vm = _userVmDao.findById(vmId);
if (vm == null) {
throw new InvalidParameterValueException(String.format("VM not found, please check the VM to which this volume %d is attached", volumeId));
}

View File

@ -1682,7 +1682,7 @@ public class VolumeApiServiceImplTest {
when(volume.getId()).thenReturn(1L);
when(volumeDaoMock.getHypervisorType(1L)).thenReturn(HypervisorType.KVM);
volumeApiServiceImpl.validationsForCheckVolumeOperation(1L);
volumeApiServiceImpl.validationsForCheckVolumeOperation(volume);
}
@Test(expected = InvalidParameterValueException.class)
@ -1701,7 +1701,7 @@ public class VolumeApiServiceImplTest {
when(userVmDaoMock.findById(1L)).thenReturn(vm);
when(vm.getState()).thenReturn(State.Running);
volumeApiServiceImpl.validationsForCheckVolumeOperation(1L);
volumeApiServiceImpl.validationsForCheckVolumeOperation(volume);
}
@Test(expected = InvalidParameterValueException.class)
@ -1718,7 +1718,7 @@ public class VolumeApiServiceImplTest {
when(volume.getInstanceId()).thenReturn(1L);
when(userVmDaoMock.findById(1L)).thenReturn(null);
volumeApiServiceImpl.validationsForCheckVolumeOperation(1L);
volumeApiServiceImpl.validationsForCheckVolumeOperation(volume);
}
@Test(expected = InvalidParameterValueException.class)
@ -1738,7 +1738,7 @@ public class VolumeApiServiceImplTest {
when(vm.getState()).thenReturn(State.Stopped);
when(volume.getState()).thenReturn(Volume.State.Allocated);
volumeApiServiceImpl.validationsForCheckVolumeOperation(1L);
volumeApiServiceImpl.validationsForCheckVolumeOperation(volume);
}
@Test(expected = InvalidParameterValueException.class)
@ -1760,7 +1760,7 @@ public class VolumeApiServiceImplTest {
when(volume.getId()).thenReturn(1L);
when(volumeDaoMock.getHypervisorType(1L)).thenReturn(HypervisorType.VMware);
volumeApiServiceImpl.validationsForCheckVolumeOperation(1L);
volumeApiServiceImpl.validationsForCheckVolumeOperation(volume);
}
@Test

View File

@ -291,7 +291,7 @@ public class StringUtils {
ObjectMapper objectMapper = new ObjectMapper();
Map<String, String> mapResult = new HashMap<>();
if (org.apache.commons.lang3.StringUtils.isNotEmpty(jsonString)) {
if (org.apache.commons.lang3.StringUtils.isNotBlank(jsonString)) {
try {
JsonNode jsonNode = objectMapper.readTree(jsonString);
jsonNode.fields().forEachRemaining(entry -> {