[Sheepdog] [PATCH] sheep: remove unregister_event from process_event_queue()

[Christoph Hellwig]

On Wed, May 16, 2012 at 01:54:06AM +0800, Yunkai Zhang wrote:
> +static void get_vdi_bitmap_from_sd_list(struct sd_node *nodes, size_t nr_nodes)
>  {
>  	int i;
>  	/* fixme: we need this until starting up. */
>  
> -	for (i = 0; i < sys->nr_nodes; i++)
> -		get_vdi_bitmap_from(sys->nodes + i);
> +	for (i = 0; i < nr_nodes; i++)
> +		get_vdi_bitmap_from(nodes + i);
>  }

I'd suggest to simply kill get_vdi_bitmap_from_sd_list and opencode it
in __sd_join to make the code cleaner.

>  	/*
> +	 * w->member_list contains joining node, we should
> +	 * exclude it in following operation.
> +	 */
> +	for (i = 0, nr_nodes = 0; i < w->member_list_entries; i++) {
> +		if (node_eq(w->member_list + i, &w->joined))
> +			continue;
> +		nodes[nr_nodes++] = w->member_list[i];
> +	}
> +
> +	/*
>  	* If a new comer try to join the running cluster, it only need read
>  	* one copy of bitmap from the first member.
>  	*/
>  	if (sys_stat_wait_format())
> -		get_vdi_bitmap_from(w->member_list);
> +		get_vdi_bitmap_from(nodes);
>  	else {
> -		get_vdi_bitmap_from_sd_list();
> +		get_vdi_bitmap_from_sd_list(nodes, nr_nodes);
>  		for (i = 0; i < w->member_list_entries; i++)
>  			get_vdi_bitmap_from(w->member_list + i);

The else clause here iterates over member_list_entries twice, also
why create the local nodes array if we could simplify skip it in
the for loop.  E.g. why not:

	for (i = 0; i < w->member_list_entries; i++) {
		if (node_eq(&w->member_list[i], &w->joined))
			continue;
		get_vdi_bitmap_from(&w->member_list[i]);

		/*
		 * A newly joining node only needs to read a copy of the
		 * bitmap from one single member node.
		 */
	  	if (sys_stat_wait_format())
			break;
	}

> +	if (result == CJ_RES_SUCCESS || result == CJ_RES_MASTER_TRANSFER)
> +		update_cluster_info(jm, joined, members, nr_members);
> +

Previously we would only queue up an event for the CJ_RES_SUCCESS case,
while the CJ_RES_MASTER_TRANSFER case handles the updates itself already,
I think with your patch CJ_RES_MASTER_TRANSFER is handled twice now.