Re: [Xen-devel] e820_host default value and libxl (not xl)

2016-05-23 Thread Wei Liu 
On Mon, May 23, 2016 at 11:59:10AM +0100, Andrew Cooper wrote:
> On 23/05/16 11:47, Wei Liu wrote:
> > On Sat, May 21, 2016 at 04:42:11AM +0200, Marek Marczykowski-Górecki wrote:
> >> Hi,
> >>
> >> According to xl.cfg(5) " This option defaults to true (1) if any PCI
> >> passthrough devices are configured and false (0) otherwise."
> >> And indeed this behaviour is implemented in xl. But not in libxl, which
> >> means other libxl based toolstacks (libvirt) will not take advantage of
> >> this directly.
> >>
> >> What would be the best approach here? Duplicate that behaviour in
> >> libvirt (currently libvirt knows nothing about this option), or move
> >> that default handling to libxl? I think the later makes more sense, but
> >> maybe there is some reason against it?
> >>
> > The latter.
> >
> > I wouldn't be surprised if the boundary between xl and libxl was
> > overlooked when implementing this flag. I've done similar things to push
> > xsm label handling logic from xl to libxl.
> 
> Please don't propage this bandaid any further than it currently is.  It
> is not appropriate for libxl to set this by default.
> 
> The reason it is currently used is because libxl/libxc doesn't know how
> to lay out a guests physmap.  This is something I am working on
> resolving for some ballooning issues we are having in XenServer.
> 

This is not entirely true. It knows a version of it, but not the final
version, which is even worse than not knowing at all. :-)

> xl can get away defaulting this on, because xl is inherently a single
> host toolstack.  However, using host_e820 is wrong for any multi-host
> setup where the VM might plausibly migrate (which includes the
> passthrough case here).
> 

Libxl has to determine the value one way or another:

1. get that value from upper layer
2. derive from internal logic

#1 takes precedence. Currently the "internal logic" is "always off".
This makes life harder for toolstack other than xl, so making that
internal logic better looks acceptable to me.

On the other hand I agree this is only bandaid to bigger issue. The
objection is also quite convincing.

So living it as is is probably the safest option.

Wei.

> ~Andrew

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

Re: [Xen-devel] e820_host default value and libxl (not xl)

2016-05-23 Thread Konrad Rzeszutek Wilk 
On Mon, May 23, 2016 at 01:27:31PM +0200, Marek Marczykowski-Górecki wrote:
> On Mon, May 23, 2016 at 11:59:10AM +0100, Andrew Cooper wrote:
> > On 23/05/16 11:47, Wei Liu wrote:
> > > On Sat, May 21, 2016 at 04:42:11AM +0200, Marek Marczykowski-Górecki 
> > > wrote:
> > >> Hi,
> > >>
> > >> According to xl.cfg(5) " This option defaults to true (1) if any PCI
> > >> passthrough devices are configured and false (0) otherwise."
> > >> And indeed this behaviour is implemented in xl. But not in libxl, which
> > >> means other libxl based toolstacks (libvirt) will not take advantage of
> > >> this directly.
> > >>
> > >> What would be the best approach here? Duplicate that behaviour in
> > >> libvirt (currently libvirt knows nothing about this option), or move
> > >> that default handling to libxl? I think the later makes more sense, but
> > >> maybe there is some reason against it?
> > >>
> > > The latter.
> > >
> > > I wouldn't be surprised if the boundary between xl and libxl was
> > > overlooked when implementing this flag. I've done similar things to push
> > > xsm label handling logic from xl to libxl.
> > 
> > Please don't propage this bandaid any further than it currently is.  It
> > is not appropriate for libxl to set this by default.
> > 
> > The reason it is currently used is because libxl/libxc doesn't know how
> > to lay out a guests physmap.  This is something I am working on
> > resolving for some ballooning issues we are having in XenServer.
> 
> Does it mean e820_host wouldn't be needed anymore?

Presumarily also the other flags we have: mmio_hole ?
> 
> > xl can get away defaulting this on, because xl is inherently a single
> > host toolstack.  However, using host_e820 is wrong for any multi-host
> > setup where the VM might plausibly migrate (which includes the
> > passthrough case here).

It would be better if you could give the libxl/xl a e820 array record.

As in - one could jam an e820 that would be the same across various
servers.

> 
> But *currently* having host_e820 disabled makes it impossible to
> passthrough some devices, even when no migration is involved. Libvirt
> does not support e820_host. So, I'm looking for a solution for the
> problem, which makes it impossible to use some devices at all when using
> libvirt.
> 
> If e820_host will not be needed anymore in the near future, I can wait,
> or simply carry a local patch for this. But otherwise I think it would
> be good to fix this in either libxl or libvirt.
> 
> -- 
> 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.xen.org
> http://lists.xen.org/xen-devel


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

Re: [Xen-devel] e820_host default value and libxl (not xl)

2016-05-23 Thread Konrad Rzeszutek Wilk 
On Mon, May 23, 2016 at 11:47:48AM +0100, Wei Liu wrote:
> On Sat, May 21, 2016 at 04:42:11AM +0200, Marek Marczykowski-Górecki wrote:
> > Hi,
> > 
> > According to xl.cfg(5) " This option defaults to true (1) if any PCI
> > passthrough devices are configured and false (0) otherwise."
> > And indeed this behaviour is implemented in xl. But not in libxl, which
> > means other libxl based toolstacks (libvirt) will not take advantage of
> > this directly.
> > 
> > What would be the best approach here? Duplicate that behaviour in
> > libvirt (currently libvirt knows nothing about this option), or move
> > that default handling to libxl? I think the later makes more sense, but
> > maybe there is some reason against it?
> > 
> 
> The latter.
> 
> I wouldn't be surprised if the boundary between xl and libxl was
> overlooked when implementing this flag. I've done similar things to push

It never occurred to me when I wrote that code sadly :-(

> xsm label handling logic from xl to libxl.
> 
> Wei.
> 
> > -- 
> > 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.xen.org
> > http://lists.xen.org/xen-devel
> 
> 
> ___
> 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

Re: [Xen-devel] e820_host default value and libxl (not xl)

2016-05-23 Thread Marek Marczykowski-Górecki 
On Mon, May 23, 2016 at 11:59:10AM +0100, Andrew Cooper wrote:
> On 23/05/16 11:47, Wei Liu wrote:
> > On Sat, May 21, 2016 at 04:42:11AM +0200, Marek Marczykowski-Górecki wrote:
> >> Hi,
> >>
> >> According to xl.cfg(5) " This option defaults to true (1) if any PCI
> >> passthrough devices are configured and false (0) otherwise."
> >> And indeed this behaviour is implemented in xl. But not in libxl, which
> >> means other libxl based toolstacks (libvirt) will not take advantage of
> >> this directly.
> >>
> >> What would be the best approach here? Duplicate that behaviour in
> >> libvirt (currently libvirt knows nothing about this option), or move
> >> that default handling to libxl? I think the later makes more sense, but
> >> maybe there is some reason against it?
> >>
> > The latter.
> >
> > I wouldn't be surprised if the boundary between xl and libxl was
> > overlooked when implementing this flag. I've done similar things to push
> > xsm label handling logic from xl to libxl.
> 
> Please don't propage this bandaid any further than it currently is.  It
> is not appropriate for libxl to set this by default.
> 
> The reason it is currently used is because libxl/libxc doesn't know how
> to lay out a guests physmap.  This is something I am working on
> resolving for some ballooning issues we are having in XenServer.

Does it mean e820_host wouldn't be needed anymore?

> xl can get away defaulting this on, because xl is inherently a single
> host toolstack.  However, using host_e820 is wrong for any multi-host
> setup where the VM might plausibly migrate (which includes the
> passthrough case here).

But *currently* having host_e820 disabled makes it impossible to
passthrough some devices, even when no migration is involved. Libvirt
does not support e820_host. So, I'm looking for a solution for the
problem, which makes it impossible to use some devices at all when using
libvirt.

If e820_host will not be needed anymore in the near future, I can wait,
or simply carry a local patch for this. But otherwise I think it would
be good to fix this in either libxl or libvirt.

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


signature.asc
Description: PGP signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] e820_host default value and libxl (not xl)

2016-05-23 Thread Andrew Cooper 
On 23/05/16 11:47, Wei Liu wrote:
> On Sat, May 21, 2016 at 04:42:11AM +0200, Marek Marczykowski-Górecki wrote:
>> Hi,
>>
>> According to xl.cfg(5) " This option defaults to true (1) if any PCI
>> passthrough devices are configured and false (0) otherwise."
>> And indeed this behaviour is implemented in xl. But not in libxl, which
>> means other libxl based toolstacks (libvirt) will not take advantage of
>> this directly.
>>
>> What would be the best approach here? Duplicate that behaviour in
>> libvirt (currently libvirt knows nothing about this option), or move
>> that default handling to libxl? I think the later makes more sense, but
>> maybe there is some reason against it?
>>
> The latter.
>
> I wouldn't be surprised if the boundary between xl and libxl was
> overlooked when implementing this flag. I've done similar things to push
> xsm label handling logic from xl to libxl.

Please don't propage this bandaid any further than it currently is.  It
is not appropriate for libxl to set this by default.

The reason it is currently used is because libxl/libxc doesn't know how
to lay out a guests physmap.  This is something I am working on
resolving for some ballooning issues we are having in XenServer.

xl can get away defaulting this on, because xl is inherently a single
host toolstack.  However, using host_e820 is wrong for any multi-host
setup where the VM might plausibly migrate (which includes the
passthrough case here).

~Andrew

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

Re: [Xen-devel] e820_host default value and libxl (not xl)

2016-05-23 Thread Wei Liu 
On Sat, May 21, 2016 at 04:42:11AM +0200, Marek Marczykowski-Górecki wrote:
> Hi,
> 
> According to xl.cfg(5) " This option defaults to true (1) if any PCI
> passthrough devices are configured and false (0) otherwise."
> And indeed this behaviour is implemented in xl. But not in libxl, which
> means other libxl based toolstacks (libvirt) will not take advantage of
> this directly.
> 
> What would be the best approach here? Duplicate that behaviour in
> libvirt (currently libvirt knows nothing about this option), or move
> that default handling to libxl? I think the later makes more sense, but
> maybe there is some reason against it?
> 

The latter.

I wouldn't be surprised if the boundary between xl and libxl was
overlooked when implementing this flag. I've done similar things to push
xsm label handling logic from xl to libxl.

Wei.

> -- 
> 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.xen.org
> http://lists.xen.org/xen-devel


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

Re: [Xen-devel] e820_host default value and libxl (not xl)

2016-05-23 Thread Dario Faggioli 
On Sat, May 21, 2016 at 4:42 AM, Marek Marczykowski-Górecki
 wrote:
> Hi,
>
> According to xl.cfg(5) " This option defaults to true (1) if any PCI
> passthrough devices are configured and false (0) otherwise."
> And indeed this behaviour is implemented in xl. But not in libxl, which
> means other libxl based toolstacks (libvirt) will not take advantage of
> this directly.
>
> What would be the best approach here? Duplicate that behaviour in
> libvirt (currently libvirt knows nothing about this option), or move
> that default handling to libxl? I think the later makes more sense, but
> maybe there is some reason against it?
>
I'm no tools maintainers (and in fact I'm Cc-ing them), but I'd say
that moving this to libxl is what looks to me to make more sense...

Dario
-- 
<> (Raistlin Majere)
---
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R Ltd., Cambridge (UK)

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