On 2014年09月19日 10:46, Hitoshi Mitake wrote:
> At Fri, 19 Sep 2014 10:40:38 +0800,
> Ruoyu wrote:
>>
>> 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
> Ah, sorry. The empty snapshot tag "" is a thing which should be
> ignored. So the above behavior is correct. Current master forbids it
> and Valerio reported it as a problem.
>
> v2 patch forbids command sequence like this:
>
> $ dog/dog vdi snapshot test -s asdf
> $ dog/dog vdi snapshot test -s asdf
> Failed to create snapshot for test, maybe snapshot id (0) or tag (asdf) is
> existed
>
> Actually, v2 can pass 030 test which consists a test case for
> duplicated snapshot tags.
It is OK if empty snapshot tag "" is allowed.
I am afraid whether empty snapshot tag could cause an unexpected problem.
>
> Thanks,
> Hitoshi
>
>>>> 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 <lian...@ucweb.com>
>>>>> Signed-off-by: Hitoshi Mitake <mitake.hito...@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");
>>
--
sheepdog mailing list
sheepdog@lists.wpkg.org
http://lists.wpkg.org/mailman/listinfo/sheepdog
