-
Notifications
You must be signed in to change notification settings - Fork 1.3k
CLOUDSTACK-8608: [VMware] System VMs failed to start due to permissions issue. Provide permissions to template folder when mounted on management server. #1875
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.
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
Conversation
|
@rhtyd Can you run VMware CI on this PR. |
|
@blueorangutan package |
|
@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
|
Packaging result: ✔centos6 ✔centos7 ✔debian. JID-511 |
|
@blueorangutan test centos7 vmware-55u3 |
|
@rhtyd a Trillian-Jenkins test job (centos7 mgmt + vmware-55u3) has been kicked to run smoke tests |
|
Trillian test result (tid-867)
|
|
@rhtyd Thanks for running tests. The test failures/errors above are failing in other PRs as well, not related to the changes in this PR. |
|
@sateesh-chodapuneedi @rhtyd Please review the changes. |
| s_logger.debug("Set permissions for " + mountPoint); | ||
| String result = null; | ||
| Script script = new Script(true, "chmod", _timeout, s_logger); | ||
| script.add("-R", "0777", mountPoint); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sureshanaparti In place of using 0777, I think it is better to declare a constant and use it.
| String result = null; | ||
| final String systemVmTmpltPermissions = "0777"; | ||
| Script script = new Script(true, "chmod", _timeout, s_logger); | ||
| script.add("-R", systemVmTmpltPermissions, mountPoint); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@SudharmaJain Used the string as suggested instead of hardcoded value for permissions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sureshanaparti It would be better if you can move it to TemplateConstants.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@SudharmaJain Moved to TemplateConstants.
|
LGTM on the code changes. |
|
LGTM. |
|
LGTM, @sureshanaparti since this is a critical bugfix wher systemvm can fail to start, can you change PR's base branch to 4.9? |
|
@rhtyd Changed PR base branch to 4.9. |
|
Thanks @sureshanaparti |
|
@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
|
Packaging result: ✖centos6 ✔centos7 ✔debian. JID-675 |
|
@blueorangutan test centos7 vmware-55u3 |
|
@rhtyd a Trillian-Jenkins test job (centos7 mgmt + vmware-55u3) has been kicked to run smoke tests |
|
Trillian test result (tid-1033)
|
|
The above intermitten failures are not related to this PR changes. |
…ns issue. Provide permissions to template folder when mounted on management server.
|
@sureshanaparti please rebase/fix the branch |
|
@sureshanaparti ping, can you fix the conflicts? |
|
@sureshanaparti this is marked blocker however no activity or discussion is seen, I'll remove the 'blocker' label and ask the author to engage in a conversation. Thanks. |
|
@sureshanaparti please rebase and re-open if still relevant |
[VMware] System VMs failed to start due to permissions issue. Provide permissions to template folder when mounted on management server.
This closes #555