Re: host events - unused/unneeded values

[Federico Simoncelli]

----- Original Message -----
>  From: "David Teigland" <teigland(a)redhat.com&gt;
>  To: "Nir Soffer" <nsoffer(a)redhat.com&gt;
>  Cc: "Allon Mureinik" <amureini(a)redhat.com&gt;,
> sanlock-devel(a)lists.fedorahosted.org
>  Sent: Monday, August 11, 2014 5:49:28 PM
>  Subject: Re: host events - unused/unneeded values
>  
>  On Sun, Aug 10, 2014 at 01:44:23PM -0400, Nir Soffer wrote:
>  > Hi David,
>  > 
>  > While testing the python bindings, I noticed that get_event always return
>  > host_id=0:
>  > 
>  > {'data': 4,
>  >  'event': 3,
>  >  'from_generation': 13,
>  >  'from_host_id': 3,
>  >  'generation': 22,
>  >  'host_id': 0}
>  > 
>  > If I'm not wrong, sanlock needs the host_id only when sending events, so
>  > this is
>  > field is not used when sending events to listeners.
>  > 
>  > The generation field also seems unneeded, as sanlock filter evnets with
>  > wrong
>  > generation, so you will never get an event with generation other then the
>  > current
>  > generation - right?
>  > 
>  > Maybe we should change the api so:
>  > 
>  > When sending events:
>  > 
>  >     host_id = recipient host id (to)
>  >     generation = recipient generation
>  > 
>  > When receiving events:
>  > 
>  >     host_id = sender host id (from)
>  >     generation = sender generation
>  > 
>  > So we can eliminate the from_host_id and from_generation in get_event,
>  > simplifying
>  > the api and eliminating some reads/writes.
>  > 
>  > I can eliminate the unneeded fields in the python bindings, but I prefer to
>  > return
>  > the same data you get in the c api.
>  > 
>  > what do you think?
>  
>  I that approach when writing it, and it became messy and confusing, so I
>  chose instead that the sanlk_host_event struct that is received from
>  get_event is the same sanlk_host_event struct that was sent in set_event.
>  
>  Technically, the host_id field should not be zero on the destination, though,
>  it should be the host_id of the destination itself.  This commit changes
>  that:
>  
>  diff --git a/src/lockspace.c b/src/lockspace.c
>  index 8309d25a11f5..316cad7e6b79 100644
>  --- a/src/lockspace.c
>  +++ b/src/lockspace.c
>  @@ -337,6 +337,7 @@ void check_other_leases(struct space *sp, char *buf)
>                   */
>   
>                  memset(&he, 0, sizeof(he));
>  +               he.host_id = sp->host_id;
>                  he.generation = leader->write_id;
>                  he.event = leader->write_generation;
>                  he.data = leader->write_timestamp; 
+1

Minor suggestion, names I'd prefer are:

{'data': 4,
 'event': 3,
 'src_generation': 13,
 'src_host_id': 3,
 'dst_generation': 22,
 'dst_host_id': 0}

-- 
Federico