* [PATCH 0/7] Rationalize usage of xc_domain_getinfo{,list}()
@ 2023-04-26 14:59 Alejandro Vallejo
2023-04-26 14:59 ` [PATCH 1/7] tools: Make some callers of xc_domain_getinfo use xc_domain_getinfolist Alejandro Vallejo
` (7 more replies)
0 siblings, 8 replies; 20+ messages in thread
From: Alejandro Vallejo @ 2023-04-26 14:59 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
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 ammended, 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 the 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 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 | 79 +++++--------------------
tools/libs/ctrl/xc_pagetab.c | 7 +--
tools/libs/ctrl/xc_private.c | 7 +--
tools/libs/ctrl/xc_private.h | 6 +-
tools/libs/guest/xg_core.c | 21 +++----
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 | 28 +++++----
tools/libs/guest/xg_dom_boot.c | 12 +---
tools/libs/guest/xg_domain.c | 6 +-
tools/libs/guest/xg_offline_page.c | 10 ++--
tools/libs/guest/xg_private.h | 1 +
tools/libs/guest/xg_resume.c | 17 +++---
tools/libs/guest/xg_sr_common.h | 2 +-
tools/libs/guest/xg_sr_restore.c | 14 ++---
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 | 15 ++---
tools/libs/light/libxl_dom_suspend.c | 7 +--
tools/libs/light/libxl_domain.c | 12 ++--
tools/libs/light/libxl_mem.c | 4 +-
tools/libs/light/libxl_sched.c | 28 ++++-----
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 | 29 ++++-----
tools/vchan/vchan-socket-proxy.c | 6 +-
tools/xenmon/xenbaked.c | 6 +-
tools/xenpaging/xenpaging.c | 14 ++---
tools/xenstore/xenstored_domain.c | 15 +++--
tools/xentrace/xenctx.c | 8 +--
xen/common/domctl.c | 32 +---------
41 files changed, 245 insertions(+), 374 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 20+ messages in thread* [PATCH 1/7] tools: Make some callers of xc_domain_getinfo use xc_domain_getinfolist 2023-04-26 14:59 [PATCH 0/7] Rationalize usage of xc_domain_getinfo{,list}() Alejandro Vallejo @ 2023-04-26 14:59 ` Alejandro Vallejo 2023-04-27 9:51 ` Andrew Cooper 2023-04-26 14:59 ` [PATCH 2/7] tools: Create xc_domain_getinfo_single() Alejandro Vallejo ` (6 subsequent siblings) 7 siblings, 1 reply; 20+ messages in thread From: Alejandro Vallejo @ 2023-04-26 14:59 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 | 5 +++++ tools/python/xen/lowlevel/xc/xc.c | 29 +++++++++++++++-------------- tools/xenmon/xenbaked.c | 6 +++--- 3 files changed, 23 insertions(+), 17 deletions(-) diff --git a/tools/include/xenctrl.h b/tools/include/xenctrl.h index 05967ecc92..90b33aa3a7 100644 --- a/tools/include/xenctrl.h +++ b/tools/include/xenctrl.h @@ -468,6 +468,11 @@ typedef struct xc_dominfo { typedef xen_domctl_getdomaininfo_t xc_domaininfo_t; +static inline unsigned int dominfo_shutdown_reason(const xc_domaininfo_t *info) +{ + return (info->flags >> XEN_DOMINF_shutdownshift) & XEN_DOMINF_shutdownmask; +} + typedef union { #if defined(__i386__) || defined(__x86_64__) diff --git a/tools/python/xen/lowlevel/xc/xc.c b/tools/python/xen/lowlevel/xc/xc.c index 35901c2d63..38212e8091 100644 --- a/tools/python/xen/lowlevel/xc/xc.c +++ b/tools/python/xen/lowlevel/xc/xc.c @@ -342,7 +342,7 @@ static PyObject *pyxc_domain_getinfo(XcObject *self, uint32_t first_dom = 0; int max_doms = 1024, nr_doms, i; size_t j; - xc_dominfo_t *info; + xc_domaininfo_t *info; static char *kwd_list[] = { "first_dom", "max_doms", NULL }; @@ -350,11 +350,11 @@ static PyObject *pyxc_domain_getinfo(XcObject *self, &first_dom, &max_doms) ) return NULL; - info = calloc(max_doms, sizeof(xc_dominfo_t)); + info = calloc(max_doms, sizeof(*info)); if (info == NULL) return PyErr_NoMemory(); - nr_doms = xc_domain_getinfo(self->xc_handle, first_dom, max_doms, info); + nr_doms = xc_domain_getinfolist(self->xc_handle, first_dom, max_doms, info); if (nr_doms < 0) { @@ -368,21 +368,22 @@ static PyObject *pyxc_domain_getinfo(XcObject *self, info_dict = Py_BuildValue( "{s:i,s:i,s:i,s:i,s:i,s:i,s:i,s:i,s:i,s:i" ",s:L,s:L,s:L,s:i,s:i,s:i}", - "domid", (int)info[i].domid, + "domid", (int)info[i].domain, "online_vcpus", info[i].nr_online_vcpus, "max_vcpu_id", info[i].max_vcpu_id, - "hvm", info[i].hvm, - "dying", info[i].dying, - "crashed", info[i].crashed, - "shutdown", info[i].shutdown, - "paused", info[i].paused, - "blocked", info[i].blocked, - "running", info[i].running, - "mem_kb", (long long)info[i].nr_pages*(XC_PAGE_SIZE/1024), + "hvm", !!(info[i].flags & XEN_DOMINF_hvm_guest), + "dying", !!(info[i].flags & XEN_DOMINF_dying), + "crashed", (info[i].flags & XEN_DOMINF_shutdown) && + (dominfo_shutdown_reason(&info[i]) == SHUTDOWN_crash), + "shutdown", !!(info[i].flags & XEN_DOMINF_shutdown), + "paused", !!(info[i].flags & XEN_DOMINF_paused), + "blocked", !!(info[i].flags & XEN_DOMINF_blocked), + "running", !!(info[i].flags & XEN_DOMINF_running), + "mem_kb", (long long)info[i].tot_pages*(XC_PAGE_SIZE/1024), "cpu_time", (long long)info[i].cpu_time, - "maxmem_kb", (long long)info[i].max_memkb, + "maxmem_kb", (long long)(info[i].max_pages << (XC_PAGE_SHIFT - 10)), "ssidref", (int)info[i].ssidref, - "shutdown_reason", info[i].shutdown_reason, + "shutdown_reason", dominfo_shutdown_reason(&info[i]), "cpupool", (int)info[i].cpupool); pyhandle = PyList_New(sizeof(xen_domain_handle_t)); if ( (pyhandle == NULL) || (info_dict == NULL) ) diff --git a/tools/xenmon/xenbaked.c b/tools/xenmon/xenbaked.c index 4dddbd20e2..8632b10ea4 100644 --- a/tools/xenmon/xenbaked.c +++ b/tools/xenmon/xenbaked.c @@ -775,7 +775,7 @@ static void global_init_domain(int domid, int idx) static int indexof(int domid) { int idx; - xc_dominfo_t dominfo[NDOMAINS]; + xc_domaininfo_t dominfo[NDOMAINS]; xc_interface *xc_handle; int ndomains; @@ -797,7 +797,7 @@ static int indexof(int domid) // call domaininfo hypercall to try and garbage collect unused entries xc_handle = xc_interface_open(0,0,0); - ndomains = xc_domain_getinfo(xc_handle, 0, NDOMAINS, dominfo); + ndomains = xc_domain_getinfolist(xc_handle, 0, NDOMAINS, dominfo); xc_interface_close(xc_handle); // for each domain in our data, look for it in the system dominfo structure @@ -808,7 +808,7 @@ static int indexof(int domid) int jdx; for (jdx=0; jdx<ndomains; jdx++) { - if (dominfo[jdx].domid == domid) + if (dominfo[jdx].domain == domid) break; } if (jdx == ndomains) // we didn't find domid in the dominfo struct -- 2.34.1 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 1/7] tools: Make some callers of xc_domain_getinfo use xc_domain_getinfolist 2023-04-26 14:59 ` [PATCH 1/7] tools: Make some callers of xc_domain_getinfo use xc_domain_getinfolist Alejandro Vallejo @ 2023-04-27 9:51 ` Andrew Cooper 2023-04-27 14:37 ` Alejandro Vallejo 0 siblings, 1 reply; 20+ messages in thread From: Andrew Cooper @ 2023-04-27 9:51 UTC (permalink / raw) To: Alejandro Vallejo, Xen-devel; +Cc: Wei Liu, Anthony PERARD, Juergen Gross Just as a note for the subject, we more commonly write function names with ()'s. On 26/04/2023 3:59 pm, Alejandro Vallejo wrote: > diff --git a/tools/include/xenctrl.h b/tools/include/xenctrl.h > index 05967ecc92..90b33aa3a7 100644 > --- a/tools/include/xenctrl.h > +++ b/tools/include/xenctrl.h > @@ -468,6 +468,11 @@ typedef struct xc_dominfo { > > typedef xen_domctl_getdomaininfo_t xc_domaininfo_t; > > +static inline unsigned int dominfo_shutdown_reason(const xc_domaininfo_t *info) > +{ > + return (info->flags >> XEN_DOMINF_shutdownshift) & XEN_DOMINF_shutdownmask; > +} > + > typedef union > { > #if defined(__i386__) || defined(__x86_64__) > diff --git a/tools/python/xen/lowlevel/xc/xc.c b/tools/python/xen/lowlevel/xc/xc.c > index 35901c2d63..38212e8091 100644 > --- a/tools/python/xen/lowlevel/xc/xc.c > +++ b/tools/python/xen/lowlevel/xc/xc.c > @@ -368,21 +368,22 @@ static PyObject *pyxc_domain_getinfo(XcObject *self, > info_dict = Py_BuildValue( > "{s:i,s:i,s:i,s:i,s:i,s:i,s:i,s:i,s:i,s:i" > ",s:L,s:L,s:L,s:i,s:i,s:i}", > - "domid", (int)info[i].domid, > + "domid", (int)info[i].domain, > "online_vcpus", info[i].nr_online_vcpus, > "max_vcpu_id", info[i].max_vcpu_id, > - "hvm", info[i].hvm, > - "dying", info[i].dying, > - "crashed", info[i].crashed, > - "shutdown", info[i].shutdown, > - "paused", info[i].paused, > - "blocked", info[i].blocked, > - "running", info[i].running, > - "mem_kb", (long long)info[i].nr_pages*(XC_PAGE_SIZE/1024), > + "hvm", !!(info[i].flags & XEN_DOMINF_hvm_guest), > + "dying", !!(info[i].flags & XEN_DOMINF_dying), > + "crashed", (info[i].flags & XEN_DOMINF_shutdown) && > + (dominfo_shutdown_reason(&info[i]) == SHUTDOWN_crash), Isn't this your dominfo_shutdown_with() from patch 6 ? I'd pull that forward to this patch too, and use it here. > + "shutdown", !!(info[i].flags & XEN_DOMINF_shutdown), > + "paused", !!(info[i].flags & XEN_DOMINF_paused), > + "blocked", !!(info[i].flags & XEN_DOMINF_blocked), > + "running", !!(info[i].flags & XEN_DOMINF_running), > + "mem_kb", (long long)info[i].tot_pages*(XC_PAGE_SIZE/1024), > "cpu_time", (long long)info[i].cpu_time, > - "maxmem_kb", (long long)info[i].max_memkb, > + "maxmem_kb", (long long)(info[i].max_pages << (XC_PAGE_SHIFT - 10)), > "ssidref", (int)info[i].ssidref, > - "shutdown_reason", info[i].shutdown_reason, > + "shutdown_reason", dominfo_shutdown_reason(&info[i]), > "cpupool", (int)info[i].cpupool); > pyhandle = PyList_New(sizeof(xen_domain_handle_t)); > if ( (pyhandle == NULL) || (info_dict == NULL) ) > diff --git a/tools/xenmon/xenbaked.c b/tools/xenmon/xenbaked.c > index 4dddbd20e2..8632b10ea4 100644 > --- a/tools/xenmon/xenbaked.c > +++ b/tools/xenmon/xenbaked.c > @@ -775,7 +775,7 @@ static void global_init_domain(int domid, int idx) > static int indexof(int domid) > { > int idx; > - xc_dominfo_t dominfo[NDOMAINS]; > + xc_domaininfo_t dominfo[NDOMAINS]; > xc_interface *xc_handle; > int ndomains; > > @@ -797,7 +797,7 @@ static int indexof(int domid) > > // call domaininfo hypercall to try and garbage collect unused entries > xc_handle = xc_interface_open(0,0,0); > - ndomains = xc_domain_getinfo(xc_handle, 0, NDOMAINS, dominfo); > + ndomains = xc_domain_getinfolist(xc_handle, 0, NDOMAINS, dominfo); > xc_interface_close(xc_handle); Not to do with your patch, but this is logic is mad. xenbaked open and closes a xenctrl handle every time its set of domids changes. I'm very seriously tempted to delete all of tools/xenmon because it shows no signs of being in use, and right now all it does is spit out an unending stream of gotten<100ns in qos_switchout(domid=32767) gotten<100ns in qos_switchout(domid=0) to stdout, which is antisocial for something calling itself a daemon. ~Andrew ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/7] tools: Make some callers of xc_domain_getinfo use xc_domain_getinfolist 2023-04-27 9:51 ` Andrew Cooper @ 2023-04-27 14:37 ` Alejandro Vallejo 0 siblings, 0 replies; 20+ messages in thread From: Alejandro Vallejo @ 2023-04-27 14:37 UTC (permalink / raw) To: Andrew Cooper; +Cc: Xen-devel, Wei Liu, Anthony PERARD, Juergen Gross Answers inlined On Thu, Apr 27, 2023 at 10:51:03AM +0100, Andrew Cooper wrote: > Just as a note for the subject, we more commonly write function names > with ()'s. No harm in abiding by that. Done on v2. > > + "crashed", (info[i].flags & XEN_DOMINF_shutdown) && > > + (dominfo_shutdown_reason(&info[i]) == SHUTDOWN_crash), > > Isn't this your dominfo_shutdown_with() from patch 6 ? > > I'd pull that forward to this patch too, and use it here. It is indeed. Done locally on v2. Cheers, Alejandro ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 2/7] tools: Create xc_domain_getinfo_single() 2023-04-26 14:59 [PATCH 0/7] Rationalize usage of xc_domain_getinfo{,list}() Alejandro Vallejo 2023-04-26 14:59 ` [PATCH 1/7] tools: Make some callers of xc_domain_getinfo use xc_domain_getinfolist Alejandro Vallejo @ 2023-04-26 14:59 ` Alejandro Vallejo 2023-04-27 9:53 ` Andrew Cooper 2023-04-26 14:59 ` [PATCH 3/7] tools: Refactor the console/io.c to avoid using xc_domain_getinfo() Alejandro Vallejo ` (5 subsequent siblings) 7 siblings, 1 reply; 20+ messages in thread From: Alejandro Vallejo @ 2023-04-26 14:59 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 | 22 ++++++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/tools/include/xenctrl.h b/tools/include/xenctrl.h index 90b33aa3a7..73b07955c6 100644 --- a/tools/include/xenctrl.h +++ b/tools/include/xenctrl.h @@ -696,6 +696,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..3ff91023bf 100644 --- a/tools/libs/ctrl/xc_domain.c +++ b/tools/libs/ctrl/xc_domain.c @@ -345,6 +345,28 @@ 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, + }; + + int rc = do_domctl(xch, &domctl); + if (rc < 0) + return rc; + + if (domctl.u.getdomaininfo.domain != domid) + return -ESRCH; + + if (info) + *info = domctl.u.getdomaininfo; + + return rc; +} + 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
* Re: [PATCH 2/7] tools: Create xc_domain_getinfo_single() 2023-04-26 14:59 ` [PATCH 2/7] tools: Create xc_domain_getinfo_single() Alejandro Vallejo @ 2023-04-27 9:53 ` Andrew Cooper 0 siblings, 0 replies; 20+ messages in thread From: Andrew Cooper @ 2023-04-27 9:53 UTC (permalink / raw) To: Alejandro Vallejo, Xen-devel; +Cc: Wei Liu, Anthony PERARD, Juergen Gross On 26/04/2023 3:59 pm, Alejandro Vallejo wrote: > 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 | 22 ++++++++++++++++++++++ > 2 files changed, 38 insertions(+) > > diff --git a/tools/include/xenctrl.h b/tools/include/xenctrl.h > index 90b33aa3a7..73b07955c6 100644 > --- a/tools/include/xenctrl.h > +++ b/tools/include/xenctrl.h > @@ -696,6 +696,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..3ff91023bf 100644 > --- a/tools/libs/ctrl/xc_domain.c > +++ b/tools/libs/ctrl/xc_domain.c > @@ -345,6 +345,28 @@ 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, > + }; > + > + int rc = do_domctl(xch, &domctl); Minor style. Should have a newline here, and drop the one 2 lines up. By and large, this library is mostly Xen style and we're trying to make it more consistent than it is, so we want extra spaces in the if conditions below. Otherwise, LGTM. Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> > + if (rc < 0) > + return rc; > + > + if (domctl.u.getdomaininfo.domain != domid) > + return -ESRCH; > + > + if (info) > + *info = domctl.u.getdomaininfo; > + > + return rc; > +} > + > int xc_domain_getinfo(xc_interface *xch, > uint32_t first_domid, > unsigned int max_doms, ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 3/7] tools: Refactor the console/io.c to avoid using xc_domain_getinfo() 2023-04-26 14:59 [PATCH 0/7] Rationalize usage of xc_domain_getinfo{,list}() Alejandro Vallejo 2023-04-26 14:59 ` [PATCH 1/7] tools: Make some callers of xc_domain_getinfo use xc_domain_getinfolist Alejandro Vallejo 2023-04-26 14:59 ` [PATCH 2/7] tools: Create xc_domain_getinfo_single() Alejandro Vallejo @ 2023-04-26 14:59 ` Alejandro Vallejo 2023-04-27 10:36 ` Andrew Cooper 2023-04-26 14:59 ` [PATCH 4/7] tools: Make init-xenstore-domain use xc_domain_getinfolist() Alejandro Vallejo ` (4 subsequent siblings) 7 siblings, 1 reply; 20+ messages in thread From: Alejandro Vallejo @ 2023-04-26 14:59 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..1fc56f6643 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) @@ -959,26 +953,35 @@ static void shutdown_domain(struct domain *d) static unsigned enum_pass = 0; +/** + * 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]; + static void enum_domains(void) { - int domid = 1; - xc_dominfo_t dominfo; + 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
* Re: [PATCH 3/7] tools: Refactor the console/io.c to avoid using xc_domain_getinfo() 2023-04-26 14:59 ` [PATCH 3/7] tools: Refactor the console/io.c to avoid using xc_domain_getinfo() Alejandro Vallejo @ 2023-04-27 10:36 ` Andrew Cooper 0 siblings, 0 replies; 20+ messages in thread From: Andrew Cooper @ 2023-04-27 10:36 UTC (permalink / raw) To: Alejandro Vallejo, Xen-devel; +Cc: Wei Liu, Anthony PERARD On 26/04/2023 3:59 pm, 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(). > > 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..1fc56f6643 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) > @@ -959,26 +953,35 @@ static void shutdown_domain(struct domain *d) > > static unsigned enum_pass = 0; > > +/** > + * 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]; > + We prefer to reduce scope where possible, and in this case it's fine to have this declared inside enum_domains(). Preferred style for that would be: static void enum_domains(void) { /** * 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; i.e. one blank line between the static and local variable declarations. > static void enum_domains(void) > { > - int domid = 1; > - xc_dominfo_t dominfo; > + 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); This is a correct translation of the prior logic. But it does highlight that xenconsoled currently depends on running in dom0. I bet this is going to be fun bug for someone down the line... Also, while going for 32k entries is absolutely the right thing to do, the entire buffer will be bounced twice to make it hypercall safe. Bordering on 4M of data, I think this is quickly going to become a second improvement to work on. ~Andrew ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 4/7] tools: Make init-xenstore-domain use xc_domain_getinfolist() 2023-04-26 14:59 [PATCH 0/7] Rationalize usage of xc_domain_getinfo{,list}() Alejandro Vallejo ` (2 preceding siblings ...) 2023-04-26 14:59 ` [PATCH 3/7] tools: Refactor the console/io.c to avoid using xc_domain_getinfo() Alejandro Vallejo @ 2023-04-26 14:59 ` Alejandro Vallejo 2023-04-26 15:20 ` Juergen Gross 2023-04-26 14:59 ` [PATCH 5/7] tools: Modify single-domid callers of xc_domain_getinfolist Alejandro Vallejo ` (3 subsequent siblings) 7 siblings, 1 reply; 20+ messages in thread From: Alejandro Vallejo @ 2023-04-26 14:59 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..5f40901d31 100644 --- a/tools/helpers/init-xenstore-domain.c +++ b/tools/helpers/init-xenstore-domain.c @@ -21,6 +21,7 @@ #define LAPIC_BASE_ADDRESS 0xfee00000UL #define MB(x) ((uint64_t)x << 20) #define GB(x) ((uint64_t)x << 30) +#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0])) static uint32_t domid = ~0; static char *kernel; @@ -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
* Re: [PATCH 4/7] tools: Make init-xenstore-domain use xc_domain_getinfolist() 2023-04-26 14:59 ` [PATCH 4/7] tools: Make init-xenstore-domain use xc_domain_getinfolist() Alejandro Vallejo @ 2023-04-26 15:20 ` Juergen Gross 2023-04-27 14:29 ` Alejandro Vallejo 0 siblings, 1 reply; 20+ messages in thread From: Juergen Gross @ 2023-04-26 15:20 UTC (permalink / raw) To: Alejandro Vallejo, Xen-devel; +Cc: Andrew Cooper, Wei Liu, Juergen Gross [-- Attachment #1.1.1: Type: text/plain, Size: 1361 bytes --] On 26.04.23 16:59, 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> > --- > 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..5f40901d31 100644 > --- a/tools/helpers/init-xenstore-domain.c > +++ b/tools/helpers/init-xenstore-domain.c > @@ -21,6 +21,7 @@ > #define LAPIC_BASE_ADDRESS 0xfee00000UL > #define MB(x) ((uint64_t)x << 20) > #define GB(x) ((uint64_t)x << 30) > +#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0])) Please include <xen-tools/common-macros.h> instead of defining ARRAY_SIZE(). With that changed: Reviewed-by: Juergen Gross <jgross@suse.com> Juergen [-- Attachment #1.1.2: OpenPGP public key --] [-- Type: application/pgp-keys, Size: 3149 bytes --] [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 495 bytes --] ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 4/7] tools: Make init-xenstore-domain use xc_domain_getinfolist() 2023-04-26 15:20 ` Juergen Gross @ 2023-04-27 14:29 ` Alejandro Vallejo 0 siblings, 0 replies; 20+ messages in thread From: Alejandro Vallejo @ 2023-04-27 14:29 UTC (permalink / raw) To: Juergen Gross; +Cc: Xen-devel, Andrew Cooper, Wei Liu, Juergen Gross Ah, I didn't notice that header. Sure, added locally on v2. Cheers, Alejandro On Wed, Apr 26, 2023 at 05:20:01PM +0200, Juergen Gross wrote: > Please include <xen-tools/common-macros.h> instead of defining ARRAY_SIZE(). > > With that changed: > > Reviewed-by: Juergen Gross <jgross@suse.com> > > > Juergen ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 5/7] tools: Modify single-domid callers of xc_domain_getinfolist 2023-04-26 14:59 [PATCH 0/7] Rationalize usage of xc_domain_getinfo{,list}() Alejandro Vallejo ` (3 preceding siblings ...) 2023-04-26 14:59 ` [PATCH 4/7] tools: Make init-xenstore-domain use xc_domain_getinfolist() Alejandro Vallejo @ 2023-04-26 14:59 ` Alejandro Vallejo 2023-04-27 11:14 ` Andrew Cooper 2023-04-26 14:59 ` [PATCH 6/7] tools: Use new xc function for some xc_domain_getinfo calls Alejandro Vallejo ` (2 subsequent siblings) 7 siblings, 1 reply; 20+ messages in thread From: Alejandro Vallejo @ 2023-04-26 14:59 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 | 12 ++++-------- tools/libs/light/libxl_mem.c | 4 ++-- tools/libs/light/libxl_sched.c | 12 ++++-------- tools/xenpaging/xenpaging.c | 14 +++++++------- 6 files changed, 23 insertions(+), 41 deletions(-) diff --git a/tools/libs/light/libxl_dom.c b/tools/libs/light/libxl_dom.c index 25fb716084..482e04b38c 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); + LOGE(ERROR, "getinfo_single 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..33ac8e9ce8 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 single"); 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); @@ -1663,8 +1659,8 @@ libxl_vcpuinfo *libxl_list_vcpu(libxl_ctx *ctx, uint32_t domid, xc_vcpuinfo_t vcpuinfo; unsigned int nr_vcpus; - if (xc_domain_getinfolist(ctx->xch, domid, 1, &domaininfo) != 1) { - LOGED(ERROR, domid, "Getting infolist"); + if (xc_domain_getinfo_single(ctx->xch, domid, &domaininfo) < 0) { + LOGED(ERROR, domid, "Getting info single"); 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..19da7c49ea 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 single"); 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 single"); 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..023f2bf295 100644 --- a/tools/xenpaging/xenpaging.c +++ b/tools/xenpaging/xenpaging.c @@ -169,10 +169,10 @@ 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"); + PERROR("Error getting domain info single"); return -1; } return domain_info.tot_pages; @@ -424,11 +424,11 @@ 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"); + PERROR("Error getting domain info single"); goto err; } -- 2.34.1 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 5/7] tools: Modify single-domid callers of xc_domain_getinfolist 2023-04-26 14:59 ` [PATCH 5/7] tools: Modify single-domid callers of xc_domain_getinfolist Alejandro Vallejo @ 2023-04-27 11:14 ` Andrew Cooper 0 siblings, 0 replies; 20+ messages in thread From: Andrew Cooper @ 2023-04-27 11:14 UTC (permalink / raw) To: Alejandro Vallejo, Xen-devel; +Cc: Wei Liu, Anthony PERARD, Juergen Gross On 26/04/2023 3:59 pm, Alejandro Vallejo wrote: > diff --git a/tools/libs/light/libxl_dom.c b/tools/libs/light/libxl_dom.c > index 25fb716084..482e04b38c 100644 > --- a/tools/libs/light/libxl_dom.c > +++ b/tools/libs/light/libxl_dom.c > @@ -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); > + LOGE(ERROR, "getinfo_single failed %d", ret); These are vaguely for human consumption. This one wants to be LOGED(ERROR, domid, "get dominfo failed: %d", ret); I think. (This code quite possibly predates LOGED() being introduced.) > diff --git a/tools/libs/light/libxl_domain.c b/tools/libs/light/libxl_domain.c > index 7f0986c185..33ac8e9ce8 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 single"); Swapping list for single really isn't very helpful here. "Getting domain info" would be better than either of these, but all of these ought to be updated to print ret, because right now I don't think there's any qualifying information. Interpreting -ESRCH is the important thing here, because that's the common "your domain doesn't (/no longer) exists" case. > diff --git a/tools/xenpaging/xenpaging.c b/tools/xenpaging/xenpaging.c > index 6e5490315d..023f2bf295 100644 > --- a/tools/xenpaging/xenpaging.c > +++ b/tools/xenpaging/xenpaging.c > @@ -169,10 +169,10 @@ 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"); > + PERROR("Error getting domain info single"); These messages I'd just be tempted to leave as-are. xenpaging hasn't left experimental status... ~Andrew ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 6/7] tools: Use new xc function for some xc_domain_getinfo calls 2023-04-26 14:59 [PATCH 0/7] Rationalize usage of xc_domain_getinfo{,list}() Alejandro Vallejo ` (4 preceding siblings ...) 2023-04-26 14:59 ` [PATCH 5/7] tools: Modify single-domid callers of xc_domain_getinfolist Alejandro Vallejo @ 2023-04-26 14:59 ` Alejandro Vallejo 2023-04-27 12:35 ` Andrew Cooper 2023-04-26 14:59 ` [PATCH 7/7] domctl: Modify getdomaininfo to fail if domid is not found Alejandro Vallejo 2023-04-26 22:30 ` Gitlab curiosity: Was [PATCH 0/7] Rationalize usage of xc_domain_getinfo{,list}() Andrew Cooper 7 siblings, 1 reply; 20+ messages in thread From: Alejandro Vallejo @ 2023-04-26 14:59 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/include/xenctrl.h | 7 +++++++ tools/libs/ctrl/xc_domain.c | 5 ++--- tools/libs/ctrl/xc_pagetab.c | 7 +++---- tools/libs/ctrl/xc_private.c | 7 +++---- tools/libs/ctrl/xc_private.h | 6 +++--- tools/libs/guest/xg_core.c | 21 +++++++------------ 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 | 28 +++++++++++++------------ tools/libs/guest/xg_dom_boot.c | 12 ++--------- tools/libs/guest/xg_domain.c | 6 +++--- tools/libs/guest/xg_offline_page.c | 10 ++++----- tools/libs/guest/xg_private.h | 1 + tools/libs/guest/xg_resume.c | 17 +++++++-------- tools/libs/guest/xg_sr_common.h | 2 +- tools/libs/guest/xg_sr_restore.c | 14 +++++-------- 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_sched.c | 16 +++++++------- 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 +++---- 31 files changed, 146 insertions(+), 167 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/include/xenctrl.h b/tools/include/xenctrl.h index 73b07955c6..755759f0fe 100644 --- a/tools/include/xenctrl.h +++ b/tools/include/xenctrl.h @@ -473,6 +473,13 @@ 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/libs/ctrl/xc_domain.c b/tools/libs/ctrl/xc_domain.c index 3ff91023bf..5c37dbe200 100644 --- a/tools/libs/ctrl/xc_domain.c +++ b/tools/libs/ctrl/xc_domain.c @@ -1958,12 +1958,11 @@ 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; 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..abd557e861 100644 --- a/tools/libs/ctrl/xc_private.h +++ b/tools/libs/ctrl/xc_private.h @@ -420,12 +420,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..33d35715c9 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,7 +469,7 @@ 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"); goto out; @@ -476,7 +477,7 @@ xc_domain_dumpcore_via_callback(xc_interface *xch, /* 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..6d260d2cff 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 is_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,13 @@ 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 ( xc_domain_getinfo_single(xch, domid, &di) < 0 ) { ERROR("Failed to obtain d%d info", domid); rc = -ESRCH; goto fail; } + is_hvm = di.flags & XEN_DOMINF_hvm_guest; rc = xc_cpu_policy_get_size(xch, &nr_leaves, &nr_msrs); if ( rc ) @@ -330,12 +331,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, is_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", is_hvm ? "hvm" : "pv"); rc = -errno; goto fail; } @@ -428,7 +429,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 is_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 +438,13 @@ 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 ( xc_domain_getinfo_single(xch, domid, &di) < 0 ) { ERROR("Failed to obtain d%d info", domid); rc = -ESRCH; goto out; } + is_hvm = di.flags & XEN_DOMINF_hvm_guest; rc = xc_cpu_policy_get_size(xch, &nr_leaves, &nr_msrs); if ( rc ) @@ -475,12 +477,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, is_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", is_hvm ? "hvm" : "pv"); rc = -errno; goto out; } @@ -514,7 +516,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 ( is_hvm ) { p->feat.mpx = test_bit(X86_FEATURE_MPX, host_featureset); } @@ -571,7 +573,7 @@ int xc_cpuid_apply_policy(xc_interface *xch, uint32_t domid, bool restore, { p->extd.itsc = itsc; - if ( di.hvm ) + if ( is_hvm ) { p->basic.pae = pae; p->basic.vmx = nested_virt; @@ -579,7 +581,7 @@ int xc_cpuid_apply_policy(xc_interface *xch, uint32_t domid, bool restore, } } - if ( !di.hvm ) + if ( !is_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..59b4d641c9 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,21 +174,13 @@ 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); + rc = xc_domain_getinfo_single(dom->xch, dom->guest_domid, &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 ) - { - 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); - return -1; - } dom->shared_info_mfn = info.shared_info_frame; /* initial mm setup */ diff --git a/tools/libs/guest/xg_domain.c b/tools/libs/guest/xg_domain.c index f0e7748449..49e532ee33 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,7 +49,7 @@ 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"); 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..0ca9ca32ef 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"); 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..60d682c746 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"); 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..a03183f4b9 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 is_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); - return -1; - } - + is_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, is_hvm, stream_type); ctx.domid = dom; @@ -914,7 +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 + ctx.restore.ops = is_hvm ? restore_ops_x86_hvm : restore_ops_x86_pv; if ( restore(&ctx) ) 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..8fc8e9d3b2 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 is_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; - } + is_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 ( is_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, is_hvm); ctx.domid = dom; - if ( ctx.dominfo.hvm ) + if ( is_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_sched.c b/tools/libs/light/libxl_sched.c index 19da7c49ea..ad81f15a97 100644 --- a/tools/libs/light/libxl_sched.c +++ b/tools/libs/light/libxl_sched.c @@ -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..b483f63fdc 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
* Re: [PATCH 6/7] tools: Use new xc function for some xc_domain_getinfo calls 2023-04-26 14:59 ` [PATCH 6/7] tools: Use new xc function for some xc_domain_getinfo calls Alejandro Vallejo @ 2023-04-27 12:35 ` Andrew Cooper 2023-04-27 18:07 ` Alejandro Vallejo 0 siblings, 1 reply; 20+ messages in thread From: Andrew Cooper @ 2023-04-27 12:35 UTC (permalink / raw) To: Alejandro Vallejo, Xen-devel Cc: Wei Liu, Anthony PERARD, Tim Deegan, George Dunlap, Juergen Gross On 26/04/2023 3:59 pm, Alejandro Vallejo wrote: > diff --git a/tools/libs/guest/xg_cpuid_x86.c b/tools/libs/guest/xg_cpuid_x86.c > index bd16a87e48..6d260d2cff 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 is_hvm; I know it makes a slightly larger diff, but simply "bool hvm" is what we use more commonly elsewhere. > + xc_domaininfo_t di; > unsigned int nr_leaves, nr_msrs; > uint32_t err_leaf = -1, err_subleaf = -1, err_msr = -1; > /* > @@ -291,13 +292,13 @@ 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 ( xc_domain_getinfo_single(xch, domid, &di) < 0 ) > { > ERROR("Failed to obtain d%d info", domid); > rc = -ESRCH; Now that xc_domain_getinfo_single() has a sane return value, you want to set it in the if(), and not override to ESRCH here. These two comments repeat several other times. > diff --git a/tools/libs/guest/xg_dom_boot.c b/tools/libs/guest/xg_dom_boot.c > index 263a3f4c85..59b4d641c9 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,21 +174,13 @@ 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); > + rc = xc_domain_getinfo_single(dom->xch, dom->guest_domid, &info); > if ( rc < 0 ) > { > xc_dom_panic(dom->xch, XC_INTERNAL_ERROR, > "%s: getdomaininfo failed (rc=%d)", __FUNCTION__, rc); > return rc; This need to change to -1, or you've broken the error convention of this function. (Yes, libxc is a giant mess of error handling...) > diff --git a/tools/libs/guest/xg_resume.c b/tools/libs/guest/xg_resume.c > index 77e2451a3c..60d682c746 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"); > return -1; > } > > - if ( !info.shutdown || (info.shutdown_reason != SHUTDOWN_suspend) ) > + if ( !dominfo_shutdown_with(&info, SHUTDOWN_suspend)) Needs a space at the end. > diff --git a/tools/libs/guest/xg_sr_restore.c b/tools/libs/guest/xg_sr_restore.c > index 7314a24cf9..a03183f4b9 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); > - return -1; > - } > - > + is_hvm = !!(ctx.dominfo.flags & XEN_DOMINF_hvm_guest); No need for !! now you switched this to bool. > diff --git a/tools/libs/guest/xg_sr_save.c b/tools/libs/guest/xg_sr_save.c > index 9853d8d846..8fc8e9d3b2 100644 > --- a/tools/libs/guest/xg_sr_save.c > +++ b/tools/libs/guest/xg_sr_save.c > @@ -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; > - } > + is_hvm = !!(ctx.dominfo.flags & XEN_DOMINF_hvm_guest); Same here. Can drop the !!. > diff --git a/tools/misc/xen-lowmemd.c b/tools/misc/xen-lowmemd.c > index a3a2741242..b483f63fdc 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; Use this opportunity to remove the double space. > 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 > @@ -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) Oh wow... so this bit of vchan was written expecting sane semantics out of xc_domain_getinfo() in the first place... This needs adjusting too because of the -1/errno -> -errno change. ~Andrew ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 6/7] tools: Use new xc function for some xc_domain_getinfo calls 2023-04-27 12:35 ` Andrew Cooper @ 2023-04-27 18:07 ` Alejandro Vallejo 0 siblings, 0 replies; 20+ messages in thread From: Alejandro Vallejo @ 2023-04-27 18:07 UTC (permalink / raw) To: Andrew Cooper Cc: Xen-devel, Wei Liu, Anthony PERARD, Tim Deegan, George Dunlap, Juergen Gross On Thu, Apr 27, 2023 at 01:35:18PM +0100, Andrew Cooper wrote: > > > + xc_domaininfo_t di; > > unsigned int nr_leaves, nr_msrs; > > uint32_t err_leaf = -1, err_subleaf = -1, err_msr = -1; > > /* > > @@ -291,13 +292,13 @@ 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 ( xc_domain_getinfo_single(xch, domid, &di) < 0 ) > > { > > ERROR("Failed to obtain d%d info", domid); > > rc = -ESRCH; > > Now that xc_domain_getinfo_single() has a sane return value, you want to > set it in the if(), and not override to ESRCH here. > > These two comments repeat several other times. That override shouldn't be done, true. Removed on v2. That said, looking again through the callers it appears some/many of them rely on PERROR() to print the error code to stderr on failure. At the bottom of the invocation there's xencall1(), which itself is defined in xencall.h to return -1 on error and sets errno. I think I'll also modify the error handlers that this patch touches to improve their debug. i.e: ERROR() -> PERROR() and also print the domid that failed to be found where it's not set. And patch 2 to define the new wrapper as returning 0 or -1 exclusively rather than forwarding whatever comes from below. > > > diff --git a/tools/libs/guest/xg_dom_boot.c b/tools/libs/guest/xg_dom_boot.c > > index 263a3f4c85..59b4d641c9 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,21 +174,13 @@ 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); > > + rc = xc_domain_getinfo_single(dom->xch, dom->guest_domid, &info); > > if ( rc < 0 ) > > { > > xc_dom_panic(dom->xch, XC_INTERNAL_ERROR, > > "%s: getdomaininfo failed (rc=%d)", __FUNCTION__, rc); > > return rc; > > This need to change to -1, or you've broken the error convention of this > function. (Yes, libxc is a giant mess of error handling...) I think you meant xc_exchange_page() instead? I've also modified this one so errno lands in the error message. > > 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 > > @@ -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) > > Oh wow... so this bit of vchan was written expecting sane semantics out > of xc_domain_getinfo() in the first place... > > This needs adjusting too because of the -1/errno -> -errno change. > > ~Andrew With the code changed to assume -1/errno (so PERROR() can still be used) this line will be correct and can be left as-is. Does this all sound ok? Cheers, Alejandro ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 7/7] domctl: Modify getdomaininfo to fail if domid is not found 2023-04-26 14:59 [PATCH 0/7] Rationalize usage of xc_domain_getinfo{,list}() Alejandro Vallejo ` (5 preceding siblings ...) 2023-04-26 14:59 ` [PATCH 6/7] tools: Use new xc function for some xc_domain_getinfo calls Alejandro Vallejo @ 2023-04-26 14:59 ` Alejandro Vallejo 2023-04-27 11:15 ` Andrew Cooper 2023-04-26 22:30 ` Gitlab curiosity: Was [PATCH 0/7] Rationalize usage of xc_domain_getinfo{,list}() Andrew Cooper 7 siblings, 1 reply; 20+ messages in thread From: Alejandro Vallejo @ 2023-04-26 14:59 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 | 70 ------------------------------------- xen/common/domctl.c | 32 ++--------------- 3 files changed, 2 insertions(+), 143 deletions(-) diff --git a/tools/include/xenctrl.h b/tools/include/xenctrl.h index 755759f0fe..7ed22eff0d 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 5c37dbe200..581e7529e5 100644 --- a/tools/libs/ctrl/xc_domain.c +++ b/tools/libs/ctrl/xc_domain.c @@ -358,82 +358,12 @@ int xc_domain_getinfo_single(xc_interface *xch, if (rc < 0) return rc; - if (domctl.u.getdomaininfo.domain != domid) - return -ESRCH; - if (info) *info = domctl.u.getdomaininfo; return rc; } -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/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 7/7] domctl: Modify getdomaininfo to fail if domid is not found 2023-04-26 14:59 ` [PATCH 7/7] domctl: Modify getdomaininfo to fail if domid is not found Alejandro Vallejo @ 2023-04-27 11:15 ` Andrew Cooper 0 siblings, 0 replies; 20+ messages in thread From: Andrew Cooper @ 2023-04-27 11:15 UTC (permalink / raw) To: Alejandro Vallejo, Xen-devel Cc: George Dunlap, Jan Beulich, Julien Grall, Stefano Stabellini, Wei Liu, Anthony PERARD, Juergen Gross On 26/04/2023 3:59 pm, 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> > --- > 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 | 70 ------------------------------------- > xen/common/domctl.c | 32 ++--------------- > 3 files changed, 2 insertions(+), 143 deletions(-) Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> Good riddance to this disaster of an interface... ^ permalink raw reply [flat|nested] 20+ messages in thread
* Gitlab curiosity: Was [PATCH 0/7] Rationalize usage of xc_domain_getinfo{,list}() 2023-04-26 14:59 [PATCH 0/7] Rationalize usage of xc_domain_getinfo{,list}() Alejandro Vallejo ` (6 preceding siblings ...) 2023-04-26 14:59 ` [PATCH 7/7] domctl: Modify getdomaininfo to fail if domid is not found Alejandro Vallejo @ 2023-04-26 22:30 ` Andrew Cooper 2023-04-27 9:26 ` Andrew Cooper 7 siblings, 1 reply; 20+ messages in thread From: Andrew Cooper @ 2023-04-26 22:30 UTC (permalink / raw) To: Alejandro Vallejo, Xen-devel Cc: George Dunlap, Jan Beulich, Julien Grall, Stefano Stabellini, Wei Liu, Anthony PERARD, Juergen Gross, Tim Deegan, Michal Orzel On 26/04/2023 3:59 pm, Alejandro Vallejo wrote: > 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 ammended, 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 the 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 getdomaininfo to fail if domid is not found The patchew run found one single failure, https://gitlab.com/xen-project/patchew/xen/-/jobs/4183881202 This part looks reasonably fatal: * Starting local ... * Executing "/etc/local.d/xen.start" ...Starting /usr/local/sbin/xenstored... /etc/xen/scripts/launch-xenstore: line 90: echo: write error: Invalid argument Setting domain 0 name, domid and JSON config... Done setting up Dom0 Starting xenconsoled... except it was only the part trying to set the OOM score after starting xenstored, and the only way that plausibly fails is if the pidfile was bad. And then the other print messages are clearly out of order. I've rerun the pipeline a second time, https://gitlab.com/xen-project/patchew/xen/-/pipelines/850230144, to see if gitlab thinks it is a reliable or unreliable failure. But, there's a plenty of other stuff in this log which is concerning. Stefano, Michal: * Starting networking ...awk: /etc/network/interfaces: No such file or directory * ERROR: networking failed to start The domains ought to have a interfaces file with "auto eth0", or even nothing. Alpine clearly isn't expecting the absence of the file at all. The fact the ping test passes usually means that this error isn't as fatal as it makes out. Next, * Executing: /lib/rc/sh/openrc-run.sh /lib/rc/sh/openrc-run.sh /etc/init.d/modloop start * Mounting modloop ... [ !! ] * ERROR: modloop failed to start Not sure what modloop is, but this doesn't look healthy. Next, * Loading modules ... * Processing /etc/modules modprobe: can't change directory to '/lib/modules': No such file or directory This probably just wants an empty dir in the filesystem. I could go on, but I wont. One thing that we do need however is /var/log/xen/* pulled into the artefacts of the job, because if there really is a real xenstored problem hiding in this series, there's no way to debug it with the current artefacts collected. ~Andrew ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: Gitlab curiosity: Was [PATCH 0/7] Rationalize usage of xc_domain_getinfo{,list}() 2023-04-26 22:30 ` Gitlab curiosity: Was [PATCH 0/7] Rationalize usage of xc_domain_getinfo{,list}() Andrew Cooper @ 2023-04-27 9:26 ` Andrew Cooper 0 siblings, 0 replies; 20+ messages in thread From: Andrew Cooper @ 2023-04-27 9:26 UTC (permalink / raw) To: Alejandro Vallejo, Xen-devel Cc: George Dunlap, Jan Beulich, Julien Grall, Stefano Stabellini, Wei Liu, Anthony PERARD, Juergen Gross, Tim Deegan, Michal Orzel On 26/04/2023 11:30 pm, Andrew Cooper wrote: > I've rerun the pipeline a second time, > https://gitlab.com/xen-project/patchew/xen/-/pipelines/850230144, to see > if gitlab thinks it is a reliable or unreliable failure. It passed the second time, so I'm pretty confident this is a buggy test with a rare race condition, rather than a bug in the series under test. ~Andrew ^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2023-04-27 18:07 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-04-26 14:59 [PATCH 0/7] Rationalize usage of xc_domain_getinfo{,list}() Alejandro Vallejo
2023-04-26 14:59 ` [PATCH 1/7] tools: Make some callers of xc_domain_getinfo use xc_domain_getinfolist Alejandro Vallejo
2023-04-27 9:51 ` Andrew Cooper
2023-04-27 14:37 ` Alejandro Vallejo
2023-04-26 14:59 ` [PATCH 2/7] tools: Create xc_domain_getinfo_single() Alejandro Vallejo
2023-04-27 9:53 ` Andrew Cooper
2023-04-26 14:59 ` [PATCH 3/7] tools: Refactor the console/io.c to avoid using xc_domain_getinfo() Alejandro Vallejo
2023-04-27 10:36 ` Andrew Cooper
2023-04-26 14:59 ` [PATCH 4/7] tools: Make init-xenstore-domain use xc_domain_getinfolist() Alejandro Vallejo
2023-04-26 15:20 ` Juergen Gross
2023-04-27 14:29 ` Alejandro Vallejo
2023-04-26 14:59 ` [PATCH 5/7] tools: Modify single-domid callers of xc_domain_getinfolist Alejandro Vallejo
2023-04-27 11:14 ` Andrew Cooper
2023-04-26 14:59 ` [PATCH 6/7] tools: Use new xc function for some xc_domain_getinfo calls Alejandro Vallejo
2023-04-27 12:35 ` Andrew Cooper
2023-04-27 18:07 ` Alejandro Vallejo
2023-04-26 14:59 ` [PATCH 7/7] domctl: Modify getdomaininfo to fail if domid is not found Alejandro Vallejo
2023-04-27 11:15 ` Andrew Cooper
2023-04-26 22:30 ` Gitlab curiosity: Was [PATCH 0/7] Rationalize usage of xc_domain_getinfo{,list}() Andrew Cooper
2023-04-27 9:26 ` 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.