Re: [libvirt] [PATCH v2 1/2] make qemu dump memory in kdump-compressed format

2014-02-26 Thread Qiao Nuohan 

On 02/25/2014 06:56 PM, Jiri Denemark wrote:

On Tue, Feb 25, 2014 at 17:54:30 +0800, Qiao Nuohan 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 used to add --compress and
[--compression-format]string to virsh dump --memory-only and send
dump-guest-memory command to qemu with dump format specified to one of elf,
kdump-zlib, kdump-lzo and kdump-snappy.

Signed-off-by: Qiao Nuohanqiaonuo...@cn.fujitsu.com
---
   include/libvirt/libvirt.h.in | 15 ++-
   src/driver.h |  3 ++-
   src/libvirt.c| 10 --
   src/qemu/qemu_driver.c   | 30 +
   src/qemu/qemu_monitor.c  |  6 +++---
   src/qemu/qemu_monitor.h  |  3 ++-
   src/qemu/qemu_monitor_json.c |  4 +++-
   src/qemu/qemu_monitor_json.h |  3 ++-
   src/remote/remote_protocol.x |  1 +
   src/test/test_driver.c   | 12 +++-
   tests/qemumonitorjsontest.c  |  2 +-
   tools/virsh-domain.c | 45 
+++-
   12 files changed, 113 insertions(+), 21 deletions(-)

diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 295d551..aae3c49 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -1180,6 +1180,18 @@ typedef enum {
   VIR_DUMP_MEMORY_ONLY  = (1  4), /* use dump-guest-memory */
   } virDomainCoreDumpFlags;

+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 */
@@ -1728,7 +1740,8 @@ int
virDomainManagedSaveRemove(virDomainPtr dom,
*/
   int virDomainCoreDump   (virDomainPtr domain,
const char *to,
- unsigned int flags);
+ unsigned int flags,
+ const unsigned int
memory_dump_format);


This is not allowed and I believe make syntax-check should fail on the
corresponding change to remote_protocol.x. If you want to add a new
parameter to an existing API, you have keep the API as is and instead
create a new API with a different name that will have all the parameters
you need.


Hello Jirka,

Thanks for pointing it out for me, I finally found it's libvirt's policy never
to change a committed public API. But driver handling methods are allowed to be
changed, am I right?

--
Regards
Qiao Nuohan

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

Re: [libvirt] [PATCH v2 1/2] make qemu dump memory in kdump-compressed format

2014-02-26 Thread Eric Blake 
On 02/26/2014 01:08 AM, Qiao Nuohan wrote:

int virDomainCoreDump   (virDomainPtr domain,
 const char *to,
 - unsigned int flags);
 + unsigned int flags,
 + const unsigned int
 memory_dump_format);

 This is not allowed and I believe make syntax-check should fail on the
 corresponding change to remote_protocol.x. If you want to add a new

I don't know if syntax-check would catch it, but the fact that you would
have had to modify src/remote_protocol-structs is indeed a red flag that
reviewers will catch.

 parameter to an existing API, you have keep the API as is and instead
 create a new API with a different name that will have all the parameters
 you need.
 
 Hello Jirka,
 
 Thanks for pointing it out for me, I finally found it's libvirt's policy
 never
 to change a committed public API. But driver handling methods are
 allowed to be
 changed, am I right?

Changing driver handling methods is fine.  That said, how are you going
to provide the extra parameter to the driver handler without also having
a public API?  We have a script that validates that all the driver
function names are close derivatives of the public API names, because
that's the only way to programmatically enforce that we have proper ACL
checks in all APIs.  So really, the easiest would be adding a new API
and new driver methods, with a better signature.

-- 
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

Re: [libvirt] [PATCH v2 1/2] make qemu dump memory in kdump-compressed format

2014-02-26 Thread Daniel P. Berrange 
On Wed, Feb 26, 2014 at 04:08:11PM +0800, Qiao Nuohan wrote:
 On 02/25/2014 06:56 PM, Jiri Denemark wrote:
 On Tue, Feb 25, 2014 at 17:54:30 +0800, Qiao Nuohan 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 used to add --compress and
 [--compression-format]string to virsh dump --memory-only and send
 dump-guest-memory command to qemu with dump format specified to one of elf,
 kdump-zlib, kdump-lzo and kdump-snappy.
 
 Signed-off-by: Qiao Nuohanqiaonuo...@cn.fujitsu.com
 ---
include/libvirt/libvirt.h.in | 15 ++-
src/driver.h |  3 ++-
src/libvirt.c| 10 --
src/qemu/qemu_driver.c   | 30 +
src/qemu/qemu_monitor.c  |  6 +++---
src/qemu/qemu_monitor.h  |  3 ++-
src/qemu/qemu_monitor_json.c |  4 +++-
src/qemu/qemu_monitor_json.h |  3 ++-
src/remote/remote_protocol.x |  1 +
src/test/test_driver.c   | 12 +++-
tests/qemumonitorjsontest.c  |  2 +-
tools/virsh-domain.c | 45 
  +++-
12 files changed, 113 insertions(+), 21 deletions(-)
 
 diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
 index 295d551..aae3c49 100644
 --- a/include/libvirt/libvirt.h.in
 +++ b/include/libvirt/libvirt.h.in
 @@ -1180,6 +1180,18 @@ typedef enum {
VIR_DUMP_MEMORY_ONLY  = (1  4), /* use dump-guest-memory */
} virDomainCoreDumpFlags;
 
 +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 */
 @@ -1728,7 +1740,8 @@ int
 virDomainManagedSaveRemove(virDomainPtr dom,
 */
int virDomainCoreDump   (virDomainPtr domain,
 const char *to,
 - unsigned int flags);
 + unsigned int flags,
 + const unsigned int
 memory_dump_format);
 
 This is not allowed and I believe make syntax-check should fail on the
 corresponding change to remote_protocol.x. If you want to add a new
 parameter to an existing API, you have keep the API as is and instead
 create a new API with a different name that will have all the parameters
 you need.
 
 Hello Jirka,
 
 Thanks for pointing it out for me, I finally found it's libvirt's policy never
 to change a committed public API. But driver handling methods are allowed to 
 be
 changed, am I right?

Yes  no. There should be a 1-1 mapping of APIs in libvirt.h to driver
callbacks defined in src/driver.h. The driver.h maps into the remote
RPC protocol and that is considered part of the ABI and thus must not
change.

What is permitted is in the implementation of the new APIs is to share
code with the impl of the old API. ie in qemu_driver.c

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 v2 1/2] make qemu dump memory in kdump-compressed format

2014-02-25 Thread Jiri Denemark 
On Tue, Feb 25, 2014 at 17:54:30 +0800, Qiao Nuohan 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 used to add --compress and
 [--compression-format] string to virsh dump --memory-only and send
 dump-guest-memory command to qemu with dump format specified to one of elf,
 kdump-zlib, kdump-lzo and kdump-snappy.
 
 Signed-off-by: Qiao Nuohan qiaonuo...@cn.fujitsu.com
 ---
   include/libvirt/libvirt.h.in | 15 ++-
   src/driver.h |  3 ++-
   src/libvirt.c| 10 --
   src/qemu/qemu_driver.c   | 30 +
   src/qemu/qemu_monitor.c  |  6 +++---
   src/qemu/qemu_monitor.h  |  3 ++-
   src/qemu/qemu_monitor_json.c |  4 +++-
   src/qemu/qemu_monitor_json.h |  3 ++-
   src/remote/remote_protocol.x |  1 +
   src/test/test_driver.c   | 12 +++-
   tests/qemumonitorjsontest.c  |  2 +-
   tools/virsh-domain.c | 45 
 +++-
   12 files changed, 113 insertions(+), 21 deletions(-)
 
 diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
 index 295d551..aae3c49 100644
 --- a/include/libvirt/libvirt.h.in
 +++ b/include/libvirt/libvirt.h.in
 @@ -1180,6 +1180,18 @@ typedef enum {
   VIR_DUMP_MEMORY_ONLY  = (1  4), /* use dump-guest-memory */
   } virDomainCoreDumpFlags;
 
 +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 */
 @@ -1728,7 +1740,8 @@ int 
 virDomainManagedSaveRemove(virDomainPtr dom,
*/
   int virDomainCoreDump   (virDomainPtr domain,
const char *to,
 - unsigned int flags);
 + unsigned int flags,
 + const unsigned int 
 memory_dump_format);

This is not allowed and I believe make syntax-check should fail on the
corresponding change to remote_protocol.x. If you want to add a new
parameter to an existing API, you have keep the API as is and instead
create a new API with a different name that will have all the parameters
you need.

Jirka

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