From: nathan Date: Tue, 23 Sep 2008 18:05:02 +0000 (+0000) Subject: b=14836 X-Git-Tag: v1_9_80~66 X-Git-Url: https://git.whamcloud.com/?a=commitdiff_plain;h=87d57fbf6b04fa3d644e2b197c59c1796197f122;p=fs%2Flustre-release.git b=14836 i=johann (att 19293) i=nathan (att 19334) i=jc.lafoucriere att 19293: locking fixes, error condition checks, remove alloc/free in qos_calc_rr att 19334: remove code duplication between llapi_file_create{,_pool}; deprecate positional setstripe params --- diff --git a/lustre/lov/lov_obd.c b/lustre/lov/lov_obd.c index 2456372..5d3f3cc 100644 --- a/lustre/lov/lov_obd.c +++ b/lustre/lov/lov_obd.c @@ -890,9 +890,6 @@ static int lov_setup(struct obd_device *obd, struct lustre_cfg *lcfg) desc->ld_active_tgt_count = 0; lov->desc = *desc; lov->lov_tgt_size = 0; - rc = lov_ost_pool_init(&lov->lov_packed, 0); - if (rc) - RETURN(rc); sema_init(&lov->lov_lock, 1); atomic_set(&lov->lov_refcount, 0); @@ -905,11 +902,17 @@ static int lov_setup(struct obd_device *obd, struct lustre_cfg *lcfg) lov->lov_qos.lq_prio_free = 232; lov->lov_pools_hash_body = lustre_hash_init("POOLS", 128, 128, - &pool_hash_operations, - 0); - + &pool_hash_operations, 0); CFS_INIT_LIST_HEAD(&lov->lov_pool_list); lov->lov_pool_count = 0; + rc = lov_ost_pool_init(&lov->lov_packed, 0); + if (rc) + RETURN(rc); + rc = lov_ost_pool_init(&lov->lov_qos.lq_rr.lqr_pool, 0); + if (rc) { + lov_ost_pool_free(&lov->lov_packed); + RETURN(rc); + } lprocfs_lov_init_vars(&lvars); lprocfs_obd_setup(obd, lvars.obd_vars); @@ -962,19 +965,18 @@ static int lov_cleanup(struct obd_device *obd) struct list_head *pos, *tmp; struct pool_desc *pool; + lprocfs_obd_cleanup(obd); + list_for_each_safe(pos, tmp, &lov->lov_pool_list) { pool = list_entry(pos, struct pool_desc, pool_list); list_del(&pool->pool_list); - lustre_hash_del_key(lov->lov_pools_hash_body, pool->pool_name); lov_ost_pool_free(&(pool->pool_rr.lqr_pool)); lov_ost_pool_free(&(pool->pool_obds)); - OBD_FREE(pool, sizeof(*pool)); + OBD_FREE_PTR(pool); } - lustre_hash_exit(lov->lov_pools_hash_body); - - lprocfs_obd_cleanup(obd); - + lov_ost_pool_free(&(lov->lov_qos.lq_rr.lqr_pool)); lov_ost_pool_free(&lov->lov_packed); + lustre_hash_exit(lov->lov_pools_hash_body); if (lov->lov_tgts) { int i; @@ -999,8 +1001,6 @@ static int lov_cleanup(struct obd_device *obd) lov->lov_tgt_size = 0; } - lov_ost_pool_free(&(lov->lov_qos.lq_rr.lqr_pool)); - RETURN(0); } diff --git a/lustre/lov/lov_pool.c b/lustre/lov/lov_pool.c index 05fde47..95846ce 100644 --- a/lustre/lov/lov_pool.c +++ b/lustre/lov/lov_pool.c @@ -289,6 +289,7 @@ int lov_ost_pool_init(struct ost_pool *op, unsigned int count) return 0; } +/* Caller must hold write op_rwlock */ int lov_ost_pool_extend(struct ost_pool *op, unsigned int max_count) { __u32 *new; @@ -306,52 +307,51 @@ int lov_ost_pool_extend(struct ost_pool *op, unsigned int max_count) /* copy old array to new one */ memcpy(new, op->op_array, op->op_size * sizeof(op->op_array[0])); - write_lock(&op->op_rwlock); OBD_FREE(op->op_array, op->op_size * sizeof(op->op_array[0])); op->op_array = new; op->op_size = new_size; - write_unlock(&op->op_rwlock); return 0; } int lov_ost_pool_add(struct ost_pool *op, __u32 idx, unsigned int max_count) { - int rc, i; + int rc = 0, i; + ENTRY; + + write_lock(&op->op_rwlock); rc = lov_ost_pool_extend(op, max_count); if (rc) - return rc; + GOTO(out, rc); /* search ost in pool array */ - read_lock(&op->op_rwlock); for (i = 0; i < op->op_count; i++) { - if (op->op_array[i] == idx) { - read_unlock(&op->op_rwlock); - return -EEXIST; - } + if (op->op_array[i] == idx) + GOTO(out, rc = -EEXIST); } /* ost not found we add it */ op->op_array[op->op_count] = idx; op->op_count++; - read_unlock(&op->op_rwlock); - return 0; +out: + write_unlock(&op->op_rwlock); + return rc; } int lov_ost_pool_remove(struct ost_pool *op, __u32 idx) { int i; - read_lock(&op->op_rwlock); + write_lock(&op->op_rwlock); for (i = 0; i < op->op_count; i++) { if (op->op_array[i] == idx) { memmove(&op->op_array[i], &op->op_array[i + 1], (op->op_count - i - 1) * sizeof(op->op_array[0])); op->op_count--; - read_unlock(&op->op_rwlock); + write_unlock(&op->op_rwlock); return 0; } } - read_unlock(&op->op_rwlock); + write_unlock(&op->op_rwlock); return -EINVAL; } @@ -378,34 +378,34 @@ int lov_pool_new(struct obd_device *obd, char *poolname) lov = &(obd->u.lov); - OBD_ALLOC(new_pool, sizeof(*new_pool)); + if (strlen(poolname) > MAXPOOLNAME) + return -ENAMETOOLONG; + OBD_ALLOC_PTR(new_pool); if (new_pool == NULL) return -ENOMEM; - if (strlen(poolname) > MAXPOOLNAME) - return -ENAMETOOLONG; - strncpy(new_pool->pool_name, poolname, MAXPOOLNAME); new_pool->pool_name[MAXPOOLNAME] = '\0'; new_pool->pool_lov = lov; rc = lov_ost_pool_init(&new_pool->pool_obds, 0); if (rc) - return rc; + GOTO(out_err, rc); memset(&(new_pool->pool_rr), 0, sizeof(struct lov_qos_rr)); rc = lov_ost_pool_init(&new_pool->pool_rr.lqr_pool, 0); - if (rc) - return rc; + if (rc) { + lov_ost_pool_free(&new_pool->pool_obds); + GOTO(out_err, rc); + } spin_lock(&obd->obd_dev_lock); - /* check if pool alreaddy exists */ - if (lustre_hash_lookup(lov->lov_pools_hash_body, - poolname) != NULL) { + /* check if pool already exists */ + if (lustre_hash_lookup(lov->lov_pools_hash_body, poolname) != NULL) { spin_unlock(&obd->obd_dev_lock); + lov_ost_pool_free(&new_pool->pool_rr.lqr_pool); lov_ost_pool_free(&new_pool->pool_obds); - OBD_FREE(new_pool, sizeof(*new_pool)); - return -EEXIST; + GOTO(out_err, rc = -EEXIST); } INIT_HLIST_NODE(&new_pool->pool_hash); @@ -421,8 +421,7 @@ int lov_pool_new(struct obd_device *obd, char *poolname) #ifdef LPROCFS /* ifdef needed for liblustre */ new_pool->pool_proc_entry = lprocfs_add_simple(lov->lov_pool_proc_entry, - poolname, - NULL, NULL, + poolname, NULL, NULL, new_pool, &pool_proc_operations); #endif @@ -433,6 +432,10 @@ int lov_pool_new(struct obd_device *obd, char *poolname) } return 0; + +out_err: + OBD_FREE_PTR(new_pool); + return rc; } int lov_pool_del(struct obd_device *obd, char *poolname) @@ -456,9 +459,6 @@ int lov_pool_del(struct obd_device *obd, char *poolname) pool->pool_proc_entry->parent); #endif - /* pool is kept in the list to be freed by lov_cleanup() - * list_del(&pool->pool_list); - */ lustre_hash_del_key(lov->lov_pools_hash_body, poolname); lov->lov_pool_count--; @@ -466,14 +466,7 @@ int lov_pool_del(struct obd_device *obd, char *poolname) spin_unlock(&obd->obd_dev_lock); /* pool struct is not freed because it may be used by - * some open in /proc - * the struct is freed at lov_cleanup() - */ - /* - if (pool->pool_rr.lqr_size != 0) - OBD_FREE(pool->pool_rr.lqr_array, pool->pool_rr.lqr_size); - lov_ost_pool_free(&pool->pool_obds); - OBD_FREE(pool, sizeof(*pool)); + * some open in /proc. The struct is freed at lov_cleanup() */ return 0; } @@ -490,39 +483,27 @@ int lov_pool_add(struct obd_device *obd, char *poolname, char *ostname) lov = &(obd->u.lov); pool = lustre_hash_lookup(lov->lov_pools_hash_body, poolname); - if (pool == NULL) { + if (pool == NULL) return -ENOENT; - } - - /* allocate pool tgt array if needed */ - mutex_down(&lov->lov_lock); - rc = lov_ost_pool_extend(&pool->pool_obds, lov->lov_tgt_size); - if (rc) { - mutex_up(&lov->lov_lock); - return rc; - } - mutex_up(&lov->lov_lock); obd_str2uuid(&ost_uuid, ostname); - spin_lock(&obd->obd_dev_lock); /* search ost in lov array */ + mutex_down(&lov->lov_lock); for (i = 0; i < lov->desc.ld_tgt_count; i++) { if (!lov->lov_tgts[i]) continue; - if (obd_uuid_equals(&ost_uuid, &(lov->lov_tgts[i]->ltd_uuid))) break; } /* test if ost found in lov */ if (i == lov->desc.ld_tgt_count) { - spin_unlock(&obd->obd_dev_lock); + mutex_up(&lov->lov_lock); return -EINVAL; } - - spin_unlock(&obd->obd_dev_lock); + mutex_up(&lov->lov_lock); lov_idx = i; diff --git a/lustre/lov/lov_qos.c b/lustre/lov/lov_qos.c index b6455b1..dd8d526 100644 --- a/lustre/lov/lov_qos.c +++ b/lustre/lov/lov_qos.c @@ -366,20 +366,22 @@ static int qos_calc_rr(struct lov_obd *lov, struct ost_pool *src_pool, RETURN(0); } - if (lqr->lqr_pool.op_size) - lov_ost_pool_free(&lqr->lqr_pool); - rc = lov_ost_pool_init(&lqr->lqr_pool, src_pool->op_count); + real_count = src_pool->op_count; + + /* Zero the pool array */ + /* alloc_rr is holding a read lock on the pool, so nobody is adding/ + deleting from the pool. The lq_rw_sem insures that nobody else + is reading. */ + lqr->lqr_pool.op_count = real_count; + rc = lov_ost_pool_extend(&lqr->lqr_pool, real_count); if (rc) { up_write(&lov->lov_qos.lq_rw_sem); RETURN(rc); } - - for (i = 0; i < src_pool->op_count; i++) + for (i = 0; i < lqr->lqr_pool.op_count; i++) lqr->lqr_pool.op_array[i] = LOV_QOS_EMPTY; - lqr->lqr_pool.op_count = src_pool->op_count; /* Place all the OSTs from 1 OSS at the same time. */ - real_count = lqr->lqr_pool.op_count; placed = 0; list_for_each_entry(oss, &lov->lov_qos.lq_oss_list, lqo_oss_list) { int j = 0; diff --git a/lustre/tests/sanity.sh b/lustre/tests/sanity.sh index 4ab40df..3a458ec 100644 --- a/lustre/tests/sanity.sh +++ b/lustre/tests/sanity.sh @@ -823,7 +823,7 @@ run_test 27c "create two stripe file f01 =======================" test_27d() { mkdir -p $DIR/d27 - $SETSTRIPE $DIR/d27/fdef 0 -1 0 || error "lstripe failed" + $SETSTRIPE -c0 -i-1 -s0 $DIR/d27/fdef || error "lstripe failed" $CHECKSTAT -t file $DIR/d27/fdef || error "checkstat failed" dd if=/dev/zero of=$DIR/d27/fdef bs=4k count=4 || error } @@ -2859,7 +2859,7 @@ test_65e() { touch $DIR/d65/f6 $LVERIFY $DIR/d65 $DIR/d65/f6 || error "lverify failed" } -run_test 65e "directory setstripe 0 -1 0 =======================" +run_test 65e "directory setstripe defaults =======================" test_65f() { mkdir -p $DIR/d65f diff --git a/lustre/utils/lfs.c b/lustre/utils/lfs.c index 2479e93..922e812 100644 --- a/lustre/utils/lfs.c +++ b/lustre/utils/lfs.c @@ -200,6 +200,21 @@ command_t cmdlist[] = { { 0, 0, 0, NULL } }; +static int isnumber(const char *str) +{ + const char *ptr; + + if (str[0] != '-' && !isdigit(str[0])) + return 0; + + for (ptr = str + 1; *ptr != '\0'; ptr++) { + if (!isdigit(*ptr)) + return 0; + } + + return 1; +} + /* functions */ static int lfs_setstripe(int argc, char **argv) { @@ -229,15 +244,12 @@ static int lfs_setstripe(int argc, char **argv) st_size = 0; st_offset = -1; st_count = 0; - if (argc == 3 && strcmp(argv[1], "-d") == 0) { - /* for compatibility with the existing positional parameter - * usage */ - fname = argv[2]; - optind = 2; - } else if (argc == 5 && - (argv[2][0] != '-' || isdigit(argv[2][1])) && - (argv[3][0] != '-' || isdigit(argv[3][1])) && - (argv[4][0] != '-' || isdigit(argv[4][1])) ) { + +#if LUSTRE_VERSION < OBD_OCD_VERSION(2,1,0,0) + if (argc == 5 && argv[1][0] != '-' && + isnumber(argv[2]) && isnumber(argv[3]) && isnumber(argv[4])) { + fprintf(stderr, "warning: deprecated usage of setstripe " + "positional parameters. Use -c, -i, -s instead.\n"); /* for compatibility with the existing positional parameter * usage */ fname = argv[1]; @@ -245,7 +257,11 @@ static int lfs_setstripe(int argc, char **argv) stripe_off_arg = argv[3]; stripe_count_arg = argv[4]; optind = 4; - } else { + } else +#else +#warning "remove obsolete positional parameter code" +#endif + { optind = 0; while ((c = getopt_long(argc, argv, "c:di:o:s:p:", long_opts, NULL)) >= 0) { @@ -279,11 +295,8 @@ static int lfs_setstripe(int argc, char **argv) return CMD_HELP; } } - if (optind < argc) - fname = argv[optind]; - else - return CMD_HELP; + fname = argv[optind]; if (delete && (stripe_size_arg != NULL || stripe_off_arg != NULL || @@ -294,10 +307,10 @@ static int lfs_setstripe(int argc, char **argv) return CMD_HELP; } } - if (optind != argc - 1) { - fprintf(stderr, "error: %s: only 1 filename|dirname can be " - "specified: '%s'\n", - argv[0], argv[argc - 1]); + + if (optind == argc) { + fprintf(stderr, "error: %s: missing filename|dirname\n", + argv[0]); return CMD_HELP; } @@ -305,8 +318,8 @@ static int lfs_setstripe(int argc, char **argv) if (stripe_size_arg != NULL) { result = parse_size(stripe_size_arg, &st_size, &size_units); if (result) { - fprintf(stderr,"error: bad size '%s'\n", - stripe_size_arg); + fprintf(stderr, "error: %s: bad size '%s'\n", + argv[0], stripe_size_arg); return result; } } @@ -329,15 +342,16 @@ static int lfs_setstripe(int argc, char **argv) } } - if (pool_name_arg == NULL) - result = llapi_file_create(fname, st_size, st_offset, st_count, 0); - else + do { result = llapi_file_create_pool(fname, st_size, st_offset, st_count, 0, pool_name_arg); - - if (result) - fprintf(stderr, "error: %s: create stripe file failed\n", - argv[0]); + if (result) { + fprintf(stderr,"error: %s: create stripe file '%s' " + "failed\n", argv[0], fname); + break; + } + fname = argv[++optind]; + } while (fname != NULL); return result; } @@ -474,6 +488,7 @@ static int lfs_find(int argc, char **argv) time(&t); optind = 0; + /* when getopt_long_only() hits '!' it returns 1 and puts "!" in optarg */ while ((c = getopt_long_only(argc, argv, "-A:C:D:g:G:M:n:PpO:qrs:t:u:U:v", long_opts, NULL)) >= 0) { xtime = NULL; @@ -481,6 +496,12 @@ static int lfs_find(int argc, char **argv) if (neg_opt) --neg_opt; /* '!' is part of option */ + /* when getopt_long_only() finds a string which is not + * an option nor a known option argument it returns 1 + * in that case if we already have found pathstart and pathend + * (i.e. we have the list of pathnames), + * the only supported value is "!" + */ isoption = (c != 1) || (strcmp(optarg, "!") == 0); if (!isoption && pathend != -1) { fprintf(stderr, "err: %s: filename|dirname must either " @@ -502,6 +523,9 @@ static int lfs_find(int argc, char **argv) /* Long options. */ break; case 1: + /* unknown; opt is "!" or path component, + * checking done above. + */ if (strcmp(optarg, "!") == 0) neg_opt = 2; break; @@ -621,7 +645,7 @@ static int lfs_find(int argc, char **argv) strcpy(buf, (char *)optarg); if (param.num_alloc_obds == 0) { - param.obduuid = (struct obd_uuid *)malloc(FIND_MAX_OSTS * + param.obduuid = malloc(FIND_MAX_OSTS * sizeof(struct obd_uuid)); if (param.obduuid == NULL) return -ENOMEM; diff --git a/lustre/utils/liblustreapi.c b/lustre/utils/liblustreapi.c index f824151..0d9e055 100644 --- a/lustre/utils/liblustreapi.c +++ b/lustre/utils/liblustreapi.c @@ -232,16 +232,19 @@ int llapi_stripe_limit_check(unsigned long stripe_size, int stripe_offset, return -EINVAL; } if (stripe_offset < -1 || stripe_offset > MAX_OBD_DEVICES) { + errno = -EINVAL; llapi_err(LLAPI_MSG_ERROR, "error: bad stripe offset %d", stripe_offset); return -EINVAL; } if (stripe_count < -1 || stripe_count > LOV_MAX_STRIPE_COUNT) { + errno = -EINVAL; llapi_err(LLAPI_MSG_ERROR, "error: bad stripe count %d", stripe_count); return -EINVAL; } if (stripe_count > 0 && (__u64)stripe_size * stripe_count > 0xffffffff){ + errno = -EINVAL; llapi_err(LLAPI_MSG_ERROR, "error: stripe_size %lu * " "stripe_count %u exceeds 4GB", stripe_size, stripe_count); @@ -250,58 +253,6 @@ int llapi_stripe_limit_check(unsigned long stripe_size, int stripe_offset, return 0; } -int llapi_file_open(const char *name, int flags, int mode, - unsigned long stripe_size, int stripe_offset, - int stripe_count, int stripe_pattern) -{ - struct lov_user_md lum = { 0 }; - int fd, rc = 0; - int isdir = 0; - - fd = open(name, flags | O_LOV_DELAY_CREATE, mode); - if (fd < 0 && errno == EISDIR) { - fd = open(name, O_DIRECTORY | O_RDONLY); - isdir++; - } - - if (fd < 0) { - rc = -errno; - llapi_err(LLAPI_MSG_ERROR, "unable to open '%s'", name); - return rc; - } - - if ((rc = llapi_stripe_limit_check(stripe_size, stripe_offset, - stripe_count, stripe_pattern)) != 0) { - errno = rc; - goto out; - } - - /* Initialize IOCTL striping pattern structure */ - lum.lmm_magic = LOV_USER_MAGIC; - lum.lmm_pattern = stripe_pattern; - lum.lmm_stripe_size = stripe_size; - lum.lmm_stripe_count = stripe_count; - lum.lmm_stripe_offset = stripe_offset; - - if (ioctl(fd, LL_IOC_LOV_SETSTRIPE, &lum)) { - char *errmsg = "stripe already set"; - rc = -errno; - if (errno != EEXIST && errno != EALREADY) - errmsg = strerror(errno); - - llapi_err_noerrno(LLAPI_MSG_ERROR, - "error on ioctl "LPX64" for '%s' (%d): %s", - (__u64)LL_IOC_LOV_SETSTRIPE, name, fd, errmsg); - } -out: - if (rc) { - close(fd); - fd = rc; - } - - return fd; -} - static int poolpath(char *fsname, char *pathname, char *pool_pathname); int llapi_file_open_pool(const char *name, int flags, int mode, @@ -326,30 +277,35 @@ int llapi_file_open_pool(const char *name, int flags, int mode, } if ((rc = llapi_stripe_limit_check(stripe_size, stripe_offset, - stripe_count, stripe_pattern)) != 0) { + stripe_count, stripe_pattern)) != 0){ errno = rc; goto out; } - /* in case user give the full pool name ., skip - * the fsname */ - ptr = strchr(pool_name, '.'); - if (ptr != NULL) { - strncpy(fsname, pool_name, ptr - pool_name); - fsname[ptr - pool_name] = '\0'; - /* if fsname matches a fs skip it - * if not keep the poolname as is */ - if (poolpath(fsname, NULL, NULL) == 0) - pool_name = ptr + 1; - } - /* Initialize IOCTL striping pattern structure */ lum.lmm_magic = LOV_USER_MAGIC_V3; lum.lmm_pattern = stripe_pattern; lum.lmm_stripe_size = stripe_size; lum.lmm_stripe_count = stripe_count; lum.lmm_stripe_offset = stripe_offset; - strncpy(lum.lmm_pool_name, pool_name, MAXPOOLNAME); + + /* in case user give the full pool name ., skip + * the fsname */ + if (pool_name != NULL) { + ptr = strchr(pool_name, '.'); + if (ptr != NULL) { + strncpy(fsname, pool_name, ptr - pool_name); + fsname[ptr - pool_name] = '\0'; + /* if fsname matches a filesystem skip it + * if not keep the poolname as is */ + if (poolpath(fsname, NULL, NULL) == 0) + pool_name = ptr + 1; + } + strncpy(lum.lmm_pool_name, pool_name, MAXPOOLNAME); + } else { + /* If no pool is specified at all, use V1 request */ + lum.lmm_magic = LOV_USER_MAGIC_V1; + } if (ioctl(fd, LL_IOC_LOV_SETSTRIPE, &lum)) { char *errmsg = "stripe already set"; @@ -359,7 +315,7 @@ int llapi_file_open_pool(const char *name, int flags, int mode, llapi_err_noerrno(LLAPI_MSG_ERROR, "error on ioctl "LPX64" for '%s' (%d): %s", - (__u64)LL_IOC_LOV_SETSTRIPE, name, fd, errmsg); + (__u64)LL_IOC_LOV_SETSTRIPE, name, fd,errmsg); } out: if (rc) { @@ -370,13 +326,23 @@ out: return fd; } +int llapi_file_open(const char *name, int flags, int mode, + unsigned long stripe_size, int stripe_offset, + int stripe_count, int stripe_pattern) +{ + return llapi_file_open_pool(name, flags, mode, stripe_size, + stripe_offset, stripe_count, + stripe_pattern, NULL); +} + int llapi_file_create(const char *name, unsigned long stripe_size, int stripe_offset, int stripe_count, int stripe_pattern) { int fd; - fd = llapi_file_open(name, O_CREAT | O_WRONLY, 0644, stripe_size, - stripe_offset, stripe_count, stripe_pattern); + fd = llapi_file_open_pool(name, O_CREAT | O_WRONLY, 0644, stripe_size, + stripe_offset, stripe_count, stripe_pattern, + NULL); if (fd < 0) return fd;