Skip to content

Add list_bitmaps command - #393

Closed
dupondje wants to merge 1 commit into
oVirt:masterfrom
dupondje:add_list_bitmaps
Closed

Add list_bitmaps command#393
dupondje wants to merge 1 commit into
oVirt:masterfrom
dupondje:add_list_bitmaps

Conversation

@dupondje

Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Member Author

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

@bennyz

bennyz commented Jun 12, 2023

Copy link
Copy Markdown
Contributor

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
Copy Markdown
Contributor

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
Copy Markdown
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