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

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(a)redhat.com&gt; > --- > > 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(a)redhat.com&gt; 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