Re: [libvirt] [PATCH] [RFC] xen domctl version 6

2010-01-07 Thread Jim Fehlig 
Jim Fehlig wrote:
 Daniel Veillard wrote:
   
 On Wed, Dec 23, 2009 at 10:59:09AM -0700, Jim Fehlig wrote:
   
 
 xen-unstable c/s 20685 changed the domctl interface, adding a field to
 xen_domctl_getdomaininfo structure.  This additional field causes stack
 corruption in libvirt.  xen-unstable c/s 20711 rightly bumped the domctl
 interface version so it is at least possible to handle the new field.

 The attached patch accounts for shr_pages field added to
 xen_domctl_getdomaininfo structure.  I'm not thrilled about the changes
 to all the macros - suggestions for improvement welcomed.

 Tested with domctl version 5 and 6.
 
   
   Yeah, we don't really have much choice there I think, but I would
 make the macros a bit more forward optimistic, i.e replacing

   (dom_interface_version == 6 ?

 with

   (dom_interface_version = 6 ?

 if there is a new bump, the call should not fail because we didn't
 updated all the macros.
   
 

 Yes, good idea.  Revised patch attached.
   

Committed now.  Thanks!

Regards,
Jim

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

Re: [libvirt] [PATCH] [RFC] xen domctl version 6

2010-01-06 Thread Daniel Veillard 
On Wed, Dec 23, 2009 at 10:59:09AM -0700, Jim Fehlig wrote:
 xen-unstable c/s 20685 changed the domctl interface, adding a field to
 xen_domctl_getdomaininfo structure.  This additional field causes stack
 corruption in libvirt.  xen-unstable c/s 20711 rightly bumped the domctl
 interface version so it is at least possible to handle the new field.
 
 The attached patch accounts for shr_pages field added to
 xen_domctl_getdomaininfo structure.  I'm not thrilled about the changes
 to all the macros - suggestions for improvement welcomed.
 
 Tested with domctl version 5 and 6.

  Yeah, we don't really have much choice there I think, but I would
make the macros a bit more forward optimistic, i.e replacing

  (dom_interface_version == 6 ?

with

  (dom_interface_version = 6 ?

if there is a new bump, the call should not fail because we didn't
updated all the macros.

  otherwise looks fine, ACK to a new patch with this,

Daniel

 Index: libvirt-0.7.4/src/xen/xen_hypervisor.c
 ===
 --- libvirt-0.7.4.orig/src/xen/xen_hypervisor.c
 +++ libvirt-0.7.4/src/xen/xen_hypervisor.c
 @@ -215,10 +215,26 @@ struct xen_v2d5_getdomaininfo {
  };
  typedef struct xen_v2d5_getdomaininfo xen_v2d5_getdomaininfo;
  
 +struct xen_v2d6_getdomaininfo {
 +domid_t  domain; /* the domain number */
 +uint32_t flags;  /* flags, see before */
 +uint64_t tot_pages ALIGN_64; /* total number of pages used */
 +uint64_t max_pages ALIGN_64; /* maximum number of pages allowed */
 +uint64_t shr_pages ALIGN_64;/* number of shared pages */
 +uint64_t shared_info_frame ALIGN_64; /* MFN of shared_info struct */
 +uint64_t cpu_time ALIGN_64;  /* CPU time used */
 +uint32_t nr_online_vcpus;  /* Number of VCPUs currently online. */
 +uint32_t max_vcpu_id; /* Maximum VCPUID in use by this domain. */
 +uint32_t ssidref;
 +xen_domain_handle_t handle;
 +};
 +typedef struct xen_v2d6_getdomaininfo xen_v2d6_getdomaininfo;
 +
  union xen_getdomaininfo {
  struct xen_v0_getdomaininfo v0;
  struct xen_v2_getdomaininfo v2;
  struct xen_v2d5_getdomaininfo v2d5;
 +struct xen_v2d6_getdomaininfo v2d6;
  };
  typedef union xen_getdomaininfo xen_getdomaininfo;
  
 @@ -226,6 +242,7 @@ union xen_getdomaininfolist {
  struct xen_v0_getdomaininfo *v0;
  struct xen_v2_getdomaininfo *v2;
  struct xen_v2d5_getdomaininfo *v2d5;
 +struct xen_v2d6_getdomaininfo *v2d6;
  };
  typedef union xen_getdomaininfolist xen_getdomaininfolist;
  
 @@ -263,114 +280,147 @@ typedef struct xen_v2s5_availheap  xen_v
  #define XEN_GETDOMAININFOLIST_ALLOC(domlist, size)  \
  (hypervisor_version  2 ?   \
   (VIR_ALLOC_N(domlist.v0, (size)) == 0) :   \
 - (dom_interface_version  5 ?   \
 -  (VIR_ALLOC_N(domlist.v2, (size)) == 0) :  \
 -  (VIR_ALLOC_N(domlist.v2d5, (size)) == 0)))
 + (dom_interface_version == 6 ?  \
 +  (VIR_ALLOC_N(domlist.v2d6, (size)) == 0) :\
 + (dom_interface_version == 5 ?  \
 +  (VIR_ALLOC_N(domlist.v2d5, (size)) == 0) :\
 +  (VIR_ALLOC_N(domlist.v2, (size)) == 0
  
  #define XEN_GETDOMAININFOLIST_FREE(domlist)\
  (hypervisor_version  2 ?  \
   VIR_FREE(domlist.v0) :\
 - (dom_interface_version  5 ?  \
 -  VIR_FREE(domlist.v2) :   \
 -  VIR_FREE(domlist.v2d5)))
 + (dom_interface_version == 6 ? \
 +  VIR_FREE(domlist.v2d6) : \
 + (dom_interface_version == 5 ? \
 +  VIR_FREE(domlist.v2d5) : \
 +  VIR_FREE(domlist.v2
  
  #define XEN_GETDOMAININFOLIST_CLEAR(domlist, size)\
  (hypervisor_version  2 ? \
   memset(domlist.v0, 0, sizeof(*domlist.v0) * size) :  \
 - (dom_interface_version  5 ? \
 -  memset(domlist.v2, 0, sizeof(*domlist.v2) * size) : \
 -  memset(domlist.v2d5, 0, sizeof(*domlist.v2d5) * size)))
 + (dom_interface_version == 6 ?\
 +  memset(domlist.v2d6, 0, sizeof(*domlist.v2d6) * size) : \
 + (dom_interface_version == 5 ?\
 +  memset(domlist.v2d5, 0, sizeof(*domlist.v2d5) * size) : \
 +  memset(domlist.v2, 0, sizeof(*domlist.v2) * size
  
  #define XEN_GETDOMAININFOLIST_DOMAIN(domlist, n)\
  (hypervisor_version  2 ?   \
   domlist.v0[n].domain : \
 - (dom_interface_version  5 ?   \
 -  domlist.v2[n].domain :

Re: [libvirt] [PATCH] [RFC] xen domctl version 6

2010-01-06 Thread Jim Fehlig 
Daniel Veillard wrote:
 On Wed, Dec 23, 2009 at 10:59:09AM -0700, Jim Fehlig wrote:
   
 xen-unstable c/s 20685 changed the domctl interface, adding a field to
 xen_domctl_getdomaininfo structure.  This additional field causes stack
 corruption in libvirt.  xen-unstable c/s 20711 rightly bumped the domctl
 interface version so it is at least possible to handle the new field.

 The attached patch accounts for shr_pages field added to
 xen_domctl_getdomaininfo structure.  I'm not thrilled about the changes
 to all the macros - suggestions for improvement welcomed.

 Tested with domctl version 5 and 6.
 

   Yeah, we don't really have much choice there I think, but I would
 make the macros a bit more forward optimistic, i.e replacing

   (dom_interface_version == 6 ?

 with

   (dom_interface_version = 6 ?

 if there is a new bump, the call should not fail because we didn't
 updated all the macros.
   

Yes, good idea.  Revised patch attached.

Regards,
Jim

commit 66363353ce4db1ed07c18f8191bfd26906bddaec
Author: Jim Fehlig jfeh...@novell.com
Date:   Wed Jan 6 14:41:24 2010 -0700

xen hypervisor: xen domctl version 6

xen-unstable c/s 20685 changed the domctl interface, adding a field to
xen_domctl_getdomaininfo structure.  This additional field causes stack
corruption in libvirt.  xen-unstable c/s 20711 rightly bumped the domctl
interface version so it is at least possible to handle the new field.
This change accounts for shr_pages field added to xen_domctl_getdomaininfo
structure.

diff --git a/src/xen/xen_hypervisor.c b/src/xen/xen_hypervisor.c
index 8279a74..6d8accc 100644
--- a/src/xen/xen_hypervisor.c
+++ b/src/xen/xen_hypervisor.c
@@ -215,10 +215,26 @@ struct xen_v2d5_getdomaininfo {
 };
 typedef struct xen_v2d5_getdomaininfo xen_v2d5_getdomaininfo;
 
+struct xen_v2d6_getdomaininfo {
+domid_t  domain;	/* the domain number */
+uint32_t flags;	/* flags, see before */
+uint64_t tot_pages ALIGN_64;	/* total number of pages used */
+uint64_t max_pages ALIGN_64;	/* maximum number of pages allowed */
+uint64_t shr_pages ALIGN_64;/* number of shared pages */
+uint64_t shared_info_frame ALIGN_64; /* MFN of shared_info struct */
+uint64_t cpu_time ALIGN_64;  /* CPU time used */
+uint32_t nr_online_vcpus;  /* Number of VCPUs currently online. */
+uint32_t max_vcpu_id; /* Maximum VCPUID in use by this domain. */
+uint32_t ssidref;
+xen_domain_handle_t handle;
+};
+typedef struct xen_v2d6_getdomaininfo xen_v2d6_getdomaininfo;
+
 union xen_getdomaininfo {
 struct xen_v0_getdomaininfo v0;
 struct xen_v2_getdomaininfo v2;
 struct xen_v2d5_getdomaininfo v2d5;
+struct xen_v2d6_getdomaininfo v2d6;
 };
 typedef union xen_getdomaininfo xen_getdomaininfo;
 
@@ -226,6 +242,7 @@ union xen_getdomaininfolist {
 struct xen_v0_getdomaininfo *v0;
 struct xen_v2_getdomaininfo *v2;
 struct xen_v2d5_getdomaininfo *v2d5;
+struct xen_v2d6_getdomaininfo *v2d6;
 };
 typedef union xen_getdomaininfolist xen_getdomaininfolist;
 
@@ -263,114 +280,147 @@ typedef struct xen_v2s5_availheap  xen_v2s5_availheap;
 #define XEN_GETDOMAININFOLIST_ALLOC(domlist, size)  \
 (hypervisor_version  2 ?   \
  (VIR_ALLOC_N(domlist.v0, (size)) == 0) :   \
- (dom_interface_version  5 ?   \
-  (VIR_ALLOC_N(domlist.v2, (size)) == 0) :  \
-  (VIR_ALLOC_N(domlist.v2d5, (size)) == 0)))
+ (dom_interface_version = 6 ?  \
+  (VIR_ALLOC_N(domlist.v2d6, (size)) == 0) :\
+ (dom_interface_version == 5 ?  \
+  (VIR_ALLOC_N(domlist.v2d5, (size)) == 0) :\
+  (VIR_ALLOC_N(domlist.v2, (size)) == 0
 
 #define XEN_GETDOMAININFOLIST_FREE(domlist)\
 (hypervisor_version  2 ?  \
  VIR_FREE(domlist.v0) :\
- (dom_interface_version  5 ?  \
-  VIR_FREE(domlist.v2) :   \
-  VIR_FREE(domlist.v2d5)))
+ (dom_interface_version = 6 ? \
+  VIR_FREE(domlist.v2d6) : \
+ (dom_interface_version == 5 ? \
+  VIR_FREE(domlist.v2d5) : \
+  VIR_FREE(domlist.v2
 
 #define XEN_GETDOMAININFOLIST_CLEAR(domlist, size)\
 (hypervisor_version  2 ? \
  memset(domlist.v0, 0, sizeof(*domlist.v0) * size) :  \
- (dom_interface_version  5 ? \
-  memset(domlist.v2, 0, sizeof(*domlist.v2) * size) : \
-  memset(domlist.v2d5, 0, sizeof(*domlist.v2d5) * size)))
+ (dom_interface_version = 6 ?\
+  memset(domlist.v2d6, 0,

[libvirt] [PATCH] [RFC] xen domctl version 6

2009-12-23 Thread Jim Fehlig 
xen-unstable c/s 20685 changed the domctl interface, adding a field to
xen_domctl_getdomaininfo structure.  This additional field causes stack
corruption in libvirt.  xen-unstable c/s 20711 rightly bumped the domctl
interface version so it is at least possible to handle the new field.

The attached patch accounts for shr_pages field added to
xen_domctl_getdomaininfo structure.  I'm not thrilled about the changes
to all the macros - suggestions for improvement welcomed.

Tested with domctl version 5 and 6.

Regards,
Jim
Index: libvirt-0.7.4/src/xen/xen_hypervisor.c
===
--- libvirt-0.7.4.orig/src/xen/xen_hypervisor.c
+++ libvirt-0.7.4/src/xen/xen_hypervisor.c
@@ -215,10 +215,26 @@ struct xen_v2d5_getdomaininfo {
 };
 typedef struct xen_v2d5_getdomaininfo xen_v2d5_getdomaininfo;
 
+struct xen_v2d6_getdomaininfo {
+domid_t  domain;	/* the domain number */
+uint32_t flags;	/* flags, see before */
+uint64_t tot_pages ALIGN_64;	/* total number of pages used */
+uint64_t max_pages ALIGN_64;	/* maximum number of pages allowed */
+uint64_t shr_pages ALIGN_64;/* number of shared pages */
+uint64_t shared_info_frame ALIGN_64; /* MFN of shared_info struct */
+uint64_t cpu_time ALIGN_64;  /* CPU time used */
+uint32_t nr_online_vcpus;  /* Number of VCPUs currently online. */
+uint32_t max_vcpu_id; /* Maximum VCPUID in use by this domain. */
+uint32_t ssidref;
+xen_domain_handle_t handle;
+};
+typedef struct xen_v2d6_getdomaininfo xen_v2d6_getdomaininfo;
+
 union xen_getdomaininfo {
 struct xen_v0_getdomaininfo v0;
 struct xen_v2_getdomaininfo v2;
 struct xen_v2d5_getdomaininfo v2d5;
+struct xen_v2d6_getdomaininfo v2d6;
 };
 typedef union xen_getdomaininfo xen_getdomaininfo;
 
@@ -226,6 +242,7 @@ union xen_getdomaininfolist {
 struct xen_v0_getdomaininfo *v0;
 struct xen_v2_getdomaininfo *v2;
 struct xen_v2d5_getdomaininfo *v2d5;
+struct xen_v2d6_getdomaininfo *v2d6;
 };
 typedef union xen_getdomaininfolist xen_getdomaininfolist;
 
@@ -263,114 +280,147 @@ typedef struct xen_v2s5_availheap  xen_v
 #define XEN_GETDOMAININFOLIST_ALLOC(domlist, size)  \
 (hypervisor_version  2 ?   \
  (VIR_ALLOC_N(domlist.v0, (size)) == 0) :   \
- (dom_interface_version  5 ?   \
-  (VIR_ALLOC_N(domlist.v2, (size)) == 0) :  \
-  (VIR_ALLOC_N(domlist.v2d5, (size)) == 0)))
+ (dom_interface_version == 6 ?  \
+  (VIR_ALLOC_N(domlist.v2d6, (size)) == 0) :\
+ (dom_interface_version == 5 ?  \
+  (VIR_ALLOC_N(domlist.v2d5, (size)) == 0) :\
+  (VIR_ALLOC_N(domlist.v2, (size)) == 0
 
 #define XEN_GETDOMAININFOLIST_FREE(domlist)\
 (hypervisor_version  2 ?  \
  VIR_FREE(domlist.v0) :\
- (dom_interface_version  5 ?  \
-  VIR_FREE(domlist.v2) :   \
-  VIR_FREE(domlist.v2d5)))
+ (dom_interface_version == 6 ? \
+  VIR_FREE(domlist.v2d6) : \
+ (dom_interface_version == 5 ? \
+  VIR_FREE(domlist.v2d5) : \
+  VIR_FREE(domlist.v2
 
 #define XEN_GETDOMAININFOLIST_CLEAR(domlist, size)\
 (hypervisor_version  2 ? \
  memset(domlist.v0, 0, sizeof(*domlist.v0) * size) :  \
- (dom_interface_version  5 ? \
-  memset(domlist.v2, 0, sizeof(*domlist.v2) * size) : \
-  memset(domlist.v2d5, 0, sizeof(*domlist.v2d5) * size)))
+ (dom_interface_version == 6 ?\
+  memset(domlist.v2d6, 0, sizeof(*domlist.v2d6) * size) : \
+ (dom_interface_version == 5 ?\
+  memset(domlist.v2d5, 0, sizeof(*domlist.v2d5) * size) : \
+  memset(domlist.v2, 0, sizeof(*domlist.v2) * size
 
 #define XEN_GETDOMAININFOLIST_DOMAIN(domlist, n)\
 (hypervisor_version  2 ?   \
  domlist.v0[n].domain : \
- (dom_interface_version  5 ?   \
-  domlist.v2[n].domain :\
-  domlist.v2d5[n].domain))
+ (dom_interface_version == 6 ?  \
+  domlist.v2d6[n].domain :  \
+ (dom_interface_version == 5 ?  \
+  domlist.v2d5[n].domain :  \
+  domlist.v2[n].domain)))
 
 #define XEN_GETDOMAININFOLIST_UUID(domlist, n)  \
 (hypervisor_version  2 ?   \
  domlist.v0[n].handle : \
- (dom_interface_version  5 ?