Re: [libvirt] [PATCH] RFC: audit: add shmem resource type
On Sun, Jul 12, 2015 at 12:36 PM, Martin Kletzander mklet...@redhat.com wrote: As said in previous attempt by Luyao to do this, the auditing should be handled differently, there's also lot more info to audit. Thanks for the patch, but this must be done in another way. Could you please give me a pointer? thanks -- Marc-André Lureau -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/3] Qemu: add CMT support
Hi Ren, Thank you for clarifying. I really do not have any more comments on the patch. Regards, Prerna On Thu, Jul 9, 2015 at 12:27 PM, Ren, Qiaowei qiaowei@intel.com wrote: On Jul 7, 2015 15:51, Ren, Qiaowei wrote: On Jul 6, 2015 14:49, Prerna wrote: On Sun, Jul 5, 2015 at 5:13 PM, Qiaowei Ren qiaowei@intel.com mailto:qiaowei@intel.com wrote: One RFC in https://www.redhat.com/archives/libvir-list/2015-June/msg01509.html CMT (Cache Monitoring Technology) can be used to measure the usage of cache by VM running on the host. This patch will extend the bulk stats API (virDomainListGetStats) to add this field. Applications based on libvirt can use this API to achieve cache usage of VM. Because CMT implementation in Linux kernel is based on perf mechanism, this patch will enable perf event for CMT when VM is created and disable it when VM is destroyed. Hi Ren, One query wrt this implementation. I see you make a perf ioctl to gather CMT stats each time the stats API is invoked. If the CMT stats are exposed by a hardware counter, then this implies logging on a per-cpu (or per-socket ???) basis. This also implies that the value read will vary as the CPU (or socket) on which it is being called changes. Now, with this background, if we need real-world stats on a VM, we need this perf ioctl executed on all CPUs/ sockets on which the VM ran. Also, once done, we will need to aggregate results from each of these sources. In this implementation, I am missing this -- there seems no control over which physical CPU the libvirt worker thread will run and collect the perf data from. Data collected from this implementation might not accurately model the system state. I _think_ libvirt currently has no way of directing a worker thread to collect stats from a given CPU -- if we do, I would be happy to learn about it :) Prerna, thanks for your reply. I checked the CMT implementation in kernel, and noticed that the series implement new -count() of pmu driver which can aggregate the results from each cpu if perf type is PERF_TYPE_INTEL_CQM . The following is the link for the patch: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?i d=bfe1fc d2688f557a6b6a88f59ea7619228728bd7 So I guess that this patch just need to set right perf type and cpu=-1. Do you think this is ok? Hi Prerna, Do you have more comments on this patch series? I would be glad to update my implementation. ^-^ Qiaowei -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/3] Qemu: add CMT support
On Jul 13, 2015 09:58, Prerna wrote: Hi Ren, Thank you for clarifying. I really do not have any more comments on the patch. Ok. Thanks for your reply. So do you know how to get more feedback from libvirt maintainer, or how to contact with them? Now I could not open libvirt.org and don't know anything but this maillist. ^-^ Thanks, Qiaowei -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/3] Qemu: add CMT support
On Jul 7, 2015 15:51, Ren, Qiaowei wrote: On Jul 6, 2015 14:49, Prerna wrote: On Sun, Jul 5, 2015 at 5:13 PM, Qiaowei Ren qiaowei@intel.com mailto:qiaowei@intel.com wrote: One RFC in https://www.redhat.com/archives/libvir-list/2015-June/msg01509.html CMT (Cache Monitoring Technology) can be used to measure the usage of cache by VM running on the host. This patch will extend the bulk stats API (virDomainListGetStats) to add this field. Applications based on libvirt can use this API to achieve cache usage of VM. Because CMT implementation in Linux kernel is based on perf mechanism, this patch will enable perf event for CMT when VM is created and disable it when VM is destroyed. Hi Ren, One query wrt this implementation. I see you make a perf ioctl to gather CMT stats each time the stats API is invoked. If the CMT stats are exposed by a hardware counter, then this implies logging on a per-cpu (or per-socket ???) basis. This also implies that the value read will vary as the CPU (or socket) on which it is being called changes. Now, with this background, if we need real-world stats on a VM, we need this perf ioctl executed on all CPUs/ sockets on which the VM ran. Also, once done, we will need to aggregate results from each of these sources. In this implementation, I am missing this -- there seems no control over which physical CPU the libvirt worker thread will run and collect the perf data from. Data collected from this implementation might not accurately model the system state. I _think_ libvirt currently has no way of directing a worker thread to collect stats from a given CPU -- if we do, I would be happy to learn about it :) Prerna, thanks for your reply. I checked the CMT implementation in kernel, and noticed that the series implement new -count() of pmu driver which can aggregate the results from each cpu if perf type is PERF_TYPE_INTEL_CQM . The following is the link for the patch: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?i d=bfe1fc d2688f557a6b6a88f59ea7619228728bd7 So I guess that this patch just need to set right perf type and cpu=-1. Do you think this is ok? Thanks, Qiaowei Could anyone help review this patch series? I would be glad to get more comments. ^-^ Thanks, Qiaowei -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] RFC: audit: add shmem resource type
Hi Marc-André: On 07/13/2015 05:49 AM, Marc-André Lureau wrote: On Sun, Jul 12, 2015 at 12:36 PM, Martin Kletzander mklet...@redhat.com wrote: As said in previous attempt by Luyao to do this, the auditing should be handled differently, there's also lot more info to audit. Thanks for the patch, but this must be done in another way. Could you please give me a pointer? Here : http://www.redhat.com/archives/libvir-list/2015-July/msg00316.html As Martin's reply in this mail, the memory size is not enought, we need audit more information (just like shmobj name, the server socket path). Thanks a lot for your patch. thanks Luyao -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] RFC: audit: add shmem resource type
On Fri, Jul 10, 2015 at 06:11:35PM +0200, Marc-André Lureau wrote: Provide information about shared memory resources in audit log. Notes: - the same shm used several times will add up. This is a very uncommon case, but we may want to account only the different shm names instead. - the shm may exist before the VMs was started, so the shm may not actually be created by the VM (it can be there before, or created by the server for instance). https://bugzilla.redhat.com/show_bug.cgi?id=1218603 Signed-off-by: Marc-André Lureau marcandre.lur...@redhat.com --- docs/auditlog.html.in| 17 + src/conf/domain_audit.c | 10 ++ src/conf/domain_audit.h | 6 ++ src/conf/domain_conf.c | 21 + src/conf/domain_conf.h | 1 + src/libvirt_private.syms | 2 ++ 6 files changed, 57 insertions(+) As said in previous attempt by Luyao to do this, the auditing should be handled differently, there's also lot more info to audit. Thanks for the patch, but this must be done in another way. diff --git a/docs/auditlog.html.in b/docs/auditlog.html.in index 8a007ca..a6e5f6d 100644 --- a/docs/auditlog.html.in +++ b/docs/auditlog.html.in @@ -172,6 +172,23 @@ ddUpdated memory size in bytes/dd /dl +h4a name=typeresourceshmemShared Memory/a/h4 + +p + The codemsg/code field will include the following sub-fields +/p + +dl + dtreason/dt + ddThe reason which caused the resource to be assigned to happen/dd + dtresrc/dt + ddThe type of resource assigned. Set to codeshmem/code/dd + dtold-shmem/dt + ddOriginal memory size in bytes, or 0/dd + dtnew-shmem/dt + ddUpdated memory size in bytes/dd +/dl + h4a name=typeresourcediskDisk/a/h4 p The codemsg/code field will include the following sub-fields diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c index caebdba..bc81aec 100644 --- a/src/conf/domain_audit.c +++ b/src/conf/domain_audit.c @@ -783,6 +783,14 @@ virDomainAuditMemory(virDomainObjPtr vm, } void +virDomainAuditShmem(virDomainObjPtr vm, +unsigned long long oldmem, unsigned long long newmem, +const char *reason, bool success) +{ +return virDomainAuditResource(vm, shmem, oldmem, newmem, reason, success); +} + +void virDomainAuditVcpu(virDomainObjPtr vm, unsigned int oldvcpu, unsigned int newvcpu, const char *reason, bool success) @@ -885,6 +893,8 @@ virDomainAuditStart(virDomainObjPtr vm, const char *reason, bool success) virDomainAuditMemory(vm, 0, virDomainDefGetMemoryActual(vm-def), start, true); +virDomainAuditShmem(vm, 0, virDomainDefGetShmem(vm-def), +start, true); virDomainAuditVcpu(vm, 0, vm-def-vcpus, start, true); if (vm-def-iothreads) virDomainAuditIOThread(vm, 0, vm-def-iothreads, start, true); diff --git a/src/conf/domain_audit.h b/src/conf/domain_audit.h index 97dadca..3db6ace 100644 --- a/src/conf/domain_audit.h +++ b/src/conf/domain_audit.h @@ -96,6 +96,12 @@ void virDomainAuditMemory(virDomainObjPtr vm, const char *reason, bool success) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(4); +void virDomainAuditShmem(virDomainObjPtr vm, + unsigned long long oldmem, + unsigned long long newmem, + const char *reason, + bool success) +ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(4); void virDomainAuditVcpu(virDomainObjPtr vm, unsigned int oldvcpu, unsigned int newvcpu, diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 5a9a88d..378aa1a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -7575,6 +7575,27 @@ virDomainDefGetMemoryActual(virDomainDefPtr def) } +/** + * virDomainDefGetShmem: + * @def: domain definition + * + * Returns the current shared memory size usable by the domain described by + * @def. + */ +unsigned long long +virDomainDefGetShmem(virDomainDefPtr def) +{ +unsigned long long ret = 0; +size_t i; + +for (i = 0; i def-nshmems; i++) { +ret += def-shmems[i]-size; +} + +return ret; +} + + static int virDomainControllerModelTypeFromString(const virDomainControllerDef *def, const char *model) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 50750c1..041d619 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2287,6 +2287,7 @@ struct _virDomainDef { unsigned long long virDomainDefGetMemoryInitial(virDomainDefPtr def); void virDomainDefSetMemoryInitial(virDomainDefPtr def, unsigned long long size); unsigned long long virDomainDefGetMemoryActual(virDomainDefPtr def); +unsigned long long virDomainDefGetShmem(virDomainDefPtr def); typedef enum { VIR_DOMAIN_KEY_WRAP_CIPHER_NAME_AES, diff