On Mon, Jan 23, 2012 at 04:21:17PM +0100, Martin Kletzander wrote:

One more parameter added into the function parsing /proc/<pid>/stat and the call of the function is fixed as well. --- src/qemu/qemu_driver.c | 25 ++++++++++++++++++------- 1 files changed, 18 insertions(+), 7 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 608e82a..e82c0d5 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1049,12 +1049,13 @@ cleanup:

static int -qemudGetProcessInfo(unsigned long long *cpuTime, int *lastCpu, int pid, - int tid) +qemudGetProcessInfo(unsigned long long *cpuTime, int *lastCpu, long *vm_rss, + int pid, int tid) { char *proc; FILE *pidinfo; unsigned long long usertime, systime; + long rss; int cpu; int ret;

@@ -1071,6 +1072,8 @@ qemudGetProcessInfo(unsigned long long *cpuTime, int *lastCpu, int pid, *cpuTime = 0; if (lastCpu) *lastCpu = 0; + if (vm_rss) + *vm_rss = 0; VIR_FREE(proc); return 0; } @@ -1082,10 +1085,10 @@ qemudGetProcessInfo(unsigned long long *cpuTime, int *lastCpu, int pid, /* pid -> stime */ "%*d %*s %*c %*d %*d %*d %*d %*d %*u %*u %*u %*u %*u %llu %llu" /* cutime -> endcode */ - "%*d %*d %*d %*d %*d %*u %*u %*d %*u %*u %*u %*u" + "%*d %*d %*d %*d %*d %*u %*u %ld %*u %*u %*u %*u" /* startstack -> processor */ "%*u %*u %*u %*u %*u %*u %*u %*u %*u %*u %*d %d", - &usertime, &systime, &cpu) != 3) { + &usertime, &systime, &rss, &cpu) != 4) { VIR_FORCE_FCLOSE(pidinfo); VIR_WARN("cannot parse process status data"); errno = -EINVAL; @@ -1102,9 +1105,16 @@ qemudGetProcessInfo(unsigned long long *cpuTime, int *lastCpu, int pid, if (lastCpu) *lastCpu = cpu;

+ /* We got pages + * We want kiloBytes + * _SC_PAGESIZE is page size in Bytes + * So calculate, but first lower the pagesize so we don't get overflow */ + if (vm_rss) + *vm_rss = rss * (sysconf(_SC_PAGESIZE) >> 10);

- VIR_DEBUG("Got status for %d/%d user=%llu sys=%llu cpu=%d", - pid, tid, usertime, systime, cpu); + + VIR_DEBUG("Got status for %d/%d user=%llu sys=%llu cpu=%d rss=%ld", + pid, tid, usertime, systime, cpu, rss);

VIR_FORCE_FCLOSE(pidinfo);

@@ -2066,7 +2076,7 @@ static int qemudDomainGetInfo(virDomainPtr dom, if (!virDomainObjIsActive(vm)) { info->cpuTime = 0; } else { - if (qemudGetProcessInfo(&(info->cpuTime), NULL, vm->pid, 0) < 0) { + if (qemudGetProcessInfo(&(info->cpuTime), NULL, NULL, vm->pid, 0) < 0) { qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", _("cannot read cputime for domain")); goto cleanup; @@ -3648,6 +3658,7 @@ qemudDomainGetVcpus(virDomainPtr dom, if (priv->vcpupids != NULL && qemudGetProcessInfo(&(info[i].cpuTime), &(info[i].cpu), + NULL, vm->pid, priv->vcpupids[i]) < 0) { virReportSystemError(errno, "%s",

Looks just fine, ACK Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

On Tue, Jan 24, 2012 at 02:25:05PM +0100, Martin Kletzander wrote:

Added RSS information gathering into qemuMemoryStats into qemu driver and the reporting into virsh dommemstat. --- v2: - fixed sign for the ret variable (can be negative)

include/libvirt/libvirt.h.in | 7 ++++++- src/qemu/qemu_driver.c | 23 ++++++++++++++++++----- tools/virsh.c | 2 ++ 3 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 958e5a6..d5ef061 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -884,11 +884,16 @@ typedef enum {

/* Current balloon value (in KB). */ VIR_DOMAIN_MEMORY_STAT_ACTUAL_BALLOON = 6, + + /* Resident Set Size of the process running the domain. This value + * is in kB */ + VIR_DOMAIN_MEMORY_STAT_RSS = 7, + /* * The number of statistics supported by this version of the interface. * To add new statistics, add them to the enum and increase this value. */ - VIR_DOMAIN_MEMORY_STAT_NR = 7, + VIR_DOMAIN_MEMORY_STAT_NR = 8,

#ifdef VIR_ENUM_SENTINELS VIR_DOMAIN_MEMORY_STAT_LAST = VIR_DOMAIN_MEMORY_STAT_NR

Change to the API is just fine

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 6600afd..20d3d84 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7978,7 +7978,7 @@ qemudDomainMemoryStats (virDomainPtr dom, { struct qemud_driver *driver = dom->conn->privateData; virDomainObjPtr vm; - unsigned int ret = -1; + int ret = -1;

I was a bit surprized by the original statement actually, that looks better to me, and needed now. Hopefully I got that right

virCheckFlags(0, -1);

@@ -7997,14 +7997,27 @@ qemudDomainMemoryStats (virDomainPtr dom, if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0) goto cleanup;

- if (virDomainObjIsActive(vm)) { + if (!virDomainObjIsActive(vm)) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("domain is not running")); + } else { qemuDomainObjPrivatePtr priv = vm->privateData; qemuDomainObjEnterMonitor(driver, vm); ret = qemuMonitorGetMemoryStats(priv->mon, stats, nr_stats); qemuDomainObjExitMonitor(driver, vm); - } else { - qemuReportError(VIR_ERR_OPERATION_INVALID, - "%s", _("domain is not running")); + + if (ret >= 0 && ret < nr_stats) { + long rss; + if (qemudGetProcessInfo(NULL, NULL, &rss, vm->pid, 0) < 0) { + qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("cannot get RSS for domain")); + } else { + stats[ret].tag = VIR_DOMAIN_MEMORY_STAT_RSS; + stats[ret].val = rss; + ret++; + } + + } }

if (qemuDomainObjEndJob(driver, vm) == 0) diff --git a/tools/virsh.c b/tools/virsh.c index d635b56..d5bbabf 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -1871,6 +1871,8 @@ cmdDomMemStat(vshControl *ctl, const vshCmd *cmd) vshPrint (ctl, "available %llu\n", stats[i].val); if (stats[i].tag == VIR_DOMAIN_MEMORY_STAT_ACTUAL_BALLOON) vshPrint (ctl, "actual %llu\n", stats[i].val); + if (stats[i].tag == VIR_DOMAIN_MEMORY_STAT_RSS) + vshPrint (ctl, "rss %llu\n", stats[i].val); }

virDomainFree(dom);

Looks fine, and works for me, ACK too I'm pushing the 2 patches but please provide one missing item: the python bindings, basically in python/libvirt-override.c you need to add the new information in libvirt_virDomainMemoryStats() it's an easy patch and we can add it while in feature freeze next week thanks ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/