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.

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

core: Remove image if CreateSnapshotFromTemplateCommand failed #609

Merged
merged 1 commit into from
Aug 24, 2022

Conversation

smelamud
Copy link
Member

In some scenarios CreateSnapshotFromTemplateCommand fails after creation
of the record in the images table. In this case the record should be
removed together with other DB records related to the disk.

Change-Id: I201b49b9acd052c733c4d418abbac3b16c9431d5
Bug-Url: https://bugzilla.redhat.com/2110351
Signed-off-by: Shmuel Melamud smelamud@redhat.com

In some scenarios CreateSnapshotFromTemplateCommand fails after creation
of the record in the images table. In this case the record should be
removed together with other DB records related to the disk.

Change-Id: I201b49b9acd052c733c4d418abbac3b16c9431d5
Bug-Url: https://bugzilla.redhat.com/2110351
Signed-off-by: Shmuel Melamud <smelamud@redhat.com>
@@ -89,6 +92,9 @@ protected void endWithFailure() {
if (diskImageDynamicDao.get(getDestinationDiskImage().getImageId()) != null) {
diskImageDynamicDao.remove(getDestinationDiskImage().getImageId());
}
if (imageDao.get(getDestinationDiskImage().getImageId()) != null) {
Copy link
Member

Choose a reason for hiding this comment

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

will it answer on some of the comment above? should you check if it succeed before removing as written?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, it won't. The comment says about checking the result of RemoveImageCommand before removing the records from the database. This is a completely different task and I doubt we should implement it this way. dbOperationScope parameter of RemoveImageCommand should rather be used, I think.

But this patch just adds a removal of one more record that refers the disk. This record does not usually exist when CreateSnapshotFromTemplateCommand fails, but in this particular case (after failure of SealVmCommand) it exists and should be removed when the disk is removed, just like records in any other table.

Copy link
Member

Choose a reason for hiding this comment

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

OK, although i do think it might be better to insert into the remove image command. but, since we already have other DB removals like that (baseDiskDao and diskImageDynamicDao)...

@smelamud smelamud merged commit 826b8e3 into oVirt:master Aug 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants