tools/python/xen/lowlevel/xc/xc.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
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
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
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
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? _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
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? _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
© 2016 - 2024 Red Hat, Inc.