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

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.

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

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