On Thu, Sep 28, 2006 at 11:37:42PM +0100, Daniel P. Berrange wrote:
> On Thu, Sep 28, 2006 at 05:22:08PM -0400, Daniel Veillard wrote:
> > On Thu, Sep 28, 2006 at 09:43:19PM +0100, Daniel P. Berrange wrote:
> > > This is really quite a nasty problem, because the struct is passed into from
> > > numerous locations in the xen_internal.h code & I didn't want to cover the
> > > entire source with conditionals.
> > >
> > > So what I've done is declared a new xen_v2_domaininfo struct which is
> > > the same as xen_v0_domaininfo, but with Jim's patch reverted again.
> > > Then provide two unions
> >
> > Okay, I really expected they didn't broke that data structure too,
> > sigh ... Yeah go ahead that seems the less uglier possible !
>
> Ok, I comitted this to CVS now.
>
> > I wonder why I didn't catch that, maybe my 3.0.3 test was done on an x86_64,
>
> Luck. On one of my 32-bit 3.0.3 boxes the 0.1.6 appeared to work fine for the
> basic operations - it was just giving bogus data (eg, 153 cpus). Depending on
> how many domains were running & what their data was things either silently
> worked (but bogus data), or completely crashed & burned.
>
> Anyway, I've now tested this on both versions of HV, and run through valgrind
> too as an extra sanity check. Lets just hope Xen doesn't break ABI yet again
> in the future....
Updating for this ABI breakage is beginning to feel like a never ending
problem :-( It seems I missed one change in my last patch - mlock()'ing
the wrong bit of data for getdomainlist. By (bad) luck it just happened
to work anyway on my tests because I guess there was sufficient allocated
memory that mlock() was still in range. Running unprivileged via the proxy
though, the proxy will always request info on 1020 domains (even if only
a couple are running), which meant we mlock() several 10's of KB of data.
THis ran in an area on unallocated mem & caused mlock() to fail, hence
my discovering the problem.
Anyway, attached is the patch I've applied to CVS...
Regards,
Dan.
--
|=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=|
|=- Perl modules: http://search.cpan.org/~danberr/ -=|
|=- Projects: http://freshmeat.net/~danielpb/ -=|
|=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|
Index: xen_internal.c
===================================================================
RCS file: /data/cvs/libvirt/src/xen_internal.c,v
retrieving revision 1.43
diff -u -r1.43 xen_internal.c
--- xen_internal.c 29 Sep 2006 16:12:08 -0000 1.43
+++ xen_internal.c 2 Oct 2006 19:25:01 -0000
@@ -145,7 +145,15 @@
domlist.v0[n].domain : \
domlist.v2[n].domain)
+#define XEN_GETDOMAININFOLIST_DATA(domlist) \
+ (hypervisor_version < 2 ? \
+ (void*)(domlist->v0) : \
+ (void*)(domlist->v2))
+#define XEN_GETDOMAININFO_SIZE \
+ (hypervisor_version < 2 ? \
+ sizeof(xen_v0_getdomaininfo) : \
+ sizeof(xen_v2_getdomaininfo))
#define XEN_GETDOMAININFO_CLEAR(dominfo) \
(hypervisor_version < 2 ? \
@@ -645,7 +653,8 @@
{
int ret = -1;
- if (mlock(dominfos, sizeof(xen_v0_getdomaininfo) * maxids) < 0) {
+ if (mlock(XEN_GETDOMAININFOLIST_DATA(dominfos),
+ XEN_GETDOMAININFO_SIZE * maxids) < 0) {
virXenError(VIR_ERR_XEN_CALL, " locking",
sizeof(xen_v0_getdomaininfo) * maxids);
return (-1);
@@ -687,7 +696,8 @@
if (ret == 0)
ret = op.u.getdomaininfolist.num_domains;
}
- if (munlock(dominfos, sizeof(xen_v0_getdomaininfo) * maxids) < 0) {
+ if (mlock(XEN_GETDOMAININFOLIST_DATA(dominfos),
+ XEN_GETDOMAININFO_SIZE * maxids) < 0) {
virXenError(VIR_ERR_XEN_CALL, " release",
sizeof(xen_v0_getdomaininfo));
ret = -1;