Skip to content

Conversation

robhoes
Copy link
Member

@robhoes robhoes commented Jul 15, 2025

The first argument of this API error is used for error codes as defined by v6d. These product-specific error codes can be matched on by clients who know about them (e.g. XenCenter for XenServer).

The new, second argument allows v6d to return an error message in English that xe can print directly, while xe remains product agnostic and does not need to know v6d's error definitions. This replaces some awkward API-message handling code in cli_operations.

The first argument of this API error is used for error codes as defined
by v6d. These product-specific error codes can be matched on by clients
who know about them (e.g. XenCenter for XenServer).

The new, second argument allows v6d to return an error message in
English that xe can print directly, while xe remains product agnostic
and does not need to know v6d's error definitions. This replaces some
awkward API-message handling code in cli_operations.

Signed-off-by: Rob Hoes <[email protected]>
@robhoes robhoes requested a review from minglumlu July 15, 2025 21:36
@robhoes
Copy link
Member Author

robhoes commented Jul 15, 2025

@psafont please note: this includes a small change to v6_interface.ml and therefore requires an update to v6d.

Comment on lines -5359 to -5362
|| msg.API.message_name = fst Api_messages.v6_rejected
|| msg.API.message_name = fst Api_messages.v6_comm_error
|| msg.API.message_name
= fst Api_messages.v6_license_server_version_obsolete
Copy link
Member

Choose a reason for hiding this comment

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

If I understood this right, xe now doesn't need to know about these messages, but they still need to be exposed so clients can still react to them, is this correct?

Copy link
Member Author

Choose a reason for hiding this comment

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

You mean the definitions in api_messages? I'd like to remove them as well, but I was worried about breaking the SDK.

date *)
| License_processing_error (** License could not be processed *)
| License_checkout_error of string (** License could not be checked out *)
| License_checkout_error of string * string
Copy link
Contributor

Choose a reason for hiding this comment

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

Since this is a breaking change anyway you could also completely rename the variant to something else?
(Then you could also keep the old one for backwards compatibility if needed)

Copy link
Member Author

Choose a reason for hiding this comment

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

As this is an host-internal interface, I don't think it is worth worrying about. In practice we update xapi+v6d at the same time.

@robhoes robhoes added this pull request to the merge queue Jul 16, 2025
Merged via the queue into xapi-project:master with commit bef6739 Jul 16, 2025
16 checks passed
@robhoes robhoes deleted the v6-errors branch July 16, 2025 10:05
@psafont
Copy link
Member

psafont commented Jul 16, 2025

therefore requires an update to v6d.

The daemon has so little code this error is never used :)

psafont added a commit to xcp-ng/xcp-featured that referenced this pull request Jul 16, 2025
I checked the code to see whether it needed changes because of xapi-project/xen-api#6592 but it all looks good.

I saw that I could make small improvements. I left some other, like code formatting out
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.

4 participants