Re: [RFC] Add uptime to API returning Dom Info

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

Attachment: signature.asc
Description: Digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]