[libvirt] [RFC] Add uptime to API returning Dom Info
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 */ }; /** 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"); + if (ret < 0) + return -1; + + pidinfo = fopen(proc, "r"); + 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); + /* 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; @@ -2595,7 +2613,7 @@ static int qemuDomainGetInfo(virDomainPtr dom, if (!virDomainObjIsActive(vm)) { info->cpuTime = 0; } else { - if (qemuGetProcessInfo(&(info->cpuTime), NULL, NULL, vm->pid, 0) < 0) { + if (qemuGetProcessInfo(&(info->cpuTime), NULL, NULL, vm->pid, 0, &(info->upTime)) < 0) { virReportError(VIR_ERR_OPERATION_FAILED, "%s", _("cannot read cputime for domain")); goto cleanup; @@ -11474,7 +11492,7 @@ qemuDomainMemoryStats(virDomainPtr dom, if (ret >= 0 && ret < nr_stats) { long rss; - if (qemuGetProcessInfo(NULL, NULL, &rss, vm->pid, 0) < 0) { + if (qemuGetProcessInfo(NULL, NULL, &rss, vm->pid, 0, NULL) < 0) { virReportError(VIR_ERR_OPERATION_FAILED, "%s", _("cannot get RSS for domain")); } else { diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index d90e6b5..193f501 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -901,6 +901,7 @@ struct remote_domain_get_info_ret { /* insert@1 */ unsigned hyper memory; unsigned short nrVirtCpu; unsigned hyper cpuTime; + unsigned hyper upTime; }; struct remote_domain_save_args { diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index e614f77..1f54d5d 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -531,6 +531,7 @@ struct remote_domain_get_info_ret { uint64_t memory; u_short nrVirtCpu; uint64_t cpuTime; + uint64_t upTime; }; struct remote_domain_save_args { remote_nonnull_domain dom; diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c index 6951db2..027790f 100644 --- a/tools/virsh-domain-monitor.c +++ b/tools/virsh-domain-monitor.c @@ -1246,6 +1246,14 @@ cmdDominfo(vshControl *ctl, const vshCmd *cmd) vshPrint(ctl, "%-15s %.1lfs\n", _("CPU time:"), cpuUsed); } + if (info.upTime != 0) { + double upTime = info.upTime; + + upTime /= 1000000000.0; + + vshPrint(ctl, "%-15s %.1lfs\n", _("Uptime:"), upTime); + } + if (info.maxMem != UINT_MAX) vshPrint(ctl, "%-15s %lu KiB\n", _("Max memory:"), info.maxMem); -- 2.1.0
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 ...
+ /* 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
On Mon, Mar 30, 2015 at 1:16 PM, Peter Krempa <pkrempa@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
On Mon, Mar 30, 2015 at 13:26:40 +0530, Nehal J Wani wrote:
On Mon, Mar 30, 2015 at 1:16 PM, Peter Krempa <pkrempa@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
4) If the guest OS reboots, it's uptime restarts
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) ?
At first, I'd not call it uptime at all. If you want to return the uptime of the guest you need to use the guest agent connection as you need to ask the actual guest for it's uptime. Guessing it is always wrong. For the calculation you are doing you should invent a different name as that definitely is not uptime and will be confused with guest uptime. Additionally I don't think that the stat would be useful. Peter
participants (2)
-
Nehal J Wani -
Peter Krempa