oVirt / vdsm

The Virtual Desktop Server Manager
GNU General Public License v2.0
160 stars 201 forks source link

Add list_bitmaps command #393

Closed dupondje closed 1 year ago

dupondje commented 1 year ago

Add list_bitmaps command to list bitmaps in the qcow2 image.

dupondje commented 1 year ago

This is still WIP, but having some questions here:

dupondje commented 1 year ago

@bennyz: See above :) This is VDSM side for https://github.com/oVirt/ovirt-engine/pull/866

bennyz commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

Let's merge https://github.com/oVirt/vdsm/pull/394 then. I fix the ovirt-engine code also then so it read's the args and uses them for validation.