Re: [Xen-devel] [PATCH v3 02/12] xenstore: call add_change_node() directly when writing node

[Juergen Gross <jgross@xxxxxxxx>]

On 12/11/16 16:10, Wei Liu wrote:
> On Fri, Nov 11, 2016 at 09:00:00AM +0100, Juergen Gross wrote:
>> Instead of calling add_change_node() at places where write_node() is
>> called, do that inside write_node().
>>
>> Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
> 
> 
> There  seems to be a subtle change in behaviour -- previously in
> create_node, there is no add_chnage_node called. Now it has.

No change. add_change_node() has been called after calling create_node()

> 
>> ---
>>  tools/xenstore/xenstored_core.c | 7 +++----
>>  1 file changed, 3 insertions(+), 4 deletions(-)
>>
>> diff --git a/tools/xenstore/xenstored_core.c 
>> b/tools/xenstore/xenstored_core.c
>> index de1a9b4..1354387 100644
>> --- a/tools/xenstore/xenstored_core.c
>> +++ b/tools/xenstore/xenstored_core.c
>> @@ -456,7 +456,7 @@ static struct node *read_node(struct connection *conn, 
>> const void *ctx,
>>      return node;
>>  }
>>  
>> -static bool write_node(struct connection *conn, const struct node *node)
>> +static bool write_node(struct connection *conn, struct node *node)
>>  {
>>      /*
>>       * conn will be null when this is called from manual_node.
>> @@ -476,6 +476,8 @@ static bool write_node(struct connection *conn, const 
>> struct node *node)
>>      if (domain_is_unprivileged(conn) && data.dsize >= quota_max_entry_size)
>>              goto error;
>>  
>> +    add_change_node(conn, node, false);
>> +
> 
> Another subtle change of behaviour -- there is another goto error after
> this, which means the change is not made as far as the caller is
> concerned if that path is taken.

That's a case which should not be of any concern: it means that
the xenstore data base is corrupt. A stale watch event is the
least problem then.

> Not saying that all these changes are wrong, but they are worth pointing
> out and probably we should put the reasoning into commit message.

I'll add some words for this case.


Juergen

> 
>>      data.dptr = talloc_size(node, data.dsize);
>>      ((uint32_t *)data.dptr)[0] = node->num_perms;
>>      ((uint32_t *)data.dptr)[1] = node->datalen;
>> @@ -976,7 +978,6 @@ static void do_write(struct connection *conn, struct 
>> buffered_data *in)
>>              }
>>      }
>>  
>> -    add_change_node(conn, node, false);
>>      fire_watches(conn, in, name, false);
>>      send_ack(conn, XS_WRITE);
>>  }
>> @@ -1007,7 +1008,6 @@ static void do_mkdir(struct connection *conn, struct 
>> buffered_data *in)
>>                      send_error(conn, errno);
>>                      return;
>>              }
>> -            add_change_node(conn, node, false);
>>              fire_watches(conn, in, name, false);
>>      }
>>      send_ack(conn, XS_MKDIR);
>> @@ -1209,7 +1209,6 @@ static void do_set_perms(struct connection *conn, 
>> struct buffered_data *in)
>>              return;
>>      }
>>  
>> -    add_change_node(conn, node, false);
>>      fire_watches(conn, in, name, false);
>>      send_ack(conn, XS_SET_PERMS);
>>  }
>> -- 
>> 2.6.6
>>
> 


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