[sheepdog] [PATCH v2] dog: allow vdi snapshot without snapshot tag

[Ruoyu]

On 2014年09月19日 10:30, Hitoshi Mitake wrote:
> At Fri, 19 Sep 2014 10:22:07 +0800,
> Ruoyu wrote:
>> The problem is still existed.
> Really? On my environment, I couldn't reproduce the problem. Can I see
> your command sequence?
$ dog/dog cluster format
__
()'`;
/\|`
/ | Caution! The cluster is not empty.
(/_)_|_ Are you sure you want to continue? [yes/no]: yes
using backend plain store
$ dog/dog vdi create test 4M -P
100.0 % [=====================================================] 4.0 MB /
4.0 MB
$ dog/dog vdi snapshot test <-- it is allowed
$ dog/dog vdi snapshot test <-- should be forbidden
$ dog/dog vdi list <-- both snapshots tagged with ""
Name Id Size Used Shared Creation time VDI id Copies Tag
s test 1 4.0 MB 4.0 MB 0.0 MB 2014-09-19 10:36 7c2b25 3
s test 2 4.0 MB 0.0 MB 4.0 MB 2014-09-19 10:36 7c2b26 3
test 0 4.0 MB 0.0 MB 4.0 MB 2014-09-19 10:36 7c2b27 3

>
>> How about disabling empty snapshot tag which will make it simple?
> It introduces big incompatibility. I want to avoid it.
>
> Thanks,
> Hitoshi
>
>> On 2014年09月19日 09:37, Hitoshi Mitake wrote:
>>> The commit a21bf27906b23448b92cca9943e1019105ffac2f makes
>>> $ dog vdi snapshot <vdi>
>>> fail if the <vdi> is an ordinal vdi because newly created VDIs have
>>> snapid 0. This patch avoids the failure with checking the VDI is
>>> snapshot or not.
>>>
>>> Cc: Ruoyu <liangry at ucweb.com>
>>> Signed-off-by: Hitoshi Mitake <mitake.hitoshi at lab.ntt.co.jp>
>>> ---
>>>   dog/vdi.c | 30 ++++++++++++++++++++++--------
>>>   1 file changed, 22 insertions(+), 8 deletions(-)
>>>
>>> v2: correct vdi existence checking
>>>
>>> diff --git a/dog/vdi.c b/dog/vdi.c
>>> index fa6130e..58b0a71 100644
>>> --- a/dog/vdi.c
>>> +++ b/dog/vdi.c
>>> @@ -567,6 +567,7 @@ static int vdi_snapshot(int argc, char **argv)
>>>   	int vs_count = 0;
>>>   	struct node_id owners[SD_MAX_COPIES];
>>>   	int nr_owners = 0, nr_issued_prevent_inode_update = 0;
>>> +	bool fail_if_snapshot = false;
>>>   
>>>   	if (vdi_cmd_data.snapshot_id != 0) {
>>>   		sd_err("Please specify a non-integer value for "
>>> @@ -584,16 +585,29 @@ static int vdi_snapshot(int argc, char **argv)
>>>   	case SD_RES_NO_TAG:
>>>   		break;
>>>   	default:
>>> -		sd_err("Failed to create snapshot for %s, maybe "
>>> -			"snapshot id (%d) or tag (%s) is existed",
>>> -			vdiname, vdi_cmd_data.snapshot_id,
>>> -			vdi_cmd_data.snapshot_tag);
>>> -		return EXIT_FAILURE;
>>> +		fail_if_snapshot = true;
>>> +		break;
>>>   	}
>>>   
>>> -	ret = read_vdi_obj(vdiname, 0, "", &vid, inode, SD_INODE_HEADER_SIZE);
>>> -	if (ret != EXIT_SUCCESS)
>>> -		return ret;
>>> +	if (fail_if_snapshot) {
>>> +		ret = dog_read_object(vid_to_vdi_oid(vid), inode,
>>> +				      SD_INODE_HEADER_SIZE, 0, true);
>>> +		if (ret != EXIT_SUCCESS)
>>> +			return ret;
>>> +
>>> +		if (vdi_is_snapshot(inode)) {
>>> +			sd_err("Failed to create snapshot for %s, maybe "
>>> +			       "snapshot id (%d) or tag (%s) is existed",
>>> +			       vdiname, vdi_cmd_data.snapshot_id,
>>> +			       vdi_cmd_data.snapshot_tag);
>>> +			return EXIT_FAILURE;
>>> +		}
>>> +	} else {
>>> +		ret = read_vdi_obj(vdiname, 0, "", &vid, inode,
>>> +				   SD_INODE_HEADER_SIZE);
>>> +		if (ret != EXIT_SUCCESS)
>>> +			return ret;
>>> +	}
>>>   
>>>   	if (inode->store_policy) {
>>>   		sd_err("creating a snapshot of hypervolume is not supported");
>>