Implement checkBitmap to check bitmap status before backup #866
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dupondje
requested review from
bennyz,
ahadas,
emesika,
mwperina,
michalskrivanek,
oliel,
sgratch,
smelamud,
didib and
ljelinkova
as code owners
bennyz
reviewed
Jun 6, 2023
...modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/backup/StartVmBackupCommand.java
Outdated
Show resolved
Hide resolved
dupondje
force-pushed
the
check_bitmap_before_backup
branch
3 times, most recently
from
d78fecc
to
7b8763f
Compare
bennyz
reviewed
Jun 11, 2023
...modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/backup/StartVmBackupCommand.java
Outdated
Show resolved
Hide resolved
bennyz
reviewed
Jun 11, 2023
...bll/src/main/java/org/ovirt/engine/core/bll/storage/disk/image/TransferDiskImageCommand.java
Show resolved
Hide resolved
|
Is there a corresponding vdsm PR? it would need to be merged first, also, I think we'll need to add this verb to host capabilities[1] so it isn't sent to older hosts |
dupondje
force-pushed
the
check_bitmap_before_backup
branch
from
7b8763f
to
4fc426d
Compare
|
Updated the code to work with the latest vdsm patch oVirt/vdsm#394 |
bennyz
reviewed
Jul 5, 2023
...modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/backup/StartVmBackupCommand.java
Show resolved
Hide resolved
sandrobonazzola
force-pushed
the
check_bitmap_before_backup
branch
from
4fc426d
to
376c883
Compare
sandrobonazzola
force-pushed
the
check_bitmap_before_backup
branch
2 times, most recently
from
06ed7ca
to
4ab605c
Compare
sandrobonazzola
requested review from
BrooklynDewolf
and removed request for
ljelinkova and
oliel
sandrobonazzola
force-pushed
the
check_bitmap_before_backup
branch
from
4ab605c
to
0eddbab
Compare
Extend the GetQemuImageInfoVDSCommand command to also provide information about the bitmaps. This data will only be valid when the hypervisor has the qemu_image_info_bitmaps capability. The bitmap information can be used to validate the engine's bitmap data with the qemu-img info. VDSM PR: oVirt/vdsm#394 Signed-off-by: Jean-Louis Dupond <jean-louis@dupond.be>
…ntsCommand If there is checkpoint inconsistency between the checkpoints in the database and those on-disk, then we need to remove all checkpoints on-disk and in the database. But the on-disk removal can fail because the checkpoint is already gone because of some corruption (for example due to an unclean shutdown). Therefor we want to allow DeleteAllVmCheckpointsCommand to continue on failure in some cases. Signed-off-by: Jean-Louis Dupond <jean-louis@dupond.be>
Instead of re-implement the functionality from DeleteAllVmCheckpoints, we just use DeleteAllVmCheckpoints and pass to force flag to allow DeleteAllVmCheckpoints to continue on failures. Signed-off-by: Jean-Louis Dupond <jean-louis@dupond.be>
Sometimes a bitmap can become invalid/corrupt without oVirt noticing this. For example if a hypervisor crashes, the active bitmap will become invalid. This means that the qcow2 volume does not contain the bitmap anymore, but oVirt thinks the bitmap still exists because it's in the database. This will currently cause oVirt to fail to create a NBDServer, as it will error with 'Bitmap does not exist in ...'. We use the QemuImageInfo to get all the on-disk bitmaps and compare them with the bitmaps in the oVirt database. If there is some inconsistency we remove all the bitmaps/checkpoints. Signed-off-by: Jean-Louis Dupond <jean-louis@dupond.be>
sandrobonazzola
force-pushed
the
check_bitmap_before_backup
branch
from
0eddbab
to
e587a87
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Sometimes a bitmap cam become invalid/corrupt without oVirt noticing this. For example if a hypervisor crashes, the active bitmap will become invalid.
This will currently cause oVirt to fail to create a NBDServer, as it will error with 'Bitmap does not exist in ...'.
We add a command to check the bitmap status before the backup is created. If the bitmap is invalid, we throw an error and remove the invalid bitmapId.
The VDSM side also needs to be implemented, but that's easy :)
Would be great if some maintainer can give his comments on the idea and the code!