[DragonFlyBSD - Submit #2773] (Closed) [PATCH] sys/vfs/hammer: fix off-by-one error in hammer volume-add ioctl

[bugtracker-admin at leaf.dragonflybsd.org]

Issue #2773 has been updated by tkusumi.

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

Applied in changeset commit:f1d8913755e416824a51d1851052e9ae399cb69e.

----------------------------------------
Submit #2773: [PATCH] sys/vfs/hammer: fix off-by-one error in hammer volume-add ioctl
http://bugs.dragonflybsd.org/issues/2773#change-12546

* Author: tkusumi
* Status: Closed
* Priority: Normal
* Assignee: 
* Category: VFS subsystem
* Target version: 
----------------------------------------
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--------------------------------
0001-sys-vfs-hammer-fix-off-by-one-error-in-hammer-volume.patch (3.44 KB)


-- 
You have received this notification because you have either subscribed to it, or are involved in it.
To change your notification preferences, please click here: http://bugs.dragonflybsd.org/my/account