| Message ID | 20230428104124.1044-5-alejandro.vallejo@cloud.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Rationalize usage of xc_domain_getinfo{,list}() | expand |
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
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
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
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 ) {
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(-)