2:56 a.m.
On Mon, Mar 30, 2015 at 1:16 PM, Peter Krempa <pkrempa(a)redhat.com> wrote:
On Mon, Mar 30, 2015 at 03:29:40 +0530, Nehal J Wani wrote:
> This is an attempt to fix: https://bugzilla.redhat.com/show_bug.cgi?id=812911
>
> Calculate vm uptime by subtracting process starttime from system uptime:
> Almost equivalent to:
> echo $(($(($(awk '{print $1}' /proc/uptime)*1e9 - $(($(cut -d " "
-f22 /proc/$PID/stat)*1e7))))/(1.0 * 1e9)))
>
> ---
> include/libvirt/libvirt-domain.h | 1 +
> src/qemu/qemu_driver.c | 32 +++++++++++++++++++++++++-------
> src/remote/remote_protocol.x | 1 +
> src/remote_protocol-structs | 1 +
> tools/virsh-domain-monitor.c | 8 ++++++++
> 5 files changed, 36 insertions(+), 7 deletions(-)
>
> diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
> index 7be4219..2df0241 100644
> --- a/include/libvirt/libvirt-domain.h
> +++ b/include/libvirt/libvirt-domain.h
> @@ -275,6 +275,7 @@ struct _virDomainInfo {
> unsigned long memory; /* the memory in KBytes used by the domain */
> unsigned short nrVirtCpu; /* the number of virtual CPUs for the domain */
> unsigned long long cpuTime; /* the CPU time used in nanoseconds */
> + unsigned long long upTime; /* the total uptime in nanoseconds */
> };
>
The public structs can't be changed once they are released. The next
best place will be the bulk stats API that is extensible.
> /**
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index f07e4fb..0b5098f 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -1309,11 +1309,12 @@ static char *qemuConnectGetCapabilities(virConnectPtr conn)
{
>
> static int
> qemuGetProcessInfo(unsigned long long *cpuTime, int *lastCpu, long *vm_rss,
> - pid_t pid, int tid)
> + pid_t pid, int tid, unsigned long long *upTime)
> {
> char *proc;
> FILE *pidinfo;
> - unsigned long long usertime = 0, systime = 0;
> + unsigned long long usertime = 0, systime = 0, starttime = 0;
> + double _uptime;
> long rss = 0;
> int cpu = 0;
> int ret;
> @@ -1337,13 +1338,29 @@ qemuGetProcessInfo(unsigned long long *cpuTime, int *lastCpu,
long *vm_rss,
> /* pid -> stime */
> "%*d %*s %*c %*d %*d %*d %*d %*d %*u %*u %*u %*u %*u %llu
%llu"
> /* cutime -> endcode */
> - "%*d %*d %*d %*d %*d %*d %*u %*u %ld %*u %*u %*u"
> + "%*d %*d %*d %*d %*d %*d %llu %*u %ld %*u %*u %*u"
> /* startstack -> processor */
> "%*u %*u %*u %*u %*u %*u %*u %*u %*u %*u %*d %d",
> - &usertime, &systime, &rss, &cpu) != 4) {
> + &usertime, &systime, &starttime, &rss, &cpu) !=
5) {
> VIR_WARN("cannot parse process status data");
> }
>
> + ret = virAsprintf(&proc, "/proc/uptime");
This copies a static string using virAsprintf?
> + if (ret < 0)
> + return -1;
> +
> + pidinfo = fopen(proc, "r");
And uses it in a place where you can use static strings? That doesn't
make sense.
> + VIR_FREE(proc);
> +
> + if (!pidinfo || fscanf(pidinfo, "%lf %*f", &_uptime) != 1) {
> + VIR_WARN("cannot parse machine uptime data");
> + }
> +
> + if (upTime)
> + *upTime = 1000ull * 1000ull * 1000ull * _uptime -
> + (1000ull * 1000ull * 1000ull * starttime)
> + / (unsigned long long)sysconf(_SC_CLK_TCK);
This certainly does not calculate uptime of the guest, merely just the
time since the process started. This will not work at least in these
cases:
1) When the VM is migrated to a different host
2) When the VM is saved
3) The uptime will not be accurate when the guest was paused
Is storing the timestamp at which the VM was started and then taking a
diff with the current timestamp right way to go?
How is uptime defined in case of (2) and (3) ?
...
> +
> /* We got jiffies
> * We want nanoseconds
> * _SC_CLK_TCK is jiffies per second
> @@ -1404,7 +1421,8 @@ qemuDomainHelperGetVcpus(virDomainObjPtr vm, virVcpuInfoPtr
info, int maxinfo,
> &(info[i].cpu),
> NULL,
> vm->pid,
> - priv->vcpupids[i]) < 0) {
> + priv->vcpupids[i],
> + NULL) < 0) {
> virReportSystemError(errno, "%s",
> _("cannot get vCPU placement &
pCPU time"));
> return -1;
Peter
--
Nehal J Wani