* [PATCH v2 0/7] Rationalize usage of xc_domain_getinfo{,list}()
@ 2023-04-28 10:41 Alejandro Vallejo
2023-04-28 10:41 ` [PATCH v2 1/7] tools: Make some callers of xc_domain_getinfo() use xc_domain_getinfolist() Alejandro Vallejo
` (6 more replies)
0 siblings, 7 replies; 20+ messages in thread
From: Alejandro Vallejo @ 2023-04-28 10:41 UTC (permalink / raw)
To: Xen-devel
Cc: Alejandro Vallejo, Andrew Cooper, George Dunlap, Jan Beulich,
Julien Grall, Stefano Stabellini, Wei Liu, Anthony PERARD,
Juergen Gross, Tim Deegan
v2:
* Various stylistic changes to match Xen style
* Removed hardcoded ARRAY_SIZE() macro in favour of common-macros.h
* Changed error handling convention of xc_domain_getinfo_single()
to return {0,-1}/errno
* Removed error handling overrides on xc_getininfo_single() callers
* Improved some debug on the error path of xc_getinfo_single() callers
* Used dominfo_shutdown_with() in lowlevel/xc/xc.c rather than extracting
the 'crashed' condition manually.
xc_domain_getinfo() returns the list of domains with domid >= first_domid.
It does so by repeatedly invoking XEN_DOMCTL_getdomaininfo, which leads to
unintuitive behaviour (asking for domid=1 might succeed returning domid=2).
Furthermore, N hypercalls are required whereas the equivalent functionality
can be achieved using with XEN_SYSCTL_getdomaininfo.
Ideally, we want a DOMCTL interface that operates over a single precisely
specified domain and a SYSCTL interface that can be used for bulk queries.
All callers of xc_domain_getinfo() that are better off using SYSCTL are
migrated to use that instead. That includes callers performing domain
discovery and those requesting info for more than 1 domain per hypercall.
A new xc_domain_getinfo_single() is introduced with stricter semantics than
xc_domain_getinfo() (failing if domid isn't found) to migrate the rest to.
With no callers left the xc_dominfo_t structure and the xc_domain_getinfo()
call itself can be cleanly removed, and the DOMCTL interface simplified to
only use its fastpath.
With the DOMCTL amended, the new xc_domain_getinfo_single() drops its
stricter check, becoming a simple wrapper to invoke the hypercall itself.
Alejandro Vallejo (7):
tools: Make some callers of xc_domain_getinfo() use
xc_domain_getinfolist()
tools: Create xc_domain_getinfo_single()
tools: Refactor console/io.c to avoid using xc_domain_getinfo()
tools: Make init-xenstore-domain use xc_domain_getinfolist()
tools: Modify single-domid callers of xc_domain_getinfolist()
tools: Use new xc function for some xc_domain_getinfo() calls
domctl: Modify XEN_DOMCTL_getdomaininfo to fail if domid is not found
tools/console/client/main.c | 7 +--
tools/console/daemon/io.c | 31 +++++-----
tools/debugger/kdd/kdd-xen.c | 6 +-
tools/helpers/init-xenstore-domain.c | 14 +++--
tools/include/xenctrl.h | 63 ++++++++-----------
tools/libs/ctrl/xc_domain.c | 82 +++++--------------------
tools/libs/ctrl/xc_pagetab.c | 7 +--
tools/libs/ctrl/xc_private.c | 7 +--
tools/libs/ctrl/xc_private.h | 7 ++-
tools/libs/guest/xg_core.c | 23 +++----
tools/libs/guest/xg_core.h | 6 +-
tools/libs/guest/xg_core_arm.c | 10 +--
tools/libs/guest/xg_core_x86.c | 18 +++---
tools/libs/guest/xg_cpuid_x86.c | 34 +++++-----
tools/libs/guest/xg_dom_boot.c | 16 ++---
tools/libs/guest/xg_domain.c | 8 +--
tools/libs/guest/xg_offline_page.c | 12 ++--
tools/libs/guest/xg_private.h | 1 +
tools/libs/guest/xg_resume.c | 19 +++---
tools/libs/guest/xg_sr_common.h | 2 +-
tools/libs/guest/xg_sr_restore.c | 17 ++---
tools/libs/guest/xg_sr_restore_x86_pv.c | 2 +-
tools/libs/guest/xg_sr_save.c | 26 +++-----
tools/libs/guest/xg_sr_save_x86_pv.c | 6 +-
tools/libs/light/libxl_dom.c | 17 ++---
tools/libs/light/libxl_dom_suspend.c | 7 +--
tools/libs/light/libxl_domain.c | 13 ++--
tools/libs/light/libxl_mem.c | 4 +-
tools/libs/light/libxl_sched.c | 26 ++++----
tools/libs/light/libxl_x86_acpi.c | 4 +-
tools/misc/xen-hvmcrash.c | 6 +-
tools/misc/xen-lowmemd.c | 6 +-
tools/misc/xen-mfndump.c | 22 +++----
tools/misc/xen-vmtrace.c | 6 +-
tools/python/xen/lowlevel/xc/xc.c | 28 ++++-----
tools/vchan/vchan-socket-proxy.c | 6 +-
tools/xenmon/xenbaked.c | 6 +-
tools/xenpaging/xenpaging.c | 10 +--
tools/xenstore/xenstored_domain.c | 15 +++--
tools/xentrace/xenctx.c | 8 +--
xen/common/domctl.c | 32 +---------
41 files changed, 254 insertions(+), 386 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 1/7] tools: Make some callers of xc_domain_getinfo() use xc_domain_getinfolist()
2023-04-28 10:41 [PATCH v2 0/7] Rationalize usage of xc_domain_getinfo{,list}() Alejandro Vallejo
@ 2023-04-28 10:41 ` Alejandro Vallejo
2023-04-28 12:19 ` Marek Marczykowski-Górecki
2023-04-28 12:21 ` Andrew Cooper
2023-04-28 10:41 ` [PATCH v2 2/7] tools: Create xc_domain_getinfo_single() Alejandro Vallejo
` (5 subsequent siblings)
6 siblings, 2 replies; 20+ messages in thread
From: Alejandro Vallejo @ 2023-04-28 10:41 UTC (permalink / raw)
To: Xen-devel
Cc: Alejandro Vallejo, Andrew Cooper, Wei Liu, Anthony PERARD,
Juergen Gross
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 | 12 ++++++++++++
tools/python/xen/lowlevel/xc/xc.c | 28 ++++++++++++++--------------
tools/xenmon/xenbaked.c | 6 +++---
3 files changed, 29 insertions(+), 17 deletions(-)
diff --git a/tools/include/xenctrl.h b/tools/include/xenctrl.h
index 05967ecc92..f5bc7f58b6 100644
--- a/tools/include/xenctrl.h
+++ b/tools/include/xenctrl.h
@@ -468,6 +468,18 @@ 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;
+}
+
+static inline bool dominfo_shutdown_with(xc_domaininfo_t *info, unsigned int expected_reason)
+{
+ /* The reason doesn't make sense unless the domain is actually shutdown */
+ return (info->flags & XEN_DOMINF_shutdown) &&
+ (dominfo_shutdown_reason(info) == expected_reason);
+}
+
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..d7ce299650 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,21 @@ 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", dominfo_shutdown_with(&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.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 2/7] tools: Create xc_domain_getinfo_single()
2023-04-28 10:41 [PATCH v2 0/7] Rationalize usage of xc_domain_getinfo{,list}() Alejandro Vallejo
2023-04-28 10:41 ` [PATCH v2 1/7] tools: Make some callers of xc_domain_getinfo() use xc_domain_getinfolist() Alejandro Vallejo
@ 2023-04-28 10:41 ` Alejandro Vallejo
2023-04-28 12:23 ` Andrew Cooper
2023-04-28 10:41 ` [PATCH v2 3/7] tools: Refactor console/io.c to avoid using xc_domain_getinfo() Alejandro Vallejo
` (4 subsequent siblings)
6 siblings, 1 reply; 20+ messages in thread
From: Alejandro Vallejo @ 2023-04-28 10:41 UTC (permalink / raw)
To: Xen-devel
Cc: Alejandro Vallejo, Andrew Cooper, Wei Liu, Anthony PERARD,
Juergen Gross
It's a stricter version of xc_domain_getinfo() where the returned domid
always matches the requested domid or the error code shows an error instead.
A few patches ahead usages of xc_domain_getinfo() are removed until only
xc_domain_getinfo_single() and xc_domain_getinfolist() remain.
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 | 16 ++++++++++++++++
tools/libs/ctrl/xc_domain.c | 23 +++++++++++++++++++++++
2 files changed, 39 insertions(+)
diff --git a/tools/include/xenctrl.h b/tools/include/xenctrl.h
index f5bc7f58b6..685df1c7ba 100644
--- a/tools/include/xenctrl.h
+++ b/tools/include/xenctrl.h
@@ -703,6 +703,22 @@ int xc_vcpu_getaffinity(xc_interface *xch,
int xc_domain_get_guest_width(xc_interface *xch, uint32_t domid,
unsigned int *guest_width);
+/**
+ * This function will return information about a single domain. It looks
+ * up the domain by the provided domid and succeeds if the domain exists
+ * and is accesible by the current domain, or fails otherwise. A buffer
+ * may optionally passed on the `info` parameter in order to retrieve
+ * information about the domain. The buffer is ignored if NULL is
+ * passed instead.
+ *
+ * @parm xch a handle to an open hypervisor interface
+ * @parm domid domid to lookup
+ * @parm info Optional domain information buffer (may be NULL)
+ * @return 0 on success, otherwise the call failed and info is undefined
+ */
+int xc_domain_getinfo_single(xc_interface *xch,
+ uint32_t domid,
+ xc_domaininfo_t *info);
/**
* This function will return information about one or more domains. It is
diff --git a/tools/libs/ctrl/xc_domain.c b/tools/libs/ctrl/xc_domain.c
index e939d07157..6b11775d4c 100644
--- a/tools/libs/ctrl/xc_domain.c
+++ b/tools/libs/ctrl/xc_domain.c
@@ -345,6 +345,29 @@ int xc_dom_vuart_init(xc_interface *xch,
return rc;
}
+int xc_domain_getinfo_single(xc_interface *xch,
+ uint32_t domid,
+ xc_domaininfo_t *info)
+{
+ struct xen_domctl domctl = {
+ .cmd = XEN_DOMCTL_getdomaininfo,
+ .domain = domid,
+ };
+
+ if ( do_domctl(xch, &domctl) < 0 )
+ return -1;
+
+ if ( domctl.u.getdomaininfo.domain != domid ) {
+ errno = ESRCH;
+ return -1;
+ }
+
+ if ( info )
+ *info = domctl.u.getdomaininfo;
+
+ return 0;
+}
+
int xc_domain_getinfo(xc_interface *xch,
uint32_t first_domid,
unsigned int max_doms,
--
2.34.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 3/7] tools: Refactor console/io.c to avoid using xc_domain_getinfo()
2023-04-28 10:41 [PATCH v2 0/7] Rationalize usage of xc_domain_getinfo{,list}() Alejandro Vallejo
2023-04-28 10:41 ` [PATCH v2 1/7] tools: Make some callers of xc_domain_getinfo() use xc_domain_getinfolist() Alejandro Vallejo
2023-04-28 10:41 ` [PATCH v2 2/7] tools: Create xc_domain_getinfo_single() Alejandro Vallejo
@ 2023-04-28 10:41 ` Alejandro Vallejo
2023-04-28 12:33 ` Andrew Cooper
2023-04-28 10:41 ` [PATCH v2 4/7] tools: Make init-xenstore-domain use xc_domain_getinfolist() Alejandro Vallejo
` (3 subsequent siblings)
6 siblings, 1 reply; 20+ messages in thread
From: Alejandro Vallejo @ 2023-04-28 10:41 UTC (permalink / raw)
To: Xen-devel; +Cc: Alejandro Vallejo, Andrew Cooper, Wei Liu, Anthony PERARD
It has 2 avoidable occurences
* Check whether a domain is valid, which can be done faster with
xc_domain_getinfo_single()
* Domain discovery, which can be done much faster with the sysctl
interface through xc_domain_getinfolist().
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>
---
tools/console/daemon/io.c | 31 +++++++++++++++++--------------
1 file changed, 17 insertions(+), 14 deletions(-)
diff --git a/tools/console/daemon/io.c b/tools/console/daemon/io.c
index 6bfe96715b..c5972cb721 100644
--- a/tools/console/daemon/io.c
+++ b/tools/console/daemon/io.c
@@ -405,13 +405,7 @@ static void buffer_advance(struct buffer *buffer, size_t len)
static bool domain_is_valid(int domid)
{
- bool ret;
- xc_dominfo_t info;
-
- ret = (xc_domain_getinfo(xc, domid, 1, &info) == 1 &&
- info.domid == domid);
-
- return ret;
+ return xc_domain_getinfo_single(xc, domid, NULL) == 0;
}
static int create_hv_log(void)
@@ -961,24 +955,33 @@ static unsigned enum_pass = 0;
static void enum_domains(void)
{
- int domid = 1;
- xc_dominfo_t dominfo;
+ /**
+ * Memory set aside to query the state of every
+ * domain in the hypervisor in a single hypercall.
+ */
+ static xc_domaininfo_t domaininfo[DOMID_FIRST_RESERVED - 1];
+
+ int ret;
struct domain *dom;
enum_pass++;
- while (xc_domain_getinfo(xc, domid, 1, &dominfo) == 1) {
- dom = lookup_domain(dominfo.domid);
- if (dominfo.dying) {
+ /* Fetch info on every valid domain except for dom0 */
+ ret = xc_domain_getinfolist(xc, 1, DOMID_FIRST_RESERVED - 1, domaininfo);
+ if (ret < 0)
+ return;
+
+ for (size_t i = 0; i < ret; i++) {
+ dom = lookup_domain(domaininfo[i].domain);
+ if (domaininfo[i].flags & XEN_DOMINF_dying) {
if (dom)
shutdown_domain(dom);
} else {
if (dom == NULL)
- dom = create_domain(dominfo.domid);
+ dom = create_domain(domaininfo[i].domain);
}
if (dom)
dom->last_seen = enum_pass;
- domid = dominfo.domid + 1;
}
}
--
2.34.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 4/7] tools: Make init-xenstore-domain use xc_domain_getinfolist()
2023-04-28 10:41 [PATCH v2 0/7] Rationalize usage of xc_domain_getinfo{,list}() Alejandro Vallejo
` (2 preceding siblings ...)
2023-04-28 10:41 ` [PATCH v2 3/7] tools: Refactor console/io.c to avoid using xc_domain_getinfo() Alejandro Vallejo
@ 2023-04-28 10:41 ` Alejandro Vallejo
2023-04-28 12:40 ` Andrew Cooper
2023-04-28 13:10 ` Andrew Cooper
2023-04-28 10:41 ` [PATCH v2 5/7] tools: Modify single-domid callers of xc_domain_getinfolist() Alejandro Vallejo
` (2 subsequent siblings)
6 siblings, 2 replies; 20+ messages in thread
From: Alejandro Vallejo @ 2023-04-28 10:41 UTC (permalink / raw)
To: Xen-devel; +Cc: Alejandro Vallejo, Andrew Cooper, Wei Liu, Juergen Gross
It currently relies on xc_domain_getinfo() returning the next available
domain past "first_domid", which is a feature that will disappear in a
future patch.
Furthermore and while at it, make it so the hypercall tries to fetch information
about more than one domain per hypercall so we can (hopefully) get away with a
single hypercall in a typical system.
Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com>
---
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Wei Liu <wl@xen.org>
Cc: Juergen Gross <jgross@suse.org>
---
tools/helpers/init-xenstore-domain.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/tools/helpers/init-xenstore-domain.c b/tools/helpers/init-xenstore-domain.c
index 0950ba7dc5..e210a2677e 100644
--- a/tools/helpers/init-xenstore-domain.c
+++ b/tools/helpers/init-xenstore-domain.c
@@ -13,6 +13,7 @@
#include <xentoollog.h>
#include <libxl.h>
#include <xen/sys/xenbus_dev.h>
+#include <xen-tools/common-macros.h>
#include <xen-xsm/flask/flask.h>
#include <xen/io/xenbus.h>
@@ -322,16 +323,19 @@ err:
static int check_domain(xc_interface *xch)
{
- xc_dominfo_t info;
+ xc_domaininfo_t info[8];
uint32_t dom;
int ret;
dom = 1;
- while ( (ret = xc_domain_getinfo(xch, dom, 1, &info)) == 1 )
+ while ( (ret = xc_domain_getinfolist(xch, dom, ARRAY_SIZE(info), info)) > 0 )
{
- if ( info.xenstore )
- return 1;
- dom = info.domid + 1;
+ for ( size_t i = 0; i < ret; i++ )
+ {
+ if ( info[i].flags & XEN_DOMINF_xs_domain )
+ return 1;
+ }
+ dom = info[ret - 1].domain + 1;
}
if ( ret < 0 && errno != ESRCH )
{
--
2.34.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 5/7] tools: Modify single-domid callers of xc_domain_getinfolist()
2023-04-28 10:41 [PATCH v2 0/7] Rationalize usage of xc_domain_getinfo{,list}() Alejandro Vallejo
` (3 preceding siblings ...)
2023-04-28 10:41 ` [PATCH v2 4/7] tools: Make init-xenstore-domain use xc_domain_getinfolist() Alejandro Vallejo
@ 2023-04-28 10:41 ` Alejandro Vallejo
2023-04-28 15:22 ` Andrew Cooper
2023-04-28 10:41 ` [PATCH v2 6/7] tools: Use new xc function for some xc_domain_getinfo() calls Alejandro Vallejo
2023-04-28 10:41 ` [PATCH v2 7/7] domctl: Modify XEN_DOMCTL_getdomaininfo to fail if domid is not found Alejandro Vallejo
6 siblings, 1 reply; 20+ messages in thread
From: Alejandro Vallejo @ 2023-04-28 10:41 UTC (permalink / raw)
To: Xen-devel
Cc: Alejandro Vallejo, Andrew Cooper, Wei Liu, Anthony PERARD,
Juergen Gross
xc_domain_getinfolist() internally relies on a sysctl that performs
a linear search for the domids. Many callers of xc_domain_getinfolist()
who require information about a precise domid are much better off calling
xc_domain_getinfo_single() instead, that will use the getdomaininfo domctl
instead and ensure the returned domid matches the requested one. The domtctl
will find the domid faster too, because that uses hashed lists.
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/libs/light/libxl_dom.c | 15 +++++----------
tools/libs/light/libxl_dom_suspend.c | 7 +------
tools/libs/light/libxl_domain.c | 13 +++++--------
tools/libs/light/libxl_mem.c | 4 ++--
tools/libs/light/libxl_sched.c | 12 ++++--------
tools/xenpaging/xenpaging.c | 10 +++++-----
6 files changed, 22 insertions(+), 39 deletions(-)
diff --git a/tools/libs/light/libxl_dom.c b/tools/libs/light/libxl_dom.c
index 25fb716084..bd5d823581 100644
--- a/tools/libs/light/libxl_dom.c
+++ b/tools/libs/light/libxl_dom.c
@@ -32,8 +32,8 @@ libxl_domain_type libxl__domain_type(libxl__gc *gc, uint32_t domid)
xc_domaininfo_t info;
int ret;
- ret = xc_domain_getinfolist(ctx->xch, domid, 1, &info);
- if (ret != 1 || info.domain != domid) {
+ ret = xc_domain_getinfo_single(ctx->xch, domid, &info);
+ if (ret < 0) {
LOG(ERROR, "unable to get domain type for domid=%"PRIu32, domid);
return LIBXL_DOMAIN_TYPE_INVALID;
}
@@ -70,15 +70,10 @@ int libxl__domain_cpupool(libxl__gc *gc, uint32_t domid)
xc_domaininfo_t info;
int ret;
- ret = xc_domain_getinfolist(CTX->xch, domid, 1, &info);
- if (ret != 1)
+ ret = xc_domain_getinfo_single(CTX->xch, domid, &info);
+ if (ret < 0)
{
- LOGE(ERROR, "getinfolist failed %d", ret);
- return ERROR_FAIL;
- }
- if (info.domain != domid)
- {
- LOGE(ERROR, "got info for dom%d, wanted dom%d\n", info.domain, domid);
+ LOGED(ERROR, domid, "get domaininfo failed: %d", ret);
return ERROR_FAIL;
}
return info.cpupool;
diff --git a/tools/libs/light/libxl_dom_suspend.c b/tools/libs/light/libxl_dom_suspend.c
index 4fa22bb739..6091a5f3f6 100644
--- a/tools/libs/light/libxl_dom_suspend.c
+++ b/tools/libs/light/libxl_dom_suspend.c
@@ -332,13 +332,8 @@ static void suspend_common_wait_guest_check(libxl__egc *egc,
/* Convenience aliases */
const uint32_t domid = dsps->domid;
- ret = xc_domain_getinfolist(CTX->xch, domid, 1, &info);
+ ret = xc_domain_getinfo_single(CTX->xch, domid, &info);
if (ret < 0) {
- LOGED(ERROR, domid, "unable to check for status of guest");
- goto err;
- }
-
- if (!(ret == 1 && info.domain == domid)) {
LOGED(ERROR, domid, "guest we were suspending has been destroyed");
goto err;
}
diff --git a/tools/libs/light/libxl_domain.c b/tools/libs/light/libxl_domain.c
index 7f0986c185..9fa4091859 100644
--- a/tools/libs/light/libxl_domain.c
+++ b/tools/libs/light/libxl_domain.c
@@ -349,16 +349,12 @@ int libxl_domain_info(libxl_ctx *ctx, libxl_dominfo *info_r,
int ret;
GC_INIT(ctx);
- ret = xc_domain_getinfolist(ctx->xch, domid, 1, &xcinfo);
+ ret = xc_domain_getinfo_single(ctx->xch, domid, &xcinfo);
if (ret<0) {
- LOGED(ERROR, domid, "Getting domain info list");
+ LOGED(ERROR, domid, "Getting domain info: %d", ret);
GC_FREE;
return ERROR_FAIL;
}
- if (ret==0 || xcinfo.domain != domid) {
- GC_FREE;
- return ERROR_DOMAIN_NOTFOUND;
- }
if (info_r)
libxl__xcinfo2xlinfo(ctx, &xcinfo, info_r);
@@ -1657,14 +1653,15 @@ int libxl__resolve_domid(libxl__gc *gc, const char *name, uint32_t *domid)
libxl_vcpuinfo *libxl_list_vcpu(libxl_ctx *ctx, uint32_t domid,
int *nr_vcpus_out, int *nr_cpus_out)
{
+ int rc;
GC_INIT(ctx);
libxl_vcpuinfo *ptr, *ret;
xc_domaininfo_t domaininfo;
xc_vcpuinfo_t vcpuinfo;
unsigned int nr_vcpus;
- if (xc_domain_getinfolist(ctx->xch, domid, 1, &domaininfo) != 1) {
- LOGED(ERROR, domid, "Getting infolist");
+ if ((rc = xc_domain_getinfo_single(ctx->xch, domid, &domaininfo)) < 0) {
+ LOGED(ERROR, domid, "Getting dominfo: %d", rc);
GC_FREE;
return NULL;
}
diff --git a/tools/libs/light/libxl_mem.c b/tools/libs/light/libxl_mem.c
index 92ec09f4cf..44e554adba 100644
--- a/tools/libs/light/libxl_mem.c
+++ b/tools/libs/light/libxl_mem.c
@@ -323,8 +323,8 @@ retry_transaction:
libxl__xs_printf(gc, t, GCSPRINTF("%s/memory/target", dompath),
"%"PRIu64, new_target_memkb);
- r = xc_domain_getinfolist(ctx->xch, domid, 1, &info);
- if (r != 1 || info.domain != domid) {
+ r = xc_domain_getinfo_single(ctx->xch, domid, &info);
+ if (r < 0) {
abort_transaction = 1;
rc = ERROR_FAIL;
goto out;
diff --git a/tools/libs/light/libxl_sched.c b/tools/libs/light/libxl_sched.c
index 7c53dc60e6..b8d0b9ccd7 100644
--- a/tools/libs/light/libxl_sched.c
+++ b/tools/libs/light/libxl_sched.c
@@ -219,13 +219,11 @@ static int sched_credit_domain_set(libxl__gc *gc, uint32_t domid,
xc_domaininfo_t domaininfo;
int rc;
- rc = xc_domain_getinfolist(CTX->xch, domid, 1, &domaininfo);
+ rc = xc_domain_getinfo_single(CTX->xch, domid, &domaininfo);
if (rc < 0) {
- LOGED(ERROR, domid, "Getting domain info list");
+ LOGED(ERROR, domid, "Getting domain info: %d", rc);
return ERROR_FAIL;
}
- if (rc != 1 || domaininfo.domain != domid)
- return ERROR_INVAL;
rc = xc_sched_credit_domain_get(CTX->xch, domid, &sdom);
if (rc != 0) {
@@ -426,13 +424,11 @@ static int sched_credit2_domain_set(libxl__gc *gc, uint32_t domid,
xc_domaininfo_t info;
int rc;
- rc = xc_domain_getinfolist(CTX->xch, domid, 1, &info);
+ rc = xc_domain_getinfo_single(CTX->xch, domid, &info);
if (rc < 0) {
- LOGED(ERROR, domid, "Getting domain info");
+ LOGED(ERROR, domid, "Getting domain info: %d", rc);
return ERROR_FAIL;
}
- if (rc != 1 || info.domain != domid)
- return ERROR_INVAL;
rc = xc_sched_credit2_domain_get(CTX->xch, domid, &sdom);
if (rc != 0) {
diff --git a/tools/xenpaging/xenpaging.c b/tools/xenpaging/xenpaging.c
index 6e5490315d..c7a9a82477 100644
--- a/tools/xenpaging/xenpaging.c
+++ b/tools/xenpaging/xenpaging.c
@@ -169,8 +169,8 @@ static int xenpaging_get_tot_pages(struct xenpaging *paging)
xc_domaininfo_t domain_info;
int rc;
- rc = xc_domain_getinfolist(xch, paging->vm_event.domain_id, 1, &domain_info);
- if ( rc != 1 )
+ rc = xc_domain_getinfo_single(xch, paging->vm_event.domain_id, &domain_info);
+ if ( rc < 0 )
{
PERROR("Error getting domain info");
return -1;
@@ -424,9 +424,9 @@ static struct xenpaging *xenpaging_init(int argc, char *argv[])
/* Get max_pages from guest if not provided via cmdline */
if ( !paging->max_pages )
{
- rc = xc_domain_getinfolist(xch, paging->vm_event.domain_id, 1,
- &domain_info);
- if ( rc != 1 )
+ rc = xc_domain_getinfo_single(xch, paging->vm_event.domain_id,
+ &domain_info);
+ if ( rc < 0 )
{
PERROR("Error getting domain info");
goto err;
--
2.34.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 6/7] tools: Use new xc function for some xc_domain_getinfo() calls
2023-04-28 10:41 [PATCH v2 0/7] Rationalize usage of xc_domain_getinfo{,list}() Alejandro Vallejo
` (4 preceding siblings ...)
2023-04-28 10:41 ` [PATCH v2 5/7] tools: Modify single-domid callers of xc_domain_getinfolist() Alejandro Vallejo
@ 2023-04-28 10:41 ` Alejandro Vallejo
2023-04-28 16:29 ` Andrew Cooper
2023-04-28 10:41 ` [PATCH v2 7/7] domctl: Modify XEN_DOMCTL_getdomaininfo to fail if domid is not found Alejandro Vallejo
6 siblings, 1 reply; 20+ messages in thread
From: Alejandro Vallejo @ 2023-04-28 10:41 UTC (permalink / raw)
To: Xen-devel
Cc: Alejandro Vallejo, Andrew Cooper, Wei Liu, Anthony PERARD,
Tim Deegan, George Dunlap, Juergen Gross
Move calls that require a information about a single precisely identified
domain to the new xc_domain_getinfo_single().
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: Tim Deegan <tim@xen.org>
Cc: George Dunlap <george.dunlap@citrix.com>
Cc: Juergen Gross <jgross@suse.com>
---
tools/console/client/main.c | 7 +++--
tools/debugger/kdd/kdd-xen.c | 6 +++--
tools/libs/ctrl/xc_domain.c | 9 +++----
tools/libs/ctrl/xc_pagetab.c | 7 +++--
tools/libs/ctrl/xc_private.c | 7 +++--
tools/libs/ctrl/xc_private.h | 7 ++---
tools/libs/guest/xg_core.c | 23 +++++++----------
tools/libs/guest/xg_core.h | 6 ++---
tools/libs/guest/xg_core_arm.c | 10 ++++----
tools/libs/guest/xg_core_x86.c | 18 ++++++-------
tools/libs/guest/xg_cpuid_x86.c | 34 ++++++++++++-------------
tools/libs/guest/xg_dom_boot.c | 16 +++---------
tools/libs/guest/xg_domain.c | 8 +++---
tools/libs/guest/xg_offline_page.c | 12 ++++-----
tools/libs/guest/xg_private.h | 1 +
tools/libs/guest/xg_resume.c | 19 +++++++-------
tools/libs/guest/xg_sr_common.h | 2 +-
tools/libs/guest/xg_sr_restore.c | 17 +++++--------
tools/libs/guest/xg_sr_restore_x86_pv.c | 2 +-
tools/libs/guest/xg_sr_save.c | 26 ++++++++-----------
tools/libs/guest/xg_sr_save_x86_pv.c | 6 ++---
tools/libs/light/libxl_dom.c | 4 +--
tools/libs/light/libxl_domain.c | 4 +--
tools/libs/light/libxl_sched.c | 20 +++++++--------
tools/libs/light/libxl_x86_acpi.c | 4 +--
tools/misc/xen-hvmcrash.c | 6 ++---
tools/misc/xen-lowmemd.c | 6 ++---
tools/misc/xen-mfndump.c | 22 +++++++---------
tools/misc/xen-vmtrace.c | 6 ++---
tools/vchan/vchan-socket-proxy.c | 6 ++---
tools/xenstore/xenstored_domain.c | 15 +++++------
tools/xentrace/xenctx.c | 8 +++---
32 files changed, 157 insertions(+), 187 deletions(-)
diff --git a/tools/console/client/main.c b/tools/console/client/main.c
index 1a6fa162f7..6775006488 100644
--- a/tools/console/client/main.c
+++ b/tools/console/client/main.c
@@ -408,17 +408,16 @@ int main(int argc, char **argv)
if (dom_path == NULL)
err(errno, "xs_get_domain_path()");
if (type == CONSOLE_INVAL) {
- xc_dominfo_t xcinfo;
+ xc_domaininfo_t xcinfo;
xc_interface *xc_handle = xc_interface_open(0,0,0);
if (xc_handle == NULL)
err(errno, "Could not open xc interface");
- if ( (xc_domain_getinfo(xc_handle, domid, 1, &xcinfo) != 1) ||
- (xcinfo.domid != domid) ) {
+ if (xc_domain_getinfo_single(xc_handle, domid, &xcinfo) < 0) {
xc_interface_close(xc_handle);
err(errno, "Failed to get domain information");
}
/* default to pv console for pv guests and serial for hvm guests */
- if (xcinfo.hvm)
+ if (xcinfo.flags & XEN_DOMINF_hvm_guest)
type = CONSOLE_SERIAL;
else
type = CONSOLE_PV;
diff --git a/tools/debugger/kdd/kdd-xen.c b/tools/debugger/kdd/kdd-xen.c
index e78c9311c4..b1a96bf4e2 100644
--- a/tools/debugger/kdd/kdd-xen.c
+++ b/tools/debugger/kdd/kdd-xen.c
@@ -570,7 +570,7 @@ kdd_guest *kdd_guest_init(char *arg, FILE *log, int verbosity)
kdd_guest *g = NULL;
xc_interface *xch = NULL;
uint32_t domid;
- xc_dominfo_t info;
+ xc_domaininfo_t info;
g = calloc(1, sizeof (kdd_guest));
if (!g)
@@ -590,7 +590,9 @@ kdd_guest *kdd_guest_init(char *arg, FILE *log, int verbosity)
g->domid = domid;
/* Check that the domain exists and is HVM */
- if (xc_domain_getinfo(xch, domid, 1, &info) != 1 || !info.hvm)
+ if (xc_domain_getinfo_single(xch, domid, &info) < 0)
+ goto err;
+ if (!(info.flags & XEN_DOMINF_hvm_guest))
goto err;
snprintf(g->id, (sizeof g->id) - 1,
diff --git a/tools/libs/ctrl/xc_domain.c b/tools/libs/ctrl/xc_domain.c
index 6b11775d4c..533e3c1314 100644
--- a/tools/libs/ctrl/xc_domain.c
+++ b/tools/libs/ctrl/xc_domain.c
@@ -1959,15 +1959,14 @@ int xc_domain_memory_mapping(
uint32_t add_mapping)
{
DECLARE_DOMCTL;
- xc_dominfo_t info;
+ xc_domaininfo_t info;
int ret = 0, rc;
unsigned long done = 0, nr, max_batch_sz;
- if ( xc_domain_getinfo(xch, domid, 1, &info) != 1 ||
- info.domid != domid )
+ if ( xc_domain_getinfo_single(xch, domid, &info) < 0 )
{
- PERROR("Could not get info for domain");
- return -EINVAL;
+ PERROR("Could not get info for dom%u", domid);
+ return -1;
}
if ( !xc_core_arch_auto_translated_physmap(&info) )
return 0;
diff --git a/tools/libs/ctrl/xc_pagetab.c b/tools/libs/ctrl/xc_pagetab.c
index db25c20247..d9f886633a 100644
--- a/tools/libs/ctrl/xc_pagetab.c
+++ b/tools/libs/ctrl/xc_pagetab.c
@@ -29,17 +29,16 @@
unsigned long xc_translate_foreign_address(xc_interface *xch, uint32_t dom,
int vcpu, unsigned long long virt)
{
- xc_dominfo_t dominfo;
+ xc_domaininfo_t dominfo;
uint64_t paddr, mask, pte = 0;
int size, level, pt_levels = 2;
void *map;
- if (xc_domain_getinfo(xch, dom, 1, &dominfo) != 1
- || dominfo.domid != dom)
+ if (xc_domain_getinfo_single(xch, dom, &dominfo) < 0)
return 0;
/* What kind of paging are we dealing with? */
- if (dominfo.hvm) {
+ if (dominfo.flags & XEN_DOMINF_hvm_guest) {
struct hvm_hw_cpu ctx;
if (xc_domain_hvm_getcontext_partial(xch, dom,
HVM_SAVE_CODE(CPU), vcpu,
diff --git a/tools/libs/ctrl/xc_private.c b/tools/libs/ctrl/xc_private.c
index 2f99a7d2cf..8dcebad401 100644
--- a/tools/libs/ctrl/xc_private.c
+++ b/tools/libs/ctrl/xc_private.c
@@ -441,11 +441,10 @@ int xc_machphys_mfn_list(xc_interface *xch,
long xc_get_tot_pages(xc_interface *xch, uint32_t domid)
{
- xc_dominfo_t info;
- if ( (xc_domain_getinfo(xch, domid, 1, &info) != 1) ||
- (info.domid != domid) )
+ xc_domaininfo_t info;
+ if ( xc_domain_getinfo_single(xch, domid, &info) < 0 )
return -1;
- return info.nr_pages;
+ return info.tot_pages;
}
int xc_copy_to_domain_page(xc_interface *xch,
diff --git a/tools/libs/ctrl/xc_private.h b/tools/libs/ctrl/xc_private.h
index 80dc464c93..8faabaea67 100644
--- a/tools/libs/ctrl/xc_private.h
+++ b/tools/libs/ctrl/xc_private.h
@@ -16,6 +16,7 @@
#ifndef XC_PRIVATE_H
#define XC_PRIVATE_H
+#include <inttypes.h>
#include <unistd.h>
#include <stdarg.h>
#include <stdio.h>
@@ -420,12 +421,12 @@ void *xc_vm_event_enable(xc_interface *xch, uint32_t domain_id, int param,
int do_dm_op(xc_interface *xch, uint32_t domid, unsigned int nr_bufs, ...);
#if defined (__i386__) || defined (__x86_64__)
-static inline int xc_core_arch_auto_translated_physmap(const xc_dominfo_t *info)
+static inline int xc_core_arch_auto_translated_physmap(const xc_domaininfo_t *info)
{
- return info->hvm;
+ return info->flags & XEN_DOMINF_hvm_guest;
}
#elif defined (__arm__) || defined(__aarch64__)
-static inline int xc_core_arch_auto_translated_physmap(const xc_dominfo_t *info)
+static inline int xc_core_arch_auto_translated_physmap(const xc_domaininfo_t *info)
{
return 1;
}
diff --git a/tools/libs/guest/xg_core.c b/tools/libs/guest/xg_core.c
index c52f1161c1..f83436d6cb 100644
--- a/tools/libs/guest/xg_core.c
+++ b/tools/libs/guest/xg_core.c
@@ -349,7 +349,7 @@ elfnote_dump_none(xc_interface *xch, void *args, dumpcore_rtn_t dump_rtn)
static int
elfnote_dump_core_header(
xc_interface *xch,
- void *args, dumpcore_rtn_t dump_rtn, const xc_dominfo_t *info,
+ void *args, dumpcore_rtn_t dump_rtn, const xc_domaininfo_t *info,
int nr_vcpus, unsigned long nr_pages)
{
int sts;
@@ -361,7 +361,8 @@ elfnote_dump_core_header(
elfnote.descsz = sizeof(header);
elfnote.type = XEN_ELFNOTE_DUMPCORE_HEADER;
- header.xch_magic = info->hvm ? XC_CORE_MAGIC_HVM : XC_CORE_MAGIC;
+ header.xch_magic = (info->flags & XEN_DOMINF_hvm_guest) ? XC_CORE_MAGIC_HVM
+ : XC_CORE_MAGIC;
header.xch_nr_vcpus = nr_vcpus;
header.xch_nr_pages = nr_pages;
header.xch_page_size = PAGE_SIZE;
@@ -423,7 +424,7 @@ xc_domain_dumpcore_via_callback(xc_interface *xch,
void *args,
dumpcore_rtn_t dump_rtn)
{
- xc_dominfo_t info;
+ xc_domaininfo_t info;
shared_info_any_t *live_shinfo = NULL;
struct domain_info_context _dinfo = {};
struct domain_info_context *dinfo = &_dinfo;
@@ -468,15 +469,15 @@ xc_domain_dumpcore_via_callback(xc_interface *xch,
goto out;
}
- if ( xc_domain_getinfo(xch, domid, 1, &info) != 1 )
+ if ( xc_domain_getinfo_single(xch, domid, &info) < 0 )
{
- PERROR("Could not get info for domain");
+ PERROR("Could not get info for dom%u", domid);
goto out;
}
/* Map the shared info frame */
live_shinfo = xc_map_foreign_range(xch, domid, PAGE_SIZE,
PROT_READ, info.shared_info_frame);
- if ( !live_shinfo && !info.hvm )
+ if ( !live_shinfo && !(info.flags & XEN_DOMINF_hvm_guest) )
{
PERROR("Couldn't map live_shinfo");
goto out;
@@ -517,12 +518,6 @@ xc_domain_dumpcore_via_callback(xc_interface *xch,
dinfo->guest_width = sizeof(unsigned long);
}
- if ( domid != info.domid )
- {
- PERROR("Domain %d does not exist", domid);
- goto out;
- }
-
ctxt = calloc(sizeof(*ctxt), info.max_vcpu_id + 1);
if ( !ctxt )
{
@@ -560,9 +555,9 @@ xc_domain_dumpcore_via_callback(xc_interface *xch,
* all the array...
*
* We don't want to use the total potential size of the memory map
- * since that is usually much higher than info.nr_pages.
+ * since that is usually much higher than info.tot_pages.
*/
- nr_pages = info.nr_pages;
+ nr_pages = info.tot_pages;
if ( !auto_translated_physmap )
{
diff --git a/tools/libs/guest/xg_core.h b/tools/libs/guest/xg_core.h
index aaca9e0a8b..ff577dad31 100644
--- a/tools/libs/guest/xg_core.h
+++ b/tools/libs/guest/xg_core.h
@@ -134,15 +134,15 @@ typedef struct xc_core_memory_map xc_core_memory_map_t;
struct xc_core_arch_context;
int xc_core_arch_memory_map_get(xc_interface *xch,
struct xc_core_arch_context *arch_ctxt,
- xc_dominfo_t *info, shared_info_any_t *live_shinfo,
+ xc_domaininfo_t *info, shared_info_any_t *live_shinfo,
xc_core_memory_map_t **mapp,
unsigned int *nr_entries);
int xc_core_arch_map_p2m(xc_interface *xch, struct domain_info_context *dinfo,
- xc_dominfo_t *info, shared_info_any_t *live_shinfo,
+ xc_domaininfo_t *info, shared_info_any_t *live_shinfo,
xen_pfn_t **live_p2m);
int xc_core_arch_map_p2m_writable(xc_interface *xch, struct domain_info_context *dinfo,
- xc_dominfo_t *info,
+ xc_domaininfo_t *info,
shared_info_any_t *live_shinfo,
xen_pfn_t **live_p2m);
diff --git a/tools/libs/guest/xg_core_arm.c b/tools/libs/guest/xg_core_arm.c
index de30cf0c31..34276152da 100644
--- a/tools/libs/guest/xg_core_arm.c
+++ b/tools/libs/guest/xg_core_arm.c
@@ -33,14 +33,14 @@ xc_core_arch_gpfn_may_present(struct xc_core_arch_context *arch_ctxt,
int
xc_core_arch_memory_map_get(xc_interface *xch, struct xc_core_arch_context *unused,
- xc_dominfo_t *info, shared_info_any_t *live_shinfo,
+ xc_domaininfo_t *info, shared_info_any_t *live_shinfo,
xc_core_memory_map_t **mapp,
unsigned int *nr_entries)
{
xen_pfn_t p2m_size = 0;
xc_core_memory_map_t *map;
- if ( xc_domain_nr_gpfns(xch, info->domid, &p2m_size) < 0 )
+ if ( xc_domain_nr_gpfns(xch, info->domain, &p2m_size) < 0 )
return -1;
map = malloc(sizeof(*map));
@@ -59,7 +59,7 @@ xc_core_arch_memory_map_get(xc_interface *xch, struct xc_core_arch_context *unus
}
static int
-xc_core_arch_map_p2m_rw(xc_interface *xch, struct domain_info_context *dinfo, xc_dominfo_t *info,
+xc_core_arch_map_p2m_rw(xc_interface *xch, struct domain_info_context *dinfo, xc_domaininfo_t *info,
shared_info_any_t *live_shinfo, xen_pfn_t **live_p2m, int rw)
{
errno = ENOSYS;
@@ -67,14 +67,14 @@ xc_core_arch_map_p2m_rw(xc_interface *xch, struct domain_info_context *dinfo, xc
}
int
-xc_core_arch_map_p2m(xc_interface *xch, struct domain_info_context *dinfo, xc_dominfo_t *info,
+xc_core_arch_map_p2m(xc_interface *xch, struct domain_info_context *dinfo, xc_domaininfo_t *info,
shared_info_any_t *live_shinfo, xen_pfn_t **live_p2m)
{
return xc_core_arch_map_p2m_rw(xch, dinfo, info, live_shinfo, live_p2m, 0);
}
int
-xc_core_arch_map_p2m_writable(xc_interface *xch, struct domain_info_context *dinfo, xc_dominfo_t *info,
+xc_core_arch_map_p2m_writable(xc_interface *xch, struct domain_info_context *dinfo, xc_domaininfo_t *info,
shared_info_any_t *live_shinfo, xen_pfn_t **live_p2m)
{
return xc_core_arch_map_p2m_rw(xch, dinfo, info, live_shinfo, live_p2m, 1);
diff --git a/tools/libs/guest/xg_core_x86.c b/tools/libs/guest/xg_core_x86.c
index c5e4542ccc..dbd3a440f7 100644
--- a/tools/libs/guest/xg_core_x86.c
+++ b/tools/libs/guest/xg_core_x86.c
@@ -49,14 +49,14 @@ xc_core_arch_gpfn_may_present(struct xc_core_arch_context *arch_ctxt,
int
xc_core_arch_memory_map_get(xc_interface *xch, struct xc_core_arch_context *unused,
- xc_dominfo_t *info, shared_info_any_t *live_shinfo,
+ xc_domaininfo_t *info, shared_info_any_t *live_shinfo,
xc_core_memory_map_t **mapp,
unsigned int *nr_entries)
{
xen_pfn_t p2m_size = 0;
xc_core_memory_map_t *map;
- if ( xc_domain_nr_gpfns(xch, info->domid, &p2m_size) < 0 )
+ if ( xc_domain_nr_gpfns(xch, info->domain, &p2m_size) < 0 )
return -1;
map = malloc(sizeof(*map));
@@ -314,24 +314,24 @@ xc_core_arch_map_p2m_tree_rw(xc_interface *xch, struct domain_info_context *dinf
}
static int
-xc_core_arch_map_p2m_rw(xc_interface *xch, struct domain_info_context *dinfo, xc_dominfo_t *info,
+xc_core_arch_map_p2m_rw(xc_interface *xch, struct domain_info_context *dinfo, xc_domaininfo_t *info,
shared_info_any_t *live_shinfo, xen_pfn_t **live_p2m, int rw)
{
xen_pfn_t *p2m_frame_list = NULL;
uint64_t p2m_cr3;
- uint32_t dom = info->domid;
+ uint32_t dom = info->domain;
int ret = -1;
int err;
- if ( xc_domain_nr_gpfns(xch, info->domid, &dinfo->p2m_size) < 0 )
+ if ( xc_domain_nr_gpfns(xch, info->domain, &dinfo->p2m_size) < 0 )
{
ERROR("Could not get maximum GPFN!");
goto out;
}
- if ( dinfo->p2m_size < info->nr_pages )
+ if ( dinfo->p2m_size < info->tot_pages )
{
- ERROR("p2m_size < nr_pages -1 (%lx < %lx", dinfo->p2m_size, info->nr_pages - 1);
+ ERROR("p2m_size < nr_pages -1 (%lx < %"PRIx64, dinfo->p2m_size, info->tot_pages - 1);
goto out;
}
@@ -366,14 +366,14 @@ out:
}
int
-xc_core_arch_map_p2m(xc_interface *xch, struct domain_info_context *dinfo, xc_dominfo_t *info,
+xc_core_arch_map_p2m(xc_interface *xch, struct domain_info_context *dinfo, xc_domaininfo_t *info,
shared_info_any_t *live_shinfo, xen_pfn_t **live_p2m)
{
return xc_core_arch_map_p2m_rw(xch, dinfo, info, live_shinfo, live_p2m, 0);
}
int
-xc_core_arch_map_p2m_writable(xc_interface *xch, struct domain_info_context *dinfo, xc_dominfo_t *info,
+xc_core_arch_map_p2m_writable(xc_interface *xch, struct domain_info_context *dinfo, xc_domaininfo_t *info,
shared_info_any_t *live_shinfo, xen_pfn_t **live_p2m)
{
return xc_core_arch_map_p2m_rw(xch, dinfo, info, live_shinfo, live_p2m, 1);
diff --git a/tools/libs/guest/xg_cpuid_x86.c b/tools/libs/guest/xg_cpuid_x86.c
index bd16a87e48..1519b5d556 100644
--- a/tools/libs/guest/xg_cpuid_x86.c
+++ b/tools/libs/guest/xg_cpuid_x86.c
@@ -281,7 +281,8 @@ static int xc_cpuid_xend_policy(
xc_interface *xch, uint32_t domid, const struct xc_xend_cpuid *xend)
{
int rc;
- xc_dominfo_t di;
+ bool hvm;
+ xc_domaininfo_t di;
unsigned int nr_leaves, nr_msrs;
uint32_t err_leaf = -1, err_subleaf = -1, err_msr = -1;
/*
@@ -291,13 +292,12 @@ static int xc_cpuid_xend_policy(
xen_cpuid_leaf_t *host = NULL, *def = NULL, *cur = NULL;
unsigned int nr_host, nr_def, nr_cur;
- if ( xc_domain_getinfo(xch, domid, 1, &di) != 1 ||
- di.domid != domid )
+ if ( (rc = xc_domain_getinfo_single(xch, domid, &di)) < 0 )
{
- ERROR("Failed to obtain d%d info", domid);
- rc = -ESRCH;
+ PERROR("Failed to obtain d%d info", domid);
goto fail;
}
+ hvm = di.flags & XEN_DOMINF_hvm_guest;
rc = xc_cpu_policy_get_size(xch, &nr_leaves, &nr_msrs);
if ( rc )
@@ -330,12 +330,12 @@ static int xc_cpuid_xend_policy(
/* Get the domain type's default policy. */
nr_msrs = 0;
nr_def = nr_leaves;
- rc = get_system_cpu_policy(xch, di.hvm ? XEN_SYSCTL_cpu_policy_hvm_default
+ rc = get_system_cpu_policy(xch, hvm ? XEN_SYSCTL_cpu_policy_hvm_default
: XEN_SYSCTL_cpu_policy_pv_default,
&nr_def, def, &nr_msrs, NULL);
if ( rc )
{
- PERROR("Failed to obtain %s def policy", di.hvm ? "hvm" : "pv");
+ PERROR("Failed to obtain %s def policy", hvm ? "hvm" : "pv");
rc = -errno;
goto fail;
}
@@ -428,7 +428,8 @@ int xc_cpuid_apply_policy(xc_interface *xch, uint32_t domid, bool restore,
const struct xc_xend_cpuid *xend)
{
int rc;
- xc_dominfo_t di;
+ bool hvm;
+ xc_domaininfo_t di;
unsigned int i, nr_leaves, nr_msrs;
xen_cpuid_leaf_t *leaves = NULL;
struct cpu_policy *p = NULL;
@@ -436,13 +437,12 @@ int xc_cpuid_apply_policy(xc_interface *xch, uint32_t domid, bool restore,
uint32_t host_featureset[FEATURESET_NR_ENTRIES] = {};
uint32_t len = ARRAY_SIZE(host_featureset);
- if ( xc_domain_getinfo(xch, domid, 1, &di) != 1 ||
- di.domid != domid )
+ if ( (rc = xc_domain_getinfo_single(xch, domid, &di)) < 0 )
{
- ERROR("Failed to obtain d%d info", domid);
- rc = -ESRCH;
+ PERROR("Failed to obtain d%d info", domid);
goto out;
}
+ hvm = di.flags & XEN_DOMINF_hvm_guest;
rc = xc_cpu_policy_get_size(xch, &nr_leaves, &nr_msrs);
if ( rc )
@@ -475,12 +475,12 @@ int xc_cpuid_apply_policy(xc_interface *xch, uint32_t domid, bool restore,
/* Get the domain's default policy. */
nr_msrs = 0;
- rc = get_system_cpu_policy(xch, di.hvm ? XEN_SYSCTL_cpu_policy_hvm_default
+ rc = get_system_cpu_policy(xch, hvm ? XEN_SYSCTL_cpu_policy_hvm_default
: XEN_SYSCTL_cpu_policy_pv_default,
&nr_leaves, leaves, &nr_msrs, NULL);
if ( rc )
{
- PERROR("Failed to obtain %s default policy", di.hvm ? "hvm" : "pv");
+ PERROR("Failed to obtain %s default policy", hvm ? "hvm" : "pv");
rc = -errno;
goto out;
}
@@ -514,7 +514,7 @@ int xc_cpuid_apply_policy(xc_interface *xch, uint32_t domid, bool restore,
p->feat.hle = test_bit(X86_FEATURE_HLE, host_featureset);
p->feat.rtm = test_bit(X86_FEATURE_RTM, host_featureset);
- if ( di.hvm )
+ if ( hvm )
{
p->feat.mpx = test_bit(X86_FEATURE_MPX, host_featureset);
}
@@ -571,7 +571,7 @@ int xc_cpuid_apply_policy(xc_interface *xch, uint32_t domid, bool restore,
{
p->extd.itsc = itsc;
- if ( di.hvm )
+ if ( hvm )
{
p->basic.pae = pae;
p->basic.vmx = nested_virt;
@@ -579,7 +579,7 @@ int xc_cpuid_apply_policy(xc_interface *xch, uint32_t domid, bool restore,
}
}
- if ( !di.hvm )
+ if ( !hvm )
{
/*
* On hardware without CPUID Faulting, PV guests see real topology.
diff --git a/tools/libs/guest/xg_dom_boot.c b/tools/libs/guest/xg_dom_boot.c
index 263a3f4c85..1dea534bba 100644
--- a/tools/libs/guest/xg_dom_boot.c
+++ b/tools/libs/guest/xg_dom_boot.c
@@ -164,7 +164,7 @@ void *xc_dom_boot_domU_map(struct xc_dom_image *dom, xen_pfn_t pfn,
int xc_dom_boot_image(struct xc_dom_image *dom)
{
- xc_dominfo_t info;
+ xc_domaininfo_t info;
int rc;
DOMPRINTF_CALLED(dom->xch);
@@ -174,19 +174,11 @@ int xc_dom_boot_image(struct xc_dom_image *dom)
return rc;
/* collect some info */
- rc = xc_domain_getinfo(dom->xch, dom->guest_domid, 1, &info);
- if ( rc < 0 )
- {
- xc_dom_panic(dom->xch, XC_INTERNAL_ERROR,
- "%s: getdomaininfo failed (rc=%d)", __FUNCTION__, rc);
- return rc;
- }
- if ( rc == 0 || info.domid != dom->guest_domid )
+ if ( xc_domain_getinfo_single(dom->xch, dom->guest_domid, &info) < 0 )
{
xc_dom_panic(dom->xch, XC_INTERNAL_ERROR,
- "%s: Huh? No domains found (nr_domains=%d) "
- "or domid mismatch (%d != %d)", __FUNCTION__,
- rc, info.domid, dom->guest_domid);
+ "%s: getdomaininfo failed (errno=%d)",
+ __FUNCTION__, rc, errno);
return -1;
}
dom->shared_info_mfn = info.shared_info_frame;
diff --git a/tools/libs/guest/xg_domain.c b/tools/libs/guest/xg_domain.c
index f0e7748449..198f6f904a 100644
--- a/tools/libs/guest/xg_domain.c
+++ b/tools/libs/guest/xg_domain.c
@@ -37,7 +37,7 @@ int xc_map_domain_meminfo(xc_interface *xch, uint32_t domid,
{
struct domain_info_context _di;
- xc_dominfo_t info;
+ xc_domaininfo_t info;
shared_info_any_t *live_shinfo;
xen_capabilities_info_t xen_caps = "";
unsigned long i;
@@ -49,9 +49,9 @@ int xc_map_domain_meminfo(xc_interface *xch, uint32_t domid,
return -1;
}
- if ( xc_domain_getinfo(xch, domid, 1, &info) != 1 )
+ if ( xc_domain_getinfo_single(xch, domid, &info) < 0 )
{
- PERROR("Could not get domain info");
+ PERROR("Could not get dominfo for dom%u", domid);
return -1;
}
@@ -86,7 +86,7 @@ int xc_map_domain_meminfo(xc_interface *xch, uint32_t domid,
info.shared_info_frame);
if ( !live_shinfo )
{
- PERROR("Could not map the shared info frame (MFN 0x%lx)",
+ PERROR("Could not map the shared info frame (MFN 0x%"PRIx64")",
info.shared_info_frame);
return -1;
}
diff --git a/tools/libs/guest/xg_offline_page.c b/tools/libs/guest/xg_offline_page.c
index ccd0299f0f..292f73a3e7 100644
--- a/tools/libs/guest/xg_offline_page.c
+++ b/tools/libs/guest/xg_offline_page.c
@@ -370,7 +370,7 @@ static int clear_pte(xc_interface *xch, uint32_t domid,
*/
static int is_page_exchangable(xc_interface *xch, uint32_t domid, xen_pfn_t mfn,
- xc_dominfo_t *info)
+ xc_domaininfo_t *info)
{
uint32_t status;
int rc;
@@ -381,7 +381,7 @@ static int is_page_exchangable(xc_interface *xch, uint32_t domid, xen_pfn_t mfn,
DPRINTF("Dom0's page can't be LM");
return 0;
}
- if (info->hvm)
+ if (info->flags & XEN_DOMINF_hvm_guest)
{
DPRINTF("Currently we can only live change PV guest's page\n");
return 0;
@@ -462,7 +462,7 @@ err0:
/* The domain should be suspended when called here */
int xc_exchange_page(xc_interface *xch, uint32_t domid, xen_pfn_t mfn)
{
- xc_dominfo_t info;
+ xc_domaininfo_t info;
struct xc_domain_meminfo minfo;
struct xc_mmu *mmu = NULL;
struct pte_backup old_ptes = {NULL, 0, 0};
@@ -477,13 +477,13 @@ int xc_exchange_page(xc_interface *xch, uint32_t domid, xen_pfn_t mfn)
xen_pfn_t *m2p_table;
unsigned long max_mfn;
- if ( xc_domain_getinfo(xch, domid, 1, &info) != 1 )
+ if ( xc_domain_getinfo_single(xch, domid, &info) < 0 )
{
- ERROR("Could not get domain info");
+ PERROR("Could not get domain info for dom%u", domid);
return -1;
}
- if (!info.shutdown || info.shutdown_reason != SHUTDOWN_suspend)
+ if (!dominfo_shutdown_with(&info, SHUTDOWN_suspend))
{
errno = EINVAL;
ERROR("Can't exchange page unless domain is suspended\n");
diff --git a/tools/libs/guest/xg_private.h b/tools/libs/guest/xg_private.h
index e729a8106c..d73947094f 100644
--- a/tools/libs/guest/xg_private.h
+++ b/tools/libs/guest/xg_private.h
@@ -16,6 +16,7 @@
#ifndef XG_PRIVATE_H
#define XG_PRIVATE_H
+#include <inttypes.h>
#include <unistd.h>
#include <errno.h>
#include <fcntl.h>
diff --git a/tools/libs/guest/xg_resume.c b/tools/libs/guest/xg_resume.c
index 77e2451a3c..2965965e5b 100644
--- a/tools/libs/guest/xg_resume.c
+++ b/tools/libs/guest/xg_resume.c
@@ -26,28 +26,27 @@
static int modify_returncode(xc_interface *xch, uint32_t domid)
{
vcpu_guest_context_any_t ctxt;
- xc_dominfo_t info;
+ xc_domaininfo_t info;
xen_capabilities_info_t caps;
struct domain_info_context _dinfo = {};
struct domain_info_context *dinfo = &_dinfo;
int rc;
- if ( xc_domain_getinfo(xch, domid, 1, &info) != 1 ||
- info.domid != domid )
+ if ( xc_domain_getinfo_single(xch, domid, &info) < 0 )
{
- PERROR("Could not get domain info");
+ PERROR("Could not get info for dom%u", domid);
return -1;
}
- if ( !info.shutdown || (info.shutdown_reason != SHUTDOWN_suspend) )
+ if ( !dominfo_shutdown_with(&info, SHUTDOWN_suspend) )
{
ERROR("Dom %d not suspended: (shutdown %d, reason %d)", domid,
- info.shutdown, info.shutdown_reason);
+ info.flags & XEN_DOMINF_shutdown, dominfo_shutdown_reason(&info));
errno = EINVAL;
return -1;
}
- if ( info.hvm )
+ if ( info.flags & XEN_DOMINF_hvm_guest )
{
/* HVM guests without PV drivers have no return code to modify. */
uint64_t irq = 0;
@@ -133,7 +132,7 @@ static int xc_domain_resume_hvm(xc_interface *xch, uint32_t domid)
static int xc_domain_resume_any(xc_interface *xch, uint32_t domid)
{
DECLARE_DOMCTL;
- xc_dominfo_t info;
+ xc_domaininfo_t info;
int i, rc = -1;
#if defined(__i386__) || defined(__x86_64__)
struct domain_info_context _dinfo = { .guest_width = 0,
@@ -146,7 +145,7 @@ static int xc_domain_resume_any(xc_interface *xch, uint32_t domid)
xen_pfn_t *p2m = NULL;
#endif
- if ( xc_domain_getinfo(xch, domid, 1, &info) != 1 )
+ if ( xc_domain_getinfo_single(xch, domid, &info) < 0 )
{
PERROR("Could not get domain info");
return rc;
@@ -156,7 +155,7 @@ static int xc_domain_resume_any(xc_interface *xch, uint32_t domid)
* (x86 only) Rewrite store_mfn and console_mfn back to MFN (from PFN).
*/
#if defined(__i386__) || defined(__x86_64__)
- if ( info.hvm )
+ if ( info.flags & XEN_DOMINF_hvm_guest )
return xc_domain_resume_hvm(xch, domid);
if ( xc_domain_get_guest_width(xch, domid, &dinfo->guest_width) != 0 )
diff --git a/tools/libs/guest/xg_sr_common.h b/tools/libs/guest/xg_sr_common.h
index 36d45ef56f..2f058ee3a6 100644
--- a/tools/libs/guest/xg_sr_common.h
+++ b/tools/libs/guest/xg_sr_common.h
@@ -220,7 +220,7 @@ struct xc_sr_context
/* Plain VM, or checkpoints over time. */
xc_stream_type_t stream_type;
- xc_dominfo_t dominfo;
+ xc_domaininfo_t dominfo;
union /* Common save or restore data. */
{
diff --git a/tools/libs/guest/xg_sr_restore.c b/tools/libs/guest/xg_sr_restore.c
index 7314a24cf9..6767c9f5cc 100644
--- a/tools/libs/guest/xg_sr_restore.c
+++ b/tools/libs/guest/xg_sr_restore.c
@@ -852,6 +852,7 @@ int xc_domain_restore(xc_interface *xch, int io_fd, uint32_t dom,
xc_stream_type_t stream_type,
struct restore_callbacks *callbacks, int send_back_fd)
{
+ bool hvm;
xen_pfn_t nr_pfns;
struct xc_sr_context ctx = {
.xch = xch,
@@ -887,20 +888,15 @@ int xc_domain_restore(xc_interface *xch, int io_fd, uint32_t dom,
break;
}
- if ( xc_domain_getinfo(xch, dom, 1, &ctx.dominfo) != 1 )
+ if ( xc_domain_getinfo_single(xch, dom, &ctx.dominfo) < 0 )
{
- PERROR("Failed to get domain info");
- return -1;
- }
-
- if ( ctx.dominfo.domid != dom )
- {
- ERROR("Domain %u does not exist", dom);
+ PERROR("Failed to get info for dom%u", dom);
return -1;
}
+ hvm = ctx.dominfo.flags & XEN_DOMINF_hvm_guest;
DPRINTF("fd %d, dom %u, hvm %u, stream_type %d",
- io_fd, dom, ctx.dominfo.hvm, stream_type);
+ io_fd, dom, hvm, stream_type);
ctx.domid = dom;
@@ -914,8 +910,7 @@ int xc_domain_restore(xc_interface *xch, int io_fd, uint32_t dom,
}
ctx.restore.p2m_size = nr_pfns;
- ctx.restore.ops = ctx.dominfo.hvm
- ? restore_ops_x86_hvm : restore_ops_x86_pv;
+ ctx.restore.ops = hvm ? restore_ops_x86_hvm : restore_ops_x86_pv;
if ( restore(&ctx) )
return -1;
diff --git a/tools/libs/guest/xg_sr_restore_x86_pv.c b/tools/libs/guest/xg_sr_restore_x86_pv.c
index dc50b0f5a8..eaeb97f4a0 100644
--- a/tools/libs/guest/xg_sr_restore_x86_pv.c
+++ b/tools/libs/guest/xg_sr_restore_x86_pv.c
@@ -903,7 +903,7 @@ static int handle_shared_info(struct xc_sr_context *ctx,
ctx->dominfo.shared_info_frame);
if ( !guest_shinfo )
{
- PERROR("Failed to map Shared Info at mfn %#lx",
+ PERROR("Failed to map Shared Info at mfn %#"PRIx64,
ctx->dominfo.shared_info_frame);
goto err;
}
diff --git a/tools/libs/guest/xg_sr_save.c b/tools/libs/guest/xg_sr_save.c
index 9853d8d846..b0b30b4bc2 100644
--- a/tools/libs/guest/xg_sr_save.c
+++ b/tools/libs/guest/xg_sr_save.c
@@ -336,19 +336,17 @@ static int suspend_domain(struct xc_sr_context *ctx)
}
/* Refresh domain information. */
- if ( (xc_domain_getinfo(xch, ctx->domid, 1, &ctx->dominfo) != 1) ||
- (ctx->dominfo.domid != ctx->domid) )
+ if ( xc_domain_getinfo_single(xch, ctx->domid, &ctx->dominfo) < 0 )
{
PERROR("Unable to refresh domain information");
return -1;
}
/* Confirm the domain has actually been paused. */
- if ( !ctx->dominfo.shutdown ||
- (ctx->dominfo.shutdown_reason != SHUTDOWN_suspend) )
+ if ( !dominfo_shutdown_with(&ctx->dominfo, SHUTDOWN_suspend) )
{
ERROR("Domain has not been suspended: shutdown %d, reason %d",
- ctx->dominfo.shutdown, ctx->dominfo.shutdown_reason);
+ ctx->dominfo.flags & XEN_DOMINF_shutdown, dominfo_shutdown_reason(&ctx->dominfo));
return -1;
}
@@ -893,8 +891,7 @@ static int save(struct xc_sr_context *ctx, uint16_t guest_type)
if ( rc )
goto err;
- if ( !ctx->dominfo.shutdown ||
- (ctx->dominfo.shutdown_reason != SHUTDOWN_suspend) )
+ if ( !dominfo_shutdown_with(&ctx->dominfo, SHUTDOWN_suspend) )
{
ERROR("Domain has not been suspended");
rc = -1;
@@ -989,6 +986,7 @@ int xc_domain_save(xc_interface *xch, int io_fd, uint32_t dom,
.fd = io_fd,
.stream_type = stream_type,
};
+ bool hvm;
/* GCC 4.4 (of CentOS 6.x vintage) can' t initialise anonymous unions. */
ctx.save.callbacks = callbacks;
@@ -996,17 +994,13 @@ int xc_domain_save(xc_interface *xch, int io_fd, uint32_t dom,
ctx.save.debug = !!(flags & XCFLAGS_DEBUG);
ctx.save.recv_fd = recv_fd;
- if ( xc_domain_getinfo(xch, dom, 1, &ctx.dominfo) != 1 )
+ if ( xc_domain_getinfo_single(xch, dom, &ctx.dominfo) < 0 )
{
PERROR("Failed to get domain info");
return -1;
}
- if ( ctx.dominfo.domid != dom )
- {
- ERROR("Domain %u does not exist", dom);
- return -1;
- }
+ hvm = ctx.dominfo.flags & XEN_DOMINF_hvm_guest;
/* Sanity check stream_type-related parameters */
switch ( stream_type )
@@ -1018,7 +1012,7 @@ int xc_domain_save(xc_interface *xch, int io_fd, uint32_t dom,
assert(callbacks->checkpoint && callbacks->postcopy);
/* Fallthrough */
case XC_STREAM_PLAIN:
- if ( ctx.dominfo.hvm )
+ if ( hvm )
assert(callbacks->switch_qemu_logdirty);
break;
@@ -1028,11 +1022,11 @@ int xc_domain_save(xc_interface *xch, int io_fd, uint32_t dom,
}
DPRINTF("fd %d, dom %u, flags %u, hvm %d",
- io_fd, dom, flags, ctx.dominfo.hvm);
+ io_fd, dom, flags, hvm);
ctx.domid = dom;
- if ( ctx.dominfo.hvm )
+ if ( hvm )
{
ctx.save.ops = save_ops_x86_hvm;
return save(&ctx, DHDR_TYPE_X86_HVM);
diff --git a/tools/libs/guest/xg_sr_save_x86_pv.c b/tools/libs/guest/xg_sr_save_x86_pv.c
index 4964f1f7b8..f3d7a7a71a 100644
--- a/tools/libs/guest/xg_sr_save_x86_pv.c
+++ b/tools/libs/guest/xg_sr_save_x86_pv.c
@@ -20,7 +20,7 @@ static int map_shinfo(struct xc_sr_context *ctx)
xch, ctx->domid, PAGE_SIZE, PROT_READ, ctx->dominfo.shared_info_frame);
if ( !ctx->x86.pv.shinfo )
{
- PERROR("Failed to map shared info frame at mfn %#lx",
+ PERROR("Failed to map shared info frame at mfn %#"PRIx64,
ctx->dominfo.shared_info_frame);
return -1;
}
@@ -943,7 +943,7 @@ static int normalise_pagetable(struct xc_sr_context *ctx, const uint64_t *src,
#ifdef __i386__
if ( mfn == INVALID_MFN )
{
- if ( !ctx->dominfo.paused )
+ if ( !(ctx->dominfo.flags & XEN_DOMINF_paused) )
errno = EAGAIN;
else
{
@@ -965,7 +965,7 @@ static int normalise_pagetable(struct xc_sr_context *ctx, const uint64_t *src,
if ( !mfn_in_pseudophysmap(ctx, mfn) )
{
- if ( !ctx->dominfo.paused )
+ if ( !(ctx->dominfo.flags & XEN_DOMINF_paused) )
errno = EAGAIN;
else
{
diff --git a/tools/libs/light/libxl_dom.c b/tools/libs/light/libxl_dom.c
index bd5d823581..94fef37401 100644
--- a/tools/libs/light/libxl_dom.c
+++ b/tools/libs/light/libxl_dom.c
@@ -34,7 +34,7 @@ libxl_domain_type libxl__domain_type(libxl__gc *gc, uint32_t domid)
ret = xc_domain_getinfo_single(ctx->xch, domid, &info);
if (ret < 0) {
- LOG(ERROR, "unable to get domain type for domid=%"PRIu32, domid);
+ LOGED(ERROR, domid, "unable to get dominfo");
return LIBXL_DOMAIN_TYPE_INVALID;
}
if (info.flags & XEN_DOMINF_hvm_guest) {
@@ -73,7 +73,7 @@ int libxl__domain_cpupool(libxl__gc *gc, uint32_t domid)
ret = xc_domain_getinfo_single(CTX->xch, domid, &info);
if (ret < 0)
{
- LOGED(ERROR, domid, "get domaininfo failed: %d", ret);
+ LOGED(ERROR, domid, "get domaininfo failed");
return ERROR_FAIL;
}
return info.cpupool;
diff --git a/tools/libs/light/libxl_domain.c b/tools/libs/light/libxl_domain.c
index 9fa4091859..5709b3e62f 100644
--- a/tools/libs/light/libxl_domain.c
+++ b/tools/libs/light/libxl_domain.c
@@ -351,7 +351,7 @@ int libxl_domain_info(libxl_ctx *ctx, libxl_dominfo *info_r,
ret = xc_domain_getinfo_single(ctx->xch, domid, &xcinfo);
if (ret<0) {
- LOGED(ERROR, domid, "Getting domain info: %d", ret);
+ LOGED(ERROR, domid, "Getting domain info");
GC_FREE;
return ERROR_FAIL;
}
@@ -1661,7 +1661,7 @@ libxl_vcpuinfo *libxl_list_vcpu(libxl_ctx *ctx, uint32_t domid,
unsigned int nr_vcpus;
if ((rc = xc_domain_getinfo_single(ctx->xch, domid, &domaininfo)) < 0) {
- LOGED(ERROR, domid, "Getting dominfo: %d", rc);
+ LOGED(ERROR, domid, "Getting dominfo");
GC_FREE;
return NULL;
}
diff --git a/tools/libs/light/libxl_sched.c b/tools/libs/light/libxl_sched.c
index b8d0b9ccd7..b87e490d12 100644
--- a/tools/libs/light/libxl_sched.c
+++ b/tools/libs/light/libxl_sched.c
@@ -221,7 +221,7 @@ static int sched_credit_domain_set(libxl__gc *gc, uint32_t domid,
rc = xc_domain_getinfo_single(CTX->xch, domid, &domaininfo);
if (rc < 0) {
- LOGED(ERROR, domid, "Getting domain info: %d", rc);
+ LOGED(ERROR, domid, "Getting domain info");
return ERROR_FAIL;
}
@@ -426,7 +426,7 @@ static int sched_credit2_domain_set(libxl__gc *gc, uint32_t domid,
rc = xc_domain_getinfo_single(CTX->xch, domid, &info);
if (rc < 0) {
- LOGED(ERROR, domid, "Getting domain info: %d", rc);
+ LOGED(ERROR, domid, "Getting domain info");
return ERROR_FAIL;
}
@@ -498,10 +498,10 @@ static int sched_rtds_vcpu_get(libxl__gc *gc, uint32_t domid,
{
uint32_t num_vcpus;
int i, r, rc;
- xc_dominfo_t info;
+ xc_domaininfo_t info;
struct xen_domctl_schedparam_vcpu *vcpus;
- r = xc_domain_getinfo(CTX->xch, domid, 1, &info);
+ r = xc_domain_getinfo_single(CTX->xch, domid, &info);
if (r < 0) {
LOGED(ERROR, domid, "Getting domain info");
rc = ERROR_FAIL;
@@ -552,10 +552,10 @@ static int sched_rtds_vcpu_get_all(libxl__gc *gc, uint32_t domid,
{
uint32_t num_vcpus;
int i, r, rc;
- xc_dominfo_t info;
+ xc_domaininfo_t info;
struct xen_domctl_schedparam_vcpu *vcpus;
- r = xc_domain_getinfo(CTX->xch, domid, 1, &info);
+ r = xc_domain_getinfo_single(CTX->xch, domid, &info);
if (r < 0) {
LOGED(ERROR, domid, "Getting domain info");
rc = ERROR_FAIL;
@@ -602,10 +602,10 @@ static int sched_rtds_vcpu_set(libxl__gc *gc, uint32_t domid,
int r, rc;
int i;
uint16_t max_vcpuid;
- xc_dominfo_t info;
+ xc_domaininfo_t info;
struct xen_domctl_schedparam_vcpu *vcpus;
- r = xc_domain_getinfo(CTX->xch, domid, 1, &info);
+ r = xc_domain_getinfo_single(CTX->xch, domid, &info);
if (r < 0) {
LOGED(ERROR, domid, "Getting domain info");
rc = ERROR_FAIL;
@@ -662,11 +662,11 @@ static int sched_rtds_vcpu_set_all(libxl__gc *gc, uint32_t domid,
int r, rc;
int i;
uint16_t max_vcpuid;
- xc_dominfo_t info;
+ xc_domaininfo_t info;
struct xen_domctl_schedparam_vcpu *vcpus;
uint32_t num_vcpus;
- r = xc_domain_getinfo(CTX->xch, domid, 1, &info);
+ r = xc_domain_getinfo_single(CTX->xch, domid, &info);
if (r < 0) {
LOGED(ERROR, domid, "Getting domain info");
rc = ERROR_FAIL;
diff --git a/tools/libs/light/libxl_x86_acpi.c b/tools/libs/light/libxl_x86_acpi.c
index 22eb160659..796b009d0c 100644
--- a/tools/libs/light/libxl_x86_acpi.c
+++ b/tools/libs/light/libxl_x86_acpi.c
@@ -87,14 +87,14 @@ static int init_acpi_config(libxl__gc *gc,
{
xc_interface *xch = dom->xch;
uint32_t domid = dom->guest_domid;
- xc_dominfo_t info;
+ xc_domaininfo_t info;
struct hvm_info_table *hvminfo;
int i, r, rc;
config->dsdt_anycpu = config->dsdt_15cpu = dsdt_pvh;
config->dsdt_anycpu_len = config->dsdt_15cpu_len = dsdt_pvh_len;
- r = xc_domain_getinfo(xch, domid, 1, &info);
+ r = xc_domain_getinfo_single(xch, domid, &info);
if (r < 0) {
LOG(ERROR, "getdomaininfo failed (rc=%d)", r);
rc = ERROR_FAIL;
diff --git a/tools/misc/xen-hvmcrash.c b/tools/misc/xen-hvmcrash.c
index 4f0dabcb18..1d058fa40a 100644
--- a/tools/misc/xen-hvmcrash.c
+++ b/tools/misc/xen-hvmcrash.c
@@ -48,7 +48,7 @@ main(int argc, char **argv)
{
int domid;
xc_interface *xch;
- xc_dominfo_t dominfo;
+ xc_domaininfo_t dominfo;
int ret;
uint32_t len;
uint8_t *buf;
@@ -66,13 +66,13 @@ main(int argc, char **argv)
exit(1);
}
- ret = xc_domain_getinfo(xch, domid, 1, &dominfo);
+ ret = xc_domain_getinfo_single(xch, domid, &dominfo);
if (ret < 0) {
perror("xc_domain_getinfo");
exit(1);
}
- if (!dominfo.hvm) {
+ if (!(dominfo.flags & XEN_DOMINF_hvm_guest)) {
fprintf(stderr, "domain %d is not HVM\n", domid);
exit(1);
}
diff --git a/tools/misc/xen-lowmemd.c b/tools/misc/xen-lowmemd.c
index a3a2741242..9d5cb549a8 100644
--- a/tools/misc/xen-lowmemd.c
+++ b/tools/misc/xen-lowmemd.c
@@ -38,7 +38,7 @@ void cleanup(void)
#define BUFSZ 512
void handle_low_mem(void)
{
- xc_dominfo_t dom0_info;
+ xc_domaininfo_t dom0_info;
xc_physinfo_t info;
unsigned long long free_pages, dom0_pages, diff, dom0_target;
char data[BUFSZ], error[BUFSZ];
@@ -58,13 +58,13 @@ void handle_low_mem(void)
return;
diff = THRESHOLD_PG - free_pages;
- if (xc_domain_getinfo(xch, 0, 1, &dom0_info) < 1)
+ if (xc_domain_getinfo_single(xch, 0, &dom0_info) < 0)
{
perror("Failed to get dom0 info");
return;
}
- dom0_pages = (unsigned long long) dom0_info.nr_pages;
+ dom0_pages = (unsigned long long) dom0_info.tot_pages;
printf("Dom0 pages: 0x%llx:%llu\n", dom0_pages, dom0_pages);
dom0_target = dom0_pages - diff;
if (dom0_target <= DOM0_FLOOR_PG)
diff --git a/tools/misc/xen-mfndump.c b/tools/misc/xen-mfndump.c
index b32c95e262..8863ece3f5 100644
--- a/tools/misc/xen-mfndump.c
+++ b/tools/misc/xen-mfndump.c
@@ -74,7 +74,7 @@ int dump_m2p_func(int argc, char *argv[])
int dump_p2m_func(int argc, char *argv[])
{
struct xc_domain_meminfo minfo;
- xc_dominfo_t info;
+ xc_domaininfo_t info;
unsigned long i;
int domid;
@@ -85,8 +85,7 @@ int dump_p2m_func(int argc, char *argv[])
}
domid = atoi(argv[0]);
- if ( xc_domain_getinfo(xch, domid, 1, &info) != 1 ||
- info.domid != domid )
+ if ( xc_domain_getinfo_single(xch, domid, &info) < 0 )
{
ERROR("Failed to obtain info for domain %d\n", domid);
return -1;
@@ -158,7 +157,7 @@ int dump_p2m_func(int argc, char *argv[])
int dump_ptes_func(int argc, char *argv[])
{
struct xc_domain_meminfo minfo;
- xc_dominfo_t info;
+ xc_domaininfo_t info;
void *page = NULL;
unsigned long i, max_mfn;
int domid, pte_num, rc = 0;
@@ -172,8 +171,7 @@ int dump_ptes_func(int argc, char *argv[])
domid = atoi(argv[0]);
mfn = strtoul(argv[1], NULL, 16);
- if ( xc_domain_getinfo(xch, domid, 1, &info) != 1 ||
- info.domid != domid )
+ if ( xc_domain_getinfo_single(xch, domid, &info) < 0 )
{
ERROR("Failed to obtain info for domain %d\n", domid);
return -1;
@@ -266,7 +264,7 @@ int dump_ptes_func(int argc, char *argv[])
int lookup_pte_func(int argc, char *argv[])
{
struct xc_domain_meminfo minfo;
- xc_dominfo_t info;
+ xc_domaininfo_t info;
void *page = NULL;
unsigned long i, j;
int domid, pte_num;
@@ -280,8 +278,7 @@ int lookup_pte_func(int argc, char *argv[])
domid = atoi(argv[0]);
mfn = strtoul(argv[1], NULL, 16);
- if ( xc_domain_getinfo(xch, domid, 1, &info) != 1 ||
- info.domid != domid )
+ if ( xc_domain_getinfo_single(xch, domid, &info) < 0 )
{
ERROR("Failed to obtain info for domain %d\n", domid);
return -1;
@@ -336,7 +333,7 @@ int lookup_pte_func(int argc, char *argv[])
int memcmp_mfns_func(int argc, char *argv[])
{
- xc_dominfo_t info1, info2;
+ xc_domaininfo_t info1, info2;
void *page1 = NULL, *page2 = NULL;
int domid1, domid2;
xen_pfn_t mfn1, mfn2;
@@ -352,9 +349,8 @@ int memcmp_mfns_func(int argc, char *argv[])
mfn1 = strtoul(argv[1], NULL, 16);
mfn2 = strtoul(argv[3], NULL, 16);
- if ( xc_domain_getinfo(xch, domid1, 1, &info1) != 1 ||
- xc_domain_getinfo(xch, domid2, 1, &info2) != 1 ||
- info1.domid != domid1 || info2.domid != domid2)
+ if ( xc_domain_getinfo_single(xch, domid1, &info1) < 0 ||
+ xc_domain_getinfo_single(xch, domid2, &info2) < 0)
{
ERROR("Failed to obtain info for domains\n");
return -1;
diff --git a/tools/misc/xen-vmtrace.c b/tools/misc/xen-vmtrace.c
index 5b688a54af..ba2ce17a17 100644
--- a/tools/misc/xen-vmtrace.c
+++ b/tools/misc/xen-vmtrace.c
@@ -133,15 +133,15 @@ int main(int argc, char **argv)
while ( !interrupted )
{
- xc_dominfo_t dominfo;
+ xc_domaininfo_t dominfo;
if ( get_more_data() )
goto out;
usleep(1000 * 100);
- if ( xc_domain_getinfo(xch, domid, 1, &dominfo) != 1 ||
- dominfo.domid != domid || dominfo.shutdown )
+ if ( xc_domain_getinfo_single(xch, domid, &dominfo) < 0 ||
+ (dominfo.flags & XEN_DOMINF_shutdown) )
{
if ( get_more_data() )
goto out;
diff --git a/tools/vchan/vchan-socket-proxy.c b/tools/vchan/vchan-socket-proxy.c
index e1d959c6d1..9c4c336b03 100644
--- a/tools/vchan/vchan-socket-proxy.c
+++ b/tools/vchan/vchan-socket-proxy.c
@@ -222,7 +222,7 @@ static struct libxenvchan *connect_vchan(int domid, const char *path) {
struct libxenvchan *ctrl = NULL;
struct xs_handle *xs = NULL;
xc_interface *xc = NULL;
- xc_dominfo_t dominfo;
+ xc_domaininfo_t dominfo;
char **watch_ret;
unsigned int watch_num;
int ret;
@@ -254,12 +254,12 @@ static struct libxenvchan *connect_vchan(int domid, const char *path) {
if (ctrl)
break;
- ret = xc_domain_getinfo(xc, domid, 1, &dominfo);
+ ret = xc_domain_getinfo_single(xc, domid, &dominfo);
/* break the loop if domain is definitely not there anymore, but
* continue if it is or the call failed (like EPERM) */
if (ret == -1 && errno == ESRCH)
break;
- if (ret == 1 && (dominfo.domid != (uint32_t)domid || dominfo.dying))
+ if (ret == 0 && (dominfo.flags & XEN_DOMINF_dying))
break;
}
diff --git a/tools/xenstore/xenstored_domain.c b/tools/xenstore/xenstored_domain.c
index f62be2245c..aeb7595ae1 100644
--- a/tools/xenstore/xenstored_domain.c
+++ b/tools/xenstore/xenstored_domain.c
@@ -339,15 +339,14 @@ static int destroy_domain(void *_domain)
return 0;
}
-static bool get_domain_info(unsigned int domid, xc_dominfo_t *dominfo)
+static bool get_domain_info(unsigned int domid, xc_domaininfo_t *dominfo)
{
- return xc_domain_getinfo(*xc_handle, domid, 1, dominfo) == 1 &&
- dominfo->domid == domid;
+ return xc_domain_getinfo_single(*xc_handle, domid, dominfo) == 0;
}
static int check_domain(const void *k, void *v, void *arg)
{
- xc_dominfo_t dominfo;
+ xc_domaininfo_t dominfo;
struct connection *conn;
bool dom_valid;
struct domain *domain = v;
@@ -360,12 +359,12 @@ static int check_domain(const void *k, void *v, void *arg)
return 0;
}
if (dom_valid) {
- if ((dominfo.crashed || dominfo.shutdown)
+ if ((dominfo.flags & XEN_DOMINF_shutdown)
&& !domain->shutdown) {
domain->shutdown = true;
*notify = true;
}
- if (!dominfo.dying)
+ if (!(dominfo.flags & XEN_DOMINF_dying))
return 0;
}
if (domain->conn) {
@@ -486,7 +485,7 @@ static struct domain *find_or_alloc_domain(const void *ctx, unsigned int domid)
static struct domain *find_or_alloc_existing_domain(unsigned int domid)
{
struct domain *domain;
- xc_dominfo_t dominfo;
+ xc_domaininfo_t dominfo;
domain = find_domain_struct(domid);
if (!domain && get_domain_info(domid, &dominfo))
@@ -1010,7 +1009,7 @@ int domain_alloc_permrefs(struct node_perms *perms)
{
unsigned int i, domid;
struct domain *d;
- xc_dominfo_t dominfo;
+ xc_domaininfo_t dominfo;
for (i = 0; i < perms->num; i++) {
domid = perms->p[i].id;
diff --git a/tools/xentrace/xenctx.c b/tools/xentrace/xenctx.c
index 85ba0c0fa6..9acb9db460 100644
--- a/tools/xentrace/xenctx.c
+++ b/tools/xentrace/xenctx.c
@@ -92,7 +92,7 @@ static struct xenctx {
int do_stack;
#endif
int kernel_start_set;
- xc_dominfo_t dominfo;
+ xc_domaininfo_t dominfo;
} xenctx;
struct symbol {
@@ -989,7 +989,7 @@ static void dump_ctx(int vcpu)
#if defined(__i386__) || defined(__x86_64__)
{
- if (xenctx.dominfo.hvm) {
+ if (xenctx.dominfo.flags & XEN_DOMINF_hvm_guest) {
struct hvm_hw_cpu cpuctx;
xen_capabilities_info_t xen_caps = "";
if (xc_domain_hvm_getcontext_partial(
@@ -1269,9 +1269,9 @@ int main(int argc, char **argv)
exit(-1);
}
- ret = xc_domain_getinfo(xenctx.xc_handle, xenctx.domid, 1, &xenctx.dominfo);
+ ret = xc_domain_getinfo_single(xenctx.xc_handle, xenctx.domid, &xenctx.dominfo);
if (ret < 0) {
- perror("xc_domain_getinfo");
+ perror("xc_domain_getinfo_single");
exit(-1);
}
--
2.34.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 7/7] domctl: Modify XEN_DOMCTL_getdomaininfo to fail if domid is not found
2023-04-28 10:41 [PATCH v2 0/7] Rationalize usage of xc_domain_getinfo{,list}() Alejandro Vallejo
` (5 preceding siblings ...)
2023-04-28 10:41 ` [PATCH v2 6/7] tools: Use new xc function for some xc_domain_getinfo() calls Alejandro Vallejo
@ 2023-04-28 10:41 ` Alejandro Vallejo
2023-04-28 14:05 ` Andrew Cooper
6 siblings, 1 reply; 20+ messages in thread
From: Alejandro Vallejo @ 2023-04-28 10:41 UTC (permalink / raw)
To: Xen-devel
Cc: Alejandro Vallejo, Andrew Cooper, George Dunlap, Jan Beulich,
Julien Grall, Stefano Stabellini, Wei Liu, Anthony PERARD,
Juergen Gross
It previously mimicked the getdomaininfo sysctl semantics by returning
the first domid higher than the requested domid that does exist. This
unintuitive behaviour causes quite a few mistakes and makes the call
needlessly slow in its error path.
This patch removes the fallback search, returning -ESRCH if the requested
domain doesn't exist. Domain discovery can still be done through the sysctl
interface as that performs a linear search on the list of domains.
With this modification the xc_domain_getinfo() function is deprecated and
removed to make sure it's not mistakenly used expecting the old behaviour.
The new xc wrapper is xc_domain_getinfo_single().
All previous callers of xc_domain_getinfo() have been updated to use
xc_domain_getinfo_single() or xc_domain_getinfolist() instead. This also
means xc_dominfo_t is no longer used by anything and can be purged.
Resolves: xen-project/xen#105
Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com>
---
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <george.dunlap@citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Julien Grall <julien@xen.org>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Wei Liu <wl@xen.org>
Cc: Anthony PERARD <anthony.perard@citrix.com>
Cc: Juergen Gross <jgross@suse.com>
---
tools/include/xenctrl.h | 43 --------------------
tools/libs/ctrl/xc_domain.c | 72 ----------------------------------
tools/libs/guest/xg_dom_boot.c | 2 +-
xen/common/domctl.c | 32 +--------------
4 files changed, 3 insertions(+), 146 deletions(-)
diff --git a/tools/include/xenctrl.h b/tools/include/xenctrl.h
index 685df1c7ba..04d33db305 100644
--- a/tools/include/xenctrl.h
+++ b/tools/include/xenctrl.h
@@ -444,28 +444,6 @@ typedef struct xc_core_header {
* DOMAIN MANAGEMENT FUNCTIONS
*/
-typedef struct xc_dominfo {
- uint32_t domid;
- uint32_t ssidref;
- unsigned int dying:1, crashed:1, shutdown:1,
- paused:1, blocked:1, running:1,
- hvm:1, debugged:1, xenstore:1, hap:1;
- unsigned int shutdown_reason; /* only meaningful if shutdown==1 */
- unsigned long nr_pages; /* current number, not maximum */
- unsigned long nr_outstanding_pages;
- unsigned long nr_shared_pages;
- unsigned long nr_paged_pages;
- unsigned long shared_info_frame;
- uint64_t cpu_time;
- unsigned long max_memkb;
- unsigned int nr_online_vcpus;
- unsigned int max_vcpu_id;
- xen_domain_handle_t handle;
- unsigned int cpupool;
- uint8_t gpaddr_bits;
- struct xen_arch_domainconfig arch_config;
-} xc_dominfo_t;
-
typedef xen_domctl_getdomaininfo_t xc_domaininfo_t;
static inline unsigned int dominfo_shutdown_reason(const xc_domaininfo_t *info)
@@ -720,27 +698,6 @@ int xc_domain_getinfo_single(xc_interface *xch,
uint32_t domid,
xc_domaininfo_t *info);
-/**
- * This function will return information about one or more domains. It is
- * designed to iterate over the list of domains. If a single domain is
- * requested, this function will return the next domain in the list - if
- * one exists. It is, therefore, important in this case to make sure the
- * domain requested was the one returned.
- *
- * @parm xch a handle to an open hypervisor interface
- * @parm first_domid the first domain to enumerate information from. Domains
- * are currently enumerate in order of creation.
- * @parm max_doms the number of elements in info
- * @parm info an array of max_doms size that will contain the information for
- * the enumerated domains.
- * @return the number of domains enumerated or -1 on error
- */
-int xc_domain_getinfo(xc_interface *xch,
- uint32_t first_domid,
- unsigned int max_doms,
- xc_dominfo_t *info);
-
-
/**
* This function will set the execution context for the specified vcpu.
*
diff --git a/tools/libs/ctrl/xc_domain.c b/tools/libs/ctrl/xc_domain.c
index 533e3c1314..724fa6f753 100644
--- a/tools/libs/ctrl/xc_domain.c
+++ b/tools/libs/ctrl/xc_domain.c
@@ -357,84 +357,12 @@ int xc_domain_getinfo_single(xc_interface *xch,
if ( do_domctl(xch, &domctl) < 0 )
return -1;
- if ( domctl.u.getdomaininfo.domain != domid ) {
- errno = ESRCH;
- return -1;
- }
-
if ( info )
*info = domctl.u.getdomaininfo;
return 0;
}
-int xc_domain_getinfo(xc_interface *xch,
- uint32_t first_domid,
- unsigned int max_doms,
- xc_dominfo_t *info)
-{
- unsigned int nr_doms;
- uint32_t next_domid = first_domid;
- DECLARE_DOMCTL;
- int rc = 0;
-
- memset(info, 0, max_doms*sizeof(xc_dominfo_t));
-
- for ( nr_doms = 0; nr_doms < max_doms; nr_doms++ )
- {
- domctl.cmd = XEN_DOMCTL_getdomaininfo;
- domctl.domain = next_domid;
- if ( (rc = do_domctl(xch, &domctl)) < 0 )
- break;
- info->domid = domctl.domain;
-
- info->dying = !!(domctl.u.getdomaininfo.flags&XEN_DOMINF_dying);
- info->shutdown = !!(domctl.u.getdomaininfo.flags&XEN_DOMINF_shutdown);
- info->paused = !!(domctl.u.getdomaininfo.flags&XEN_DOMINF_paused);
- info->blocked = !!(domctl.u.getdomaininfo.flags&XEN_DOMINF_blocked);
- info->running = !!(domctl.u.getdomaininfo.flags&XEN_DOMINF_running);
- info->hvm = !!(domctl.u.getdomaininfo.flags&XEN_DOMINF_hvm_guest);
- info->debugged = !!(domctl.u.getdomaininfo.flags&XEN_DOMINF_debugged);
- info->xenstore = !!(domctl.u.getdomaininfo.flags&XEN_DOMINF_xs_domain);
- info->hap = !!(domctl.u.getdomaininfo.flags&XEN_DOMINF_hap);
-
- info->shutdown_reason =
- (domctl.u.getdomaininfo.flags>>XEN_DOMINF_shutdownshift) &
- XEN_DOMINF_shutdownmask;
-
- if ( info->shutdown && (info->shutdown_reason == SHUTDOWN_crash) )
- {
- info->shutdown = 0;
- info->crashed = 1;
- }
-
- info->ssidref = domctl.u.getdomaininfo.ssidref;
- info->nr_pages = domctl.u.getdomaininfo.tot_pages;
- info->nr_outstanding_pages = domctl.u.getdomaininfo.outstanding_pages;
- info->nr_shared_pages = domctl.u.getdomaininfo.shr_pages;
- info->nr_paged_pages = domctl.u.getdomaininfo.paged_pages;
- info->max_memkb = domctl.u.getdomaininfo.max_pages << (PAGE_SHIFT-10);
- info->shared_info_frame = domctl.u.getdomaininfo.shared_info_frame;
- info->cpu_time = domctl.u.getdomaininfo.cpu_time;
- info->nr_online_vcpus = domctl.u.getdomaininfo.nr_online_vcpus;
- info->max_vcpu_id = domctl.u.getdomaininfo.max_vcpu_id;
- info->cpupool = domctl.u.getdomaininfo.cpupool;
- info->gpaddr_bits = domctl.u.getdomaininfo.gpaddr_bits;
- info->arch_config = domctl.u.getdomaininfo.arch_config;
-
- memcpy(info->handle, domctl.u.getdomaininfo.handle,
- sizeof(xen_domain_handle_t));
-
- next_domid = (uint16_t)domctl.domain + 1;
- info++;
- }
-
- if ( nr_doms == 0 )
- return rc;
-
- return nr_doms;
-}
-
int xc_domain_getinfolist(xc_interface *xch,
uint32_t first_domain,
unsigned int max_domains,
diff --git a/tools/libs/guest/xg_dom_boot.c b/tools/libs/guest/xg_dom_boot.c
index 1dea534bba..dc858a1567 100644
--- a/tools/libs/guest/xg_dom_boot.c
+++ b/tools/libs/guest/xg_dom_boot.c
@@ -178,7 +178,7 @@ int xc_dom_boot_image(struct xc_dom_image *dom)
{
xc_dom_panic(dom->xch, XC_INTERNAL_ERROR,
"%s: getdomaininfo failed (errno=%d)",
- __FUNCTION__, rc, errno);
+ __FUNCTION__, errno);
return -1;
}
dom->shared_info_mfn = info.shared_info_frame;
diff --git a/xen/common/domctl.c b/xen/common/domctl.c
index ad71ad8a4c..24a14996e6 100644
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -314,7 +314,7 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
/* fall through */
default:
d = rcu_lock_domain_by_id(op->domain);
- if ( !d && op->cmd != XEN_DOMCTL_getdomaininfo )
+ if ( !d )
return -ESRCH;
}
@@ -534,42 +534,14 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
case XEN_DOMCTL_getdomaininfo:
{
- domid_t dom = DOMID_INVALID;
-
- if ( !d )
- {
- ret = -EINVAL;
- if ( op->domain >= DOMID_FIRST_RESERVED )
- break;
-
- rcu_read_lock(&domlist_read_lock);
-
- dom = op->domain;
- for_each_domain ( d )
- if ( d->domain_id >= dom )
- break;
- }
-
- ret = -ESRCH;
- if ( d == NULL )
- goto getdomaininfo_out;
-
ret = xsm_getdomaininfo(XSM_HOOK, d);
if ( ret )
- goto getdomaininfo_out;
+ break;
getdomaininfo(d, &op->u.getdomaininfo);
op->domain = op->u.getdomaininfo.domain;
copyback = 1;
-
- getdomaininfo_out:
- /* When d was non-NULL upon entry, no cleanup is needed. */
- if ( dom == DOMID_INVALID )
- break;
-
- rcu_read_unlock(&domlist_read_lock);
- d = NULL;
break;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH v2 1/7] tools: Make some callers of xc_domain_getinfo() use xc_domain_getinfolist()
2023-04-28 10:41 ` [PATCH v2 1/7] tools: Make some callers of xc_domain_getinfo() use xc_domain_getinfolist() Alejandro Vallejo
@ 2023-04-28 12:19 ` Marek Marczykowski-Górecki
2023-04-28 12:21 ` Andrew Cooper
1 sibling, 0 replies; 20+ messages in thread
From: Marek Marczykowski-Górecki @ 2023-04-28 12:19 UTC (permalink / raw)
To: Alejandro Vallejo
Cc: Xen-devel, Andrew Cooper, Wei Liu, Anthony PERARD, Juergen Gross
[-- Attachment #1: Type: text/plain, Size: 6441 bytes --]
On Fri, Apr 28, 2023 at 11:41:18AM +0100, Alejandro Vallejo wrote:
> 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>
For the Python part:
Acked-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.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 | 12 ++++++++++++
> tools/python/xen/lowlevel/xc/xc.c | 28 ++++++++++++++--------------
> tools/xenmon/xenbaked.c | 6 +++---
> 3 files changed, 29 insertions(+), 17 deletions(-)
>
> diff --git a/tools/include/xenctrl.h b/tools/include/xenctrl.h
> index 05967ecc92..f5bc7f58b6 100644
> --- a/tools/include/xenctrl.h
> +++ b/tools/include/xenctrl.h
> @@ -468,6 +468,18 @@ 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;
> +}
> +
> +static inline bool dominfo_shutdown_with(xc_domaininfo_t *info, unsigned int expected_reason)
> +{
> + /* The reason doesn't make sense unless the domain is actually shutdown */
> + return (info->flags & XEN_DOMINF_shutdown) &&
> + (dominfo_shutdown_reason(info) == expected_reason);
> +}
> +
> 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..d7ce299650 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,21 @@ 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", dominfo_shutdown_with(&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.1
>
>
--
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 1/7] tools: Make some callers of xc_domain_getinfo() use xc_domain_getinfolist()
2023-04-28 10:41 ` [PATCH v2 1/7] tools: Make some callers of xc_domain_getinfo() use xc_domain_getinfolist() Alejandro Vallejo
2023-04-28 12:19 ` Marek Marczykowski-Górecki
@ 2023-04-28 12:21 ` Andrew Cooper
1 sibling, 0 replies; 20+ messages in thread
From: Andrew Cooper @ 2023-04-28 12:21 UTC (permalink / raw)
To: Alejandro Vallejo, Xen-devel; +Cc: Wei Liu, Anthony PERARD, Juergen Gross
On 28/04/2023 11:41 am, Alejandro Vallejo wrote:
> 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>
FYI, expected practice is to put your v2 notes in each patch, here in
the commit message. They're more accessible here than in the cover letter.
However, this patch is now fine, so Reviewed-by: Andrew Cooper
<andrew.cooper3@citrix.com>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 2/7] tools: Create xc_domain_getinfo_single()
2023-04-28 10:41 ` [PATCH v2 2/7] tools: Create xc_domain_getinfo_single() Alejandro Vallejo
@ 2023-04-28 12:23 ` Andrew Cooper
0 siblings, 0 replies; 20+ messages in thread
From: Andrew Cooper @ 2023-04-28 12:23 UTC (permalink / raw)
To: Alejandro Vallejo, Xen-devel; +Cc: Wei Liu, Anthony PERARD, Juergen Gross
On 28/04/2023 11:41 am, Alejandro Vallejo wrote:
> diff --git a/tools/libs/ctrl/xc_domain.c b/tools/libs/ctrl/xc_domain.c
> index e939d07157..6b11775d4c 100644
> --- a/tools/libs/ctrl/xc_domain.c
> +++ b/tools/libs/ctrl/xc_domain.c
> @@ -345,6 +345,29 @@ int xc_dom_vuart_init(xc_interface *xch,
> return rc;
> }
>
> +int xc_domain_getinfo_single(xc_interface *xch,
> + uint32_t domid,
> + xc_domaininfo_t *info)
> +{
> + struct xen_domctl domctl = {
> + .cmd = XEN_DOMCTL_getdomaininfo,
> + .domain = domid,
> + };
> +
> + if ( do_domctl(xch, &domctl) < 0 )
> + return -1;
> +
> + if ( domctl.u.getdomaininfo.domain != domid ) {
One tiny style issue. This brace should be on the next line.
I'll fix on commit.
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 3/7] tools: Refactor console/io.c to avoid using xc_domain_getinfo()
2023-04-28 10:41 ` [PATCH v2 3/7] tools: Refactor console/io.c to avoid using xc_domain_getinfo() Alejandro Vallejo
@ 2023-04-28 12:33 ` Andrew Cooper
2023-04-28 12:43 ` Andrew Cooper
2023-04-28 12:58 ` Alejandro Vallejo
0 siblings, 2 replies; 20+ messages in thread
From: Andrew Cooper @ 2023-04-28 12:33 UTC (permalink / raw)
To: Alejandro Vallejo, Xen-devel; +Cc: Wei Liu, Anthony PERARD
On 28/04/2023 11:41 am, Alejandro Vallejo wrote:
> It has 2 avoidable occurences
>
> * Check whether a domain is valid, which can be done faster with
> xc_domain_getinfo_single()
> * Domain discovery, which can be done much faster with the sysctl
> interface through xc_domain_getinfolist().
It occurs to me that this isn't really right here.
It's true in principle, but switching to requesting all domains at once
is a fix for a race condition.
I'd suggest "which can be done in a race free way through ..." and avoid
saying faster. It's likely not faster now with the 4M bounce, but we
can fix that in due course.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 4/7] tools: Make init-xenstore-domain use xc_domain_getinfolist()
2023-04-28 10:41 ` [PATCH v2 4/7] tools: Make init-xenstore-domain use xc_domain_getinfolist() Alejandro Vallejo
@ 2023-04-28 12:40 ` Andrew Cooper
2023-04-28 12:45 ` Alejandro Vallejo
2023-04-28 13:10 ` Andrew Cooper
1 sibling, 1 reply; 20+ messages in thread
From: Andrew Cooper @ 2023-04-28 12:40 UTC (permalink / raw)
To: Alejandro Vallejo, Xen-devel; +Cc: Wei Liu, Juergen Gross
On 28/04/2023 11:41 am, Alejandro Vallejo wrote:
> diff --git a/tools/helpers/init-xenstore-domain.c b/tools/helpers/init-xenstore-domain.c
> index 0950ba7dc5..e210a2677e 100644
> --- a/tools/helpers/init-xenstore-domain.c
> +++ b/tools/helpers/init-xenstore-domain.c
> @@ -322,16 +323,19 @@ err:
>
> static int check_domain(xc_interface *xch)
> {
> - xc_dominfo_t info;
> + xc_domaininfo_t info[8];
I'm recommend having a comment here, saying something like /* Commonly
dom0 is the only domain, but buffer a little for efficiency. */
Because this is also the justification for why we don't need to ask for
32k domains at once to find XEN_DOMINF_xs_domain in a race-free way.
Can be fixed on commit if you're happy with the adjustment.
~Andrew
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 3/7] tools: Refactor console/io.c to avoid using xc_domain_getinfo()
2023-04-28 12:33 ` Andrew Cooper
@ 2023-04-28 12:43 ` Andrew Cooper
2023-04-28 12:58 ` Alejandro Vallejo
1 sibling, 0 replies; 20+ messages in thread
From: Andrew Cooper @ 2023-04-28 12:43 UTC (permalink / raw)
To: Alejandro Vallejo, Xen-devel; +Cc: Wei Liu, Anthony PERARD
On 28/04/2023 1:33 pm, Andrew Cooper wrote:
> On 28/04/2023 11:41 am, Alejandro Vallejo wrote:
>> It has 2 avoidable occurences
>>
>> * Check whether a domain is valid, which can be done faster with
>> xc_domain_getinfo_single()
>> * Domain discovery, which can be done much faster with the sysctl
>> interface through xc_domain_getinfolist().
> It occurs to me that this isn't really right here.
>
> It's true in principle, but switching to requesting all domains at once
> is a fix for a race condition.
>
> I'd suggest "which can be done in a race free way through ..." and avoid
> saying faster. It's likely not faster now with the 4M bounce, but we
> can fix that in due course.
Oh, there's also one tabs/spaces indentation hiccup. That can be fixed
on commit too.
~Andrew
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 4/7] tools: Make init-xenstore-domain use xc_domain_getinfolist()
2023-04-28 12:40 ` Andrew Cooper
@ 2023-04-28 12:45 ` Alejandro Vallejo
0 siblings, 0 replies; 20+ messages in thread
From: Alejandro Vallejo @ 2023-04-28 12:45 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Xen-devel, Wei Liu, Juergen Gross
Sounds good to me.
Cheers,
Alejandro
On Fri, Apr 28, 2023 at 01:40:50PM +0100, Andrew Cooper wrote:
> I'm recommend having a comment here, saying something like /* Commonly
> dom0 is the only domain, but buffer a little for efficiency. */
>
> Because this is also the justification for why we don't need to ask for
> 32k domains at once to find XEN_DOMINF_xs_domain in a race-free way.
>
> Can be fixed on commit if you're happy with the adjustment.
>
> ~Andrew
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 3/7] tools: Refactor console/io.c to avoid using xc_domain_getinfo()
2023-04-28 12:33 ` Andrew Cooper
2023-04-28 12:43 ` Andrew Cooper
@ 2023-04-28 12:58 ` Alejandro Vallejo
1 sibling, 0 replies; 20+ messages in thread
From: Alejandro Vallejo @ 2023-04-28 12:58 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Xen-devel, Wei Liu, Anthony PERARD
On Fri, Apr 28, 2023 at 01:33:45PM +0100, Andrew Cooper wrote:
> On 28/04/2023 11:41 am, Alejandro Vallejo wrote:
> > It has 2 avoidable occurences
> >
> > * Check whether a domain is valid, which can be done faster with
> > xc_domain_getinfo_single()
> > * Domain discovery, which can be done much faster with the sysctl
> > interface through xc_domain_getinfolist().
>
> It occurs to me that this isn't really right here.
>
> It's true in principle, but switching to requesting all domains at once
> is a fix for a race condition.
>
> I'd suggest "which can be done in a race free way through ..." and avoid
> saying faster. It's likely not faster now with the 4M bounce, but we
> can fix that in due course.
I agree, yes.
Alejandro
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 4/7] tools: Make init-xenstore-domain use xc_domain_getinfolist()
2023-04-28 10:41 ` [PATCH v2 4/7] tools: Make init-xenstore-domain use xc_domain_getinfolist() Alejandro Vallejo
2023-04-28 12:40 ` Andrew Cooper
@ 2023-04-28 13:10 ` Andrew Cooper
1 sibling, 0 replies; 20+ messages in thread
From: Andrew Cooper @ 2023-04-28 13:10 UTC (permalink / raw)
To: Alejandro Vallejo, Xen-devel; +Cc: Wei Liu, Juergen Gross
On 28/04/2023 11:41 am, Alejandro Vallejo wrote:
> It currently relies on xc_domain_getinfo() returning the next available
> domain past "first_domid", which is a feature that will disappear in a
> future patch.
>
> Furthermore and while at it, make it so the hypercall tries to fetch information
> about more than one domain per hypercall so we can (hopefully) get away with a
> single hypercall in a typical system.
>
> Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com>
> ---
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> Cc: Wei Liu <wl@xen.org>
> Cc: Juergen Gross <jgross@suse.org>
Oh, also, you should have retained the Reviewed-by: that Juergen gave
you on v1, seeing as you did precisely what he asked.
Same for my R-by on patch 7, except there's a different hiccup there now...
~Andrew
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 7/7] domctl: Modify XEN_DOMCTL_getdomaininfo to fail if domid is not found
2023-04-28 10:41 ` [PATCH v2 7/7] domctl: Modify XEN_DOMCTL_getdomaininfo to fail if domid is not found Alejandro Vallejo
@ 2023-04-28 14:05 ` Andrew Cooper
0 siblings, 0 replies; 20+ messages in thread
From: Andrew Cooper @ 2023-04-28 14:05 UTC (permalink / raw)
To: Alejandro Vallejo, Xen-devel
Cc: George Dunlap, Jan Beulich, Julien Grall, Stefano Stabellini,
Wei Liu, Anthony PERARD, Juergen Gross
On 28/04/2023 11:41 am, Alejandro Vallejo wrote:
> It previously mimicked the getdomaininfo sysctl semantics by returning
> the first domid higher than the requested domid that does exist. This
> unintuitive behaviour causes quite a few mistakes and makes the call
> needlessly slow in its error path.
>
> This patch removes the fallback search, returning -ESRCH if the requested
> domain doesn't exist. Domain discovery can still be done through the sysctl
> interface as that performs a linear search on the list of domains.
>
> With this modification the xc_domain_getinfo() function is deprecated and
> removed to make sure it's not mistakenly used expecting the old behaviour.
> The new xc wrapper is xc_domain_getinfo_single().
>
> All previous callers of xc_domain_getinfo() have been updated to use
> xc_domain_getinfo_single() or xc_domain_getinfolist() instead. This also
> means xc_dominfo_t is no longer used by anything and can be purged.
>
> Resolves: xen-project/xen#105
> Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com>
You haven't (in theory) changed this patch, so should have retained my
R-by from v1, except...
> diff --git a/tools/libs/guest/xg_dom_boot.c b/tools/libs/guest/xg_dom_boot.c
> index 1dea534bba..dc858a1567 100644
> --- a/tools/libs/guest/xg_dom_boot.c
> +++ b/tools/libs/guest/xg_dom_boot.c
> @@ -178,7 +178,7 @@ int xc_dom_boot_image(struct xc_dom_image *dom)
> {
> xc_dom_panic(dom->xch, XC_INTERNAL_ERROR,
> "%s: getdomaininfo failed (errno=%d)",
> - __FUNCTION__, rc, errno);
> + __FUNCTION__, errno);
> return -1;
> }
> dom->shared_info_mfn = info.shared_info_frame;
... this hunk means the patch 6 build is broken.
~Andrew
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 5/7] tools: Modify single-domid callers of xc_domain_getinfolist()
2023-04-28 10:41 ` [PATCH v2 5/7] tools: Modify single-domid callers of xc_domain_getinfolist() Alejandro Vallejo
@ 2023-04-28 15:22 ` Andrew Cooper
0 siblings, 0 replies; 20+ messages in thread
From: Andrew Cooper @ 2023-04-28 15:22 UTC (permalink / raw)
To: Alejandro Vallejo, Xen-devel; +Cc: Wei Liu, Anthony PERARD, Juergen Gross
On 28/04/2023 11:41 am, Alejandro Vallejo wrote:
> xc_domain_getinfolist() internally relies on a sysctl that performs
> a linear search for the domids. Many callers of xc_domain_getinfolist()
> who require information about a precise domid are much better off calling
> xc_domain_getinfo_single() instead, that will use the getdomaininfo domctl
> instead and ensure the returned domid matches the requested one. The domtctl
> will find the domid faster too, because that uses hashed lists.
>
> 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/libs/light/libxl_dom.c | 15 +++++----------
> tools/libs/light/libxl_dom_suspend.c | 7 +------
> tools/libs/light/libxl_domain.c | 13 +++++--------
> tools/libs/light/libxl_mem.c | 4 ++--
> tools/libs/light/libxl_sched.c | 12 ++++--------
> tools/xenpaging/xenpaging.c | 10 +++++-----
> 6 files changed, 22 insertions(+), 39 deletions(-)
>
> diff --git a/tools/libs/light/libxl_dom.c b/tools/libs/light/libxl_dom.c
> index 25fb716084..bd5d823581 100644
> --- a/tools/libs/light/libxl_dom.c
> +++ b/tools/libs/light/libxl_dom.c
> @@ -32,8 +32,8 @@ libxl_domain_type libxl__domain_type(libxl__gc *gc, uint32_t domid)
> xc_domaininfo_t info;
> int ret;
>
> - ret = xc_domain_getinfolist(ctx->xch, domid, 1, &info);
> - if (ret != 1 || info.domain != domid) {
> + ret = xc_domain_getinfo_single(ctx->xch, domid, &info);
> + if (ret < 0) {
> LOG(ERROR, "unable to get domain type for domid=%"PRIu32, domid);
I think this LOG() would benefit from turning into a LOGED() like the
others.
Otherwise, everything LGTM. I'm happy to adjust on commit.
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 6/7] tools: Use new xc function for some xc_domain_getinfo() calls
2023-04-28 10:41 ` [PATCH v2 6/7] tools: Use new xc function for some xc_domain_getinfo() calls Alejandro Vallejo
@ 2023-04-28 16:29 ` Andrew Cooper
0 siblings, 0 replies; 20+ messages in thread
From: Andrew Cooper @ 2023-04-28 16:29 UTC (permalink / raw)
To: Alejandro Vallejo, Xen-devel
Cc: Wei Liu, Anthony PERARD, Tim Deegan, George Dunlap, Juergen Gross
On 28/04/2023 11:41 am, Alejandro Vallejo wrote:
> diff --git a/tools/libs/ctrl/xc_domain.c b/tools/libs/ctrl/xc_domain.c
> index 6b11775d4c..533e3c1314 100644
> --- a/tools/libs/ctrl/xc_domain.c
> +++ b/tools/libs/ctrl/xc_domain.c
> @@ -1959,15 +1959,14 @@ int xc_domain_memory_mapping(
> uint32_t add_mapping)
> {
> DECLARE_DOMCTL;
> - xc_dominfo_t info;
> + xc_domaininfo_t info;
> int ret = 0, rc;
> unsigned long done = 0, nr, max_batch_sz;
>
> - if ( xc_domain_getinfo(xch, domid, 1, &info) != 1 ||
> - info.domid != domid )
> + if ( xc_domain_getinfo_single(xch, domid, &info) < 0 )
> {
> - PERROR("Could not get info for domain");
> - return -EINVAL;
> + PERROR("Could not get info for dom%u", domid);
> + return -1;
I think this needs to be "return -errno" to have the same semantics as
before.
> diff --git a/tools/libs/ctrl/xc_private.c b/tools/libs/ctrl/xc_private.c
> index 2f99a7d2cf..8dcebad401 100644
> --- a/tools/libs/ctrl/xc_private.c
> +++ b/tools/libs/ctrl/xc_private.c
> @@ -441,11 +441,10 @@ int xc_machphys_mfn_list(xc_interface *xch,
>
> long xc_get_tot_pages(xc_interface *xch, uint32_t domid)
> {
> - xc_dominfo_t info;
> - if ( (xc_domain_getinfo(xch, domid, 1, &info) != 1) ||
> - (info.domid != domid) )
> + xc_domaininfo_t info;
> + if ( xc_domain_getinfo_single(xch, domid, &info) < 0 )
> return -1;
> - return info.nr_pages;
> + return info.tot_pages;
As we're modifying every line in the function, take the opportunity to
add two extra blank lines to improve readability.
> diff --git a/tools/libs/guest/xg_cpuid_x86.c b/tools/libs/guest/xg_cpuid_x86.c
> index bd16a87e48..1519b5d556 100644
> --- a/tools/libs/guest/xg_cpuid_x86.c
> +++ b/tools/libs/guest/xg_cpuid_x86.c
> @@ -281,7 +281,8 @@ static int xc_cpuid_xend_policy(
> xc_interface *xch, uint32_t domid, const struct xc_xend_cpuid *xend)
> {
> int rc;
> - xc_dominfo_t di;
> + bool hvm;
> + xc_domaininfo_t di;
> unsigned int nr_leaves, nr_msrs;
> uint32_t err_leaf = -1, err_subleaf = -1, err_msr = -1;
> /*
> @@ -291,13 +292,12 @@ static int xc_cpuid_xend_policy(
> xen_cpuid_leaf_t *host = NULL, *def = NULL, *cur = NULL;
> unsigned int nr_host, nr_def, nr_cur;
>
> - if ( xc_domain_getinfo(xch, domid, 1, &di) != 1 ||
> - di.domid != domid )
> + if ( (rc = xc_domain_getinfo_single(xch, domid, &di)) < 0 )
> {
> - ERROR("Failed to obtain d%d info", domid);
> - rc = -ESRCH;
> + PERROR("Failed to obtain d%d info", domid);
> goto fail;
Sorry, I gave you bad advice last time around. We need rc = -errno to
maintain behaviour here, and that is a pattern used out of context too.
Same in related hunks too.
> @@ -330,12 +330,12 @@ static int xc_cpuid_xend_policy(
> /* Get the domain type's default policy. */
> nr_msrs = 0;
> nr_def = nr_leaves;
> - rc = get_system_cpu_policy(xch, di.hvm ? XEN_SYSCTL_cpu_policy_hvm_default
> + rc = get_system_cpu_policy(xch, hvm ? XEN_SYSCTL_cpu_policy_hvm_default
> : XEN_SYSCTL_cpu_policy_pv_default,
We like to keep the ? and : vertically aligned if possible.
> diff --git a/tools/libs/guest/xg_dom_boot.c b/tools/libs/guest/xg_dom_boot.c
> index 263a3f4c85..1dea534bba 100644
> --- a/tools/libs/guest/xg_dom_boot.c
> +++ b/tools/libs/guest/xg_dom_boot.c
> @@ -174,19 +174,11 @@ int xc_dom_boot_image(struct xc_dom_image *dom)
> return rc;
>
> /* collect some info */
> - rc = xc_domain_getinfo(dom->xch, dom->guest_domid, 1, &info);
> - if ( rc < 0 )
> - {
> - xc_dom_panic(dom->xch, XC_INTERNAL_ERROR,
> - "%s: getdomaininfo failed (rc=%d)", __FUNCTION__, rc);
> - return rc;
> - }
> - if ( rc == 0 || info.domid != dom->guest_domid )
> + if ( xc_domain_getinfo_single(dom->xch, dom->guest_domid, &info) < 0 )
> {
> xc_dom_panic(dom->xch, XC_INTERNAL_ERROR,
> - "%s: Huh? No domains found (nr_domains=%d) "
> - "or domid mismatch (%d != %d)", __FUNCTION__,
> - rc, info.domid, dom->guest_domid);
> + "%s: getdomaininfo failed (errno=%d)",
> + __FUNCTION__, rc, errno);
Ah yes, this is where your stray hunk from patch 7 wants to live.
> diff --git a/tools/libs/guest/xg_sr_restore.c b/tools/libs/guest/xg_sr_restore.c
> index 7314a24cf9..6767c9f5cc 100644
> --- a/tools/libs/guest/xg_sr_restore.c
> +++ b/tools/libs/guest/xg_sr_restore.c
> @@ -887,20 +888,15 @@ int xc_domain_restore(xc_interface *xch, int io_fd, uint32_t dom,
> break;
> }
>
> - if ( xc_domain_getinfo(xch, dom, 1, &ctx.dominfo) != 1 )
> + if ( xc_domain_getinfo_single(xch, dom, &ctx.dominfo) < 0 )
> {
> - PERROR("Failed to get domain info");
> - return -1;
> - }
> -
> - if ( ctx.dominfo.domid != dom )
> - {
> - ERROR("Domain %u does not exist", dom);
> + PERROR("Failed to get info for dom%u", dom);
This is somewhat ambiguous, because "info" could be anything. "dominfo"
would be better.
> diff --git a/tools/libs/guest/xg_sr_save.c b/tools/libs/guest/xg_sr_save.c
> index 9853d8d846..b0b30b4bc2 100644
> --- a/tools/libs/guest/xg_sr_save.c
> +++ b/tools/libs/guest/xg_sr_save.c
> @@ -336,19 +336,17 @@ static int suspend_domain(struct xc_sr_context *ctx)
> }
>
> /* Refresh domain information. */
> - if ( (xc_domain_getinfo(xch, ctx->domid, 1, &ctx->dominfo) != 1) ||
> - (ctx->dominfo.domid != ctx->domid) )
> + if ( xc_domain_getinfo_single(xch, ctx->domid, &ctx->dominfo) < 0 )
> {
> PERROR("Unable to refresh domain information");
> return -1;
> }
>
> /* Confirm the domain has actually been paused. */
> - if ( !ctx->dominfo.shutdown ||
> - (ctx->dominfo.shutdown_reason != SHUTDOWN_suspend) )
> + if ( !dominfo_shutdown_with(&ctx->dominfo, SHUTDOWN_suspend) )
> {
> ERROR("Domain has not been suspended: shutdown %d, reason %d",
> - ctx->dominfo.shutdown, ctx->dominfo.shutdown_reason);
> + ctx->dominfo.flags & XEN_DOMINF_shutdown, dominfo_shutdown_reason(&ctx->dominfo));
This likely wants wrapping onto the next line.
> diff --git a/tools/libs/light/libxl_dom.c b/tools/libs/light/libxl_dom.c
> index bd5d823581..94fef37401 100644
> --- a/tools/libs/light/libxl_dom.c
> +++ b/tools/libs/light/libxl_dom.c
> @@ -34,7 +34,7 @@ libxl_domain_type libxl__domain_type(libxl__gc *gc, uint32_t domid)
>
> ret = xc_domain_getinfo_single(ctx->xch, domid, &info);
> if (ret < 0) {
> - LOG(ERROR, "unable to get domain type for domid=%"PRIu32, domid);
> + LOGED(ERROR, domid, "unable to get dominfo");
> return LIBXL_DOMAIN_TYPE_INVALID;
> }
Ah, this is the answer to my review on patch 5.
Quite a few of the following hunks look like want to be in patch 5 too.
Everything else looks good, best as I can tell.
~Andrew
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2023-04-28 16:29 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-04-28 10:41 [PATCH v2 0/7] Rationalize usage of xc_domain_getinfo{,list}() Alejandro Vallejo
2023-04-28 10:41 ` [PATCH v2 1/7] tools: Make some callers of xc_domain_getinfo() use xc_domain_getinfolist() Alejandro Vallejo
2023-04-28 12:19 ` Marek Marczykowski-Górecki
2023-04-28 12:21 ` Andrew Cooper
2023-04-28 10:41 ` [PATCH v2 2/7] tools: Create xc_domain_getinfo_single() Alejandro Vallejo
2023-04-28 12:23 ` Andrew Cooper
2023-04-28 10:41 ` [PATCH v2 3/7] tools: Refactor console/io.c to avoid using xc_domain_getinfo() Alejandro Vallejo
2023-04-28 12:33 ` Andrew Cooper
2023-04-28 12:43 ` Andrew Cooper
2023-04-28 12:58 ` Alejandro Vallejo
2023-04-28 10:41 ` [PATCH v2 4/7] tools: Make init-xenstore-domain use xc_domain_getinfolist() Alejandro Vallejo
2023-04-28 12:40 ` Andrew Cooper
2023-04-28 12:45 ` Alejandro Vallejo
2023-04-28 13:10 ` Andrew Cooper
2023-04-28 10:41 ` [PATCH v2 5/7] tools: Modify single-domid callers of xc_domain_getinfolist() Alejandro Vallejo
2023-04-28 15:22 ` Andrew Cooper
2023-04-28 10:41 ` [PATCH v2 6/7] tools: Use new xc function for some xc_domain_getinfo() calls Alejandro Vallejo
2023-04-28 16:29 ` Andrew Cooper
2023-04-28 10:41 ` [PATCH v2 7/7] domctl: Modify XEN_DOMCTL_getdomaininfo to fail if domid is not found Alejandro Vallejo
2023-04-28 14:05 ` Andrew Cooper
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.