Re: [Xen-devel] [PATCH] python: Adjust xc_physinfo wrapper for updated virt_caps bits

[Ian Jackson <ian.jackson@xxxxxxxxxx>]

Marek Marczykowski-Górecki writes ("[PATCH] python: Adjust xc_physinfo wrapper 
for updated virt_caps bits"):
> Commit f089fddd94 "xen: report PV capability in sysctl and use it in
> toolstack" changed meaning of virt_caps bit 1 - previously it was
> "directio", but was changed to "pv" and "directio" was moved to bit 2.
> Adjust python wrapper, and add reporting of both "pv_directio" and
> "hvm_directio".

Thanks for your attention to this...

But:

> index cc8175a11e..0a8d8f407e 100644
> --- a/tools/python/xen/lowlevel/xc/xc.c
> +++ b/tools/python/xen/lowlevel/xc/xc.c
> @@ -973,7 +973,8 @@ static PyObject *pyxc_physinfo(XcObject *self)
>      xc_physinfo_t pinfo;
>      char cpu_cap[128], virt_caps[128], *p;
>      int i;
> -    const char *virtcap_names[] = { "hvm", "hvm_directio" };
> +    const char *virtcap_names[] = { "hvm", "pv",
> +                                    "hvm_directio", "pv_directio" };

It seems quite wrong that we have no way to keep this in sync - and
not even comments in both places!  (This is not your fault...)

> @@ -989,6 +990,10 @@ static PyObject *pyxc_physinfo(XcObject *self)
>      for ( i = 0; i < 2; i++ )
>          if ( (pinfo.capabilities >> i) & 1 )
>            p += sprintf(p, "%s ", virtcap_names[i]);
> +    if (pinfo.capabilities & XEN_SYSCTL_PHYSCAP_directio)
> +        for ( i = 0; i < 2; i++ )
> +            if ( (pinfo.capabilities >> i) & 1 )
> +              p += sprintf(p, "%s ", virtcap_names[i+2]);
>      if ( p != virt_caps )
>        *(p-1) = '\0';

I'm not sure I like this.  AFAICT the +2 is magic, and you in fact
treat the two halves of this array together as a single array.  So
this should either be two arrays, or, more likely, something like this
maybe:

  +              p += sprintf(p, "%s_directio ", virtcap_names[i]);

What do you think ?

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel