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

[Wei Liu <wei.liu2@citrix.com>]

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@suse.com>


There  seems to be a subtle change in behaviour -- previously in
create_node, there is no add_chnage_node called. Now it has.

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

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

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