On Mon, Mar 16, 2015 at 09:39:35PM +0900, Hitoshi Mitake wrote: > On Mon, Mar 16, 2015 at 7:42 PM, Liu Yuan <[email protected]> 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 <[email protected]> > >> --- > >> 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 > >> [email protected] > >> https://lists.wpkg.org/mailman/listinfo/sheepdog > > > > It seems that your patch 21549a1bd4981fabcc09d062a647162127fe0637 > > > > Author: Hitoshi Mitake <[email protected]> > > 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.
How about make 'dog vdi clone --no-share' as the default clone operation? And we can add dog vdi clone --share to keep old behavior as optional. By this manner, --no-share will save us from this kind of subtle problem. And your team problem about vdi exhaustion will be achieved :). This manner is not perfect, but it will benefit us: 1. stable code base since old algorithm is long tested. 2. we won't degrate vdi_lookup Thanks, Yuan -- sheepdog mailing list [email protected] https://lists.wpkg.org/mailman/listinfo/sheepdog
