[PATCH] python: Adjust xc_physinfo wrapper for updated virt_caps bits

[PATCH] python: Adjust xc_physinfo wrapper for updated virt_caps bits

[Marek Marczykowski-Górecki]

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".

Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
---
This should be backported to 4.12
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
---
 tools/python/xen/lowlevel/xc/xc.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/tools/python/xen/lowlevel/xc/xc.c b/tools/python/xen/lowlevel/xc/xc.c
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" };
 
     if ( xc_physinfo(self->xc_handle, &pinfo) != 0 )
         return pyxc_error_to_exception(self->xc_handle);
@@ -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';
 
-- 
2.17.2


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

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

[Marek Marczykowski-Górecki]

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".

Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
---
This should be backported to 4.12
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
---
 tools/python/xen/lowlevel/xc/xc.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/tools/python/xen/lowlevel/xc/xc.c b/tools/python/xen/lowlevel/xc/xc.c
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" };
 
     if ( xc_physinfo(self->xc_handle, &pinfo) != 0 )
         return pyxc_error_to_exception(self->xc_handle);
@@ -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';
 
-- 
2.17.2


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

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

[Wei Liu]

On Sun, Apr 28, 2019 at 09:08:23PM +0200, Marek Marczykowski-Górecki wrote:
> 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".
> 
> Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>

Acked-by: Wei Liu <wei.liu2@citrix.com>

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

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

[Wei Liu]

On Sun, Apr 28, 2019 at 09:08:23PM +0200, Marek Marczykowski-Górecki wrote:
> 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".
> 
> Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>

Acked-by: Wei Liu <wei.liu2@citrix.com>

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

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

[Ian Jackson]

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@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

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

[Ian Jackson]

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@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

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

[Marek Marczykowski-Górecki]

[-- Attachment #1.1: Type: text/plain, Size: 2245 bytes --]

On Mon, Apr 29, 2019 at 11:16:26AM +0100, Ian Jackson wrote:
> 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...)

I'll add a comment...

> > @@ -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 ?

Makes sense.

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

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

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

[Marek Marczykowski-Górecki]

[-- Attachment #1.1: Type: text/plain, Size: 2245 bytes --]

On Mon, Apr 29, 2019 at 11:16:26AM +0100, Ian Jackson wrote:
> 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...)

I'll add a comment...

> > @@ -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 ?

Makes sense.

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

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

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

[Marek Marczykowski-Górecki]

[-- Attachment #1.1: Type: text/plain, Size: 2576 bytes --]

On Tue, Apr 30, 2019 at 12:08:38AM +0200, Marek Marczykowski-Górecki wrote:
> On Mon, Apr 29, 2019 at 11:16:26AM +0100, Ian Jackson wrote:
> > 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...)
> 
> I'll add a comment...

Actually, this would work much better if the loop below would use
#defines from sysctl.h, instead of hardcoded values. I'll update it this
way.

> > > @@ -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 ?
> 
> Makes sense.
> 



-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

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

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

[Marek Marczykowski-Górecki]

[-- Attachment #1.1: Type: text/plain, Size: 2576 bytes --]

On Tue, Apr 30, 2019 at 12:08:38AM +0200, Marek Marczykowski-Górecki wrote:
> On Mon, Apr 29, 2019 at 11:16:26AM +0100, Ian Jackson wrote:
> > 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...)
> 
> I'll add a comment...

Actually, this would work much better if the loop below would use
#defines from sysctl.h, instead of hardcoded values. I'll update it this
way.

> > > @@ -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 ?
> 
> Makes sense.
> 



-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

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