[libvirt] [PATCH] remote_daemon: Log host boot time

[Cole Robinson]

On 12/17/19 8:25 AM, Michal Privoznik wrote:
> This is not strictly needed, but it makes sure we initialize the
> @bootTime global variable. Thing is, in order to validate XATTRs
> and prune those set in some previous runs of the host, a
> timestamp is recorded in XATTRs. The host boot time was unique
> enough so it was chosen as the timestamp value. And to avoid
> querying and parsing /proc/uptime every time, the query function
> does that only once and stores the boot time in a global
> variable. However, the only time the query function is called is
> in a child process that does lock files and changes seclabels. So
> effectively, we are doing exactly what we wanted to prevent from
> happening.
> 
> The fix is simple, call the query function whilst the daemon is
> initializing.
> 
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
> 
> Since this will be used by security driver only, I was tempted to put
> this somewhere under src/security/, but especially because of split
> daemon I didn't. Placing this into remote_daemon.c makes sure all
> sub-daemons will have the variable initialized and won't suffer the
> problem.
> 
>  src/remote/remote_daemon.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/src/remote/remote_daemon.c b/src/remote/remote_daemon.c
> index b400b1dd10..5debb6ce97 100644
> --- a/src/remote/remote_daemon.c
> +++ b/src/remote/remote_daemon.c
> @@ -57,6 +57,7 @@
>  #include "virgettext.h"
>  #include "util/virnetdevopenvswitch.h"
>  #include "virsystemd.h"
> +#include "virhostuptime.h"
>  
>  #include "driver.h"
>  
> @@ -1020,6 +1021,7 @@ int main(int argc, char **argv) {
>      bool implicit_conf = false;
>      char *run_dir = NULL;
>      mode_t old_umask;
> +    unsigned long long bootTime;
>  
>      struct option opts[] = {
>          { "verbose", no_argument, &verbose, 'v'},
> @@ -1151,6 +1153,12 @@ int main(int argc, char **argv) {
>          exit(EXIT_FAILURE);
>      }
>  
> +    if (virHostGetBootTime(&bootTime) < 0) {
> +        VIR_DEBUG("Unable to get host boot time");
> +    } else {
> +        VIR_DEBUG("host boot time: %lld", bootTime);
> +    }
> +

Please add a comment that this is initializing some global state that we
need elsewhere, because otherwise it won't be obvious why this is here.
With that:

Reviewed-by: Cole Robinson <crobinso at redhat.com>

Better IMO would be have an explicit virHostBootTimeInit function, and
any usage of GetBootTime would fail if the init hasn't been called yet.
It would make this case more self descriptive, and make sure any new
users aren't doing it wrong

- Cole