Skip to content
This repository has been archived by the owner on Mar 26, 2020. It is now read-only.

Refactoring for Intelligent Volume Provisioning #782

Merged
merged 1 commit into from
May 24, 2018
Merged

Refactoring for Intelligent Volume Provisioning #782

merged 1 commit into from
May 24, 2018

Conversation

aravindavk
Copy link
Member

Added more device utils and fstab handling library

Updates: #466
Signed-off-by: Aravinda VK avishwan@redhat.com

func GetRedundancy(disperse uint) int {
var temp, l, mask uint
temp = disperse
l = 0
Copy link
Member

Choose a reason for hiding this comment

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

no need to set it explicitly to zero

Copy link
Member Author

Choose a reason for hiding this comment

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

This func is moved from volume-create-disperse.go to utils. Implementation not changed

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed.

@aravindavk
Copy link
Member Author

Note for reviewers:

  • Moved glusterd2/commands/volumes/events.go to glusterd2/volume/events.go

  • Moved getRedundancy function from glusterd2/commands/volumes/volume-create-disperse.go to glusterd2/volume/volume-utils.go file

  • Moved glusterd2/commands/volumes/brickutils.go to pkg/api/brickutils.go

  • Added three additional fields to Device Resp

       + VgName        string `json:"vg-name"`
       + AvailableSize uint64 `json:"available-size"`
       + Used          bool   `json:"used"`
    
  • New file added for fstab functionalities plugins/device/deviceutils/fstab.go

  • More functions added to plugins/device/deviceutils/utils.go

  • Updated AvailableSize during device add

@aravindavk aravindavk requested a review from phlogistonjohn May 23, 2018 05:58
"github.com/pborman/uuid"
)

func nodesFromVolumeCreateReq(req *api.VolCreateReq) ([]uuid.UUID, error) {
// NodesFromVolumeCreateReq extracts the list of nodes from Volume Create request
func NodesFromVolumeCreateReq(req *VolCreateReq) ([]uuid.UUID, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

You can make this as a method of VolCreateReq structure and then use it as req.Nodes()

@@ -26,7 +25,8 @@ func nodesFromVolumeCreateReq(req *api.VolCreateReq) ([]uuid.UUID, error) {
return nodes, nil
}

func nodesFromVolumeExpandReq(req *api.VolExpandReq) ([]uuid.UUID, error) {
// NodesFromVolumeExpandReq extracts list of Peer IDs from Volume Expand request
func NodesFromVolumeExpandReq(req *VolExpandReq) ([]uuid.UUID, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here.

@aravindavk
Copy link
Member Author

@prashanthpai @Madhu-1 addressed review comments, please review

Copy link
Member

@Madhu-1 Madhu-1 left a comment

Choose a reason for hiding this comment

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

LGTM

if err != nil {
return nil, err
}
defer lockfile.Close()
Copy link
Contributor

@prashanthpai prashanthpai May 24, 2018

Choose a reason for hiding this comment

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

The file is closed when the function returns. Doesn't closing file descriptor implicitly unlock ?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes, will remove this

Added more device utils and `fstab` handling library

Updates: #466
Signed-off-by: Aravinda VK <avishwan@redhat.com>
@prashanthpai
Copy link
Contributor

retest this please

1 similar comment
@prashanthpai
Copy link
Contributor

retest this please

@prashanthpai prashanthpai merged commit 368c857 into gluster:master May 24, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants