xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] Fix cpumap setting before passing to XEN
@ 2016-04-11  1:42 Zhenzhong Duan
  2016-04-11 11:27 ` Wei Liu
  0 siblings, 1 reply; 11+ messages in thread
From: Zhenzhong Duan @ 2016-04-11  1:42 UTC (permalink / raw)
  To: xen-devel

It's tool's duty to pass a correct cpumap to XEN. On a host with less 
than 64
CPUS, it just shows below error.

[root@localhost /]# xm vcpu-pin 3 all all
Error: Cannot pin vcpu: 0 to cpu: [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12,
13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31,
32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50,
51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63] - (22, 'Invalid
argument')

The fix make it same as in xl code.

Signed-off-by: Zhenzhong Duan <zhenzhong.duan@oracle.com>
---
  tools/python/xen/lowlevel/xc/xc.c |    4 +++-
  1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/tools/python/xen/lowlevel/xc/xc.c 
b/tools/python/xen/lowlevel/xc/xc.c
index c40a4e9..2f4898d 100644
--- a/tools/python/xen/lowlevel/xc/xc.c
+++ b/tools/python/xen/lowlevel/xc/xc.c
@@ -243,13 +243,15 @@ static PyObject *pyxc_vcpu_setaffinity(XcObject *self,
          for ( i = 0; i < PyList_Size(cpulist); i++ )
          {
              long cpu = PyInt_AsLong(PyList_GetItem(cpulist, i));
-            if ( cpu < 0 || cpu >= nr_cpus )
+            if ( cpu < 0 )
              {
                  free(cpumap);
                  errno = EINVAL;
                  PyErr_SetFromErrno(xc_error_obj);
                  return NULL;
              }
+            if ( cpu >= nr_cpus )
+                continue;
              cpumap[cpu / 8] |= 1 << (cpu % 8);
          }
      }
-- 
1.7.3


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH] Fix cpumap setting before passing to XEN
  2016-04-11  1:42 [PATCH] Fix cpumap setting before passing to XEN Zhenzhong Duan
@ 2016-04-11 11:27 ` Wei Liu
  2016-04-12  3:35   ` Zhenzhong Duan
  0 siblings, 1 reply; 11+ messages in thread
From: Wei Liu @ 2016-04-11 11:27 UTC (permalink / raw)
  To: Zhenzhong Duan; +Cc: Ian Jackson, Wei Liu, xen-devel

On Mon, Apr 11, 2016 at 09:42:57AM +0800, Zhenzhong Duan wrote:
> It's tool's duty to pass a correct cpumap to XEN. On a host with less than
> 64
> CPUS, it just shows below error.
> 
> [root@localhost /]# xm vcpu-pin 3 all all
> Error: Cannot pin vcpu: 0 to cpu: [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12,
> 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31,
> 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50,
> 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63] - (22, 'Invalid
> argument')
> 

This error clearly shows that the upper layer is not passing down the
right cpumap. So this example along won't convince me to that this is a
bug in python binding. It's free to reject input that it deems wrong.

I guess the question is more or less what should it do in such
situation. So ...

> The fix make it same as in xl code.
> 

Is there reference to this? I guess the commit message  or mail thread
will shed more light on the expected behaviour.

BTW please CC relevant maintainers in the future, thanks.

Wei.

> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@oracle.com>
> ---
>  tools/python/xen/lowlevel/xc/xc.c |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)
> 
> diff --git a/tools/python/xen/lowlevel/xc/xc.c
> b/tools/python/xen/lowlevel/xc/xc.c
> index c40a4e9..2f4898d 100644
> --- a/tools/python/xen/lowlevel/xc/xc.c
> +++ b/tools/python/xen/lowlevel/xc/xc.c
> @@ -243,13 +243,15 @@ static PyObject *pyxc_vcpu_setaffinity(XcObject *self,
>          for ( i = 0; i < PyList_Size(cpulist); i++ )
>          {
>              long cpu = PyInt_AsLong(PyList_GetItem(cpulist, i));
> -            if ( cpu < 0 || cpu >= nr_cpus )
> +            if ( cpu < 0 )
>              {
>                  free(cpumap);
>                  errno = EINVAL;
>                  PyErr_SetFromErrno(xc_error_obj);
>                  return NULL;
>              }
> +            if ( cpu >= nr_cpus )
> +                continue;
>              cpumap[cpu / 8] |= 1 << (cpu % 8);
>          }
>      }
> -- 
> 1.7.3
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Fix cpumap setting before passing to XEN
  2016-04-11 11:27 ` Wei Liu
@ 2016-04-12  3:35   ` Zhenzhong Duan
  2016-04-18  4:57     ` Zhenzhong Duan
  2016-04-20 14:33     ` Wei Liu
  0 siblings, 2 replies; 11+ messages in thread
From: Zhenzhong Duan @ 2016-04-12  3:35 UTC (permalink / raw)
  To: Wei Liu; +Cc: Ian Jackson, xen-devel

在 2016/4/11 19:27, Wei Liu 写道:
> On Mon, Apr 11, 2016 at 09:42:57AM +0800, Zhenzhong Duan wrote:
>> It's tool's duty to pass a correct cpumap to XEN. On a host with less than
>> 64
>> CPUS, it just shows below error.
>>
>> [root@localhost /]# xm vcpu-pin 3 all all
>> Error: Cannot pin vcpu: 0 to cpu: [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12,
>> 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31,
>> 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50,
>> 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63] - (22, 'Invalid
>> argument')
>>
> This error clearly shows that the upper layer is not passing down the
> right cpumap. So this example along won't convince me to that this is a
> bug in python binding. It's free to reject input that it deems wrong.
>
> I guess the question is more or less what should it do in such
> situation. So ...
>
>> The fix make it same as in xl code.
>>
> Is there reference to this? I guess the commit message  or mail thread
> will shed more light on the expected behaviour.
Calling path:
main_vcpupin->cpurange_parse->update_cpumap_range->libxl_bitmap_set

void libxl_bitmap_set(libxl_bitmap *bitmap, int bit)
{
     if (bit >= bitmap->size * 8)
         return;
     bitmap->map[bit / 8] |= 1 << (bit & 7);
}

I referenced above code. It just ignore the cpu number beyond the max 
CPU count rather than raise an error.

thanks
zduan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Fix cpumap setting before passing to XEN
  2016-04-12  3:35   ` Zhenzhong Duan
@ 2016-04-18  4:57     ` Zhenzhong Duan
  2016-04-20 14:33     ` Wei Liu
  1 sibling, 0 replies; 11+ messages in thread
From: Zhenzhong Duan @ 2016-04-18  4:57 UTC (permalink / raw)
  To: Wei Liu; +Cc: Ian Jackson, xen-devel

On 2016/4/12 11:35, Zhenzhong Duan wrote:
> On 2016/4/11 19:27, Wei Liu wrote:
>> On Mon, Apr 11, 2016 at 09:42:57AM +0800, Zhenzhong Duan wrote:
>>> It's tool's duty to pass a correct cpumap to XEN. On a host with 
>>> less than
>>> 64
>>> CPUS, it just shows below error.
>>>
>>> [root@localhost /]# xm vcpu-pin 3 all all
>>> Error: Cannot pin vcpu: 0 to cpu: [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 
>>> 11, 12,
>>> 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 
>>> 30, 31,
>>> 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 
>>> 49, 50,
>>> 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63] - (22, 'Invalid
>>> argument')
>>>
>> This error clearly shows that the upper layer is not passing down the
>> right cpumap. So this example along won't convince me to that this is a
>> bug in python binding. It's free to reject input that it deems wrong.
>>
>> I guess the question is more or less what should it do in such
>> situation. So ...
>>
>>> The fix make it same as in xl code.
>>>
>> Is there reference to this? I guess the commit message  or mail thread
>> will shed more light on the expected behaviour.
> Calling path:
> main_vcpupin->cpurange_parse->update_cpumap_range->libxl_bitmap_set
>
> void libxl_bitmap_set(libxl_bitmap *bitmap, int bit)
> {
>     if (bit >= bitmap->size * 8)
>         return;
>     bitmap->map[bit / 8] |= 1 << (bit & 7);
> }
>
> I referenced above code. It just ignore the cpu number beyond the max 
> CPU count rather than raise an error.
Any further comments?

thanks
zduan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Fix cpumap setting before passing to XEN
  2016-04-12  3:35   ` Zhenzhong Duan
  2016-04-18  4:57     ` Zhenzhong Duan
@ 2016-04-20 14:33     ` Wei Liu
  2016-04-22 18:15       ` Konrad Rzeszutek Wilk
  1 sibling, 1 reply; 11+ messages in thread
From: Wei Liu @ 2016-04-20 14:33 UTC (permalink / raw)
  To: Zhenzhong Duan; +Cc: Ian Jackson, Wei Liu, xen-devel

Sorry I was at the Xen hackathon in the last two days.

On Tue, Apr 12, 2016 at 11:35:20AM +0800, Zhenzhong Duan wrote:
> 在 2016/4/11 19:27, Wei Liu 写道:
> >On Mon, Apr 11, 2016 at 09:42:57AM +0800, Zhenzhong Duan wrote:
> >>It's tool's duty to pass a correct cpumap to XEN. On a host with less than
> >>64
> >>CPUS, it just shows below error.
> >>
> >>[root@localhost /]# xm vcpu-pin 3 all all
> >>Error: Cannot pin vcpu: 0 to cpu: [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12,
> >>13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31,
> >>32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50,
> >>51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63] - (22, 'Invalid
> >>argument')
> >>
> >This error clearly shows that the upper layer is not passing down the
> >right cpumap. So this example along won't convince me to that this is a
> >bug in python binding. It's free to reject input that it deems wrong.
> >
> >I guess the question is more or less what should it do in such
> >situation. So ...
> >
> >>The fix make it same as in xl code.
> >>
> >Is there reference to this? I guess the commit message  or mail thread
> >will shed more light on the expected behaviour.
> Calling path:
> main_vcpupin->cpurange_parse->update_cpumap_range->libxl_bitmap_set
> 
> void libxl_bitmap_set(libxl_bitmap *bitmap, int bit)
> {
>     if (bit >= bitmap->size * 8)
>         return;
>     bitmap->map[bit / 8] |= 1 << (bit & 7);
> }
> 
> I referenced above code. It just ignore the cpu number beyond the max CPU
> count rather than raise an error.
> 

In principle I think having python binding and xl/libxl behave more or less
the same is the right direction. I'm a bit nervous about the change of
behaviour on the other hand.

Let's wait for a few more days to see if other people have any comment on
this.

Wei.

> thanks
> zduan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Fix cpumap setting before passing to XEN
  2016-04-20 14:33     ` Wei Liu
@ 2016-04-22 18:15       ` Konrad Rzeszutek Wilk
  2016-04-25 13:26         ` Ian Jackson
  0 siblings, 1 reply; 11+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-04-22 18:15 UTC (permalink / raw)
  To: Wei Liu; +Cc: Ian Jackson, Zhenzhong Duan, xen-devel

On Wed, Apr 20, 2016 at 03:33:13PM +0100, Wei Liu wrote:
> Sorry I was at the Xen hackathon in the last two days.
> 
> On Tue, Apr 12, 2016 at 11:35:20AM +0800, Zhenzhong Duan wrote:
> > 在 2016/4/11 19:27, Wei Liu 写道:
> > >On Mon, Apr 11, 2016 at 09:42:57AM +0800, Zhenzhong Duan wrote:
> > >>It's tool's duty to pass a correct cpumap to XEN. On a host with less than
> > >>64
> > >>CPUS, it just shows below error.
> > >>
> > >>[root@localhost /]# xm vcpu-pin 3 all all
> > >>Error: Cannot pin vcpu: 0 to cpu: [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12,
> > >>13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31,
> > >>32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50,
> > >>51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63] - (22, 'Invalid
> > >>argument')
> > >>
> > >This error clearly shows that the upper layer is not passing down the
> > >right cpumap. So this example along won't convince me to that this is a
> > >bug in python binding. It's free to reject input that it deems wrong.
> > >
> > >I guess the question is more or less what should it do in such
> > >situation. So ...
> > >
> > >>The fix make it same as in xl code.
> > >>
> > >Is there reference to this? I guess the commit message  or mail thread
> > >will shed more light on the expected behaviour.
> > Calling path:
> > main_vcpupin->cpurange_parse->update_cpumap_range->libxl_bitmap_set
> > 
> > void libxl_bitmap_set(libxl_bitmap *bitmap, int bit)
> > {
> >     if (bit >= bitmap->size * 8)
> >         return;
> >     bitmap->map[bit / 8] |= 1 << (bit & 7);
> > }
> > 
> > I referenced above code. It just ignore the cpu number beyond the max CPU
> > count rather than raise an error.
> > 
> 
> In principle I think having python binding and xl/libxl behave more or less
> the same is the right direction. I'm a bit nervous about the change of
> behaviour on the other hand.
> 
> Let's wait for a few more days to see if other people have any comment on
> this.

Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> ?

> 
> Wei.
> 
> > thanks
> > zduan
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Fix cpumap setting before passing to XEN
  2016-04-22 18:15       ` Konrad Rzeszutek Wilk
@ 2016-04-25 13:26         ` Ian Jackson
  2016-04-26  3:54           ` Zhenzhong Duan
  0 siblings, 1 reply; 11+ messages in thread
From: Ian Jackson @ 2016-04-25 13:26 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk; +Cc: Wei Liu, Zhenzhong Duan, xen-devel

Konrad Rzeszutek Wilk writes ("Re: [Xen-devel] [PATCH] Fix cpumap setting before passing to XEN"):
> On Wed, Apr 20, 2016 at 03:33:13PM +0100, Wei Liu wrote:
> > In principle I think having python binding and xl/libxl behave more or less
> > the same is the right direction. I'm a bit nervous about the change of
> > behaviour on the other hand.
> > 
> > Let's wait for a few more days to see if other people have any comment on
> > this.
> 
> Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> ?

Does this bug report mean that `xm vcpu-pin ... all' has never
worked properly ?  Can that really be the case ?

Also, xm exists in Xen 4.4 and earlier, only.  Xen 4.4 is no longer
supported upstream, so we would not apply this patch to Xen 4.4.  So
whatever we do, this is not going to fix any bug in `xm vcpu-pin' in
4.4.

This doesn't necessarily mean that I object to changing the behaviour
of the python xc module in still-supported Xen releases.  But I'm not
sure the reasoning behind the behaviour of the libxl bitmap functions
applies to the Python interface.

Zhenzhong Duan, are you using an out-of-tree copy of xm and xend ?

Regards,
Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Fix cpumap setting before passing to XEN
  2016-04-25 13:26         ` Ian Jackson
@ 2016-04-26  3:54           ` Zhenzhong Duan
  2016-04-28 15:07             ` Wei Liu
  0 siblings, 1 reply; 11+ messages in thread
From: Zhenzhong Duan @ 2016-04-26  3:54 UTC (permalink / raw)
  To: Ian Jackson, Konrad Rzeszutek Wilk; +Cc: Wei Liu, xen-devel

On 2016/4/25 21:26, Ian Jackson wrote:
> Konrad Rzeszutek Wilk writes ("Re: [Xen-devel] [PATCH] Fix cpumap setting before passing to XEN"):
>> On Wed, Apr 20, 2016 at 03:33:13PM +0100, Wei Liu wrote:
>>> In principle I think having python binding and xl/libxl behave more or less
>>> the same is the right direction. I'm a bit nervous about the change of
>>> behaviour on the other hand.
>>>
>>> Let's wait for a few more days to see if other people have any comment on
>>> this.
>> Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> ?
> Does this bug report mean that `xm vcpu-pin ... all' has never
> worked properly ?  Can that really be the case ?
Xen 4.3 doesn't work, Xen 3.4 works.
I have no Xen 4.4 around to test that, but checked code, it will not.
Then I found below commit involved.

commit 41abbadef60e5fccdfd688579dd458f7f7887cf5
Author: Ian Jackson <ian.jackson@eu.citrix.com>
Date:   Wed May 29 15:48:11 2013 +0100

     libxc: limit cpu values when setting vcpu affinity

     When support for pinning more than 64 cpus was added, check for cpu
     out-of-range values was removed. This can lead to subsequent
     out-of-bounds cpumap array accesses in case the cpu number is higher
     than the actual count.

     This patch returns the check.

     This is CVE-2013-2072 / XSA-56

     Signed-off-by: Petr Matousek <pmatouse@redhat.com>
>
> Also, xm exists in Xen 4.4 and earlier, only.  Xen 4.4 is no longer
> supported upstream, so we would not apply this patch to Xen 4.4.  So
> whatever we do, this is not going to fix any bug in `xm vcpu-pin' in
> 4.4.
The only impact is upper layer or the user need to pass a correct cpumap 
param not beyond the real cpu map to avoid the error.
But I am not clear if python binding is still used or will be removed 
just as Xend.
>
> This doesn't necessarily mean that I object to changing the behaviour
> of the python xc module in still-supported Xen releases.  But I'm not
> sure the reasoning behind the behaviour of the libxl bitmap functions
> applies to the Python interface.
>
> Zhenzhong Duan, are you using an out-of-tree copy of xm and xend ?
I am using xen-4.3.0-55.el6.47.33 which is Xen 4.3 variant

thanks
zduan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Fix cpumap setting before passing to XEN
  2016-04-26  3:54           ` Zhenzhong Duan
@ 2016-04-28 15:07             ` Wei Liu
  2016-04-28 17:02               ` Ian Jackson
  0 siblings, 1 reply; 11+ messages in thread
From: Wei Liu @ 2016-04-28 15:07 UTC (permalink / raw)
  To: Zhenzhong Duan; +Cc: Wei Liu, xen-devel, Ian Jackson

On Tue, Apr 26, 2016 at 11:54:32AM +0800, Zhenzhong Duan wrote:
> On 2016/4/25 21:26, Ian Jackson wrote:
> >Konrad Rzeszutek Wilk writes ("Re: [Xen-devel] [PATCH] Fix cpumap setting before passing to XEN"):
> >>On Wed, Apr 20, 2016 at 03:33:13PM +0100, Wei Liu wrote:
> >>>In principle I think having python binding and xl/libxl behave more or less
> >>>the same is the right direction. I'm a bit nervous about the change of
> >>>behaviour on the other hand.
> >>>
> >>>Let's wait for a few more days to see if other people have any comment on
> >>>this.
> >>Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> ?
> >Does this bug report mean that `xm vcpu-pin ... all' has never
> >worked properly ?  Can that really be the case ?
> Xen 4.3 doesn't work, Xen 3.4 works.
> I have no Xen 4.4 around to test that, but checked code, it will not.
> Then I found below commit involved.
> 
> commit 41abbadef60e5fccdfd688579dd458f7f7887cf5
> Author: Ian Jackson <ian.jackson@eu.citrix.com>
> Date:   Wed May 29 15:48:11 2013 +0100
> 
>     libxc: limit cpu values when setting vcpu affinity
> 
>     When support for pinning more than 64 cpus was added, check for cpu
>     out-of-range values was removed. This can lead to subsequent
>     out-of-bounds cpumap array accesses in case the cpu number is higher
>     than the actual count.
> 
>     This patch returns the check.
> 
>     This is CVE-2013-2072 / XSA-56
> 
>     Signed-off-by: Petr Matousek <pmatouse@redhat.com>
> >
> >Also, xm exists in Xen 4.4 and earlier, only.  Xen 4.4 is no longer
> >supported upstream, so we would not apply this patch to Xen 4.4.  So
> >whatever we do, this is not going to fix any bug in `xm vcpu-pin' in
> >4.4.
> The only impact is upper layer or the user need to pass a correct cpumap
> param not beyond the real cpu map to avoid the error.
> But I am not clear if python binding is still used or will be removed just
> as Xend.

I don't think we have plan to remove it any time soon. On the other hand
because no in-tree component uses it so we don't know whether it works
in practice or not.

> >
> >This doesn't necessarily mean that I object to changing the behaviour
> >of the python xc module in still-supported Xen releases.  But I'm not
> >sure the reasoning behind the behaviour of the libxl bitmap functions
> >applies to the Python interface.
> >
> >Zhenzhong Duan, are you using an out-of-tree copy of xm and xend ?
> I am using xen-4.3.0-55.el6.47.33 which is Xen 4.3 variant
> 

So what is the conclusion of this discussion so far? I admit I'm a bit
lost here.

Wei.

> thanks
> zduan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Fix cpumap setting before passing to XEN
  2016-04-28 15:07             ` Wei Liu
@ 2016-04-28 17:02               ` Ian Jackson
  2016-05-04 20:04                 ` Konrad Rzeszutek Wilk
  0 siblings, 1 reply; 11+ messages in thread
From: Ian Jackson @ 2016-04-28 17:02 UTC (permalink / raw)
  To: Wei Liu; +Cc: xen-devel, Zhenzhong Duan

Wei Liu writes ("Re: [Xen-devel] [PATCH] Fix cpumap setting before passing to XEN"):
> So what is the conclusion of this discussion so far? I admit I'm a bit
> lost here.

Undoubtedly:

* That "xm ..." generates the reported error is definitely a bug.

* "xm" exists only in versions of Xen now no longer supported
  upstream.

* Applying the proposed patch to libxc in upstream supported versions
  of Xen will not fix "xm" for users like Zhenzhong Duan.


There are two possible views about the nature of the bug:

1. It is xm's fault for passing an invalid cpumap.

2. It is python xc's fault for failing to tolerate a cpumap with
   bits set which do not correspond to actual cpus.

In the case 1., the xm python code needs to be changed.  But there is
nothing for upstream to do because none of our supported trees contain
this code any more.

In the case 2., the in-tree python xc code should be changed.


I am somewhat reluctant to go down the path implied by case 2, because
we don't know what collateral damage there may be.  OTOH the proposed
patch is one which tolerates a wider range of inputs, which is fairly
safe.

I think that the behaviour of libxl (which has to provide a C API,
rather than a python one) is a poor guide to the best behaviour for
python xc.


So I still don't have a clear view about whether the patch should be
applied.  (I think it clearly shouldn't be backported.)

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Fix cpumap setting before passing to XEN
  2016-04-28 17:02               ` Ian Jackson
@ 2016-05-04 20:04                 ` Konrad Rzeszutek Wilk
  0 siblings, 0 replies; 11+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-05-04 20:04 UTC (permalink / raw)
  To: Ian Jackson; +Cc: Wei Liu, Zhenzhong Duan, xen-devel

On Thu, Apr 28, 2016 at 06:02:54PM +0100, Ian Jackson wrote:
> Wei Liu writes ("Re: [Xen-devel] [PATCH] Fix cpumap setting before passing to XEN"):
> > So what is the conclusion of this discussion so far? I admit I'm a bit
> > lost here.
> 
> Undoubtedly:
> 
> * That "xm ..." generates the reported error is definitely a bug.
> 
> * "xm" exists only in versions of Xen now no longer supported
>   upstream.
> 
> * Applying the proposed patch to libxc in upstream supported versions
>   of Xen will not fix "xm" for users like Zhenzhong Duan.

Wait, how will it not? It will continue without erroring out.
> 
> 
> There are two possible views about the nature of the bug:
> 
> 1. It is xm's fault for passing an invalid cpumap.
> 
> 2. It is python xc's fault for failing to tolerate a cpumap with
>    bits set which do not correspond to actual cpus.
> 
> In the case 1., the xm python code needs to be changed.  But there is
> nothing for upstream to do because none of our supported trees contain
> this code any more.
> 
> In the case 2., the in-tree python xc code should be changed.
> 
> 
> I am somewhat reluctant to go down the path implied by case 2, because
> we don't know what collateral damage there may be.  OTOH the proposed
> patch is one which tolerates a wider range of inputs, which is fairly
> safe.
> 
> I think that the behaviour of libxl (which has to provide a C API,
> rather than a python one) is a poor guide to the best behaviour for
> python xc.
> 
> 
> So I still don't have a clear view about whether the patch should be
> applied.  (I think it clearly shouldn't be backported.)

I concur on backporting - but I think having it upstream will save the
folks who bind to the Python libxc code and eventually will hit this.

> 
> Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2016-05-04 20:04 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-11  1:42 [PATCH] Fix cpumap setting before passing to XEN Zhenzhong Duan
2016-04-11 11:27 ` Wei Liu
2016-04-12  3:35   ` Zhenzhong Duan
2016-04-18  4:57     ` Zhenzhong Duan
2016-04-20 14:33     ` Wei Liu
2016-04-22 18:15       ` Konrad Rzeszutek Wilk
2016-04-25 13:26         ` Ian Jackson
2016-04-26  3:54           ` Zhenzhong Duan
2016-04-28 15:07             ` Wei Liu
2016-04-28 17:02               ` Ian Jackson
2016-05-04 20:04                 ` Konrad Rzeszutek Wilk

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).