Re: [Xen-devel] [PATCH 2/6] libxl: events: Deregister xenstore watch fd when not needed

2014-12-09 Thread Ian Campbell
On Tue, 2014-12-09 at 15:54 +, Ian Jackson wrote:
 We want to have no fd events registered when we are idle.
 In this patch, deal with the xenstore watch fd:
 
 * Track the total number of active watches.
 * When deregistering a watch, or when watch registration fails, check
   whether there are now no watches and if so deregister the fd.
 * On libxl teardown, the watch fd should therefore be unregistered.
   assert that this is the case.
 
 Signed-off-by: Ian Jackson ian.jack...@eu.citrix.com

Acked-by: Ian Campbell ian.campb...@citrix.com


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


Re: [Xen-devel] [PATCH 2/6] libxl: events: Deregister xenstore watch fd when not needed

2014-12-09 Thread Ian Jackson
Ian Campbell writes (Re: [PATCH 2/6] libxl: events: Deregister xenstore watch 
fd when not needed):
 On Tue, 2014-12-09 at 15:54 +, Ian Jackson wrote:
  We want to have no fd events registered when we are idle.
  In this patch, deal with the xenstore watch fd:
...
  Signed-off-by: Ian Jackson ian.jack...@eu.citrix.com
 
 Acked-by: Ian Campbell ian.campb...@citrix.com

Thanks.  In fact you had acked this already but somehow I have dropped
that.

Ian.

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


Re: [Xen-devel] [PATCH 2/6] libxl: events: Deregister xenstore watch fd when not needed

2014-11-28 Thread Ian Campbell
On Thu, 2014-11-27 at 18:27 +, Ian Jackson wrote:
 * On libxl teardown, the watch fd should therefore be unregistered.
   assert that this is the case.

A bunch of the patches in this series basically assume that the ctx is
idle when it is freed, i.e. it requires everything to be explicitly
cancelled rather than implicitly doing so on free.

I think this is a fine restriction, but it probably ought to be written
down.

Ian.


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


Re: [Xen-devel] [PATCH 2/6] libxl: events: Deregister xenstore watch fd when not needed

2014-11-28 Thread Ian Campbell
On Thu, 2014-11-27 at 18:27 +, Ian Jackson wrote:
 We want to have no fd events registered when we are idle.
 In this patch, deal with the xenstore watch fd:
 
 * Track the total number of active watches.
 * When deregistering a watch, or when watch registration fails, check
   whether there are now no watches and if so deregister the fd.
 * On libxl teardown, the watch fd should therefore be unregistered.
   assert that this is the case.
 
 Signed-off-by: Ian Jackson ian.jack...@eu.citrix.com

Acked-by: Ian Campbell ian.campb...@citrix.com




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


Re: [Xen-devel] [PATCH 2/6] libxl: events: Deregister xenstore watch fd when not needed

2014-11-28 Thread Ian Jackson
Ian Campbell writes (Re: [PATCH 2/6] libxl: events: Deregister xenstore watch 
fd when not needed):
 On Thu, 2014-11-27 at 18:27 +, Ian Jackson wrote:
  * On libxl teardown, the watch fd should therefore be unregistered.
assert that this is the case.
 
 A bunch of the patches in this series basically assume that the ctx is
 idle when it is freed, i.e. it requires everything to be explicitly
 cancelled rather than implicitly doing so on free.

libxl_ctx_free explicitly disables all the application-requested event
generators.  (free_disable_deaths and libxl__evdisable_disk_eject.)

Destroying the ctx during the execution of an asynchronous operation
is forbidden by this text in libxl.h (near line 813):
  * *ao_how does not need to remain valid after the initiating function
  * returns. All other parameters must remain valid for the lifetime of
  * the asynchronous operation, unless otherwise specified.
That implies that the ctx must remain valid during the ao, so it may
not be destroyed beforehand.

Those are the two ways that, even without any threads inside libxl, a
ctx can be other than idle.

It should be obvious to the application programmer that destroying the
ctx when there are other threads inside libxl is not going to work.
Indeed a programmer who tries to destroy the ctx when they have
threads which might be inside libxl cannot ensure that the ctx is
valid even on entry to libxl.

 I think this is a fine restriction, but it probably ought to be written
 down.

Does the above demonstrate that the existing restrictions are
documented ?  I'd rather avoid writing the restrictions twice if it
can be avoided - these docs are long enough as they are.

Ian.

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


Re: [Xen-devel] [PATCH 2/6] libxl: events: Deregister xenstore watch fd when not needed

2014-11-28 Thread Ian Campbell
On Fri, 2014-11-28 at 14:56 +, Ian Jackson wrote:
 Ian Campbell writes (Re: [PATCH 2/6] libxl: events: Deregister xenstore 
 watch fd when not needed):
  On Thu, 2014-11-27 at 18:27 +, Ian Jackson wrote:
   * On libxl teardown, the watch fd should therefore be unregistered.
 assert that this is the case.
  
  A bunch of the patches in this series basically assume that the ctx is
  idle when it is freed, i.e. it requires everything to be explicitly
  cancelled rather than implicitly doing so on free.
 
 libxl_ctx_free explicitly disables all the application-requested event
 generators.  (free_disable_deaths and libxl__evdisable_disk_eject.)

So it does, I was looking for that before commenting but didn't see
those calls for what they were, despite the comment right there...

 Destroying the ctx during the execution of an asynchronous operation
 is forbidden by this text in libxl.h (near line 813):
   * *ao_how does not need to remain valid after the initiating function
   * returns. All other parameters must remain valid for the lifetime of
   * the asynchronous operation, unless otherwise specified.
 That implies that the ctx must remain valid during the ao, so it may
 not be destroyed beforehand.
 
 Those are the two ways that, even without any threads inside libxl, a
 ctx can be other than idle.
 
 It should be obvious to the application programmer that destroying the
 ctx when there are other threads inside libxl is not going to work.
 Indeed a programmer who tries to destroy the ctx when they have
 threads which might be inside libxl cannot ensure that the ctx is
 valid even on entry to libxl.
 
  I think this is a fine restriction, but it probably ought to be written
  down.
 
 Does the above demonstrate that the existing restrictions are
 documented ?

Yes.

   I'd rather avoid writing the restrictions twice if it
 can be avoided - these docs are long enough as they are.

Indeed.



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