The hostname is resolved just once since commit 58125210ab3b ("Tools:
hv: cache FQDN in kvp_daemon to avoid timeouts") to make sure the VM
responds within the timeout limits to requests from the host.
If for some reason getaddrinfo fails, the string returned by the
"FullyQualifiedDomainName" request contains some error string, which is
then used by tools on the host side.
Adjust the code to resolve the current hostname in a separate thread.
This thread loops until getaddrinfo returns success. During this time
all "FullyQualifiedDomainName" requests will be answered with an empty
string.
Signed-off-by: Olaf Hering <[email protected]>
---
tools/hv/Makefile | 2 ++
tools/hv/hv_kvp_daemon.c | 69 ++++++++++++++++++++++++++++++++----------------
2 files changed, 48 insertions(+), 23 deletions(-)
diff --git a/tools/hv/Makefile b/tools/hv/Makefile
index b57143d9459c..3b5481015a84 100644
--- a/tools/hv/Makefile
+++ b/tools/hv/Makefile
@@ -22,6 +22,8 @@ ALL_PROGRAMS := $(patsubst %,$(OUTPUT)%,$(ALL_TARGETS))
ALL_SCRIPTS := hv_get_dhcp_info.sh hv_get_dns_info.sh hv_set_ifconfig.sh
+$(OUTPUT)hv_kvp_daemon: LDFLAGS += -lpthread
+
all: $(ALL_PROGRAMS)
export srctree OUTPUT CC LD CFLAGS
diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c
index e9ef4ca6a655..22cf1c4dbf5c 100644
--- a/tools/hv/hv_kvp_daemon.c
+++ b/tools/hv/hv_kvp_daemon.c
@@ -41,6 +41,7 @@
#include <net/if.h>
#include <limits.h>
#include <getopt.h>
+#include <pthread.h>
/*
* KVP protocol: The user mode component first registers with the
@@ -85,7 +86,7 @@ static char *processor_arch;
static char *os_build;
static char *os_version;
static char *lic_version = "Unknown version";
-static char full_domain_name[HV_KVP_EXCHANGE_MAX_VALUE_SIZE];
+static char *full_domain_name;
static struct utsname uts_buf;
/*
@@ -1327,27 +1328,53 @@ static int kvp_set_ip_info(char *if_name, struct hv_kvp_ipaddr_value *new_val)
return error;
}
-
-static void
-kvp_get_domain_name(char *buffer, int length)
+/*
+ * Async retrival of Fully Qualified Domain Name because getaddrinfo takes an
+ * unpredictable amount of time to finish.
+ */
+static void *kvp_getaddrinfo(void *p)
{
- struct addrinfo hints, *info ;
- int error = 0;
+ char *tmp, **str_ptr = (char **)p;
+ char hostname[HOST_NAME_MAX + 1];
+ struct addrinfo *info, hints = {
+ .ai_family = AF_INET, /* Get only ipv4 addrinfo. */
+ .ai_socktype = SOCK_STREAM,
+ .ai_flags = AI_CANONNAME,
+ };
+ int ret;
+
+ if (gethostname(hostname, sizeof(hostname) - 1) < 0)
+ goto out;
+
+ do {
+ ret = getaddrinfo(hostname, NULL, &hints, &info);
+ if (ret)
+ sleep(1);
+ } while (ret);
+
+ ret = asprintf(&tmp, "%s", info->ai_canonname);
+ freeaddrinfo(info);
+ if (ret <= 0)
+ goto out;
+
+ if (ret > HV_KVP_EXCHANGE_MAX_VALUE_SIZE)
+ tmp[HV_KVP_EXCHANGE_MAX_VALUE_SIZE - 1] = '\0';
+ *str_ptr = tmp;
- gethostname(buffer, length);
- memset(&hints, 0, sizeof(hints));
- hints.ai_family = AF_INET; /*Get only ipv4 addrinfo. */
- hints.ai_socktype = SOCK_STREAM;
- hints.ai_flags = AI_CANONNAME;
+out:
+ pthread_exit(NULL);
+}
+
+static void kvp_obtain_domain_name(char **str_ptr)
+{
+ pthread_t t;
- error = getaddrinfo(buffer, NULL, &hints, &info);
- if (error != 0) {
- snprintf(buffer, length, "getaddrinfo failed: 0x%x %s",
- error, gai_strerror(error));
+ if (pthread_create(&t, NULL, kvp_getaddrinfo, str_ptr)) {
+ syslog(LOG_ERR, "pthread_create failed; error: %d %s",
+ errno, strerror(errno));
return;
}
- snprintf(buffer, length, "%s", info->ai_canonname);
- freeaddrinfo(info);
+ pthread_detach(t);
}
void print_usage(char *argv[])
@@ -1412,11 +1439,7 @@ int main(int argc, char *argv[])
* Retrieve OS release information.
*/
kvp_get_os_info();
- /*
- * Cache Fully Qualified Domain Name because getaddrinfo takes an
- * unpredictable amount of time to finish.
- */
- kvp_get_domain_name(full_domain_name, sizeof(full_domain_name));
+ kvp_obtain_domain_name(&full_domain_name);
if (kvp_file_init()) {
syslog(LOG_ERR, "Failed to initialize the pools");
@@ -1571,7 +1594,7 @@ int main(int argc, char *argv[])
switch (hv_msg->body.kvp_enum_data.index) {
case FullyQualifiedDomainName:
- strcpy(key_value, full_domain_name);
+ strcpy(key_value, full_domain_name ? : "");
strcpy(key_name, "FullyQualifiedDomainName");
break;
case IntegrationServicesVersion:
On Thu, Oct 24, 2019 at 04:49:43PM +0200, Olaf Hering wrote:
>The hostname is resolved just once since commit 58125210ab3b ("Tools:
>hv: cache FQDN in kvp_daemon to avoid timeouts") to make sure the VM
>responds within the timeout limits to requests from the host.
>
>If for some reason getaddrinfo fails, the string returned by the
>"FullyQualifiedDomainName" request contains some error string, which is
>then used by tools on the host side.
>
>Adjust the code to resolve the current hostname in a separate thread.
>This thread loops until getaddrinfo returns success. During this time
>all "FullyQualifiedDomainName" requests will be answered with an empty
>string.
>
>Signed-off-by: Olaf Hering <[email protected]>
Thank for the patch Olaf. However, it seems to break build for me:
/usr/bin/ld: hv_kvp_daemon-in.o: in function `kvp_obtain_domain_name':
/home/sasha/linux/tools/hv/hv_kvp_daemon.c:1372: undefined reference to `pthread_create'
/usr/bin/ld: /home/sasha/linux/tools/hv/hv_kvp_daemon.c:1377: undefined reference to `pthread_detach'
collect2: error: ld returned 1 exit status
--
Thanks,
Sasha
Am Mon, 28 Oct 2019 12:17:54 -0400
schrieb Sasha Levin <[email protected]>:
> undefined reference to `pthread_create'
Does "make V=1 -C tools/hv" work for you?
This is what I use.
Olaf
Am Mon, 28 Oct 2019 18:49:55 +0100
schrieb Olaf Hering <[email protected]>:
> Am Mon, 28 Oct 2019 12:17:54 -0400
> schrieb Sasha Levin <[email protected]>:
> > undefined reference to `pthread_create'
> Does "make V=1 -C tools/hv" work for you?
Did you have a chance to check why the patch fails for you?
Olaf
On Mon, Oct 28, 2019 at 06:49:55PM +0100, Olaf Hering wrote:
>Am Mon, 28 Oct 2019 12:17:54 -0400
>schrieb Sasha Levin <[email protected]>:
>
>> undefined reference to `pthread_create'
>
>Does "make V=1 -C tools/hv" work for you?
>This is what I use.
It works before I apply this patch, but breaks after:
$ make V=1 -C tools/hv
make: Entering directory '/home/sasha/linux/tools/hv'
make -f /home/sasha/linux/tools/build/Makefile.build dir=. obj=hv_kvp_daemon
make[1]: Entering directory '/home/sasha/linux/tools/hv'
gcc -Wp,-MD,./.hv_kvp_daemon.o.d -Wp,-MT,hv_kvp_daemon.o -O2 -Wall -g -D_GNU_SOURCE -Iinclude -D"BUILD_STR(s)=#s" -c -o hv_kvp_daemon.o hv_kvp_daemon.c
ld -r -o hv_kvp_daemon-in.o hv_kvp_daemon.o
make[1]: Leaving directory '/home/sasha/linux/tools/hv'
gcc -O2 -Wall -g -D_GNU_SOURCE -Iinclude hv_kvp_daemon-in.o -o hv_kvp_daemon
make -f /home/sasha/linux/tools/build/Makefile.build dir=. obj=hv_vss_daemon
make[1]: Entering directory '/home/sasha/linux/tools/hv'
gcc -Wp,-MD,./.hv_vss_daemon.o.d -Wp,-MT,hv_vss_daemon.o -O2 -Wall -g -D_GNU_SOURCE -Iinclude -D"BUILD_STR(s)=#s" -c -o hv_vss_daemon.o hv_vss_daemon.c
ld -r -o hv_vss_daemon-in.o hv_vss_daemon.o
make[1]: Leaving directory '/home/sasha/linux/tools/hv'
gcc -O2 -Wall -g -D_GNU_SOURCE -Iinclude hv_vss_daemon-in.o -o hv_vss_daemon
make -f /home/sasha/linux/tools/build/Makefile.build dir=. obj=hv_fcopy_daemon
make[1]: Entering directory '/home/sasha/linux/tools/hv'
gcc -Wp,-MD,./.hv_fcopy_daemon.o.d -Wp,-MT,hv_fcopy_daemon.o -O2 -Wall -g -D_GNU_SOURCE -Iinclude -D"BUILD_STR(s)=#s" -c -o hv_fcopy_daemon.o hv_fcopy_daemon.c
ld -r -o hv_fcopy_daemon-in.o hv_fcopy_daemon.o
make[1]: Leaving directory '/home/sasha/linux/tools/hv'
gcc -O2 -Wall -g -D_GNU_SOURCE -Iinclude hv_fcopy_daemon-in.o -o hv_fcopy_daemon
make: Leaving directory '/home/sasha/linux/tools/hv'
$ git am ~/incoming/_PATCH_v1_tools-hv_async_name_resolution_in_kvp_daemon.patch
Applying: tools/hv: async name resolution in kvp_daemon
$ make V=1 -C tools/hv
make: Entering directory '/home/sasha/linux/tools/hv'
make -f /home/sasha/linux/tools/build/Makefile.build dir=. obj=hv_kvp_daemon
make[1]: Entering directory '/home/sasha/linux/tools/hv'
gcc -Wp,-MD,./.hv_kvp_daemon.o.d -Wp,-MT,hv_kvp_daemon.o -O2 -Wall -g -D_GNU_SOURCE -Iinclude -D"BUILD_STR(s)=#s" -c -o hv_kvp_daemon.o hv_kvp_daemon.c
ld -r -o hv_kvp_daemon-in.o hv_kvp_daemon.o
make[1]: Leaving directory '/home/sasha/linux/tools/hv'
gcc -O2 -Wall -g -D_GNU_SOURCE -Iinclude -lpthread hv_kvp_daemon-in.o -o hv_kvp_daemon
/usr/bin/ld: hv_kvp_daemon-in.o: in function `kvp_obtain_domain_name':
/home/sasha/linux/tools/hv/hv_kvp_daemon.c:1372: undefined reference to `pthread_create'
/usr/bin/ld: /home/sasha/linux/tools/hv/hv_kvp_daemon.c:1377: undefined reference to `pthread_detach'
collect2: error: ld returned 1 exit status
make: *** [Makefile:36: hv_kvp_daemon] Error 1
make: Leaving directory '/home/sasha/linux/tools/hv'
--
Thanks,
Sasha
Olaf Hering <[email protected]> writes:
> The hostname is resolved just once since commit 58125210ab3b ("Tools:
> hv: cache FQDN in kvp_daemon to avoid timeouts") to make sure the VM
> responds within the timeout limits to requests from the host.
>
> If for some reason getaddrinfo fails, the string returned by the
> "FullyQualifiedDomainName" request contains some error string, which is
> then used by tools on the host side.
>
> Adjust the code to resolve the current hostname in a separate thread.
> This thread loops until getaddrinfo returns success. During this time
> all "FullyQualifiedDomainName" requests will be answered with an empty
> string.
>
> Signed-off-by: Olaf Hering <[email protected]>
> ---
> tools/hv/Makefile | 2 ++
> tools/hv/hv_kvp_daemon.c | 69 ++++++++++++++++++++++++++++++++----------------
> 2 files changed, 48 insertions(+), 23 deletions(-)
>
> diff --git a/tools/hv/Makefile b/tools/hv/Makefile
> index b57143d9459c..3b5481015a84 100644
> --- a/tools/hv/Makefile
> +++ b/tools/hv/Makefile
> @@ -22,6 +22,8 @@ ALL_PROGRAMS := $(patsubst %,$(OUTPUT)%,$(ALL_TARGETS))
>
> ALL_SCRIPTS := hv_get_dhcp_info.sh hv_get_dns_info.sh hv_set_ifconfig.sh
>
> +$(OUTPUT)hv_kvp_daemon: LDFLAGS += -lpthread
> +
> all: $(ALL_PROGRAMS)
>
> export srctree OUTPUT CC LD CFLAGS
> diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c
> index e9ef4ca6a655..22cf1c4dbf5c 100644
> --- a/tools/hv/hv_kvp_daemon.c
> +++ b/tools/hv/hv_kvp_daemon.c
> @@ -41,6 +41,7 @@
> #include <net/if.h>
> #include <limits.h>
> #include <getopt.h>
> +#include <pthread.h>
>
> /*
> * KVP protocol: The user mode component first registers with the
> @@ -85,7 +86,7 @@ static char *processor_arch;
> static char *os_build;
> static char *os_version;
> static char *lic_version = "Unknown version";
> -static char full_domain_name[HV_KVP_EXCHANGE_MAX_VALUE_SIZE];
> +static char *full_domain_name;
> static struct utsname uts_buf;
>
> /*
> @@ -1327,27 +1328,53 @@ static int kvp_set_ip_info(char *if_name, struct hv_kvp_ipaddr_value *new_val)
> return error;
> }
>
> -
> -static void
> -kvp_get_domain_name(char *buffer, int length)
> +/*
> + * Async retrival of Fully Qualified Domain Name because getaddrinfo takes an
> + * unpredictable amount of time to finish.
> + */
> +static void *kvp_getaddrinfo(void *p)
> {
> - struct addrinfo hints, *info ;
> - int error = 0;
> + char *tmp, **str_ptr = (char **)p;
> + char hostname[HOST_NAME_MAX + 1];
> + struct addrinfo *info, hints = {
> + .ai_family = AF_INET, /* Get only ipv4 addrinfo. */
> + .ai_socktype = SOCK_STREAM,
> + .ai_flags = AI_CANONNAME,
> + };
> + int ret;
> +
> + if (gethostname(hostname, sizeof(hostname) - 1) < 0)
> + goto out;
> +
> + do {
> + ret = getaddrinfo(hostname, NULL, &hints, &info);
> + if (ret)
> + sleep(1);
Is it only EAI_AGAIN or do you see any other return values which justify
the retry? I'm afraid that in case of a e.g. non-existing hostname we'll
be infinitely looping with EAI_FAIL.
> + } while (ret);
> +
> + ret = asprintf(&tmp, "%s", info->ai_canonname);
> + freeaddrinfo(info);
> + if (ret <= 0)
> + goto out;
> +
> + if (ret > HV_KVP_EXCHANGE_MAX_VALUE_SIZE)
> + tmp[HV_KVP_EXCHANGE_MAX_VALUE_SIZE - 1] = '\0';
> + *str_ptr = tmp;
>
> - gethostname(buffer, length);
> - memset(&hints, 0, sizeof(hints));
> - hints.ai_family = AF_INET; /*Get only ipv4 addrinfo. */
> - hints.ai_socktype = SOCK_STREAM;
> - hints.ai_flags = AI_CANONNAME;
> +out:
> + pthread_exit(NULL);
> +}
> +
> +static void kvp_obtain_domain_name(char **str_ptr)
> +{
> + pthread_t t;
>
> - error = getaddrinfo(buffer, NULL, &hints, &info);
> - if (error != 0) {
> - snprintf(buffer, length, "getaddrinfo failed: 0x%x %s",
> - error, gai_strerror(error));
> + if (pthread_create(&t, NULL, kvp_getaddrinfo, str_ptr)) {
> + syslog(LOG_ERR, "pthread_create failed; error: %d %s",
> + errno, strerror(errno));
> return;
> }
> - snprintf(buffer, length, "%s", info->ai_canonname);
> - freeaddrinfo(info);
> + pthread_detach(t);
I think this should be complemented with pthread_cancel/pthread_join
before exiting main().
> }
>
> void print_usage(char *argv[])
> @@ -1412,11 +1439,7 @@ int main(int argc, char *argv[])
> * Retrieve OS release information.
> */
> kvp_get_os_info();
> - /*
> - * Cache Fully Qualified Domain Name because getaddrinfo takes an
> - * unpredictable amount of time to finish.
> - */
> - kvp_get_domain_name(full_domain_name, sizeof(full_domain_name));
> + kvp_obtain_domain_name(&full_domain_name);
>
> if (kvp_file_init()) {
> syslog(LOG_ERR, "Failed to initialize the pools");
> @@ -1571,7 +1594,7 @@ int main(int argc, char *argv[])
>
> switch (hv_msg->body.kvp_enum_data.index) {
> case FullyQualifiedDomainName:
> - strcpy(key_value, full_domain_name);
> + strcpy(key_value, full_domain_name ? : "");
> strcpy(key_name, "FullyQualifiedDomainName");
> break;
> case IntegrationServicesVersion:
--
Vitaly
Am Thu, 07 Nov 2019 14:39:11 +0100
schrieb Vitaly Kuznetsov <[email protected]>:
> Olaf Hering <[email protected]> writes:
> Is it only EAI_AGAIN or do you see any other return values which justify
> the retry? I'm afraid that in case of a e.g. non-existing hostname we'll
> be infinitely looping with EAI_FAIL.
I currently do not have a setup that reproduces the failure.
I think if this thread loops forever, so be it.
The report I have shows "getaddrinfo failed: 0xfffffffe Name or service not known" on the host side.
And that went away within the VM once "networking was fixed", whatever this means.
But hv_kvp_daemon would report the error string forever.
> > + pthread_detach(t);
> I think this should be complemented with pthread_cancel/pthread_join
> before exiting main().
If the thread is detached, it is exactly that: detached. Why do you think the main thread should wait for the detached thread?
Olaf
Olaf Hering <[email protected]> writes:
> Am Thu, 07 Nov 2019 14:39:11 +0100
> schrieb Vitaly Kuznetsov <[email protected]>:
>
>> Olaf Hering <[email protected]> writes:
>
>> Is it only EAI_AGAIN or do you see any other return values which justify
>> the retry? I'm afraid that in case of a e.g. non-existing hostname we'll
>> be infinitely looping with EAI_FAIL.
>
> I currently do not have a setup that reproduces the failure.
> I think if this thread loops forever, so be it.
>
> The report I have shows "getaddrinfo failed: 0xfffffffe Name or service not known" on the host side.
> And that went away within the VM once "networking was fixed", whatever this means.
> But hv_kvp_daemon would report the error string forever.
Looping forever with a permanent error is pretty unusual...
>
>> > + pthread_detach(t);
>> I think this should be complemented with pthread_cancel/pthread_join
>> before exiting main().
>
> If the thread is detached, it is exactly that: detached. Why do you think the main thread should wait for the detached thread?
Ah, my bad: you actually can't join a detached thread, scratch my
comment.
--
Vitaly
Am Thu, 07 Nov 2019 15:15:45 +0100
schrieb Vitaly Kuznetsov <[email protected]>:
> Looping forever with a permanent error is pretty unusual...
That might be true, but how would you detect a permanent error?
Just because the DNS server is gone for one hour does not mean it will be gone forever.
Olaf
Olaf Hering <[email protected]> writes:
> Am Thu, 07 Nov 2019 15:15:45 +0100
> schrieb Vitaly Kuznetsov <[email protected]>:
>
>> Looping forever with a permanent error is pretty unusual...
>
> That might be true, but how would you detect a permanent error?
> Just because the DNS server is gone for one hour does not mean it will be gone forever.
'man 3 getaddrinfo' lists the following:
EAI_ADDRFAMILY
EAI_AGAIN
EAI_BADFLAGS
EAI_FAIL
EAI_FAMILY
EAI_MEMORY
EAI_NODATA
EAI_NONAME
EAI_SERVICE
EAI_SOCKTYPE
EAI_SYSTEM
I *think* what you're aiming at is EAI_AGAIN and EAI_FAIL, the rest
should probably terminate the resolver thread (e.g. AF_INET is
unsupported or something like that).
--
Vitaly
Am Sat, 2 Nov 2019 00:18:56 -0400
schrieb Sasha Levin <[email protected]>:
> make[1]: Leaving directory '/home/sasha/linux/tools/hv'
> gcc -O2 -Wall -g -D_GNU_SOURCE -Iinclude -lpthread hv_kvp_daemon-in.o -o hv_kvp_daemon
> /usr/bin/ld: hv_kvp_daemon-in.o: in function `kvp_obtain_domain_name':
> /home/sasha/linux/tools/hv/hv_kvp_daemon.c:1372: undefined reference to `pthread_create'
> /usr/bin/ld: /home/sasha/linux/tools/hv/hv_kvp_daemon.c:1377: undefined reference to `pthread_detach'
Is perhaps '-pthread' instead of -lpthread' required, as indicated by pthread_create(3)?
Not sure why it happens to work for me. But I will make this change for the upcoming v2.
Olaf
Am Thu, 07 Nov 2019 15:44:27 +0100
schrieb Vitaly Kuznetsov <[email protected]>:
> I *think* what you're aiming at is EAI_AGAIN and EAI_FAIL, the rest
> should probably terminate the resolver thread (e.g. AF_INET is
> unsupported or something like that).
The thread aims for success. Resolving of the hostname can take some time.
Maybe the network is not fully functional when the thread starts.
Maybe the VM admin does further tweaks while the thread is running.
IMO there is no downside to wait for success.
Olaf