Skip to content

Design proposal for supported image formats (v3) #6360

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

gthvn1
Copy link
Contributor

@gthvn1 gthvn1 commented Mar 14, 2025

In the previous version of the design some questions were always raised. In this version we added explanations about how to choose the format when the VDI is created but also the destination format during migration.
We no longer rely on the order of the parameters passed by the driver because it is not guaranteed. Instead we propose to use some existing parameters to choose the format of the VDI when it is created and the format that should be used during migration. If no parameters are specified it will be up to the driver to do what he wants.

@gthvn1 gthvn1 force-pushed the feature/supported-image-formats-v3 branch from 1c0c7c8 to abee315 Compare March 14, 2025 16:29
@gthvn1 gthvn1 requested a review from psafont March 15, 2025 14:09
Comment on lines 47 to 49
a string. The string specifies a preferred format (e.g., qcow2), ensuring that the
VDI is migrated in the correct format. If the chosen or default format cannot be
used (e.g., due to size limitations), an error will be generated.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this error be calculated before the mgiration happens, and not when the limit is reached? I'd like to have the time the check is done written down in the design

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok sure. I will check that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still need to check that ;). I will do it today I think.

@gthvn1 gthvn1 force-pushed the feature/supported-image-formats-v3 branch 2 times, most recently from e3bf586 to d237624 Compare March 20, 2025 16:17
@gthvn1 gthvn1 closed this Apr 29, 2025
@gthvn1 gthvn1 deleted the feature/supported-image-formats-v3 branch April 29, 2025 08:26
@gthvn1 gthvn1 restored the feature/supported-image-formats-v3 branch April 29, 2025 08:38
@gthvn1
Copy link
Contributor Author

gthvn1 commented Apr 29, 2025

Closed by mistake.

@gthvn1 gthvn1 reopened this Apr 29, 2025
@gthvn1 gthvn1 force-pushed the feature/supported-image-formats-v3 branch from d237624 to c81e182 Compare April 29, 2025 09:11
Comment on lines +65 to +66
the destination driver will choose the format of the VDI if none is explicitly
specified. This ensures backward compatibility with both current and future drivers.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't this mean that the origin needs to be able to produce that format? It may mean a new error message needs to be added.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yes. In fact it looks like the same issue than the one above when format is not supported. Even if we provide a tip with supported image format a client could ask for another one that is not supported. So in all cases we need to detect as early as possible that a format requested by the client is not supported. If supported image format is provided by the SM Driver we can easily check the list but if it is not provided I'm not sure how to detect the error early. Interesting.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that we can probably add checks of the size and of the format supported by destination in the migration_send'. If the information of supported image format is in the database. If the information is not provided I think that the error will be detected during the creation of the mirror+snapshot, so later unfortunately.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So as I understand it it should be ok to check in migrate_receive or migrate_send' if the supported_image_format passed as an option is valid for the SR because we can from the VDI_ref found its SR and if I'm not mistaken the SR type has a corresponding SM type. And then SM type will have it supported image format list (or empty if not provided). So I think that we can make the check at the same time as we are doing the assert_can_migrate_vdis. If it is ok I will add this information in the specification.

While I'm looking at this it makes me think that also when doing the VM migration we will need to allow to choose the format of the VDI. Because for VM live migration we can pass VDI mappings we need to specify the format as well.

So for VM migration one idea is to add an option to the mapping like vdi:<source vdi uuid>=<dest sr uuid>,vhd or vdi:<source vdi uuid>=<dest sr uuid>,qcow2 and of course you can still use vdi:<source vdi uuid>=<dest sr uuid>.
Or we can add a separate option to specify for a given VDI (selected by its source) what format should be used when moving it. Something like vdi_format:<source vdi uuid>=vhd.

Finally there are more cases than I thought :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it is ok I will add this information in the specification.

Yeah, go ahead

I'd prefer the separate option for the migration, if possible

@gthvn1 gthvn1 force-pushed the feature/supported-image-formats-v3 branch 3 times, most recently from 3ec5291 to 4bf6d5c Compare May 14, 2025 16:03
In this version we added explanation about how to choose the format when
the VDI is created but also the destination format during migration.
We also fix some typos.

Signed-off-by: Guillaume <[email protected]>
@gthvn1 gthvn1 force-pushed the feature/supported-image-formats-v3 branch from 4bf6d5c to 322f4bc Compare May 20, 2025 13:38
Copy link
Contributor

@lindig lindig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To help this get merged, will approve it (since it can't break any code)

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