Improving smbd_smb2_request_setinfo_done/smbd_smb2_request_error tracing

[Stefan Metzmacher]

Am 22.05.24 um 00:23 schrieb Andrew Bartlett via samba-technical:
> Kia Ora Team,
> 
> I've been helping a client out who is improving the Go SMB2 client
> implementation and today we got caught in the weeds just trying to set
> ACLs on Samba.
> 
> We probably have something really silly wrong, as despite being root
> and just setting an ACL with smbcacls, it fails.
> 
> Anyway, it was heavy going debugging partly as our logs are sparse in
> that code, and partly as even when we have logs, the very helpful
> 'location' macros don't work as well as we would like
> 
> So smbd2_smb2_request_error() prints the location of the failure, but
> if that is just smbd_smb2_request_setinfo_done():137 we don't get many
> more clues.
> 
> Is there a legitimate way this this be unwrapped (I see I'm not meant
> to read the guts of the tevent_req) to be
> 
> smbd_smb2_request_error_ex(req, status, NULL, subreq-
>> internal.finish_location)
> 
> I still don't do much with tevent, so I'm wondering is that legitimate
> in this code?  Will that always be filled in?  Or could it be NULL, and
> can I rely on it at least being initialised?
> 
> (eg so i can do):
> 
> smbd_smb2_request_error_ex(req, status, NULL,subreq-
>> internal.finish_location ? subreq->internal.finish_location
> : __location__)
> 
> Anyway, just wanted to give feedback and perhaps improve debugging here
> for the next person, but lost.

tevent_req_print() could be used.

If you have that please also change the other cases in:
smbd_smb2_request_getinfo_done
smbd_smb2_request_lock_done
smbd_smb2_request_flush_done
smbd_smb2_request_ioctl_done
smbd_smb2_request_find_done
smbd_smb2_request_create_done
... (for every SMB2 opcode)

But maybe a explicit debug message after the call to
the _recv function and before any TALLOC_FREE(subreq)
would be better than passing it to smbd_smb2_request_error_ex().
smbd_smb2_request_ioctl_done() has something in there
already...

I guess in mode cases we want nt_errstr(status) and tevent_req_print(subreq, subreq),
plus some additional value like in smbd_smb2_request_ioctl_donesmbd_smb2_request_ioctl_done()
there disconnect would also be useful to print...

metze