Ashwani Raina has posted comments on this change. ( http://gerrit.cloudera.org:8080/23016 )
Change subject: [server] KUDU-1457 [2/n] Add support for IPv6 ...................................................................... Patch Set 5: (6 comments) http://gerrit.cloudera.org:8080/#/c/23016/5//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/23016/5//COMMIT_MSG@7 PS5, Line 7: [server] KUDU-1457 [2/n] Add support for IPv6 > A couple of nits for your consideration w.r.t. the description of this and Done http://gerrit.cloudera.org:8080/#/c/23016/5/src/kudu/server/webserver.cc File src/kudu/server/webserver.cc: PS5: > Do you think we need to add at least a few basic/smoke tests into webserver I have a separate patch for webserver changes that includes testing for corresponding changes. Additionally, I am planning to have a separate patch for IPv6 changes that is applicable to all the tests making use of IP addresses in any way. Since that change is going to be of global scope in terms of number of test files that require change, it would make sense to have a separate patch to do it. http://gerrit.cloudera.org:8080/#/c/23016/5/src/kudu/server/webserver.cc@499 PS5, Line 499: * > style nit for here and elsewhere: the asterisk sticks to the type, not the Done http://gerrit.cloudera.org:8080/#/c/23016/5/src/kudu/server/webserver.cc@500 PS5, Line 500: push_back(Sockaddr(*s_in)) > nit: might this be just Done http://gerrit.cloudera.org:8080/#/c/23016/5/thirdparty/patches/squeasel-support-get-bound-addresses-for-ipv6.patch File thirdparty/patches/squeasel-support-get-bound-addresses-for-ipv6.patch: PS5: > Do plan you to add this new code into the Cloudera's squeasel repo at some No, the patch is not sourced from there. This is the new code that I added to enable squeasel function i.e. sq_get_bound_addresses to return IPv6 address as well. I plan to add this code to Cloudera squeasel repo later. http://gerrit.cloudera.org:8080/#/c/23016/5/thirdparty/patches/squeasel-support-get-bound-addresses-for-ipv6.patch@46 PS5, Line 46: *) > Is this a bug in the current version without the patch? Considering out parameter type(struct sockaddr_in ***addrs), it seems sq_get_bound_addresses was implemented without taking IPv6 addresses into consideration. So, yeah, it's a bug since current version clearly supports IPv6. -- To view, visit http://gerrit.cloudera.org:8080/23016 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iee175a00a7ac9e20a31ddb8396468cfcd8f3b6f9 Gerrit-Change-Number: 23016 Gerrit-PatchSet: 5 Gerrit-Owner: Ashwani Raina <[email protected]> Gerrit-Reviewer: Abhishek Chennaka <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Ashwani Raina <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber <[email protected]> Gerrit-Comment-Date: Fri, 25 Jul 2025 15:17:06 +0000 Gerrit-HasComments: Yes
