Re: [libvirt] [PATCH v3 1/4] add new virDomainMemoryDump API

2014-02-27 Thread Daniel P. Berrange 
On Thu, Feb 27, 2014 at 03:56:42PM +0800, qiaonuohan wrote:
 --memory-only option is introduced without compression supported. Therefore,
 this is a freature regression of virsh dump. Now qemu has support dumping 
 memory
 in kdump-compressed format. This patch is adding new virDomainMemoryDump API,
 so that the format in which qemu dump domain's memory can be specified.
 
 Signed-off-by: Qiao Nuohan qiaonuo...@cn.fujitsu.com_
 ---
  include/libvirt/libvirt.h.in | 21 ++
  src/access/viraccessperm.c   |  2 +-
  src/access/viraccessperm.h   |  6 +++
  src/driver.h |  7 
  src/libvirt.c| 95 
 
  src/libvirt_public.syms  |  5 +++
  src/remote/remote_driver.c   |  1 +
  src/remote/remote_protocol.x | 15 ++-
  src/remote_protocol-structs  |  7 
  src/test/test_driver.c   | 17 +++-
  10 files changed, 172 insertions(+), 4 deletions(-)
 
 diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
 index 295d551..ab6efca 100644
 --- a/include/libvirt/libvirt.h.in
 +++ b/include/libvirt/libvirt.h.in
 @@ -1180,6 +1180,19 @@ typedef enum {
  VIR_DUMP_MEMORY_ONLY  = (1  4), /* use dump-guest-memory */
  } virDomainCoreDumpFlags;
  
 +/* Domain memory dump's format */
 +typedef enum {
 +VIR_MEMORY_DUMP_COMPRESS_ZLIB   = 1, /* dump guest memory in
 +kdump-compressed format, with
 +zlib-compressed */
 +VIR_MEMORY_DUMP_COMPRESS_LZO= 2, /* dump guest memory in
 +kdump-compressed format, with
 +lzo-compressed */
 +VIR_MEMORY_DUMP_COMPRESS_SNAPPY = 3, /* dump guest memory in
 +kdump-compressed format, with
 +snappy-compressed */
 +} virMemoryDumpFormat;

s/virMemoryDumpFormat/virDomainCoreDumpFormat/

No need to assign values to each entry here since this
is a plain enum, not bitflags.

Also  s/MEMORY_COMPRESS/DOMAIN_CORE_FORMAT/ for every member

And add  VIR_MEMORY_DUMP_FORMAT_RAW  for the non-compressed
format, which probably ought to be the default (ie first).

 +
  /* Domain migration flags. */
  typedef enum {
  VIR_MIGRATE_LIVE  = (1  0), /* live migration */
 @@ -1731,6 +1744,14 @@ int virDomainCoreDump   
 (virDomainPtr domain,
   unsigned int flags);
  
  /*
 + * Domain core dump
 + */
 +int virDomainMemoryDump (virDomainPtr domain,
 + const char *to,
 + unsigned int flags,
 + unsigned int dumpformat);

Convention is that the 'flags' parameter should always be the very last
one.

The name would be better as virDomainCoreDumpWithFormat IMHO to
show it is an extension of virDomainCoreDump.

 diff --git a/src/access/viraccessperm.h b/src/access/viraccessperm.h
 index 6d14f05..2537fbf 100644
 --- a/src/access/viraccessperm.h
 +++ b/src/access/viraccessperm.h
 @@ -205,6 +205,12 @@ typedef enum {
  VIR_ACCESS_PERM_DOMAIN_CORE_DUMP, /* Dump guest core */
  
  /**
 + * @desc: Dump domain's memory only
 + * @message: Dumping domain's memory requires authorization
 + */
 +VIR_ACCESS_PERM_DOMAIN_MEMORY_DUMP, /* Dump guest's memory only */
 +
 +/**

There's no point in inventing a new permission - the same permission
as the existing API is more than OK.


 +/**
 + * virDomainMemoryDump:
 + * @domain: a domain object
 + * @to: path for the core file
 + * @flags: bitwise-OR of virDomainCoreDumpFlags
 + * @dumpformat: format of domain memory's dump
 + *
 + * This method will only dump the memory of a domain on a given file for
 + * analysis, so @flag must includes VIR_DUMP_MEMORY_ONLY.

This restricton seems rather dubious. This new method's functionality
should be a super-set of virDomainCoreDump - ie everything that the
old method could do, should be possible in the new method. All we
do is add the ability to choose between formats.

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

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

Re: [libvirt] [PATCH v3 1/4] add new virDomainMemoryDump API

2014-02-27 Thread Eric Blake 
On 02/27/2014 07:43 AM, Daniel P. Berrange wrote:

 +/* Domain memory dump's format */

/* */ comments don't make it through to the html doc pages.  Here, you
want something like:

/**
 * virDomainCoreDumpFormat:
 * Values for specifying different formats of domain core dumps.
 */

 +typedef enum {
 +VIR_MEMORY_DUMP_COMPRESS_ZLIB   = 1, /* dump guest memory in
 +kdump-compressed format, with
 +zlib-compressed */
 +VIR_MEMORY_DUMP_COMPRESS_LZO= 2, /* dump guest memory in
 +kdump-compressed format, with
 +lzo-compressed */
 +VIR_MEMORY_DUMP_COMPRESS_SNAPPY = 3, /* dump guest memory in
 +kdump-compressed format, with
 +snappy-compressed */
 +} virMemoryDumpFormat;
 

Missing a *_LAST enum value, under proper #ifdef.

 s/virMemoryDumpFormat/virDomainCoreDumpFormat/
 
 No need to assign values to each entry here since this
 is a plain enum, not bitflags.
 
 Also  s/MEMORY_COMPRESS/DOMAIN_CORE_FORMAT/ for every member
 
 And add  VIR_MEMORY_DUMP_FORMAT_RAW  for the non-compressed
 format, which probably ought to be the default (ie first).
 

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v3 1/4] add new virDomainMemoryDump API

2014-02-26 Thread qiaonuohan 
--memory-only option is introduced without compression supported. Therefore,
this is a freature regression of virsh dump. Now qemu has support dumping memory
in kdump-compressed format. This patch is adding new virDomainMemoryDump API,
so that the format in which qemu dump domain's memory can be specified.

Signed-off-by: Qiao Nuohan qiaonuo...@cn.fujitsu.com_
---
 include/libvirt/libvirt.h.in | 21 ++
 src/access/viraccessperm.c   |  2 +-
 src/access/viraccessperm.h   |  6 +++
 src/driver.h |  7 
 src/libvirt.c| 95 
 src/libvirt_public.syms  |  5 +++
 src/remote/remote_driver.c   |  1 +
 src/remote/remote_protocol.x | 15 ++-
 src/remote_protocol-structs  |  7 
 src/test/test_driver.c   | 17 +++-
 10 files changed, 172 insertions(+), 4 deletions(-)

diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 295d551..ab6efca 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -1180,6 +1180,19 @@ typedef enum {
 VIR_DUMP_MEMORY_ONLY  = (1  4), /* use dump-guest-memory */
 } virDomainCoreDumpFlags;
 
+/* Domain memory dump's format */
+typedef enum {
+VIR_MEMORY_DUMP_COMPRESS_ZLIB   = 1, /* dump guest memory in
+kdump-compressed format, with
+zlib-compressed */
+VIR_MEMORY_DUMP_COMPRESS_LZO= 2, /* dump guest memory in
+kdump-compressed format, with
+lzo-compressed */
+VIR_MEMORY_DUMP_COMPRESS_SNAPPY = 3, /* dump guest memory in
+kdump-compressed format, with
+snappy-compressed */
+} virMemoryDumpFormat;
+
 /* Domain migration flags. */
 typedef enum {
 VIR_MIGRATE_LIVE  = (1  0), /* live migration */
@@ -1731,6 +1744,14 @@ int virDomainCoreDump   
(virDomainPtr domain,
  unsigned int flags);
 
 /*
+ * Domain core dump
+ */
+int virDomainMemoryDump (virDomainPtr domain,
+ const char *to,
+ unsigned int flags,
+ unsigned int dumpformat);
+
+/*
  * Screenshot of current domain console
  */
 char *  virDomainScreenshot (virDomainPtr domain,
diff --git a/src/access/viraccessperm.c b/src/access/viraccessperm.c
index d517c66..af4f05e 100644
--- a/src/access/viraccessperm.c
+++ b/src/access/viraccessperm.c
@@ -42,7 +42,7 @@ VIR_ENUM_IMPL(virAccessPermDomain,
   init_control, inject_nmi, send_input, send_signal, 
fs_trim,
   block_read, block_write, mem_read,
   open_graphics, open_device, screenshot,
-  open_namespace);
+  open_namespace, memory_dump);
 
 VIR_ENUM_IMPL(virAccessPermInterface,
   VIR_ACCESS_PERM_INTERFACE_LAST,
diff --git a/src/access/viraccessperm.h b/src/access/viraccessperm.h
index 6d14f05..2537fbf 100644
--- a/src/access/viraccessperm.h
+++ b/src/access/viraccessperm.h
@@ -205,6 +205,12 @@ typedef enum {
 VIR_ACCESS_PERM_DOMAIN_CORE_DUMP, /* Dump guest core */
 
 /**
+ * @desc: Dump domain's memory only
+ * @message: Dumping domain's memory requires authorization
+ */
+VIR_ACCESS_PERM_DOMAIN_MEMORY_DUMP, /* Dump guest's memory only */
+
+/**
  * @desc: Use domain power management
  * @message: Using domain power management requires authoriation
  */
diff --git a/src/driver.h b/src/driver.h
index fbfaac4..7c001e6 100644
--- a/src/driver.h
+++ b/src/driver.h
@@ -306,6 +306,12 @@ typedef int
 const char *to,
 unsigned int flags);
 
+typedef int
+(*virDrvDomainMemoryDump)(virDomainPtr domain,
+  const char *to,
+  unsigned int flags,
+  unsigned int dumpformat);
+
 typedef char *
 (*virDrvDomainScreenshot)(virDomainPtr domain,
   virStreamPtr stream,
@@ -1200,6 +1206,7 @@ struct _virDriver {
 virDrvDomainSaveImageGetXMLDesc domainSaveImageGetXMLDesc;
 virDrvDomainSaveImageDefineXML domainSaveImageDefineXML;
 virDrvDomainCoreDump domainCoreDump;
+virDrvDomainMemoryDump domainMemoryDump;
 virDrvDomainScreenshot domainScreenshot;
 virDrvDomainSetVcpus domainSetVcpus;
 virDrvDomainSetVcpusFlags domainSetVcpusFlags;
diff --git a/src/libvirt.c b/src/libvirt.c
index dcf6b53..ec106a6 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -2999,6 +2999,101 @@ error:
 return -1;
 }
 
+/**
+ * virDomainMemoryDump:
+ * @domain: a domain object
+ * @to: path for the core file
+ * @flags: bitwise-OR of virDomainCoreDumpFlags
+ *