Hi,
I have updated and rebased the patches on top of current master.
On 24.2.2012 20:35, Stephen Gallagher wrote:
On Fri, 2012-02-24 at 19:10 +0100, Jan Cholasta wrote:
> Hi,
>
> this patchset contains these patches:
>
> [PATCH 1/3] SSH: Save SSH host name aliases
>
> This is needed in order to properly generate the known_hosts file when
> user uses ssh with non-FQDN host name.
>
I fixed up a few minor issues with this patch. The check for
in_transaction should not have been conditional on ret (in the current
code it was fine, but defensively it only needs to check for
in_transaction).
It's against our coding policy to mix pointers to structs and plain
structs in the same variable declaration, so I split
+ struct addrinfo ai_hint;
+ struct addrinfo *ai = NULL;
for clarity.
You forgot to check for memory allocation error in
sss_ssh_cmd_get_host_pubkeys() when creating the new primary name. I
added it.
And I moved the SSS_SSH_KNOWN_HOSTS_PATH and
SSS_SSH_KNOWN_HOSTS_TEMP_TMPL define to patch 0003 where it belongs.
Thanks.
Functionally, this is fine, so ack with my changes (attached).
I have done this additional little change in ipa_hostid.c:
@@ -227,8 +231,7 @@ hosts_get_connect_done(struct tevent_req *subreq)
subreq = ipa_host_info_send(state, state->ev, state->sysdb,
sdap_id_op_handle(state->op),
state->ctx->sdap_id_ctx->opts,
state->name,
- state->ctx->ipa_opts->host_map,
- state->ctx->ipa_opts->hostgroup_map,
+ state->ctx->ipa_opts->host_map, NULL,
state->ctx->host_search_bases);
if (!subreq) {
tevent_req_error(req, ENOMEM);
>
> [PATCH 2/3] SSH: Move responder and client common code to util
>
> Don't manage user-specific known_hosts file in sss_ssh_knownhostsproxy.
>
> Continue connecting to SSH server even when SSSD is not running in
> sss_ssh_knownhostsproxy and don't drop the connection when the
> sss_ssh_knownhostsproxy process receives a signal.
>
>
https://fedorahosted.org/sssd/ticket/1179
>
https://fedorahosted.org/sssd/ticket/1184
>
I'm not comfortable enough in this code to review it. I'd prefer to have
Jakub's sign-off here.
It would also be really nice if you could split this into multiple
logical patches, because it's really overwhelming.
Done, I have split this patch into 3 patches:
[PATCH 2/5] SSH: Refactor responder and client common code
[PATCH 3/5] UTIL: Add function for atomic I/O
[PATCH 4/5] SSH: Continue connecting to SSH server even when SSSD is not
running in sss_ssh_knownhostsproxy
Additionally, don't drop the connection when the sss_ssh_knownhostsproxy
process receives a signal.
https://fedorahosted.org/sssd/ticket/1179
https://fedorahosted.org/sssd/ticket/1184
>
> [PATCH 3/3] SSH: Manage global known_hosts file in the responder
>
>
https://fedorahosted.org/sssd/ticket/1193
>
> The known_hosts file is stored in /var/lib/sss/pubconf/known_hosts.
Nack
You need to wrap the write() in a loop to make sure it writes
completely. It also needs to loop on EINTR.
Fixed.
I have also moved the "don't manage user-specific known_hosts" bit of
the previous patch to this patch.
Honza
--
Jan Cholasta