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

Add list_bitmaps command #393

Closed
wants to merge 1 commit into from
Closed

Conversation

dupondje
Copy link
Member

Add list_bitmaps command to list bitmaps in the qcow2 image.

Signed-off-by: Jean-Louis Dupond <jean-louis@dupond.be>
Change-Id: Ic3e078340278739745ab27ab97d9f23b482781df
@dupondje dupondje marked this pull request as draft June 12, 2023 10:59
@dupondje
Copy link
Member Author

This is still WIP, but having some questions here:

  • There is no way to have a SDM command/job return something else than just the status?
  • Or we want to extend SDM jobs to also be able to return a response?
  • It might even be better/cleaner to extend getQemuImageInfo to return also bitmaps?
  • Or just create the list_bitmaps command like getQemuImageInfo (without using SDM)

@dupondje
Copy link
Member Author

@bennyz: See above :) This is VDSM side for oVirt/ovirt-engine#866

@bennyz
Copy link
Member

bennyz commented Jun 12, 2023

This is still WIP, but having some questions here:

* There is no way to have a SDM command/job return something else than just the status?

* Or we want to extend SDM jobs to also be able to return a response?

This is intended for somewhat longer async operations, so not really

* It might even be better/cleaner to extend getQemuImageInfo to return also bitmaps?

* Or just create the list_bitmaps command like getQemuImageInfo (without using SDM)

This sounds like a better approach because this is a sync operation to perform a validation, I'll let vdsm maintainers decide

@aesteve-rh
Copy link
Member

Sorry for the delay, but was out for a few days. And I wanted to check the code before giving an answer.

I agree with @bennyz. Both extend getQemuImageInfo or adding a new command are fit solutions here. Also, they will not interfere with other flows.

Thanks for the update!

@dupondje
Copy link
Member Author

Let's merge #394 then. I fix the ovirt-engine code also then so it read's the args and uses them for validation.

@dupondje dupondje closed this Jun 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants