xc_domain_getinfo() is slow and prone to races because N hypercalls are
needed to find information about N domains. xc_domain_getinfolist() finds
the same information in a single hypercall as long as a big enough buffer
is provided. Plus, xc_domain_getinfo() is disappearing on a future patch
so migrate the callers interested in more than 1 domain to the the *list()
version.
Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com>
---
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Wei Liu <wl@xen.org>
Cc: Anthony PERARD <anthony.perard@citrix.com>
Cc: Juergen Gross <jgross@suse.com>
---
tools/include/xenctrl.h | 5 +++++
tools/python/xen/lowlevel/xc/xc.c | 29 +++++++++++++++--------------
tools/xenmon/xenbaked.c | 6 +++---
3 files changed, 23 insertions(+), 17 deletions(-)
diff --git a/tools/include/xenctrl.h b/tools/include/xenctrl.h
index 05967ecc92..90b33aa3a7 100644
--- a/tools/include/xenctrl.h
+++ b/tools/include/xenctrl.h
@@ -468,6 +468,11 @@ typedef struct xc_dominfo {
typedef xen_domctl_getdomaininfo_t xc_domaininfo_t;
+static inline unsigned int dominfo_shutdown_reason(const xc_domaininfo_t *info)
+{
+ return (info->flags >> XEN_DOMINF_shutdownshift) & XEN_DOMINF_shutdownmask;
+}
+
typedef union
{
#if defined(__i386__) || defined(__x86_64__)
diff --git a/tools/python/xen/lowlevel/xc/xc.c b/tools/python/xen/lowlevel/xc/xc.c
index 35901c2d63..38212e8091 100644
--- a/tools/python/xen/lowlevel/xc/xc.c
+++ b/tools/python/xen/lowlevel/xc/xc.c
@@ -342,7 +342,7 @@ static PyObject *pyxc_domain_getinfo(XcObject *self,
uint32_t first_dom = 0;
int max_doms = 1024, nr_doms, i;
size_t j;
- xc_dominfo_t *info;
+ xc_domaininfo_t *info;
static char *kwd_list[] = { "first_dom", "max_doms", NULL };
@@ -350,11 +350,11 @@ static PyObject *pyxc_domain_getinfo(XcObject *self,
&first_dom, &max_doms) )
return NULL;
- info = calloc(max_doms, sizeof(xc_dominfo_t));
+ info = calloc(max_doms, sizeof(*info));
if (info == NULL)
return PyErr_NoMemory();
- nr_doms = xc_domain_getinfo(self->xc_handle, first_dom, max_doms, info);
+ nr_doms = xc_domain_getinfolist(self->xc_handle, first_dom, max_doms, info);
if (nr_doms < 0)
{
@@ -368,21 +368,22 @@ static PyObject *pyxc_domain_getinfo(XcObject *self,
info_dict = Py_BuildValue(
"{s:i,s:i,s:i,s:i,s:i,s:i,s:i,s:i,s:i,s:i"
",s:L,s:L,s:L,s:i,s:i,s:i}",
- "domid", (int)info[i].domid,
+ "domid", (int)info[i].domain,
"online_vcpus", info[i].nr_online_vcpus,
"max_vcpu_id", info[i].max_vcpu_id,
- "hvm", info[i].hvm,
- "dying", info[i].dying,
- "crashed", info[i].crashed,
- "shutdown", info[i].shutdown,
- "paused", info[i].paused,
- "blocked", info[i].blocked,
- "running", info[i].running,
- "mem_kb", (long long)info[i].nr_pages*(XC_PAGE_SIZE/1024),
+ "hvm", !!(info[i].flags & XEN_DOMINF_hvm_guest),
+ "dying", !!(info[i].flags & XEN_DOMINF_dying),
+ "crashed", (info[i].flags & XEN_DOMINF_shutdown) &&
+ (dominfo_shutdown_reason(&info[i]) == SHUTDOWN_crash),
+ "shutdown", !!(info[i].flags & XEN_DOMINF_shutdown),
+ "paused", !!(info[i].flags & XEN_DOMINF_paused),
+ "blocked", !!(info[i].flags & XEN_DOMINF_blocked),
+ "running", !!(info[i].flags & XEN_DOMINF_running),
+ "mem_kb", (long long)info[i].tot_pages*(XC_PAGE_SIZE/1024),
"cpu_time", (long long)info[i].cpu_time,
- "maxmem_kb", (long long)info[i].max_memkb,
+ "maxmem_kb", (long long)(info[i].max_pages << (XC_PAGE_SHIFT - 10)),
"ssidref", (int)info[i].ssidref,
- "shutdown_reason", info[i].shutdown_reason,
+ "shutdown_reason", dominfo_shutdown_reason(&info[i]),
"cpupool", (int)info[i].cpupool);
pyhandle = PyList_New(sizeof(xen_domain_handle_t));
if ( (pyhandle == NULL) || (info_dict == NULL) )
diff --git a/tools/xenmon/xenbaked.c b/tools/xenmon/xenbaked.c
index 4dddbd20e2..8632b10ea4 100644
--- a/tools/xenmon/xenbaked.c
+++ b/tools/xenmon/xenbaked.c
@@ -775,7 +775,7 @@ static void global_init_domain(int domid, int idx)
static int indexof(int domid)
{
int idx;
- xc_dominfo_t dominfo[NDOMAINS];
+ xc_domaininfo_t dominfo[NDOMAINS];
xc_interface *xc_handle;
int ndomains;
@@ -797,7 +797,7 @@ static int indexof(int domid)
// call domaininfo hypercall to try and garbage collect unused entries
xc_handle = xc_interface_open(0,0,0);
- ndomains = xc_domain_getinfo(xc_handle, 0, NDOMAINS, dominfo);
+ ndomains = xc_domain_getinfolist(xc_handle, 0, NDOMAINS, dominfo);
xc_interface_close(xc_handle);
// for each domain in our data, look for it in the system dominfo structure
@@ -808,7 +808,7 @@ static int indexof(int domid)
int jdx;
for (jdx=0; jdx<ndomains; jdx++) {
- if (dominfo[jdx].domid == domid)
+ if (dominfo[jdx].domain == domid)
break;
}
if (jdx == ndomains) // we didn't find domid in the dominfo struct
--
2.34.1Just as a note for the subject, we more commonly write function names
with ()'s.
On 26/04/2023 3:59 pm, Alejandro Vallejo wrote:
> diff --git a/tools/include/xenctrl.h b/tools/include/xenctrl.h
> index 05967ecc92..90b33aa3a7 100644
> --- a/tools/include/xenctrl.h
> +++ b/tools/include/xenctrl.h
> @@ -468,6 +468,11 @@ typedef struct xc_dominfo {
>
> typedef xen_domctl_getdomaininfo_t xc_domaininfo_t;
>
> +static inline unsigned int dominfo_shutdown_reason(const xc_domaininfo_t *info)
> +{
> + return (info->flags >> XEN_DOMINF_shutdownshift) & XEN_DOMINF_shutdownmask;
> +}
> +
> typedef union
> {
> #if defined(__i386__) || defined(__x86_64__)
> diff --git a/tools/python/xen/lowlevel/xc/xc.c b/tools/python/xen/lowlevel/xc/xc.c
> index 35901c2d63..38212e8091 100644
> --- a/tools/python/xen/lowlevel/xc/xc.c
> +++ b/tools/python/xen/lowlevel/xc/xc.c
> @@ -368,21 +368,22 @@ static PyObject *pyxc_domain_getinfo(XcObject *self,
> info_dict = Py_BuildValue(
> "{s:i,s:i,s:i,s:i,s:i,s:i,s:i,s:i,s:i,s:i"
> ",s:L,s:L,s:L,s:i,s:i,s:i}",
> - "domid", (int)info[i].domid,
> + "domid", (int)info[i].domain,
> "online_vcpus", info[i].nr_online_vcpus,
> "max_vcpu_id", info[i].max_vcpu_id,
> - "hvm", info[i].hvm,
> - "dying", info[i].dying,
> - "crashed", info[i].crashed,
> - "shutdown", info[i].shutdown,
> - "paused", info[i].paused,
> - "blocked", info[i].blocked,
> - "running", info[i].running,
> - "mem_kb", (long long)info[i].nr_pages*(XC_PAGE_SIZE/1024),
> + "hvm", !!(info[i].flags & XEN_DOMINF_hvm_guest),
> + "dying", !!(info[i].flags & XEN_DOMINF_dying),
> + "crashed", (info[i].flags & XEN_DOMINF_shutdown) &&
> + (dominfo_shutdown_reason(&info[i]) == SHUTDOWN_crash),
Isn't this your dominfo_shutdown_with() from patch 6 ?
I'd pull that forward to this patch too, and use it here.
> + "shutdown", !!(info[i].flags & XEN_DOMINF_shutdown),
> + "paused", !!(info[i].flags & XEN_DOMINF_paused),
> + "blocked", !!(info[i].flags & XEN_DOMINF_blocked),
> + "running", !!(info[i].flags & XEN_DOMINF_running),
> + "mem_kb", (long long)info[i].tot_pages*(XC_PAGE_SIZE/1024),
> "cpu_time", (long long)info[i].cpu_time,
> - "maxmem_kb", (long long)info[i].max_memkb,
> + "maxmem_kb", (long long)(info[i].max_pages << (XC_PAGE_SHIFT - 10)),
> "ssidref", (int)info[i].ssidref,
> - "shutdown_reason", info[i].shutdown_reason,
> + "shutdown_reason", dominfo_shutdown_reason(&info[i]),
> "cpupool", (int)info[i].cpupool);
> pyhandle = PyList_New(sizeof(xen_domain_handle_t));
> if ( (pyhandle == NULL) || (info_dict == NULL) )
> diff --git a/tools/xenmon/xenbaked.c b/tools/xenmon/xenbaked.c
> index 4dddbd20e2..8632b10ea4 100644
> --- a/tools/xenmon/xenbaked.c
> +++ b/tools/xenmon/xenbaked.c
> @@ -775,7 +775,7 @@ static void global_init_domain(int domid, int idx)
> static int indexof(int domid)
> {
> int idx;
> - xc_dominfo_t dominfo[NDOMAINS];
> + xc_domaininfo_t dominfo[NDOMAINS];
> xc_interface *xc_handle;
> int ndomains;
>
> @@ -797,7 +797,7 @@ static int indexof(int domid)
>
> // call domaininfo hypercall to try and garbage collect unused entries
> xc_handle = xc_interface_open(0,0,0);
> - ndomains = xc_domain_getinfo(xc_handle, 0, NDOMAINS, dominfo);
> + ndomains = xc_domain_getinfolist(xc_handle, 0, NDOMAINS, dominfo);
> xc_interface_close(xc_handle);
Not to do with your patch, but this is logic is mad. xenbaked open and
closes a xenctrl handle every time its set of domids changes.
I'm very seriously tempted to delete all of tools/xenmon because it
shows no signs of being in use, and right now all it does is spit out an
unending stream of
gotten<100ns in qos_switchout(domid=32767)
gotten<100ns in qos_switchout(domid=0)
to stdout, which is antisocial for something calling itself a daemon.
~Andrew
Answers inlined On Thu, Apr 27, 2023 at 10:51:03AM +0100, Andrew Cooper wrote: > Just as a note for the subject, we more commonly write function names > with ()'s. No harm in abiding by that. Done on v2. > > + "crashed", (info[i].flags & XEN_DOMINF_shutdown) && > > + (dominfo_shutdown_reason(&info[i]) == SHUTDOWN_crash), > > Isn't this your dominfo_shutdown_with() from patch 6 ? > > I'd pull that forward to this patch too, and use it here. It is indeed. Done locally on v2. Cheers, Alejandro
© 2016 - 2026 Red Hat, Inc.