govc: Add support for providing size while we create a VMFS datastore #3528
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@HaruChebrolu, you must sign our contributor license agreement before your changes are merged. Click here to sign the agreement. If you are a VMware employee, read this for further instruction. |
HaruChebrolu
changed the title
HaruChebrolu
force-pushed
the
datastore_size
branch
from
b6f822e
to
c259513
Compare
|
@HaruChebrolu, we have received your signed contributor license agreement. The review is usually completed within a week, but may take longer under certain circumstances. Another comment will be added to the pull request to notify you when the merge can proceed. |
HaruChebrolu
force-pushed
the
datastore_size
branch
from
c259513
to
373cd6e
Compare
dougm
reviewed
Aug 28, 2024
There was a problem hiding this comment.
Thank you @HaruChebrolu
A few general items, please
- Run
make docto update USAGE.md - Code needs to be gofmt'd
- Amend commit message to include
Closes #3519, this will create a link in generated release notes and auto-close the issue when we merge your change.
One more suggestion for the size flag, to use units.ByteSize. This allows value to be specified as (for example) -size 30M , -size 2T, etc. See patch below.
diff --git a/govc/datastore/create.go b/govc/datastore/create.go
index 0432e46b..fc5e11e8 100644
--- a/govc/datastore/create.go
+++ b/govc/datastore/create.go
@@ -28,13 +28,14 @@ import (
"github.com/vmware/govmomi/govc/cli"
"github.com/vmware/govmomi/govc/flags"
"github.com/vmware/govmomi/object"
+ "github.com/vmware/govmomi/units"
"github.com/vmware/govmomi/vim25/soap"
"github.com/vmware/govmomi/vim25/types"
)
const (
- sectorSize = 512 // Sector size in bytes
- startSector = 2048 // Start sector (typically fixed)
+ sectorSize = 512 // Sector size in bytes
+ startSector = 2048 // Start sector (typically fixed)
)
type create struct {
@@ -55,7 +56,7 @@ type create struct {
// Options for VMFS
DiskCanonicalName string
Version *int32
- Size int64
+ Size units.ByteSize
// Options for local
Path string
@@ -150,7 +151,7 @@ func (cmd *create) Register(ctx context.Context, f *flag.FlagSet) {
// Options for VMFS
f.StringVar(&cmd.DiskCanonicalName, "disk", "", "Canonical name of disk (VMFS only)")
f.Var(flags.NewOptionalInt32(&cmd.Version), "version", "VMFS major version")
- f.Int64Var(&cmd.Size, "size", -1, "Size of new disk")
+ f.Var(&cmd.Size, "size", "Size of new disk")
// Options for Local
f.StringVar(&cmd.Path, "path", "", "Local directory path for the datastore (local only)")
@@ -296,7 +297,7 @@ func (cmd *create) CreateVmfsDatastore(ctx context.Context, hosts []*object.Host
spec := *option.Spec.(*types.VmfsDatastoreCreateSpec)
spec.Vmfs.VolumeName = cmd.Name
if cmd.Size > 0 {
- endSector := CalculateSectors(cmd.Size)
+ endSector := CalculateSectors(int64(cmd.Size))
// set values for Sectors
spec.Partition.Partition[0].StartSector = startSector
spec.Partition.Partition[0].EndSector = endSector
@@ -314,13 +315,11 @@ func (cmd *create) CreateVmfsDatastore(ctx context.Context, hosts []*object.Host
}
// CalculateSectors calculates the start and end sectors based on the given size.
-func CalculateSectors(sizeGB int64) (endSector int64) {
- sizeInBytes := sizeGB * 1024 * 1024 * 1024
+func CalculateSectors(sizeInBytes int64) (endSector int64) {
+ totalSectors := sizeInBytes / sectorSize
+ endSector = startSector + totalSectors - 1
- totalSectors := sizeInBytes / sectorSize
- endSector = startSector + totalSectors - 1
-
- return endSector
+ return endSector
}
func (cmd *create) CreateLocalDatastore(ctx context.Context, hosts []*object.HostSystem) error {
HaruChebrolu
force-pushed
the
datastore_size
branch
from
373cd6e
to
790ca92
Compare
Done |
dougm
reviewed
Aug 28, 2024
There was a problem hiding this comment.
Thanks for updating @HaruChebrolu
But still missing govc/USAGE.md from make doc (you can see the GH Action check failed)
See this doc on the commit message. Still want the govc: subject prefix as you had it before, Closes should be in the body:
govc: Add '-size' flag to datastore.create
Closes #3519
790ca92
to
f38b98d
Compare
HaruChebrolu
force-pushed
the
datastore_size
branch
from
f38b98d
to
1ae50b0
Compare
HaruChebrolu
force-pushed
the
datastore_size
branch
from
5c4423f
to
373217e
Compare
13 tasks
|
@HaruChebrolu, VMware has approved your signed contributor license agreement. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
govc: Add support for providing size while we create a VMFS datastore
Closes: #(issue-number)
#3519
Type of change
Please mark options that are relevant:
not work as expected)
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide
instructions so we can reproduce. If applicable, please also list any relevant
details for your test configuration.
Tried to create a VMFS datastore with some size in govc cli
go run main.go datastore.create -type VMFS -name vmfs61 -version 6 -disk eui.b8fa2f975ecc490ea090a46404568540 -size 30 ocsqe-virt07.lab.eng.blr.redhat.comTried to create a VMFS datastore without size argument in govc cli, so that it takes the entire size of disk into consideration(default case)
go run main.go datastore.create -type VMFS -name vmfs61 -version 6 -disk eui.b8fa2f975ecc490ea090a46404568540 ocsqe-virt07.lab.eng.blr.redhat.comChecklist:
CONTRIBUTIONguidelines of this project