Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix extract snapshot from vm snapshot on kvm #6422

Merged
merged 2 commits into from May 31, 2022
Merged

Fix extract snapshot from vm snapshot on kvm #6422

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f04e662
Fix extract snapshot from VM snapshot on KVM
May 30, 2022
39c1572
Fix validation expression - does not need to escape the slash
May 30, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions ...s/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java
View file
Open in desktop
Expand Up @@ -930,7 +930,7 @@ public Answer backupSnapshot(final CopyCommand cmd) {
final String secondaryStoragePoolUrl = nfsImageStore.getUrl();
// NOTE: snapshot name is encoded in snapshot path
final int index = snapshot.getPath().lastIndexOf("/");
final boolean isCreatedFromVmSnapshot = (index == -1) ? true: false; // -1 means the snapshot is created from existing vm snapshot
final boolean isCreatedFromVmSnapshot = index == -1; // -1 means the snapshot is created from existing vm snapshot

final String snapshotName = snapshot.getPath().substring(index + 1);
String descName = snapshotName;
Expand Down Expand Up @@ -1002,7 +1002,7 @@ public Answer backupSnapshot(final CopyCommand cmd) {
}
} else {
final Script command = new Script(_manageSnapshotPath, cmd.getWaitInMillSeconds(), s_logger);
command.add("-b", snapshot.getPath());
command.add("-b", isCreatedFromVmSnapshot ? snapshotDisk.getPath() : snapshot.getPath());
command.add(NAME_OPTION, snapshotName);
command.add("-p", snapshotDestPath);
if (isCreatedFromVmSnapshot) {
Expand Down
46 changes: 40 additions & 6 deletions scripts/storage/qcow2/managesnapshot.sh
View file
Open in desktop
Expand Up @@ -226,14 +226,48 @@ backup_snapshot() {
return 2
fi
elif [ -f ${disk} ]; then
if [[ $disk == *"/snapshots/"* ]]; then
#Backup volume snapshot
cp "$disk" "${destPath}/${destName}"
ret_code=$?

cp "$disk" "${destPath}/${destName}"
ret_code=$?
if [ $ret_code -gt 0 ]
then
printf "Failed to backup $snapshotname for disk $disk to $destPath\n" >&2
return 2
fi
else
# Backup VM snapshot
qemuimg_ret=$($qemu_img snapshot $forceShareFlag -l $disk 2>&1)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@GutoVeronezi
can this be replaced with qemu-img -V ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@weizhouapache, as this is a fix of PR #5297 (specifically this commit: f5180dd), and I'm only returning the removed code to VM snapshots, I'd rather not refactoring anything right now.

In other hand, this entire script could be removed and the workflow implemented via Java, with documentation and unit tests. We could create an issue to do this in the future. What do you think?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@GutoVeronezi
looks good to me. let's fix the issue now and refactor in next releases.

ret_code=$?
if [ $ret_code -gt 0 ] && [[ $qemuimg_ret == *"snapshot: invalid option -- 'U'"* ]]; then
forceShareFlag=""
qemuimg_ret=$($qemu_img snapshot $forceShareFlag -l $disk)
ret_code=$?
fi

if [ $ret_code -gt 0 ]
then
printf "Failed to backup $snapshotname for disk $disk to $destPath\n" >&2
return 2
if [ $ret_code -gt 0 ] || [[ ! $qemuimg_ret == *"$snapshotname"* ]]; then
printf "there is no $snapshotname on disk $disk\n" >&2
return 1
fi

qemuimg_ret=$($qemu_img convert $forceShareFlag -f qcow2 -O qcow2 -l snapshot.name=$snapshotname $disk $destPath/$destName 2>&1 > /dev/null)
ret_code=$?
if [ $ret_code -gt 0 ] && [[ $qemuimg_ret == *"convert: invalid option -- 'U'"* ]]; then
forceShareFlag=""
qemuimg_ret=$($qemu_img convert $forceShareFlag -f qcow2 -O qcow2 -l snapshot.name=$snapshotname $disk $destPath/$destName 2>&1 > /dev/null)
ret_code=$?
fi

if [ $ret_code -gt 0 ] && [[ $qemuimg_ret == *"convert: invalid option -- 'l'"* ]]; then
$qemu_img convert $forceShareFlag -f qcow2 -O qcow2 -s $snapshotname $disk $destPath/$destName >& /dev/null
ret_code=$?
fi

if [ $ret_code -gt 0 ]; then
printf "Failed to backup $snapshotname for disk $disk to $destPath\n" >&2
return 2
fi
fi
else
printf "***Failed to backup snapshot $snapshotname, undefined type $disk\n" >&2
Expand Down