From afb7e22c9cd0ab2232f75608642d73ae45a94aea Mon Sep 17 00:00:00 2001 From: Tomohiro Kusumi Date: Mon, 19 Jan 2015 03:26:47 +0900 Subject: [PATCH] sys/vfs/hammer: fix off-by-one error in hammer volume-add ioctl This patch should fix off-by-one error regarding maximum volume number if I'm taking the hammer's spec right. hammer_ioc_volume_add() tests current # of volumes before it adds a new volume, however this part seems wrong by off-by-one error. It should be "if (hmp->nvolumes >= HAMMER_MAX_VOLUMES)" given that hmp->nvolumes is # of volumes before adding, and HAMMER_MAX_VOLUMES(256) is the maximum # of volumes it can actually become. if (hmp->nvolumes + 1 >= HAMMER_MAX_VOLUMES) { kprintf("Max number of HAMMER volumes exceeded\n"); return (EINVAL); } I tried to somehow get to that limit using vnconfig but I couldn't, so instead of doing that I did following with a latest master compiled with 3 for HAMMER_MAX_VOLUMES and saw what happened. newfs with 3 devices works as expected. \# mkdir -p /HAMMER \# newfs_hammer -f -L TEST /dev/ad1 /dev/ad2 /dev/ad3 Volume 0 DEVICE /dev/ad1 size 50.00GB Volume 1 DEVICE /dev/ad2 size 50.00GB Volume 2 DEVICE /dev/ad3 size 50.00GB initialize freemap volume 0 initializing the undo map (504 MB) initialize freemap volume 1 initialize freemap volume 2 --------------------------------------------- 3 volumes total size 150.00GB version 6 boot-area-size: 64.00MB memory-log-size: 128.00MB undo-buffer-size: 504.00MB total-pre-allocated: 0.51GB fsid: 113e753f-9f3d-11e4-9538-0900270047c3 NOTE: Please remember that you may have to manually set up a cron(8) job to prune and reblock the filesystem regularly. By default, the system automatically runs 'hammer cleanup' on a nightly basis. The periodic.conf(5) variable 'daily_clean_hammer_enable' can be unset to disable this. Also see 'man hammer' and 'man HAMMER' for more information. \# mount_hammer /dev/ad1:/dev/ad2:/dev/ad3 /HAMMER \# hammer volume-list /HAMMER /dev/ad1 /dev/ad2 /dev/ad3 However when I delete /dev/ad3 and try to add again it fails with "Max number of HAMMER volumes exceeded" in dmesg. \# hammer volume-del /dev/ad3 /HAMMER \# hammer volume-list /HAMMER /dev/ad1 /dev/ad2 \# hammer volume-add /dev/ad3 /HAMMER hammer volume-add ioctl: Invalid argument \# echo $? 1 \# hammer volume-list /HAMMER /dev/ad1 /dev/ad2 Delete /dev/ad2 and add /dev/ad2 again works as expected \# hammer volume-del /dev/ad2 /HAMMER \# hammer volume-list /HAMMER /dev/ad1 \# hammer volume-add /dev/ad2 /HAMMER \# hammer volume-list /HAMMER /dev/ad1 /dev/ad2 But again it can't go back to 3 volumes (maximum # of volumes) although it can newfs with 3 volumes, which I think shows this isn't what is expected to happen. \# hammer volume-add /dev/ad3 /HAMMER hammer volume-add ioctl: Invalid argument \# echo $? 1 --- sys/vfs/hammer/hammer_volume.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sys/vfs/hammer/hammer_volume.c b/sys/vfs/hammer/hammer_volume.c index 79d3120..dbd750b 100644 --- a/sys/vfs/hammer/hammer_volume.c +++ b/sys/vfs/hammer/hammer_volume.c @@ -83,7 +83,7 @@ hammer_ioc_volume_add(hammer_transaction_t trans, hammer_inode_t ip, return (EINVAL); } - if (hmp->nvolumes + 1 >= HAMMER_MAX_VOLUMES) { + if (hmp->nvolumes >= HAMMER_MAX_VOLUMES) { kprintf("Max number of HAMMER volumes exceeded\n"); return (EINVAL); } -- 2.1.2