Re: [PATCH rfc 2/2] fabrics: allow user to pass hostname instead of traddr

[Sagi Grimberg <sagi@grimberg.me>]

On 3/24/20 2:46 AM, Max Gurtovoy wrote:
> 
> On 3/24/2020 11:03 AM, Sagi Grimberg wrote:
>> Some users would like to use well known hostnames instead
>> of remembering ip addresses. So, allow users to pass --hostname
>> and we will attempt to resolve against the DNS.
>>
>> Signed-off-by: Sagi Grimberg <sagi@grimberg.me>
>> ---
>>   fabrics.c | 75 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 75 insertions(+)
>>
>> diff --git a/fabrics.c b/fabrics.c
>> index a7d628b1f0c9..7bd95c4b0d10 100644
>> --- a/fabrics.c
>> +++ b/fabrics.c
>> @@ -33,6 +33,10 @@
>>   #include <sys/stat.h>
>>   #include <stddef.h>
>> +#include <sys/types.h>
>> +#include <arpa/inet.h>
>> +#include <netdb.h>
>> +
>>   #include "util/parser.h"
>>   #include "nvme-ioctl.h"
>>   #include "nvme-status.h"
>> @@ -60,6 +64,7 @@ static struct config {
>>       char *nqn;
>>       char *transport;
>>       char *traddr;
>> +    char *hostname;
>>       char *trsvcid;
>>       char *host_traddr;
>>       char *hostnqn;
>> @@ -857,6 +862,54 @@ static int build_options(char *argstr, int 
>> max_len, bool discover)
>>       return 0;
>>   }
>> +static int hostname2traddr(struct config *cfg)
>> +{
>> +    struct addrinfo *host_info, hints = {.ai_family = AF_UNSPEC};
>> +    char addrstr[NVMF_TRADDR_SIZE];
>> +    const char *p;
>> +    int ret;
>> +
>> +    if (cfg->traddr) {
>> +        fprintf(stderr, "hostname and traddr cannot be passed 
>> together\n");
>> +        return -EINVAL;
>> +    }
>> +
>> +    ret = getaddrinfo(cfg->hostname, NULL, &hints, &host_info);
>> +    if (ret) {
>> +        fprintf(stderr, "failed to get host info for %s\n", 
>> cfg->hostname);
>> +        return ret;
>> +    }
>> +
>> +    switch (host_info->ai_family) {
>> +    case AF_INET:
>> +        p = inet_ntop(host_info->ai_family,
>> +            &(((struct sockaddr_in *)host_info->ai_addr)->sin_addr),
>> +            addrstr, NVMF_TRADDR_SIZE);
>> +        break;
>> +    case AF_INET6:
>> +        p = inet_ntop(host_info->ai_family,
>> +            &(((struct sockaddr_in6 *)host_info->ai_addr)->sin6_addr),
>> +            addrstr, NVMF_TRADDR_SIZE);
>> +        break;
>> +    default:
>> +        fprintf(stderr, "unrecognized address family (%d) %s\n",
>> +            host_info->ai_family, cfg->hostname);
>> +        ret = -EINVAL;
>> +        goto free_addrinfo;
>> +    }
>> +
>> +    if (!p) {
>> +        fprintf(stderr, "failed to get traddr for %s\n", cfg->hostname);
>> +        ret = -errno;
>> +        goto free_addrinfo;
>> +    }
>> +    cfg->traddr = strdup(addrstr);
>> +
>> +free_addrinfo:
>> +    freeaddrinfo(host_info);
>> +    return ret;
>> +}
>> +
>>   static int connect_ctrl(struct nvmf_disc_rsp_page_entry *e)
>>   {
>>       char argstr[BUF_SIZE], *p;
>> @@ -1230,6 +1283,13 @@ static int discover_from_conf_file(const char 
>> *desc, char *argstr,
>>           if (cfg.persistent && !cfg.keep_alive_tmo)
>>               cfg.keep_alive_tmo = NVMF_DEF_DISC_TMO;
>> +        if (cfg.hostname) {
>> +            ret = hostname2traddr(&cfg);
>> +            if (ret)
>> +                goto out;
>> +            cfg.hostname = NULL;
>> +        }
>> +
> 
> I guess you add the NULL to check mutual exclusion later ?

The NULL assignment here is because we are looping on the conf
file entries which can have a different hostname. The other
locations are a one-shot so this assignment is not needed.

> Can we check it in build_options ?

When we get a hostname we resolve the traddr which is what
build_options needs (hostname not an option that is passed to
the driver).

Does that answer your question?

_______________________________________________
linux-nvme mailing list
linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme