[libvirt] getFreeMemory reporting incorrectly?

[Daniel P. Berrange]

On Mon, Jun 15, 2009 at 12:09:13PM +0200, Daniel Veillard wrote:
> On Fri, Jun 12, 2009 at 03:44:50PM +0100, Daniel P. Berrange wrote:
> > On Thu, Jun 11, 2009 at 02:37:57PM -0400, Hany Fahim wrote:
> > > Thanks Daniel,
> > > Is it fair to assume this fix may appear in the next release of libvirt?
> > 
> > If you want to build from source, I'd appreciate feedback on whether
> > the following patch fixes the problem
> > 
> > diff -r f204769dd197 src/xen_internal.c
> > --- a/src/xen_internal.c	Wed Jun 03 13:52:06 2009 +0000
> > +++ b/src/xen_internal.c	Fri Jun 12 15:44:05 2009 +0100
> > @@ -241,6 +241,15 @@ struct xen_v2s4_availheap {
> >  
> >  typedef struct xen_v2s4_availheap  xen_v2s4_availheap;
> >  
> > +struct xen_v2s5_availheap {
> > +    uint32_t min_bitwidth;  /* Smallest address width (zero if don't care). */
> > +    uint32_t max_bitwidth;  /* Largest address width (zero if don't care). */
> > +    int32_t  node;          /* NUMA node (-1 for sum across all nodes). */
> > +    uint64_t avail_bytes ALIGN_64;   /* Bytes available in the specified region. */
> > +};
> > +
> > +typedef struct xen_v2s5_availheap  xen_v2s5_availheap;
> > +
> >  
> >  #define XEN_GETDOMAININFOLIST_ALLOC(domlist, size)                      \
> >      (hypervisor_version < 2 ?                                           \
> > @@ -650,6 +659,7 @@ struct xen_op_v2_sys {
> >          xen_v2s3_getdomaininfolistop getdomaininfolists3;
> >          xen_v2_getschedulerid        getschedulerid;
> >          xen_v2s4_availheap           availheap;
> > +        xen_v2s5_availheap           availheap5;
> >          uint8_t padding[128];
> >      } u;
> >  };
> > @@ -3125,12 +3135,18 @@ xenHypervisorNodeGetCellsFreeMemory(virC
> >      op_sys.cmd = XEN_V2_OP_GETAVAILHEAP;
> >  
> >      for (i = startCell, j = 0;(i < priv->nbNodeCells) && (j < maxCells);i++,j++) {
> > -        op_sys.u.availheap.node = i;
> > +        if (sys_interface_version >= 5)
> > +            op_sys.u.availheap5.node = i;
> > +        else
> > +            op_sys.u.availheap.node = i;
> >          ret = xenHypervisorDoV2Sys(priv->handle, &op_sys);
> >          if (ret < 0) {
> >              return(-1);
> >          }
> > -        freeMems[j] = op_sys.u.availheap.avail_bytes;
> > +        if (sys_interface_version >= 5)
> > +            freeMems[j] = op_sys.u.availheap5.avail_bytes;
> > +        else
> > +            freeMems[j] = op_sys.u.availheap.avail_bytes;
> >      }
> >      return (j);
> >  }
> 
> 
>   Looks fine to me, ACK.
>   Just one small worry, did we precisely identify when the change
> happened ? In xenHypervisorInit we go directly from sys_interface_version
> of 4 to 6, we never try 5 as far as I can see.

That's not a problem, because version 5 only ever appeared in the
xen-unstable.hg tree for a month or two. 3.1.0 was version 4, and
3.2.0 went to version 6. So there's no compelling reason to check
for version 5 really.

Regards,
Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|