Skip to content

restapi: Add watchdog support to update VM API - #1021

Open
Artemi10 wants to merge 1 commit into
oVirt:masterfrom
Artemi10:feature/update-watchdog
Open

restapi: Add watchdog support to update VM API#1021
Artemi10 wants to merge 1 commit into
oVirt:masterfrom
Artemi10:feature/update-watchdog

Conversation

@Artemi10

Copy link
Copy Markdown

Updating the watchdog via the VM Watchdog API bypass validation checks, such as verifying the presence of a next-run configuration for running VMs. This can lead to inconsistent or incorrect watchdog settings.

Changes introduced with this PR

  • Add watchdog support to the update VM API which already performs the necessary validations. This ensures that watchdog updates are applied safely and consistently, especially for running VMs with pending configurations.

Are you the owner of the code you are sending in, or do you have permission of the owner?

Yes

Fixes an issue where the VM watchdog API bypass validation for running VMs, such as verifying the presence of a next-run configuration for running VMs, which can lead to inconsistent or incorrect watchdog settings. Watchdog updates are now handled via the update VM API to ensure proper checks.

Signed-off-by: Liakh Artemii <aliakh@orionsoft.ru>
@Artemi10
Artemi10 force-pushed the feature/update-watchdog branch from 26a43a6 to c39d194 Compare May 20, 2025 08:04

@dupondje dupondje left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for the PR.

So the current implementation doesn't do any validation, but you add some additional implementation without a lot of validation? And you don't touch the existing flow?
This seems like a bad idea ...

I think it should be handled in for example backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendVmWatchdogResource.java. And if there some validation is missing it should be fixed there.

@Artemi10

Artemi10 commented Jun 5, 2025

Copy link
Copy Markdown
Author

Thanks for the PR.

So the current implementation doesn't do any validation, but you add some additional implementation without a lot of validation? And you don't touch the existing flow? This seems like a bad idea ...

I think it should be handled in for example backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendVmWatchdogResource.java. And if there some validation is missing it should be fixed there.

@dupondje
Thanks for the feedback.

This implementation follows the existing approach used for RngDevice, where the logic also relies on UpdateVmCommand for validation. Since UpdateVmCommand already contains the necessary state checks, I reused this mechanism to avoid duplicating validation logic and to stay consistent with similar features.

Adding validation directly in BackendVmWatchdogResource would significantly increase complexity, as all necessary checks are already implemented within UpdateVmCommand. Integrating UpdateVmCommand into BackendVmWatchdogResource would be non-trivial due to the current classes structure.

By reusing UpdateVmCommand, we ensure consistency with the existing VM update flow and avoid duplicating validation logic in multiple layers.

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.

2 participants