diff --git a/lustre/snapfs/snapfs_internal.h b/lustre/snapfs/snapfs_internal.h
index 04ead20f974cebe42b752da8f72e0bb7442300a5..93583f9ff407b71c14ff5337fdc9fdf068098482 100644
--- a/lustre/snapfs/snapfs_internal.h
+++ b/lustre/snapfs/snapfs_internal.h
@@ -1,7 +1,8 @@
 #ifndef __LINUX_SNAPFS_H
 #define __LINUX_SNAPFS_H
 /* maximum number of snapshot tables we maintain in the kernel */
-#define SNAP_MAX_TABLES 	32
+#define SNAP_MAX		32	
+#define SNAP_MAX_TABLES 	32	
 #define SNAP_MAX_NAMELEN	64
 
 /* ioctls for manipulating snapshots 40 - 60 */
@@ -23,7 +24,6 @@
 
 #define IOC_SNAP_MAX_NR                 51 
 
-
 struct snap {
 	time_t 		time;
 	unsigned int 	index;
@@ -32,19 +32,22 @@ struct snap {
 	char 	name[SNAP_MAX_NAMELEN];
 };
 
-/* snap ioctl data for table fiddling */
-struct snap_table_data {
-	int 		tblcmd_no;		/* which table */
-	unsigned long	dev;
-	unsigned int 	tblcmd_count;		/* how many snaps */
-	struct snap 	tblcmd_snaps[0];	/* sorted times! */
-};
+
 /*FIXME, use ioc_data temporary, will use obd_ioc_data later*/
 struct ioc_data {
 	unsigned int ioc_inlen;
 	char 	     *ioc_inbuf;
 	char	     ioc_bulk[0];
 };
+
+/* snap ioctl data for table fiddling */
+struct ioc_snap_tbl_data {
+	int 		no;		/* which table */
+	unsigned long	dev;
+	unsigned int 	count;		/* how many snaps */
+	struct snap 	snaps[0];	/* sorted times! */
+};
+
 /* we have just a single snapshot control device
    it contains a list of all the snap_current info's
 */
@@ -74,7 +77,6 @@ typedef ino_t	snap_id;
 //#define OBD_OBDMDSZ  54
 //#define SNAP_MAX ((OBD_OBDMDSZ - sizeof(uint32_t))/sizeof(snap_id))
 
-#define SNAP_MAX	50
 
 
 /* if time is 0 this designates the "current" snapshot, i.e.
@@ -203,14 +205,6 @@ struct snap_obd_data {
 	unsigned int snap_index;/* which snapshot is ours */
 	unsigned int snap_table;/* which table do we use */
 };
-
-struct snap_table {
-	struct semaphore    tbl_sema;
-	spinlock_t          tbl_lock;
-	unsigned int 	    tbl_count; /* how many snapshots exist in this table*/
-	unsigned int	    generation;
-	struct snap    	    snap_items[SNAP_MAX]; 
-};
 #define DISK_SNAPTABLE_ATTR     "Snaptable"
 #define DISK_SNAP_TABLE_MAGIC	0x1976
 struct snap_disk_table {
@@ -220,6 +214,15 @@ struct snap_disk_table {
 	struct  snap_disk  	snap_items[SNAP_MAX];
 };
 
+/*Snap Table*/
+struct snap_table {
+	struct semaphore    tbl_sema;
+	spinlock_t          tbl_lock;
+	unsigned int 	    tbl_count; /* how many snapshots exist in this table*/
+	unsigned int	    generation;
+	struct snap    	    snap_items[SNAP_MAX]; 
+};
+
 struct snap_iterdata {
 	kdev_t dev;	/* snap current device number */ 
 	int index;
diff --git a/lustre/snapfs/snaptable.c b/lustre/snapfs/snaptable.c
index fae06f462f4ff9584251d430084333421339c2a9..84c414bdab4f8ff5f3cdfaa22d03eb6165459016 100644
--- a/lustre/snapfs/snaptable.c
+++ b/lustre/snapfs/snaptable.c
@@ -117,65 +117,61 @@ int snap_needs_cow(struct inode *inode)
 	RETURN(index);
 } /* snap_needs_cow */
 
-int snap_print_table(struct snap_table_data *data, char *buf, int *buflen)
+int snap_print_table(struct ioc_snap_tbl_data *data, char *buf, int *buflen)
 {
 	struct snap_table *table;
-	int tableno = data->tblcmd_no;
-	int i, l, rc = 0, nleft = (*buflen);
+	struct ioc_snap_tbl_data *stbl_out;
+	int tableno = data->no;
+	int i, rc = 0, nleft = (*buflen);
+
 	char *buf_ptr;
 
-	if ( tableno < 0 || tableno > SNAP_MAX_TABLES ) {
+	if (tableno < 0 || tableno > SNAP_MAX_TABLES) {
 		CERROR("invalid table number %d\n", tableno);
 		RETURN(-EINVAL);
 	}
-
+	
 	table = &snap_tables[tableno];
-
-	buf_ptr = buf;
-	l = snprintf(buf_ptr, nleft, "snap table %d snap count %d \n", 
-		     tableno, table->tbl_count);
-	nleft -= l;
-	if(nleft < 0) { 
-		CERROR("can not get enough space to print snaptable\n");
-		rc = -ERANGE;
-		goto exit; 
-	} else {
-		buf_ptr += l;
-	}	
-
+	stbl_out = (struct ioc_snap_tbl_data *)buf;
+	stbl_out->count = table->tbl_count;
+	stbl_out->no = tableno;	
+	buf_ptr = (char*)stbl_out->snaps; 
+	nleft -= buf_ptr - buf; 
 	for (i = 0; i < table->tbl_count; i++) {
-		/*FIXME later, will convert time to time string later */
-		l = snprintf(buf_ptr, nleft,
-			"-- slot %d, idx %d, time %lu, name %s\n", i, 
-			table->snap_items[i+1].index, table->snap_items[i+1].time, 
-			&table->snap_items[i+1].name[0]);
+		memcpy(buf_ptr, &table->snap_items[i+1], sizeof(struct snap));
 		
-		nleft -= l;
+		nleft -= sizeof(struct snap);
 		if(nleft < 0) { 
 			CERROR("can not get enough space to print snaptable\n");
 			rc = -ERANGE;
 			goto exit; 
 		} else {
-			buf_ptr += l;
+			buf_ptr += sizeof(struct snap);
 		}	
 	}
 exit:
 	if(nleft > 0) 
 		(*buflen) = (*buflen) - nleft;
-
 	return 0;
 }
-static int inline get_index_of_item(struct snap_table *table)
+static int inline get_index_of_item(struct snap_table *table, char *name)
 {
 	int count = table->tbl_count;
 	int i, j;
-
-	for (i = 0; i < SNAP_MAX; i ++) {
-		for (j = 1; j < count; j++) {
-			if (table->snap_items[j].index == i)
-				break;
+	
+	for (i = 0; i < SNAP_MAX; i++) { 
+		if (!strcmp(name, table->snap_items[i].name)) 
+			return -EINVAL;	
+	}
+	for (i = 0; i < SNAP_MAX; i++) {
+		int found = 0;
+		for (j = 0; j < (count + 1); j++) {
+			if (table->snap_items[j].index == i) {
+				found = 1;
+				break;	
+			}
                 }
-		if (j >= count) 
+		if (!found)
 			return i;
 	}
 	return -EINVAL;
@@ -183,7 +179,7 @@ static int inline get_index_of_item(struct snap_table *table)
 /* This function will write one item(a snapshot) to snaptable  
  * and will also write to disk.
  */
-int snaptable_add_item(struct snap_table_data *data)
+static int snaptable_add_item(struct ioc_snap_tbl_data *data)
 {
 	struct snap_table 		*table;
 	struct snap_disk_table 		*disk_snap_table;
@@ -198,7 +194,7 @@ int snaptable_add_item(struct snap_table_data *data)
 	if (!snapops || !snapops->set_meta_attr)
 		RETURN(-EINVAL);
 
-	tableno = data->tblcmd_no;
+	tableno = data->no;
 	if (tableno < 0 || tableno > SNAP_MAX_TABLES) {
 		CERROR("invalid table number %d\n", tableno);
 		RETURN(-EINVAL);
@@ -207,20 +203,20 @@ int snaptable_add_item(struct snap_table_data *data)
 	count = table->tbl_count;
 
 	/* XXX Is down this sema necessary*/
-	//down_interruptible(&table->tbl_sema);
+	down_interruptible(&table->tbl_sema);
 
 	/*add item in snap_table*/
 	table->snap_items[count+1].gen = table->generation;
 	table->snap_items[count+1].time = CURRENT_TIME;
 	/* find table index */
-	index = get_index_of_item(table);
+	index = get_index_of_item(table, data->snaps[0].name);
 	if (index < 0)
 		RETURN(-EINVAL);
-
+	
 	table->snap_items[count+1].index = index;
 	table->snap_items[count+1].flags = 0;
 	memcpy(&table->snap_items[count + 1].name[0], 
-	       &data->tblcmd_snaps[0].name[0], SNAP_MAX_NAMELEN);
+	       data->snaps[0].name, SNAP_MAX_NAMELEN);
 	/* we will write the whole snap_table to disk */
 	SNAP_ALLOC(disk_snap_table, sizeof(struct snap_disk_table));
 	if (!disk_snap_table)
@@ -246,7 +242,7 @@ int snaptable_add_item(struct snap_table_data *data)
 	table->tbl_count++;
 	table->generation++;
 	
-	//up(&table->tbl_sema);
+	up(&table->tbl_sema);
 	
 	return 0;
 }
diff --git a/lustre/snapfs/utils/snapctl.c b/lustre/snapfs/utils/snapctl.c
index a7828bae2df91bfaa338f9e3863115993bc840f1..5edb4fa8bd225a4945859cc8b68450f4f37d16b4 100644
--- a/lustre/snapfs/utils/snapctl.c
+++ b/lustre/snapfs/utils/snapctl.c
@@ -17,16 +17,21 @@
 #define IOC_BUF_MAX_LEN 8192 
 static char rawbuf[IOC_BUF_MAX_LEN];
 static char *buf = rawbuf;
-
 /*FIXME add this temporary, will use obd_ioc_data later*/
-#define IOC_PACK(buffer, length)  	\
+#define IOC_INIT(ptr)					\
+do{							\
+	struct ioc_data* pbuf;				\
+	memset(buf, 0, sizeof(rawbuf));	 		\
+	pbuf = (struct ioc_data*)buf;			\
+	pbuf->ioc_inbuf = pbuf->ioc_bulk;		\
+	ptr = (struct ioc_snap_tbl_data *)pbuf->ioc_bulk; \
+} while(0)
+
+#define IOC_PACK(length)  		\
 do{                             	 	\
 	struct ioc_data* pbuf;			\
-	memset(buf, 0, sizeof(rawbuf));	 	\
 	pbuf = (struct ioc_data*)buf;		\
-	pbuf->ioc_inbuf = pbuf->ioc_bulk;	\
 	pbuf->ioc_inlen = length;		\
-	memcpy(pbuf->ioc_inbuf, buffer, length);\
 } while (0)
 
 static struct list_head snap_list;
@@ -217,6 +222,26 @@ int snap_dev_list(int argc, char **argv)
 	release_snap_list();
 	return 0;
 }
+static inline void print_snap_table(void * buf)
+{
+	struct ioc_snap_tbl_data *ptable;
+	int    i;
+
+	ptable = (struct ioc_snap_tbl_data*)buf;
+	
+	printf("There are %d snapshot in the system\n", ptable->count);
+	printf("index\t\tname\t\t\ttime\t\t\n"); 
+	for (i = 0; i < ptable->count; i++) {
+		struct	tm* local_time; 	
+		char	time[128];
+		
+		memset (time, 0, sizeof(time));
+		local_time = localtime(&ptable->snaps[i].time);
+		if (local_time) 
+			strftime(time, sizeof(time), "%a %b %d %Y %H:%M:%S", local_time);			
+		printf("%-10d\t%-20s\t%s\n", ptable->snaps[i].index, ptable->snaps[i].name, time); 
+	}
+}
 int snap_snap_list(int argc, char **argv)
 {
 	int i, rc = 0;
@@ -231,15 +256,17 @@ int snap_snap_list(int argc, char **argv)
 	}
 	
 	for (i = 0; i < open_device_table.count; i++) {
-		struct snap_table_data	snap_ioc_data;
-	
+		struct ioc_snap_tbl_data *snap_ioc_data;
+
+		IOC_INIT(snap_ioc_data);
+
 		if (argc == 2) { 
-			snap_ioc_data.tblcmd_no = atoi(argv[1]);
+			snap_ioc_data->no = atoi(argv[1]);
 		} else { 
-			snap_ioc_data.tblcmd_no = 0;
+			snap_ioc_data->no = 0;
 		}
 		
-		IOC_PACK((char*)&snap_ioc_data, sizeof(struct snap_table_data));
+		IOC_PACK(sizeof(struct ioc_snap_tbl_data));
 		
 		if ((rc = ioctl(open_device_table.device[i].fd, 
 				IOC_SNAP_PRINTTABLE, buf))) {
@@ -247,7 +274,8 @@ int snap_snap_list(int argc, char **argv)
 				&open_device_table.device[i].name[0], rc);
 			return (rc);
 		}
-		printf("%s", ((struct ioc_data*)buf)->ioc_bulk);
+		if(((struct ioc_data*)buf)->ioc_bulk)
+			print_snap_table(((struct ioc_data*)buf)->ioc_bulk);	
 	}
 	return rc;
 }
@@ -265,31 +293,31 @@ int snap_snap_add(int argc, char **argv)
 		return (-EINVAL);
 	}
 	for (i = 0; i < open_device_table.count; i++) {
-		struct snap_table_data	snap_ioc_data;
+		struct ioc_snap_tbl_data *snap_ioc_data;
+
+		IOC_INIT(snap_ioc_data);
 
-		snap_ioc_data.tblcmd_count = 1;
-		snap_ioc_data.dev = open_device_table.device[i].dev;
+		snap_ioc_data->count = 1;
+		snap_ioc_data->dev = open_device_table.device[i].dev;
 
 		if (argc == 3) { 
-			snap_ioc_data.tblcmd_no = atoi(argv[1]);
-			memcpy(&snap_ioc_data.tblcmd_snaps[0].name[0], 
+			snap_ioc_data->no = atoi(argv[1]);
+			memcpy(snap_ioc_data->snaps[0].name, 
 			       argv[2], strlen(argv[2]));
 		} else { 
-			snap_ioc_data.tblcmd_no = 0;
-			memcpy(&snap_ioc_data.tblcmd_snaps[0].name[0], 
+			snap_ioc_data->no = 0;
+			memcpy(snap_ioc_data->snaps[0].name, 
 			       argv[1], strlen(argv[1]));
 		}
-		snap_ioc_data.tblcmd_snaps[0].time = time(NULL);
+		snap_ioc_data->snaps[0].time = time(NULL);
 		
-		IOC_PACK(&snap_ioc_data, sizeof(struct snap_table_data));
+		IOC_PACK(sizeof(struct ioc_snap_tbl_data) + sizeof(struct snap));
 
 		if ((rc = ioctl(open_device_table.device[i].fd, 
-				IOC_SNAP_ADD, buf))) {
-			fprintf(stderr, "add snapshot %s failed %d \n", 
-				&open_device_table.device[i].name[0], rc);
+					IOC_SNAP_ADD, buf))) {
+			fprintf(stderr, "add %s failed \n", argv[1]);
 		} else {
-			fprintf(stderr, "add snapshot %s success\n", 
-				&open_device_table.device[i].name[0]); 
+			fprintf(stderr, "add %s success\n", argv[1]);
 		}
 	}
 	return rc;