abh1sar commented on code in PR #11560:
URL: https://github.com/apache/cloudstack/pull/11560#discussion_r2320764918
##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapper.java:
##########
@@ -140,52 +146,59 @@ private void restoreVolume(String backupPath, String
backupRepoType, String back
throw new CloudRuntimeException(String.format("Failed to
attach volume to VM: %s", vmNameAndState.first()));
}
}
- } catch (Exception e) {
- throw new CloudRuntimeException("Failed to restore volume", e);
} finally {
unmountBackupDirectory(mountDirectory);
deleteTemporaryDirectory(mountDirectory);
}
}
- private String mountBackupDirectory(String backupRepoAddress, String
backupRepoType, String mountOptions) {
+ private String mountBackupDirectory(String backupRepoAddress, String
backupRepoType, String mountOptions, Integer mountTimeout) {
String randomChars = RandomStringUtils.random(5, true, false);
String mountDirectory = String.format("%s.%s",BACKUP_TEMP_FILE_PREFIX
, randomChars);
+
try {
mountDirectory =
Files.createTempDirectory(mountDirectory).toString();
- String mount = String.format(MOUNT_COMMAND, backupRepoType,
backupRepoAddress, mountDirectory);
- if ("cifs".equals(backupRepoType)) {
- if (Objects.isNull(mountOptions) ||
mountOptions.trim().isEmpty()) {
- mountOptions = "nobrl";
- } else {
- mountOptions += ",nobrl";
- }
- }
- if (Objects.nonNull(mountOptions) &&
!mountOptions.trim().isEmpty()) {
- mount += " -o " + mountOptions;
+ } catch (IOException e) {
+ logger.error(String.format("Failed to create the tmp mount
directory {} for restore", mountDirectory), e);
+ throw new CloudRuntimeException("Failed to create the tmp mount
directory for restore on the KVM host");
+ }
+
+ String mount = String.format(MOUNT_COMMAND, backupRepoType,
backupRepoAddress, mountDirectory);
+ if ("cifs".equals(backupRepoType)) {
+ if (Objects.isNull(mountOptions) || mountOptions.trim().isEmpty())
{
+ mountOptions = "nobrl";
+ } else {
+ mountOptions += ",nobrl";
}
- Script.runSimpleBashScript(mount);
- } catch (Exception e) {
- throw new CloudRuntimeException(String.format("Failed to mount %s
to %s", backupRepoType, backupRepoAddress), e);
+ }
+ if (Objects.nonNull(mountOptions) && !mountOptions.trim().isEmpty()) {
+ mount += " -o " + mountOptions;
+ }
+
+ int exitValue = Script.runSimpleBashScriptForExitValue(mount,
mountTimeout, false);
+ if (exitValue != 0) {
+ logger.error(String.format("Failed to mount repository {} of type
{} to the directory {}", backupRepoAddress, backupRepoType, mountDirectory));
+ throw new CloudRuntimeException("Failed to mount the backup
repository on the KVM host");
}
return mountDirectory;
}
private void unmountBackupDirectory(String backupDirectory) {
- try {
- String umountCmd = String.format(UMOUNT_COMMAND, backupDirectory);
- Script.runSimpleBashScript(umountCmd);
- } catch (Exception e) {
- throw new CloudRuntimeException(String.format("Failed to unmount
backup directory: %s", backupDirectory), e);
+ String umountCmd = String.format(UMOUNT_COMMAND, backupDirectory);
+ int exitValue = Script.runSimpleBashScriptForExitValue(umountCmd);
Review Comment:
fyi - same here. runSimpleBashScript doesn't throw any exception.
##########
server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java:
##########
@@ -1310,18 +1331,21 @@ public boolean restoreBackupToVM(final Long backupId,
final Long vmId) throws Re
host = restoreInfo.first().getPrivateIpAddress();
dataStore = restoreInfo.second().getUuid();
}
- if (!backupProvider.restoreBackupToVM(vm, backup, host,
dataStore)) {
- throw new CloudRuntimeException(String.format("Error restoring
backup [%s] to VM %s.", backupDetailsInMessage, vm.getUuid()));
- }
+ result = backupProvider.restoreBackupToVM(vm, backup, host,
dataStore);
+
} catch (Exception e) {
- updateVolumeState(vm, Volume.Event.RestoreFailed,
Volume.State.Ready);
- updateVmState(vm, VirtualMachine.Event.RestoringFailed,
VirtualMachine.State.Stopped);
- logger.error(String.format("Failed to create Instance [%s] from
backup [%s] due to: [%s].", vm.getInstanceName(), backupDetailsInMessage,
e.getMessage()), e);
- ActionEventUtils.onCompletedActionEvent(User.UID_SYSTEM,
vm.getAccountId(), EventVO.LEVEL_ERROR, EventTypes.EVENT_VM_CREATE_FROM_BACKUP,
- String.format("Failed to create Instance %s from backup
%s", vm.getInstanceName(), backup.getUuid()),
- vm.getId(),
ApiCommandResourceType.VirtualMachine.toString(), eventId);
+ logger.error(String.format("Failed to create Instance [%s] from
backup [%s] due to: [%s]", vm.getInstanceName(), backupDetailsInMessage,
e.getMessage()), e);
+ processRestoreBackupToVMFailure(vm, backup, eventId);
throw new CloudRuntimeException(String.format("Error while
creating Instance [%s] from backup [%s].", vm.getUuid(),
backupDetailsInMessage));
}
+
+ if (result != null && !result.first()) {
+ String error_msg = String.format("Failed to create Instance [%s]
from backup [%s] due to: %s.", vm.getInstanceName(), backupDetailsInMessage,
result.second());
+ logger.error(error_msg);
+ processRestoreBackupToVMFailure(vm, backup, eventId);
+ throw new CloudRuntimeException(error_msg);
Review Comment:
fyi - this is done to pass on the error passed by the provider.
--
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]
