Re: [libvirt] [RFC] Add uptime to API returning Dom Info
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 pkre...@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
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC] Add uptime to API returning Dom Info
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
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC] Add uptime to API returning Dom Info
On Mon, Mar 30, 2015 at 1:16 PM, Peter Krempa pkre...@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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
