search

openzfs / zfs

OpenZFS on Linux and FreeBSD
https://openzfs.github.io/openzfs-docs
Other
10.58k stars 1.75k forks source link

zvol_id ioctl() failure #172

Closed behlendorf closed 13 years ago

behlendorf commented 13 years ago

On arch Linux:

[root@l0cutus ~]#  /usr/bin/zvol_id /dev/zd0
ioctl_get_msg failed:-1
(���7

Alright for some reason the ioctl() is failing but we are not printing the errno so it's unclear why. Minimally zvol_id should be updated to print the errno, and then not print the garbage which was left in the buffer. The errno should shed some light on what's going wrong.

L0cutus commented 13 years ago

let me know when there are news so i can try since on my archlinux distro i have problems with this...

behlendorf commented 13 years ago

Sure, we'll try and get a patch together to get a better error message. I'll let you know when it's available.

fajarnugraha commented 13 years ago

Good news: I found the problem

Seems that:

- kernel 2.6.32 allows read parameters (copy_to_user) with _IO, 
  while newer kernels (tested Archlinux's 2.6.37 kernel) enforces 
  _IOR (which is the correct thing to do)
- default installation from source puts binaries 
  (zfs, zpool, zvol_id, etc) in /usr/local instead of /usr, 
  so udev rule path does not match (it specifically use /usr/bin)

I also added a fix to return correct error code and message in case of ioctl error. Fix is available at https://github.com/fajarnugraha/zfs/commit/c1a192c50d3fadfe8c3005cfc9954c028401a261

You might also want to modify 60-zpool.rules to use relative path for zpool_id

L0cutus commented 13 years ago

the path was corrected by the aur package mantainer also, as you can see here: http://aur.archlinux.org/packages.php?ID=47434 so the udev rules & bins are put into correct path on installation (i hope ;-)) infact :

[root@l0cutus ~]# which zpool_id /usr/bin/zpool_id

package installation path: http://pastebin.com/bMvG1MLM hope i'm able to install your git version, i'm not a dev, only a simple user ;-) hope also behlendorf can import it so i can use his git (i've see it is used by the AUR package mantainer). thanks a lot for now !

fajarnugraha commented 13 years ago

Scratch that. My testing was incomplete, udev will NOT work if PROGRAM is not full path. If you've already downloaded my patch, you should revert changes to etc/udev/rules.d/60-zvol.rules (basically specifying full path to zvol_id). Is there a clean way to achieve this (by changing etc/udev/rules.d/Makefile, perhaps)?

I'll test this on my RHEL5 test box and post a revised patch later.

user318 commented 13 years ago

This patch does not helped me. Not because udev path issue. The zvol_id tool by itself is not working. Without patch:

# zvol_id /dev/zd0 
ioctl_get_msg failed:-1
(A��A

With this patch:

# ./zvol_id /dev/zd0 
ioctl_get_msg failed:Inappropriate ioctl for device

Strace of patched version:

# strace ./zvol_id /dev/zd0
...
stat("/dev/zd0", {st_mode=S_IFBLK|0660, st_rdev=makedev(230, 0), ...}) = 0
open("/dev/zd0", O_RDONLY)              = 3
ioctl(3, 0x80ff127d, 0x7fff1e74c4b0)    = -1 ENOTTY (Inappropriate ioctl for device)
fstat(1, {st_mode=S_IFCHR|0620, st_rdev=makedev(136, 2), ...}) = 0
mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7f7f691dd000
write(1, "ioctl_get_msg failed:Inappropria"..., 52ioctl_get_msg failed:Inappropriate ioctl for device
) = 52
exit_group(25)                          = ?

Also, I noted that udev catches not only stdout, but stderr too (at least when running udevadm test /block/zd0). It results in making of symlinks:

/dev/ioctl_get_msg -> zd0
/dev/zvol/ioctl_get_msg -> ../zd0
fajarnugraha commented 13 years ago

Did you also recompile zfs module?

Changes from IO to IOR in zfs.h means ioctl number has changed. It also means zfs.ko (or to be specific, zvol.c) is changed as well. After installing the new modules, you need to rmmod-modprobe zfs (or just run "depmod -a", and reboot)

For the stderr part, IIRC the error message is spitted out to stdout, so udev works correctly nwhen it created those links. Part of the change I commit is to return non-zero exit code on error, so udev SHOULD ignore the error messages (instead of creating symlinks) IF you're running the patched version.

user318 commented 13 years ago

Did you also recompile zfs module?

Sorry. Didn't know that it touches module too. With new module it is working now. Thank You. Waiting in upstream.

About udev & stdout You are right too.

L0cutus commented 13 years ago

thanks for the patch ! now, can you tell me if this is the normal behaviour ?

zpool create tank /dev/sdd1 -> ok zfs create -V 100G tank/fish -> /dev/tank/fish created ok /dev/zvol/tank/fish -> created sfdisk /dev/zvol/tank/fish << EOF 0, EOF create this: /dev/tank/fish -> disappeared /dev/tank/fish-part1 -> created /dev/zvol/tank/fish-part1 -> created and point to /dev/zd0p1 sfdisk -l /dev/tank/fish -> error not exists

[root@l0cutus ~]# sfdisk -l /dev/tank/fish-part1 Disco /dev/tank/fish-part1: 208048 cilindri, 16 testine, 63 settori/traccia sfdisk: ERRORE: il settore 0 non ha una firma msdos /dev/tank/fish-part1: tipo di tabella delle partizioni non riconosciuto Non si è trovata alcuna partizione

mkfs.ext2 -q /dev/tank/fish-part1 -> crash since never stop.

L0cutus commented 13 years ago

after a reboot and a zpool export tank;zpool import tank, all dirs reappeared, i mean: /dev/tank/fish /dev/tank/fish-part1

after the mks2fs /dev/tank/fish-part1 was done (all ok), /dev/fish-part1 disappeared again so i have only /dev/tank/fish, after i've export/reimport tank pool, all reappeared again, is this ok or there is a problem ?

thanks !

behlendorf commented 13 years ago

Fajar, thanks for looking at this issue. I've pulled your original patch in to the following topic branch. It includes your two fixes and an additional patch with some Makefile magic to ensure the path in 60-zvol matches where 'make install' will place zvol_id. Can you review/test it, if it looks good I'm happy to apply it.

https://github.com/behlendorf/zfs/tree/zvol_id

fajarnugraha commented 13 years ago

@L0cutus: no, that's not normal. It doesn't happen on my RHEL5 testbox.

Upon some more testing, I see that UDEV emits event like this when creating a partition

KERNEL[1301001596.462869] change   /devices/virtual/block/zd0 (block)
KERNEL[1301001596.462907] add      /devices/virtual/block/zd0/zd0p1 (block)
KERNEL[1301001596.463213] change   /devices/virtual/block/zd0 (block)
UDEV  [1301001596.518597] change   /devices/virtual/block/zd0 (block)
UDEV  [1301001596.539798] add      /devices/virtual/block/zd0/zd0p1 (block)
UDEV  [1301001596.563277] change   /devices/virtual/block/zd0 (block)

The "change" event was previously unhandled. Try changing /etc/udev/rules.d/60-zvol.rules, modifying

ACTION=="add"

to

ACTION=="add|change"

@Brian: I'll test it (with this addition) sometime later today

L0cutus commented 13 years ago

perfect ! just tested and it does work ! the only a thing i need to know, when i create a snapshot of tank/fish, is correct to have this under /dev/tank: [root@l0cutus ~]# ls -la /dev/tank/ totale 0 0 drwxr-xr-x 2 root root 120 24 mar 23.32 . 0 drwxr-xr-x 19 root root 5,6K 24 mar 23.32 .. 0 lrwxrwxrwx 1 root root 6 24 mar 23.30 fish -> ../zd0 0 lrwxrwxrwx 1 root root 8 24 mar 23.31 fish-part1 -> ../zd0p1 0 lrwxrwxrwx 1 root root 7 24 mar 23.32 fish@pristine1 -> ../zd16 0 lrwxrwxrwx 1 root root 9 24 mar 23.32 fish@pristine1-part1 -> ../zd16p1

is that correct ?

thanks again !

fajarnugraha commented 13 years ago

Yes, it's correct. You're able to mount those snapshot vol/partitions read only.

Also just a small note, the naming for partitions inside a zvol changed a bit from rc1 (before udev was introduced, the name was fish@pristine1p1, where now it's fish@pristine1-part1). The idea is for simplicity, since "--part1" is more generic compared to "1" (if the volname ends with letter) or "p1" (if the volname ends with number).

Another note, creating a partition is not mandatory. You can just treat zvols like LVs, and create filesystem directly on it. It will allow easy fs/volume resize later.

L0cutus commented 13 years ago

great, thanks again for info/fix/tips ! :-)

fajarnugraha commented 13 years ago

@Brian: I tested the branch zvol_id on RHEL5 + kernel 2.6.32, some more changes needed to be made:

With those changes in place it works as expected

behlendorf commented 13 years ago

Thanks for the review! I've updated the patches with all but one of the above changes and will apply them. The reason you needed to revert 81e97e2 "Linux 2.6.29 compat, credentials" is because there's is a matching changes needed in the spl. Pull the latest spl source and that will resolve you build issue.

behlendorf commented 13 years ago

Applied to master, closing issue.