Re: [PATCH 1/7] tools: Make some callers of xc_domain_getinfo use xc_domain_getinfolist

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

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