Skip to content
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.

Sign up for GitHub

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

Jump to bottom

NAS-133751 / 25.10 / Allow importing/exporting disk images in virt plugin #15690

Merged
merged 2 commits into from
Feb 14, 2025
Merged

NAS-133751 / 25.10 / Allow importing/exporting disk images in virt plugin #15690

merged 2 commits into from
Feb 14, 2025
+226 −2

Conversation

Qubad786
Copy link
Contributor

Context

It was requested that we port importing/exporting disk images functionality to virt plugin and that has been done. (Similar changes from VM plugin are being removed in another PR #15658).

@Qubad786 Qubad786 requested review from william-gr and a team February 12, 2025 20:52
@bugclerk bugclerk changed the title Allow importing/exporting disk images in virt plugin NAS-133751 / 25.10 / Allow importing/exporting disk images in virt plugin Feb 12, 2025
@bugclerk
Copy link
Contributor

Jira URL: https://ixsystems.atlassian.net/browse/NAS-133751

william-gr
william-gr reviewed Feb 13, 2025
View reviewed changes
Copy link
Member

@william-gr william-gr left a comment

Choose a reason for hiding this comment

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

Functionality looks good to me! In the future we can support client download/upload but don't think its a requirement for now.
I will let someone more familiar with the codebase to approve.

Thanks!

yocalebo
yocalebo requested changes Feb 13, 2025
View reviewed changes
Copy link
Contributor

@yocalebo yocalebo left a comment •
edited
Loading

Choose a reason for hiding this comment

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

These 2 methods are basically a copy and paste of one another. (i.e. there is too much duplicated code here). We should condense the validation logic to 1 method, and then condense the subprocess logic from each endpoint into 1 method.

src/middlewared/middlewared/plugins/virt/disk.py Outdated Show resolved Hide resolved
src/middlewared/middlewared/plugins/virt/disk.py Outdated Show resolved Hide resolved
src/middlewared/middlewared/plugins/virt/disk.py Outdated Show resolved Hide resolved
@Qubad786 Qubad786 requested a review from yocalebo February 14, 2025 11:32
@Qubad786 Qubad786 merged commit a992c09 into master Feb 14, 2025
2 checks passed
@Qubad786 Qubad786 deleted the NAS-133751 branch February 14, 2025 20:06
@bugclerk bugclerk mentioned this pull request Feb 14, 2025
Merged
@bugclerk
Copy link
Contributor

This PR has been merged and conversations have been locked.
If you would like to discuss more about this issue please use our forums or raise a Jira ticket.

@truenas truenas locked as resolved and limited conversation to collaborators Feb 14, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@william-gr william-gr william-gr left review comments

@yocalebo yocalebo yocalebo approved these changes

Assignees
No one assigned
Labels
backport-25.04-RC.1 backported no-time-tracked
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Qubad786 @bugclerk @william-gr @yocalebo