-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
mount.glusterfs: Increase the max characters allow in the hostnames of glusterservers #4331
base: devel
Are you sure you want to change the base?
Conversation
Can one of the admins verify this patch? |
2 similar comments
Can one of the admins verify this patch? |
Can one of the admins verify this patch? |
/run regression |
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.
Looks good. Please sign the commit using git commit --amend -s
0 test(s) failed 1 test(s) generated core 2 test(s) needed retry |
…f glusterservers Signed-off-by: David Pascoe-Deslauriers <dpascoed@coldfrontlabs.ca>
@aravindavk I've amended the commit to include the sign-off. Thanks! |
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.
LGTM
@@ -35,7 +35,7 @@ _init () | |||
LOG_DEBUG=DEBUG; | |||
LOG_TRACE=TRACE; | |||
|
|||
HOST_NAME_MAX=64; | |||
HOST_NAME_MAX=255; |
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.
Do we need to change at ctx.c(https://github.com/gluster/glusterfs/blob/517d75e611322dd5757d5d9231e98356273c15f9/libglusterfs/src/ctx.c#L53C5-L53C42) also. After this change maximum hostname length would be 255 chars but ctx->hostname still would be sysconf(_SC_HOST_NAME_MAX) and on my system the value is 64.
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.
I did some quick research to try and understand what's going on.
From what I've read, it looks like HOST_NAME_MAX (_SC_HOST_NAME_MAX) does not refer to the FQDN, just the hostname.
The numbers do (sort of) line up with https://www.rfc-editor.org/rfc/rfc1035, which limits the FQDN to 255 bytes, but any labels to a max of 63 bytes. But looks like rfc1035 labels are 63 bytes + null and linux hostnames can be 64 bytes + null, so maybe I'm interpreting things wrong.
Pragmatically, with the glusters setup where we ran into issues, we only ran into issues with the mount commands and with this change the mounts work correctly. Checking gluster, the peers are connected via FQDNs longer than 64 characters and it's working correctly.
So I'm not sure how the MAX length in ctx plays in with all of the rest unfortunately.
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.
I think you are correct, we don;t need to change it.
/run regression |
The mount scripts validate hostnames by checking that they are less than 64 characters long. Host names on many cloud environments are generally much longer than this, but are still valid.
This change increases the hostname length to allow for these valid hostname.