bernardodemarco commented on code in PR #12758:
URL: https://github.com/apache/cloudstack/pull/12758#discussion_r3430131770
##########
api/src/main/java/com/cloud/vm/VirtualMachine.java:
##########
@@ -58,7 +58,10 @@ public enum State {
Error(false, "VM is in error"),
Unknown(false, "VM state is unknown."),
Shutdown(false, "VM state is shutdown from inside"),
- Restoring(true, "VM is being restored from backup");
+ Restoring(true, "VM is being restored from backup"),
+ BackingUp(true, "VM is being backed up"),
+ BackupError(false, "VM backup is in a inconsistent state. Operator
should analyse the logs and restore the VM"),
+ RestoreError(false, "VM restore left the VM in a inconsistent state.
Operator should analyse the logs and restore the VM");
Review Comment:
```suggestion
RestoreError(false, "VM restore left the VM in an inconsistent
state. Operator should analyse the logs and restore the VM");
```
##########
agent/conf/agent.properties:
##########
@@ -480,3 +480,16 @@ iscsi.session.cleanup.enabled=false
# Optional vCenter SHA1 thumbprint for VMware to KVM conversion via VDDK,
passed as
# -io vddk-thumbprint=<value>. If unset, CloudStack computes it on the KVM
host via openssl.
#vddk.thumbprint=
+
+# Timeout (in seconds) for QCOW2 delta merge operations, mainly used for
classic volume snapshots, disk-only VM snapshots on file-based storage, and the
KBOSS plugin.
+# If a value of 0 or less is informed, the default will be used.
+# qcow2.delta.merge.timeout=259200
+
+
Review Comment:
```suggestion
```
##########
server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java:
##########
@@ -338,6 +348,76 @@ public List<Long> getBackupOfferingDomains(Long
offeringId) {
return backupOfferingDetailsDao.findDomainIds(offeringId);
}
+ @Override
+ public BackupOffering createBackupOffering(CreateBackupOfferingCmd cmd) {
+ validateBackupForZone(cmd.getZoneId());
+ if (backupOfferingDao.findByName(cmd.getName(), cmd.getZoneId()) !=
null) {
+ throw new CloudRuntimeException("A backup offering with the same
name already exists in this zone");
+ }
+
+ if (CollectionUtils.isNotEmpty(cmd.getDomainIds())) {
+ for (final Long domainId: cmd.getDomainIds()) {
+ if (domainDao.findById(domainId) == null) {
+ throw new InvalidParameterValueException("Please specify a
valid domain id");
Review Comment:
```suggestion
throw new InvalidParameterValueException("Please specify
a valid domain ID");
```
##########
api/src/main/java/com/cloud/vm/VirtualMachine.java:
##########
@@ -58,7 +58,10 @@ public enum State {
Error(false, "VM is in error"),
Unknown(false, "VM state is unknown."),
Shutdown(false, "VM state is shutdown from inside"),
- Restoring(true, "VM is being restored from backup");
+ Restoring(true, "VM is being restored from backup"),
+ BackingUp(true, "VM is being backed up"),
+ BackupError(false, "VM backup is in a inconsistent state. Operator
should analyse the logs and restore the VM"),
Review Comment:
```suggestion
BackupError(false, "VM backup is in an inconsistent state. Operator
should analyse the logs and restore the VM"),
```
##########
server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java:
##########
@@ -1602,19 +1727,26 @@ public boolean restoreBackupVolumeAndAttachToVM(final
String backedUpVolumeUuid,
throw new CloudRuntimeException("Failed to find volume with Id " +
backedUpVolumeUuid + " in the backed-up volumes metadata");
}
- accountManager.checkAccess(CallContext.current().getCallingAccount(),
null, true, vm);
+ accountManager.checkAccess(CallContext.current().getCallingAccount(),
null, true, vmFromBackup);
final BackupOffering offering =
backupOfferingDao.findByIdIncludingRemoved(backup.getBackupOfferingId());
if (offering == null) {
throw new CloudRuntimeException("Failed to find Instance Backup
Offering");
}
+ if (!StringUtils.equals(KBOSS_BACKUP_PROVIDER, offering.getProvider())
&& !VirtualMachine.PowerState.PowerOff.equals(vm.getPowerState())) {
+ throw new CloudRuntimeException(String.format("VM [%s] needs to be
powered off to restore the volume [%s].", vm.getUuid(), backedUpVolumeUuid));
+ }
+
BackupProvider backupProvider =
getBackupProvider(offering.getProvider());
- VolumeVO backedUpVolume = volumeDao.findByUuid(backedUpVolumeUuid);
+ VolumeVO backedUpVolume =
volumeDao.findByUuidIncludingRemoved(backedUpVolumeUuid);
Pair<HostVO, StoragePoolVO> restoreInfo;
- if (!"nas".equals(offering.getProvider()) || (backedUpVolume == null))
{
- restoreInfo = getRestoreVolumeHostAndDatastore(vm);
- } else {
+
+ if ("nas".equals(offering.getProvider()) && backedUpVolume != null) {
restoreInfo = getRestoreVolumeHostAndDatastoreForNas(vm,
backedUpVolume);
+ } else if (KBOSS_BACKUP_PROVIDER.equals(offering.getProvider())){
Review Comment:
```suggestion
} else if (KBOSS_BACKUP_PROVIDER.equals(offering.getProvider())) {
```
##########
api/src/main/java/org/apache/cloudstack/api/command/user/backup/DownloadValidationScreenshotCmd.java:
##########
@@ -0,0 +1,94 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.api.command.user.backup;
+
+import com.cloud.event.EventTypes;
+import com.cloud.exception.InvalidParameterValueException;
+import com.cloud.user.Account;
+import org.apache.cloudstack.api.ACL;
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.BaseAsyncCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.response.BackupResponse;
+import org.apache.cloudstack.api.response.ExtractResponse;
+import org.apache.cloudstack.backup.Backup;
+import org.apache.cloudstack.backup.InternalBackupService;
+
+import javax.inject.Inject;
+
+@APICommand(name = "downloadValidationScreenshot", description = "Download
validation screenshot of given backup.",
+ responseObject = ExtractResponse.class, since = "4.23.0.0",
requestHasSensitiveInfo = false,
Review Comment:
```suggestion
responseObject = ExtractResponse.class, since = "4.23.0",
requestHasSensitiveInfo = false,
```
##########
api/src/main/java/org/apache/cloudstack/api/command/user/backup/RestoreBackupCmd.java:
##########
@@ -59,6 +61,14 @@ public class RestoreBackupCmd extends BaseAsyncCmd {
description = "ID of the backup")
private Long backupId;
+ @Parameter(name = ApiConstants.QUICK_RESTORE, type = CommandType.BOOLEAN,
entityType = BackupResponse.class, description = "Whether to use the quick
restore process or not. " +
+ "Currently this parameter is only supported by the KBOSS
provider.", since = "4.23.0")
+ private Boolean quickRestore;
+
+ @Parameter(name = ApiConstants.HOST_ID, type = CommandType.UUID,
entityType = HostResponse.class, description = "If quickrestore is true, which
host to start the VM on;" +
+ " otherwise, ignored. Currently this parameter is only supported
by the KBOSS provider.", since = "4.23.0")
+ private Long hostId;
Review Comment:
We could add the `authorized` field here
##########
api/src/main/java/org/apache/cloudstack/api/command/user/backup/DownloadValidationScreenshotCmd.java:
##########
@@ -0,0 +1,94 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.api.command.user.backup;
+
+import com.cloud.event.EventTypes;
+import com.cloud.exception.InvalidParameterValueException;
+import com.cloud.user.Account;
+import org.apache.cloudstack.api.ACL;
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.BaseAsyncCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.response.BackupResponse;
+import org.apache.cloudstack.api.response.ExtractResponse;
+import org.apache.cloudstack.backup.Backup;
+import org.apache.cloudstack.backup.InternalBackupService;
+
+import javax.inject.Inject;
+
+@APICommand(name = "downloadValidationScreenshot", description = "Download
validation screenshot of given backup.",
+ responseObject = ExtractResponse.class, since = "4.23.0.0",
requestHasSensitiveInfo = false,
+ responseHasSensitiveInfo = false)
+public class DownloadValidationScreenshotCmd extends BaseAsyncCmd {
+
+ @Inject
+ private InternalBackupService internalBackupService;
+
+ /////////////////////////////////////////////////////
+ //////////////// API parameters /////////////////////
+ /////////////////////////////////////////////////////
+
+ @ACL
+ @Parameter(name = ApiConstants.BACKUP_ID, type = CommandType.UUID,
entityType = BackupResponse.class, required = true,
+ description = "Id of the backup.")
Review Comment:
```suggestion
description = "ID of the backup.")
```
##########
ui/src/config/section/compute.js:
##########
@@ -292,6 +292,22 @@ export default {
}
}
},
+ {
+ api: 'finishBackupChain',
+ icon: 'vertical-align-middle-outlined',
+ label: 'label.backup.chain.finish',
+ dataView: true,
+ args: ['virtualmachineid'],
+ show: (record) => {
+ console.log(record)
Review Comment:
```suggestion
```
##########
api/src/main/java/org/apache/cloudstack/api/command/user/backup/RestoreVolumeFromBackupAndAttachToVMCmd.java:
##########
@@ -78,6 +79,14 @@ public class RestoreVolumeFromBackupAndAttachToVMCmd extends
BaseAsyncCmd {
description = "ID of the Instance where to attach the restored
volume")
private Long vmId;
+ @Parameter(name = ApiConstants.QUICK_RESTORE, type = CommandType.BOOLEAN,
description = "Whether to use the quick restore process or not. " +
+ "Currently this parameter is only supported by the KBOSS
provider.", since = "4.23.0")
+ private Boolean quickRestore;
+
+ @Parameter(name = ApiConstants.HOST_ID, type = CommandType.UUID,
entityType = HostResponse.class, description = "If quickrestore is true, which
host to start the VM on;" +
+ " otherwise, ignored. Currently this parameter is only supported
by the KBOSS provider.", since = "4.23.0")
+ private Long hostId;
Review Comment:
We could add the `authorized` field here
##########
api/src/main/java/org/apache/cloudstack/api/response/BackupResponse.java:
##########
@@ -71,10 +71,22 @@ public class BackupResponse extends BaseResponse {
@Param(description = "Backup protected (virtual) size in bytes")
private Long protectedSize;
+ @SerializedName(ApiConstants.UNCOMPRESSED_SIZE)
+ @Param(description = "backup uncompressed size in bytes. Only set if
backup is compressed")
Review Comment:
```suggestion
@Param(description = "Backup uncompressed size in bytes. Only defined if
backup is compressed.")
```
--
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]
