[sheepdog] [PATCH] sheep: cleaning vdi deletion process of sheep daemon

[Liu Yuan]

On Thu, Dec 26, 2013 at 03:53:51PM +0900, Hitoshi Mitake wrote:
> Current code of vdi deletion process in sheep deamon is a real
> disaster. Especially, its irregular usage of work queue is terrible
> thing for maintenance. In addition, the vdi deletion process can
> potentially notify a completion of deletion to a dog process before
> actual completion.
> 
> This patch cleans the VDI deletion code and removes the above bug.
> 
> Signed-off-by: Hitoshi Mitake <mitake.hitoshi at lab.ntt.co.jp>
> ---
>  sheep/vdi.c |  301 ++++++++++++++++++++++++++++++++++-------------------------
>  1 file changed, 176 insertions(+), 125 deletions(-)
> 
> diff --git a/sheep/vdi.c b/sheep/vdi.c
> index eca4f43..49038c5 100644
> --- a/sheep/vdi.c
> +++ b/sheep/vdi.c
> @@ -754,22 +754,6 @@ int vdi_snapshot(const struct vdi_iocb *iocb, uint32_t *new_vid)
>  				  info.vid);
>  }
>  
> -static int start_deletion(struct request *req, uint32_t vid);
> -
> -int vdi_delete(const struct vdi_iocb *iocb, struct request *req)
> -{
> -	struct vdi_info info;
> -	int ret;
> -
> -	ret = vdi_lookup(iocb, &info);
> -	if (ret != SD_RES_SUCCESS)
> -		goto out;
> -
> -	ret = start_deletion(req, info.vid);
> -out:
> -	return ret;
> -}
> -
>  int read_vdis(char *data, int len, unsigned int *rsp_len)
>  {
>  	if (len != sizeof(sys->vdi_inuse))
> @@ -781,28 +765,35 @@ int read_vdis(char *data, int len, unsigned int *rsp_len)
>  	return SD_RES_SUCCESS;
>  }
>  
> -struct deletion_work {
> -	uint32_t done;
> +struct deletion_info {
> +	uint32_t target_vid;
> +
> +	int delete_vid_count;
> +	uint32_t *delete_vid_array;
> +
> +	int done_deletion;
>  
> +	int finish_fd;
> +	struct list_node list;	/* connected deletion_info_queue */
> +};
> +
> +struct deletion_work {
>  	struct work work;
> -	struct list_node list;
> -	struct request *req;
>  
>  	uint32_t vid;
> -
> -	int count;
> -	uint32_t *buf;
> +	struct deletion_info *di;
>  };
>  
> -static LIST_HEAD(deletion_work_list);
> +static pthread_mutex_t deletion_info_queue_lock = PTHREAD_MUTEX_INITIALIZER;
> +static LIST_HEAD(deletion_info_queue);
>  
> -static int delete_inode(struct deletion_work *dw)
> +static int delete_inode(uint32_t vid)
>  {
>  	struct sd_inode *inode = NULL;
>  	int ret = SD_RES_SUCCESS;
>  
>  	inode = xzalloc(sizeof(*inode));
> -	ret = sd_read_object(vid_to_vdi_oid(dw->vid), (char *)inode,
> +	ret = sd_read_object(vid_to_vdi_oid(vid), (char *)inode,
>  			     SD_INODE_HEADER_SIZE, 0);
>  	if (ret != SD_RES_SUCCESS) {
>  		ret = SD_RES_EIO;
> @@ -811,7 +802,7 @@ static int delete_inode(struct deletion_work *dw)
>  
>  	memset(inode->name, 0, sizeof(inode->name));
>  
> -	ret = sd_write_object(vid_to_vdi_oid(dw->vid), (char *)inode,
> +	ret = sd_write_object(vid_to_vdi_oid(vid), (char *)inode,
>  			      SD_INODE_HEADER_SIZE, 0, false);
>  	if (ret != 0) {
>  		ret = SD_RES_EIO;
> @@ -871,16 +862,15 @@ static void delete_cb(void *data, enum btree_node_type type, void *arg)
>  	}
>  }
>  
> -static void delete_one(struct work *work)
> +static void delete_one_vdi(struct work *work)
>  {
> -	struct deletion_work *dw = container_of(work, struct deletion_work, work);
> -	uint32_t vdi_id = *(dw->buf + dw->count - dw->done - 1);
> +	struct deletion_work *dw =
> +		container_of(work, struct deletion_work, work);
> +	uint32_t vdi_id = dw->vid;
>  	int ret;
>  	uint32_t i, nr_deleted, nr_objs;
>  	struct sd_inode *inode = NULL;
>  
> -	sd_debug("%d %d, %16x", dw->done, dw->count, vdi_id);
> -
>  	inode = malloc(sizeof(*inode));
>  	if (!inode) {
>  		sd_err("failed to allocate memory");
> @@ -942,74 +932,111 @@ out:
>  	free(inode);
>  }
>  
> -static void delete_one_done(struct work *work)
> +static void delete_one_vdi_done(struct work *work);
> +
> +static struct deletion_work *create_next_deletion_work(struct deletion_info *di)
>  {
> -	struct deletion_work *dw = container_of(work, struct deletion_work, work);
> -	struct request *req = dw->req;
> +	struct deletion_work *dw = NULL;
>  
> -	dw->done++;
> -	if (dw->done < dw->count) {
> -		queue_work(sys->deletion_wqueue, &dw->work);
> -		return;
> +	if (!(di->done_deletion < di->delete_vid_count))
> +		/* no more deletion work, this deletion info is completed */

simply check di->done_deletion >= di->delete_vid_count

Thanks
Yuan