On Thu, Sep 28, 2006 at 07:20:35PM +0100, Daniel P. Berrange wrote:
> On Tue, Sep 19, 2006 at 12:12:19PM -0400, Daniel Veillard wrote:
> > On Wed, Sep 13, 2006 at 09:13:35PM -0400, Daniel Veillard wrote:
> > > On Wed, Sep 13, 2006 at 12:55:01PM -0600, Jim Fehlig wrote:
> > > > I found that the buffer provided for XEN_V1_OP_GETDOMAININFOLIST
> > > > hypercall differs slightly from the buffer in xen/dom0_ops.h.
> > >
> > > ohh, that's quite possible I made a mistake in rebuilding the code, yes
> > >
> > > > Attached is a patch against current cvs that works for me, but I'm not
> > > > familiar with this part of the code so not sure if this is the proper fix.
> > >
> > > I will try to double check today or tomorrow as I'm on the road,
> > > thanks a lot for the patch.
> >
> > Okay, I'm finally back, confirmed my error (sorry I should have tried a
> > 32bit box too but had none handy with the old code). So applied and commited,
>
> Urm, but this has now broken things on 32-bit 3.0.3 based Xen HV.
>
> # virsh dominfo Domain-0 | grep CPU
> CPU(s): 115
>
> And also
>
> # virsh vcpuinfo Domain-0
> libvir: Xen error : failed Xen syscall ioctl 3166208
>
> Looks like we need different versions of this struct depending on which
> Xen we're working against.
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
union xen_getdomaininfo {
struct xen_v0_getdomaininfo v0;
struct xen_v2_getdomaininfo v2;
};
typedef union xen_getdomaininfo xen_getdomaininfo;
union xen_getdomaininfolist {
struct xen_v0_getdomaininfo *v0;
struct xen_v2_getdomaininfo *v2;
};
typedef union xen_getdomaininfolist xen_getdomaininfolist;
The caller must populate & read either v0, or v2 as apropriate - to avoid
ugly if (hypervisor_version < 2) ...v0... else ...v2... I define a bunhc
of macros for accessing fields in these two unions. eg
#define XEN_GETDOMAININFOLIST_DOMAIN(domlist, n) \
(hypervisor_version < 2 ? \
domlist.v0[n].domain : \
domlist.v2[n].domain)
Or
#define XEN_GETDOMAININFOLIST_CLEAR(domlist, size) \
(hypervisor_version < 2 ? \
memset(domlist.v0, 0, sizeof(xen_v0_getdomaininfo) * size) : \
memset(domlist.v2, 0, sizeof(xen_v2_getdomaininfo) * size))
Anyway, I'm attaching a patch which I've tested against 32-bit HV on both
Xen 3.0.2 and 3.0.3, and also a 64-bit HV on 3.0.2 and 3.0.3 and all the
operations now work correctly again...
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: src/xen_internal.c
===================================================================
RCS file: /data/cvs/libvirt/src/xen_internal.c,v
retrieving revision 1.41
diff -u -b -r1.41 xen_internal.c
--- src/xen_internal.c 21 Sep 2006 15:24:37 -0000 1.41
+++ src/xen_internal.c 28 Sep 2006 21:23:02 -0000
@@ -99,13 +99,109 @@
};
typedef struct xen_v0_getdomaininfo xen_v0_getdomaininfo;
-struct xen_v0_getdomaininfolist {
+struct xen_v2_getdomaininfo {
+ domid_t domain; /* the domain number */
+ uint32_t flags; /* falgs, see before */
+ uint64_t tot_pages; /* total number of pages used */
+ uint64_t max_pages; /* maximum number of pages allowed */
+ uint64_t shared_info_frame; /* MFN of shared_info struct */
+ uint64_t cpu_time; /* 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_v2_getdomaininfo xen_v2_getdomaininfo;
+
+union xen_getdomaininfo {
+ struct xen_v0_getdomaininfo v0;
+ struct xen_v2_getdomaininfo v2;
+};
+typedef union xen_getdomaininfo xen_getdomaininfo;
+
+union xen_getdomaininfolist {
+ struct xen_v0_getdomaininfo *v0;
+ struct xen_v2_getdomaininfo *v2;
+};
+typedef union xen_getdomaininfolist xen_getdomaininfolist;
+
+#define XEN_GETDOMAININFOLIST_ALLOC(domlist, size) \
+ (hypervisor_version < 2 ? \
+ ((domlist.v0 = malloc(sizeof(xen_v0_getdomaininfo)*(size))) != NULL) : \
+ ((domlist.v2 = malloc(sizeof(xen_v2_getdomaininfo)*(size))) != NULL))
+
+#define XEN_GETDOMAININFOLIST_FREE(domlist) \
+ (hypervisor_version < 2 ? \
+ free(domlist.v0) : \
+ free(domlist.v2))
+
+#define XEN_GETDOMAININFOLIST_CLEAR(domlist, size) \
+ (hypervisor_version < 2 ? \
+ memset(domlist.v0, 0, sizeof(xen_v0_getdomaininfo) * size) : \
+ memset(domlist.v2, 0, sizeof(xen_v2_getdomaininfo) * size))
+
+#define XEN_GETDOMAININFOLIST_DOMAIN(domlist, n) \
+ (hypervisor_version < 2 ? \
+ domlist.v0[n].domain : \
+ domlist.v2[n].domain)
+
+
+
+#define XEN_GETDOMAININFO_CLEAR(dominfo) \
+ (hypervisor_version < 2 ? \
+ memset(&(dominfo.v0), 0, sizeof(xen_v0_getdomaininfo)) : \
+ memset(&(dominfo.v2), 0, sizeof(xen_v2_getdomaininfo)))
+
+#define XEN_GETDOMAININFO_DOMAIN(dominfo) \
+ (hypervisor_version < 2 ? \
+ dominfo.v0.domain : \
+ dominfo.v2.domain)
+
+#define XEN_GETDOMAININFO_CPUTIME(dominfo) \
+ (hypervisor_version < 2 ? \
+ dominfo.v0.cpu_time : \
+ dominfo.v2.cpu_time)
+
+#define XEN_GETDOMAININFO_CPUCOUNT(dominfo) \
+ (hypervisor_version < 2 ? \
+ dominfo.v0.nr_online_vcpus : \
+ dominfo.v2.nr_online_vcpus)
+
+#define XEN_GETDOMAININFO_FLAGS(dominfo) \
+ (hypervisor_version < 2 ? \
+ dominfo.v0.flags : \
+ dominfo.v2.flags)
+
+#define XEN_GETDOMAININFO_TOT_PAGES(dominfo) \
+ (hypervisor_version < 2 ? \
+ dominfo.v0.tot_pages : \
+ dominfo.v2.tot_pages)
+
+#define XEN_GETDOMAININFO_MAX_PAGES(dominfo) \
+ (hypervisor_version < 2 ? \
+ dominfo.v0.max_pages : \
+ dominfo.v2.max_pages)
+
+
+
+struct xen_v0_getdomaininfolistop {
domid_t first_domain;
uint32_t max_domains;
struct xen_v0_getdomaininfo *buffer;
uint32_t num_domains;
};
-typedef struct xen_v0_getdomaininfolist xen_v0_getdomaininfolist;
+typedef struct xen_v0_getdomaininfolistop xen_v0_getdomaininfolistop;
+
+
+struct xen_v2_getdomaininfolistop {
+ domid_t first_domain;
+ uint32_t max_domains;
+ struct xen_v2_getdomaininfo *buffer;
+ uint32_t num_domains;
+};
+typedef struct xen_v2_getdomaininfolistop xen_v2_getdomaininfolistop;
+
+
struct xen_v0_domainop {
domid_t domain;
@@ -244,7 +340,7 @@
uint32_t cmd;
uint32_t interface_version;
union {
- xen_v0_getdomaininfolist getdomaininfolist;
+ xen_v0_getdomaininfolistop getdomaininfolist;
xen_v0_domainop domain;
xen_v0_setmaxmem setmaxmem;
xen_v0_setmaxvcpu setmaxvcpu;
@@ -261,7 +357,7 @@
uint32_t cmd;
uint32_t interface_version;
union {
- xen_v0_getdomaininfolist getdomaininfolist;
+ xen_v2_getdomaininfolistop getdomaininfolist;
uint8_t padding[128];
} u;
};
@@ -545,7 +641,7 @@
*/
static int
virXen_getdomaininfolist(int handle, int first_domain, int maxids,
- xen_v0_getdomaininfo *dominfos)
+ xen_getdomaininfolist *dominfos)
{
int ret = -1;
@@ -561,7 +657,7 @@
op.cmd = XEN_V2_OP_GETDOMAININFOLIST;
op.u.getdomaininfolist.first_domain = (domid_t) first_domain;
op.u.getdomaininfolist.max_domains = maxids;
- op.u.getdomaininfolist.buffer = dominfos;
+ op.u.getdomaininfolist.buffer = dominfos->v2;
op.u.getdomaininfolist.num_domains = maxids;
ret = xenHypervisorDoV2Sys(handle, &op);
if (ret == 0)
@@ -573,7 +669,7 @@
op.cmd = XEN_V1_OP_GETDOMAININFOLIST;
op.u.getdomaininfolist.first_domain = (domid_t) first_domain;
op.u.getdomaininfolist.max_domains = maxids;
- op.u.getdomaininfolist.buffer = dominfos;
+ op.u.getdomaininfolist.buffer = dominfos->v0;
op.u.getdomaininfolist.num_domains = maxids;
ret = xenHypervisorDoV1Op(handle, &op);
if (ret == 0)
@@ -585,7 +681,7 @@
op.cmd = XEN_V0_OP_GETDOMAININFOLIST;
op.u.getdomaininfolist.first_domain = (domid_t) first_domain;
op.u.getdomaininfolist.max_domains = maxids;
- op.u.getdomaininfolist.buffer = dominfos;
+ op.u.getdomaininfolist.buffer = dominfos->v0;
op.u.getdomaininfolist.num_domains = maxids;
ret = xenHypervisorDoV0Op(handle, &op);
if (ret == 0)
@@ -599,6 +695,21 @@
return(ret);
}
+static int
+virXen_getdomaininfo(int handle, int first_domain,
+ xen_getdomaininfo *dominfo) {
+ xen_getdomaininfolist dominfos;
+
+ if (hypervisor_version < 2) {
+ dominfos.v0 = &(dominfo->v0);
+ } else {
+ dominfos.v2 = &(dominfo->v2);
+ }
+
+ return virXen_getdomaininfolist(handle, first_domain, 1, &dominfos);
+}
+
+
#ifndef PROXY
/**
* virXen_pausedomain:
@@ -998,7 +1109,7 @@
int fd, ret, cmd;
hypercall_t hc;
v0_hypercall_t v0_hc;
- xen_v0_getdomaininfo info;
+ xen_getdomaininfo info;
if (initialized) {
if (hypervisor_version == -1)
@@ -1073,7 +1184,7 @@
/* TODO: one probably will need to autodetect thse subversions too */
sys_interface_version = 2; /* XEN_SYSCTL_INTERFACE_VERSION */
dom_interface_version = 3; /* XEN_DOMCTL_INTERFACE_VERSION */
- if (virXen_getdomaininfolist(fd, 0, 1, &info) == 1) {
+ if (virXen_getdomaininfo(fd, 0, &info) == 1) {
#ifdef DEBUG
fprintf(stderr, "Using hypervisor call v2, sys version 2\n");
#endif
@@ -1081,7 +1192,7 @@
}
hypervisor_version = 1;
sys_interface_version = -1;
- if (virXen_getdomaininfolist(fd, 0, 1, &info) == 1) {
+ if (virXen_getdomaininfo(fd, 0, &info) == 1) {
#ifdef DEBUG
fprintf(stderr, "Using hypervisor call v1\n");
#endif
@@ -1227,7 +1338,7 @@
int
xenHypervisorNumOfDomains(virConnectPtr conn)
{
- xen_v0_getdomaininfo *dominfos;
+ xen_getdomaininfolist dominfos;
int ret, nbids;
static int last_maxids = 2;
int maxids = last_maxids;
@@ -1236,18 +1347,17 @@
return (-1);
retry:
- dominfos = malloc(maxids * sizeof(xen_v0_getdomaininfo));
- if (dominfos == NULL) {
+ if (!(XEN_GETDOMAININFOLIST_ALLOC(dominfos, maxids))) {
virXenError(VIR_ERR_NO_MEMORY, _("allocating %d domain info"),
maxids);
return(-1);
}
- memset(dominfos, 0, sizeof(xen_v0_getdomaininfo) * maxids);
+ XEN_GETDOMAININFOLIST_CLEAR(dominfos, maxids);
- ret = virXen_getdomaininfolist(conn->handle, 0, maxids, dominfos);
+ ret = virXen_getdomaininfolist(conn->handle, 0, maxids, &dominfos);
- free(dominfos);
+ XEN_GETDOMAININFOLIST_FREE(dominfos);
if (ret < 0)
return (-1);
@@ -1276,41 +1386,40 @@
int
xenHypervisorListDomains(virConnectPtr conn, int *ids, int maxids)
{
- xen_v0_getdomaininfo *dominfos;
+ xen_getdomaininfolist dominfos;
int ret, nbids, i;
if ((conn == NULL) || (conn->handle < 0) ||
(ids == NULL) || (maxids < 1))
return (-1);
- dominfos = malloc(maxids * sizeof(xen_v0_getdomaininfo));
- if (dominfos == NULL) {
+ if (!(XEN_GETDOMAININFOLIST_ALLOC(dominfos, maxids))) {
virXenError(VIR_ERR_NO_MEMORY, "allocating %d domain info",
maxids);
return(-1);
}
- memset(dominfos, 0, sizeof(xen_v0_getdomaininfo) * maxids);
+ XEN_GETDOMAININFOLIST_CLEAR(dominfos, maxids);
memset(ids, 0, maxids * sizeof(int));
- ret = virXen_getdomaininfolist(conn->handle, 0, maxids, dominfos);
+ ret = virXen_getdomaininfolist(conn->handle, 0, maxids, &dominfos);
if (ret < 0) {
- free(dominfos);
+ XEN_GETDOMAININFOLIST_FREE(dominfos);
return (-1);
}
nbids = ret;
if ((nbids < 0) || (nbids > maxids)) {
- free(dominfos);
+ XEN_GETDOMAININFOLIST_FREE(dominfos);
return(-1);
}
for (i = 0;i < nbids;i++) {
- ids[i] = dominfos[i].domain;
+ ids[i] = XEN_GETDOMAININFOLIST_DOMAIN(dominfos, i);
}
- free(dominfos);
+ XEN_GETDOMAININFOLIST_FREE(dominfos);
return (nbids);
}
@@ -1327,21 +1436,20 @@
unsigned long
xenHypervisorGetDomMaxMemory(virConnectPtr conn, int id)
{
- xen_v0_getdomaininfo dominfo;
+ xen_getdomaininfo dominfo;
int ret;
if ((conn == NULL) || (conn->handle < 0))
return (0);
- memset(&dominfo, 0, sizeof(xen_v0_getdomaininfo));
+ XEN_GETDOMAININFO_CLEAR(dominfo);
- dominfo.domain = id;
- ret = virXen_getdomaininfolist(conn->handle, id, 1, &dominfo);
+ ret = virXen_getdomaininfo(conn->handle, id, &dominfo);
- if ((ret < 0) || (dominfo.domain != id))
+ if ((ret < 0) || (XEN_GETDOMAININFO_DOMAIN(dominfo) != id))
return (0);
- return((unsigned long) dominfo.max_pages * 4);
+ return((unsigned long) XEN_GETDOMAININFO_MAX_PAGES(dominfo) * 4);
}
#ifndef PROXY
@@ -1379,21 +1487,21 @@
int
xenHypervisorGetDomInfo(virConnectPtr conn, int id, virDomainInfoPtr info)
{
- xen_v0_getdomaininfo dominfo;
+ xen_getdomaininfo dominfo;
int ret;
if ((conn == NULL) || (conn->handle < 0) || (info == NULL))
return (-1);
memset(info, 0, sizeof(virDomainInfo));
- memset(&dominfo, 0, sizeof(xen_v0_getdomaininfo));
+ XEN_GETDOMAININFO_CLEAR(dominfo);
- ret = virXen_getdomaininfolist(conn->handle, id, 1, &dominfo);
+ ret = virXen_getdomaininfo(conn->handle, id, &dominfo);
- if ((ret < 0) || (dominfo.domain != id))
+ if ((ret < 0) || (XEN_GETDOMAININFO_DOMAIN(dominfo) != id))
return (-1);
- switch (dominfo.flags & 0xFF) {
+ switch (XEN_GETDOMAININFO_FLAGS(dominfo) & 0xFF) {
case DOMFLAGS_DYING:
info->state = VIR_DOMAIN_SHUTDOWN;
break;
@@ -1418,10 +1526,10 @@
* convert to microseconds, same thing convert to
* kilobytes from page counts
*/
- info->cpuTime = dominfo.cpu_time;
- info->memory = dominfo.tot_pages * 4;
- info->maxMem = dominfo.max_pages * 4;
- info->nrVirtCpu = dominfo.nr_online_vcpus;
+ info->cpuTime = XEN_GETDOMAININFO_CPUTIME(dominfo);
+ info->memory = XEN_GETDOMAININFO_TOT_PAGES(dominfo) * 4;
+ info->maxMem = XEN_GETDOMAININFO_MAX_PAGES(dominfo) * 4;
+ info->nrVirtCpu = XEN_GETDOMAININFO_CPUCOUNT(dominfo);
return (0);
}
@@ -1620,7 +1728,7 @@
unsigned char *cpumaps, int maplen)
{
#ifndef PROXY
- xen_v0_getdomaininfo dominfo;
+ xen_getdomaininfo dominfo;
int ret;
virVcpuInfoPtr ipt;
@@ -1634,13 +1742,13 @@
return -1;
/* first get the number of virtual CPUs in this domain */
- memset(&dominfo, 0, sizeof(xen_v0_getdomaininfo));
- ret = virXen_getdomaininfolist(domain->conn->handle, domain->handle,
- 1, &dominfo);
+ XEN_GETDOMAININFO_CLEAR(dominfo);
+ ret = virXen_getdomaininfo(domain->conn->handle, domain->handle,
+ &dominfo);
- if ((ret < 0) || (dominfo.domain != domain->handle))
+ if ((ret < 0) || (XEN_GETDOMAININFO_DOMAIN(dominfo) != domain->handle))
return (-1);
- nbinfo = dominfo.max_vcpu_id + 1;
+ nbinfo = XEN_GETDOMAININFO_CPUCOUNT(dominfo) + 1;
if (nbinfo > maxinfo) nbinfo = maxinfo;
if (cpumaps != NULL)
@@ -1666,3 +1774,13 @@
return(-1);
#endif
}
+
+
+/*
+ * Local variables:
+ * indent-tabs-mode: nil
+ * c-indent-level: 4
+ * c-basic-offset: 4
+ * tab-width: 4
+ * End:
+ */