[sheepdog] [PATCH v2 1/2] sheep, dog: make recycling VID selectable

[Hitoshi Mitake]

On Mon, Mar 16, 2015 at 7:42 PM, Liu Yuan <namei.unix at gmail.com> wrote:
> On Mon, Mar 16, 2015 at 03:57:40PM +0900, Hitoshi Mitake wrote:
>> This patch adds a new option -R to "dog cluster format". If user
>> specifies the option during cluster format, the cluster will enable
>> recycling VID (disabled in default).
>>
>> Signed-off-by: Hitoshi Mitake <mitake.hitoshi at lab.ntt.co.jp>
>> ---
>>  dog/cluster.c            | 9 ++++++++-
>>  include/internal_proto.h | 1 +
>>  sheep/ops.c              | 4 +++-
>>  sheep/vdi.c              | 2 +-
>>  4 files changed, 13 insertions(+), 3 deletions(-)
>>
>> diff --git a/dog/cluster.c b/dog/cluster.c
>> index e252316..bb61fcc 100644
>> --- a/dog/cluster.c
>> +++ b/dog/cluster.c
>> @@ -25,6 +25,7 @@ static struct sd_option cluster_options[] = {
>>       {'l', "lock", false, "Lock vdi to exclude multiple users"},
>>       {'m', "multithread", false,
>>        "use multi-thread for 'cluster snapshot save'"},
>> +     {'R', "recyclevid", false, "enable recycling of VID"},
>>       {'t', "strict", false,
>>        "do not serve write request if number of nodes is not sufficient"},
>>       {'z', "block_size_shift", true, "specify the shift num of default"
>> @@ -43,6 +44,7 @@ static struct cluster_cmd_data {
>>       char name[STORE_LEN];
>>       bool fixed_vnodes;
>>       bool use_lock;
>> +     bool recycle_vid;
>>  } cluster_cmd_data;
>>
>>  #define DEFAULT_STORE        "plain"
>> @@ -169,6 +171,8 @@ static int cluster_format(int argc, char **argv)
>>               hdr.cluster.flags |= SD_CLUSTER_FLAG_STRICT;
>>       if (cluster_cmd_data.use_lock)
>>               hdr.cluster.flags |= SD_CLUSTER_FLAG_USE_LOCK;
>> +     if (cluster_cmd_data.recycle_vid)
>> +             hdr.cluster.flags |= SD_CLUSTER_FLAG_RECYCLE_VID;
>>
>>  #ifdef HAVE_DISKVNODES
>>       hdr.cluster.flags |= SD_CLUSTER_FLAG_DISKMODE;
>> @@ -820,7 +824,7 @@ failure:
>>  static struct subcommand cluster_cmd[] = {
>>       {"info", NULL, "aprhvT", "show cluster information",
>>        NULL, CMD_NEED_NODELIST, cluster_info, cluster_options},
>> -     {"format", NULL, "bcltaphzTV", "create a Sheepdog store",
>> +     {"format", NULL, "bcltaphzTVR", "create a Sheepdog store",
>>        NULL, CMD_NEED_NODELIST, cluster_format, cluster_options},
>>       {"shutdown", NULL, "aphT", "stop Sheepdog",
>>        NULL, 0, cluster_shutdown, cluster_options},
>> @@ -890,6 +894,9 @@ static int cluster_parser(int ch, const char *opt)
>>       case 'V':
>>               cluster_cmd_data.fixed_vnodes = true;
>>               break;
>> +     case 'R':
>> +             cluster_cmd_data.recycle_vid = true;
>> +             break;
>>       }
>>
>>       return 0;
>> diff --git a/include/internal_proto.h b/include/internal_proto.h
>> index defbe6d..3a370b6 100644
>> --- a/include/internal_proto.h
>> +++ b/include/internal_proto.h
>> @@ -157,6 +157,7 @@
>>  #define SD_CLUSTER_FLAG_DISKMODE     0x0002 /* Disk mode for cluster */
>>  #define SD_CLUSTER_FLAG_AUTO_VNODES  0x0004 /* Cluster vnodes strategy */
>>  #define SD_CLUSTER_FLAG_USE_LOCK     0x0008 /* Lock/Unlock vdi */
>> +#define SD_CLUSTER_FLAG_RECYCLE_VID  0x0010 /* Enable recycling of VID */
>>
>>
>>  enum sd_status {
>> diff --git a/sheep/ops.c b/sheep/ops.c
>> index 0e5ac64..8c79a84 100644
>> --- a/sheep/ops.c
>> +++ b/sheep/ops.c
>> @@ -198,7 +198,9 @@ static int post_cluster_del_vdi(const struct sd_req *req, struct sd_rsp *rsp,
>>       if (ret == SD_RES_SUCCESS) {
>>               atomic_set_bit(vid, sys->vdi_deleted);
>>               vdi_mark_deleted(vid);
>> -             run_vid_gc(vid);
>> +
>> +             if (sys->cinfo.flags & SD_CLUSTER_FLAG_RECYCLE_VID)
>> +                     run_vid_gc(vid);
>>       }
>>
>>       if (!sys->enable_object_cache)
>> diff --git a/sheep/vdi.c b/sheep/vdi.c
>> index 8114fb5..eac0367 100644
>> --- a/sheep/vdi.c
>> +++ b/sheep/vdi.c
>> @@ -384,7 +384,7 @@ static int do_add_vdi_state(uint32_t vid, int nr_copies, bool snapshot,
>>               already_exists = true;
>>       }
>>
>> -     if (!already_exists)
>> +     if (sys->cinfo.flags & SD_CLUSTER_FLAG_RECYCLE_VID && !already_exists)
>>               update_vdi_family(parent_vid, entry, unordered);
>>
>>       sd_rw_unlock(&vdi_state_lock);
>> --
>> 1.9.1
>>
>> --
>> sheepdog mailing list
>> sheepdog at lists.wpkg.org
>> https://lists.wpkg.org/mailman/listinfo/sheepdog
>
> It seems that your patch 21549a1bd4981fabcc09d062a647162127fe0637
>
> Author: Hitoshi Mitake <mitake.hitoshi at gmail.com>
> Date:   Sun Jun 1 23:23:18 2014 +0900
>
>     sheep: don't recycle VDI ID
>
>     Recycling VDI IDs of deleted VDIs is a completely wrong idea. It
>     breaks relations between inode objects and data objects. For example,
>     it can cause a problem of corrupting cloned VDIs (see related
>     issue). This patch forbids the recycling.
>
>     Related issue:
>     https://bugs.launchpad.net/sheepdog-project/+bug/1317755
>
> conflict with your current vid recycling. I think it is not hard to allow
> vid recycle for snapshot create, I'd give it a try.
>

No, they don't conflict. The old policy of recycling has a possibility
of data corruption (the issue of launchpad describes). And new one
avoid the problem because it recycle VIDs with considering VDI family
relations.

Thanks,
Hitoshi