Project

General

Profile

Actions

Submit #2773

closed

[PATCH] sys/vfs/hammer: fix off-by-one error in hammer volume-add ioctl

Added by tkusumi about 9 years ago. Updated about 9 years ago.

Status:
Closed
Priority:
Normal
Assignee:
-
Category:
VFS subsystem
Target version:
-
Start date:
01/18/2015
Due date:
% Done:

100%

Estimated time:

Description

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


Files

Actions #1

Updated by tkusumi about 9 years ago

  • Status changed from New to Closed
  • % Done changed from 0 to 100
Actions

Also available in: Atom PDF