winterhazel commented on code in PR #12571:
URL: https://github.com/apache/cloudstack/pull/12571#discussion_r2763493235
##########
engine/components-api/src/main/java/com/cloud/agent/AgentManager.java:
##########
@@ -54,6 +54,9 @@ public interface AgentManager {
"This timeout overrides the wait global config. This holds a comma
separated key value pairs containing timeout (in seconds) for specific
commands. " +
"For example: DhcpEntryCommand=600,
SavePasswordCommand=300, VmDataCommand=300", false);
+ ConfigKey<Integer> KVMHostDiscoverySshPort = new
ConfigKey<>(ConfigKey.CATEGORY_ADVANCED, Integer.class,
Review Comment:
> You are not asking to remove a higher level setting are you? just to add a
per host parameter..
@DaanHoogland yes, essentially just changing it to a "host-scoped" setting
that can be configured while adding/editing the host.
> it's always better to have all these hosts accessible on the same port
@sureshanaparti I agree, but not allowing it to be configured on a more
granular level, requiring the same port to be used across all zones for
instance, when there was a use case for changing the port already (I imagine)
seems too stiff for me.
Out of curiosity, in the use case that prompted the introduction of this
setting, was there more than a single zone? Or was this intended for a specific
zone/cluster?
> a new host parameter (that can be updated through add or update host call)
can provide flexibility, but it's mostly NULL/empty (when not defined or
default port is used)
My idea was defining it through an entry in `host_details`, as this would be
for exceptions. There wouldn't be a column that is mostly empty this way.
> and is not applicable for VMware hosts
The current setting is not applicable for VMware hosts already, and we have
other resource-specific settings not applicable for different hypervisors.
> I think @winterhazel has a use case, and we can discuss. I only doubt if
it should be in this PR or block it. It seems like a new enhancement/feature to
me.
@DaanHoogland not wanting to block it either, just suggesting that there may
be an use case for it, so it might be good to introduce it in a granular
fashion already.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
