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

Perform cleanup after insufficient free space error during LSM #568

Merged
merged 2 commits into from
Aug 3, 2022
Merged

Perform cleanup after insufficient free space error during LSM #568

merged 2 commits into from
Aug 3, 2022

Conversation

barpavel
Copy link
Member

@barpavel barpavel commented Aug 2, 2022 •
edited

In case of Live Storage Migration failure during LiveDiskMigrateStage.CLONE_IMAGE_STRUCTURE the remove image cleanup was skipped.
It should be performed in order to free the already allocated space on the target Storage Domain.

Bug-Url: https://bugzilla.redhat.com/2102149

@barpavel barpavel requested a review from mkemel August 2, 2022 14:59
@barpavel
Copy link
Member Author

barpavel commented Aug 2, 2022

/ost

@ahadas ahadas force-pushed the no_cleanup_after_insufficient_free_space branch from c5ad592 to 211d4e7 Compare August 3, 2022 07:03
@barpavel barpavel marked this pull request as draft August 3, 2022 07:38
@barpavel barpavel force-pushed the no_cleanup_after_insufficient_free_space branch 3 times, most recently from 83dee51 to 0612c36 Compare August 3, 2022 07:57
@barpavel barpavel marked this pull request as ready for review August 3, 2022 07:59
@barpavel barpavel requested a review from ahadas August 3, 2022 07:59
@barpavel
core: code improvements of LSM cleanup method
cd08c02 
Code improvements for methods that handle destination
disk cleanups in case of Live Storage Migration failure.

1. Renaming cleanup method to "cleanupDestDiskAfterFailure()".
2. "removeImage()" method calls reformatting for for readability.

Signed-off-by: Pavel Bar <pbar@redhat.com>
Bug-Url: https://bugzilla.redhat.com/2102149
@barpavel
core: perform cleanup for failure during clone LSM phase
733e883 
In case of Live Storage Migration failure during
"LiveDiskMigrateStage.CLONE_IMAGE_STRUCTURE" the "removeImage()"
cleanup was skipped. It should be performed in order to free
the already allocated space on the target Storage Domain.

Signed-off-by: Pavel Bar <pbar@redhat.com>
Bug-Url: https://bugzilla.redhat.com/2102149
@barpavel barpavel force-pushed the no_cleanup_after_insufficient_free_space branch from 0612c36 to 733e883 Compare August 3, 2022 08:02
@barpavel
Copy link
Member Author

barpavel commented Aug 3, 2022

/ost

@barpavel
Copy link
Member Author

barpavel commented Aug 3, 2022 •
edited

Tested it again with my latest (today's) changes and synced with latest master.
Cleanup works.
Actually the only change vs. the old code is to call LiveMigrateDiskCommand::removeImage() for cleanup flow that is called from endWithFailure() (that means that LSM operation failed already anyway) also for LiveDiskMigrateStage.CLONE_IMAGE_STRUCTURE phase, so in general this shouldn't make things worse :)

@ahadas
Copy link
Member

ahadas commented Aug 3, 2022

Tested it again with my latest (today's) changes and synced with latest master. Cleanup works. Actually the only change vs. the old code is to call LiveMigrateDiskCommand::removeImage() for cleanup flow that is called from endWithFailure() (that means that LSM operation failed already anyway) also for LiveDiskMigrateStage.CLONE_IMAGE_STRUCTURE phase, so in general this shouldn't make things worse :)

that's a fair point @barpavel :)

@ahadas ahadas merged commit 1359c3d into oVirt:master Aug 3, 2022
@barpavel barpavel deleted the no_cleanup_after_insufficient_free_space branch August 4, 2022 08:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ahadas ahadas ahadas approved these changes

@bennyz bennyz Awaiting requested review from bennyz bennyz is a code owner

@mkemel mkemel Awaiting requested review from mkemel

Assignees
No one assigned
Labels
bug storage verified
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@barpavel @ahadas @bennyz