[sheepdog] [PATCH 4/6] sheep: add a helper to get vdi's inode

[MORITA Kazutaka]

> +
> +bool vdi_exist(uint32_t vid)
> +{
> +	struct sd_inode *inode;
> +	bool ret = false;
> +
> +	inode = vdi_get_inode(vid);
> +	if (!inode || vdi_is_deleted(inode))
>  		goto out;
> -	}
> -	ret = 1;
> +	ret = true;;

Redundant ';'.


> @@ -615,19 +619,9 @@ static void delete_one(struct work *work)
>  
>  	sd_dprintf("%d %d, %16x", dw->done, dw->count, vdi_id);
>  
> -	inode = malloc(sizeof(*inode));
> -	if (!inode) {
> -		sd_eprintf("failed to allocate memory");
> +	inode = vdi_get_inode(vdi_id);
> +	if (!inode)
>  		return;
> -	}
> -
> -	ret = read_backend_object(vid_to_vdi_oid(vdi_id),
> -			  (void *)inode, sizeof(*inode), 0);
> -
> -	if (ret != SD_RES_SUCCESS) {
> -		sd_eprintf("cannot find VDI object");
> -		goto out;
> -	}
>  
>  	if (inode->vdi_size == 0 && vdi_is_deleted(inode))
>  		goto out;

Is this change really correct?  read_backend_object() was introduced
to read the inode from the backend store directly.

    commit a0cd37999758318ea38fb36e1e9ba8a0fd1e970f
    Author: levin li <xingke.lwp at taobao.com>
    Date:   Wed Sep 5 19:33:56 2012 +0800
    
        sheep: read inode directly from backend in VDI deletion work
    
        This patch is preparation for next patch that deletes object cache
        when deleting VDI, if object cache deletion work is processed in
        notify, then it may causes race between cache deletion and next VDI
        deletion work, because VDI deletion work may needs to read its parent
        VDI inode data, so we can not read the inode data from cache

If we can safely read the inode from cache, please explain the reason
in the commit log.

Perhaps, vdi_get_inode() has to call read_backend_object() instead of
read_object()?

> @@ -697,31 +691,23 @@ static void delete_one_done(struct work *work)
>  
>  static int fill_vdi_list(struct deletion_work *dw, uint32_t root_vid)
>  {
> -	int ret, i;
> +	int i;
>  	struct sd_inode *inode = NULL;
>  	int done = dw->count;
>  	uint32_t vid;
>  
> -	inode = malloc(SD_INODE_HEADER_SIZE);
> -	if (!inode) {
> -		sd_eprintf("failed to allocate memory");
> -		goto err;
> -	}
> -
>  	dw->buf[dw->count++] = root_vid;
>  again:
>  	vid = dw->buf[done++];
> -	ret = read_backend_object(vid_to_vdi_oid(vid), (char *)inode,
> -			  SD_INODE_HEADER_SIZE, 0);
> +	inode = vdi_get_inode(vid);
> +	if (!inode)
> +		return 0;

We don't need the entire inode here.  Should we introduce
vdi_get_inode_header()?


>  static uint64_t get_vdi_root(uint32_t vid, bool *cloned)
>  {
> -	int ret;
>  	struct sd_inode *inode = NULL;
>  
>  	*cloned = false;
> -
> -	inode = malloc(SD_INODE_HEADER_SIZE);
> -	if (!inode) {
> -		sd_eprintf("failed to allocate memory");
> -		vid = 0;
> -		goto out;
> -	}
>  next:
> -	ret = read_backend_object(vid_to_vdi_oid(vid), (char *)inode,
> -			  SD_INODE_HEADER_SIZE, 0);
> +	inode = vdi_get_inode(vid);
> +	if (!inode)
> +		return 0;

Same here.

Thanks,

Kazutaka