Re: [libvirt] [PATCH] RFC: audit: add shmem resource type

2015-07-12 Thread Marc-André Lureau
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

2015-07-12 Thread Prerna
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

2015-07-12 Thread Ren, Qiaowei
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

2015-07-12 Thread Ren, Qiaowei
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

2015-07-12 Thread lhuang

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

2015-07-12 Thread Martin Kletzander

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