Skip to content

Add API for Disk Resize#66

Merged
openshift-merge-bot[bot] merged 1 commit intocontainers:mainfrom
baude:diskresize
Dec 7, 2023
Merged

Add API for Disk Resize#66
openshift-merge-bot[bot] merged 1 commit intocontainers:mainfrom
baude:diskresize

Conversation

@baude
Copy link
Copy Markdown
Member

@baude baude commented Nov 27, 2023

Add disk resize api for pkg/hypvctl that takes a disk path and a strongly typed new size. No error protection for invalid disk resizes (like trying to descrease the size of a disk).

Fixes: #54

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Nov 27, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: baude

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Comment thread pkg/hypervctl/disk_resize.go Outdated
@baude
Copy link
Copy Markdown
Member Author

baude commented Nov 30, 2023

once #67 merges, i will write an integration test with the current code and then update to the suggested code. that will make me feel more comfy with the change

@baude
Copy link
Copy Markdown
Member Author

baude commented Dec 5, 2023

@n1hility I think we are good to go now ... wdyt?

@cfergeau double checking this is what you need as well...

@cfergeau
Copy link
Copy Markdown

cfergeau commented Dec 5, 2023

Yes, we want to replace this:

	return cmd("Hyper-V\\Resize-VHD",
		"-Path",
		quote(diskPath),
		"-SizeBytes",
		fmt.Sprintf("%d", newSize))

The new API seems to fit.
We'd also need to be able to get the disk size for a disk image (out, err := cmdOut(fmt.Sprintf("@(Get-VHD -Path %s).Size", quote(diskPath)))), I don't know if it's already possible in libhvee?

@n1hility
Copy link
Copy Markdown
Member

n1hility commented Dec 5, 2023

LGTM

@n1hility
Copy link
Copy Markdown
Member

n1hility commented Dec 5, 2023

Yes, we want to replace this:

	return cmd("Hyper-V\\Resize-VHD",
		"-Path",
		quote(diskPath),
		"-SizeBytes",
		fmt.Sprintf("%d", newSize))

The new API seems to fit. We'd also need to be able to get the disk size for a disk image (out, err := cmdOut(fmt.Sprintf("@(Get-VHD -Path %s).Size", quote(diskPath)))), I don't know if it's already possible in libhvee?

Not yet, although there is an API call we can get it from here:
https://learn.microsoft.com/en-us/windows/win32/hyperv_v2/getvirtualharddisksettingdata-msvm-imagemanagementservice

@baude
Copy link
Copy Markdown
Member Author

baude commented Dec 5, 2023

ill add the api now

@n1hility
Copy link
Copy Markdown
Member

n1hility commented Dec 6, 2023

LGTM other than the go.sum conflict

Add disk resize api for pkg/hypvctl that takes a disk path and a
strongly typed new size.  No error protection for invalid disk resizes
(like trying to descrease the size of a disk).

Fixes: containers#54

Signed-off-by: Brent Baude <bbaude@redhat.com>
Comment thread test/e2e/utils_test.go

// PowerShellCommand is a wrapper to run something in powershell. It takes a command and args.
// And it returns stdout as a string
func PowerShellCommand(commandName string, args []string) (string, error) {
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.

Is this still used?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

No, but a the test suite gets filled out, I'm certain it will be.

@cfergeau
Copy link
Copy Markdown

cfergeau commented Dec 7, 2023

/lgtm

@openshift-ci openshift-ci Bot added the lgtm label Dec 7, 2023
@openshift-merge-bot openshift-merge-bot Bot merged commit 920a56e into containers:main Dec 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

create exported function for resizing disk

5 participants