Renaming a zvol which is opened does not move/rename the symlink in /dev/Pool/ and we lose access to zvol device

[arturpzol]

System information

Type	Version/Name
Distribution Name	Debian Jessie
Distribution Version	8
Linux Kernel	4.4.45, 3.10
Architecture	x86_64
ZFS Version	0.7.2, 0.6.5.6
SPL Version	0.7.2, 0.6.5.6

Describe the problem you're observing

When we rename the zvol which is opened, udev does not change zvol symlink in /dev/Pool/.
Access to zvol device is also not possible after it.

Describe how to reproduce the problem

ZFS 0.6.5.6 and 0.7.2:

zpool create Pool-0 mirror scsi-SQEMU_QEMU_HARDDISK_24121-1 scsi-SQEMU_QEMU_HARDDISK_24121-2 -m /Pools/Pool-0 -f
zfs create -s -b 131072  -V 1073741824 Pool-0/1

ls /dev/zvol/Pool-0/  
1

zfs rename Pool-0/1 Pool-0/2
ls /dev/zvol/Pool-0/
2

zfs get type -t volume 
NAME      PROPERTY  VALUE   SOURCE
Pool-0/2  type      volume  -

-----

zfs rename Pool-0/2 Pool-0/3
ls /dev/zvol/Pool-0/
3

zfs get type -t volume 
NAME      PROPERTY  VALUE   SOURCE
Pool-0/3  type      volume  -

-----

dd if=/dev/zero of=/dev/zvol/Pool-0/3 &
[1] 11040

zfs rename Pool-0/3 Pool-0/4
ls /dev/zvol/Pool-0/
3

zfs get type -t volume 
NAME      PROPERTY  VALUE   SOURCE
Pool-0/4  type      volume  -

then access to the zvol is not possible:

ls /dev/zvol/Pool-0/
3

zfs get type -t volume 
NAME      PROPERTY  VALUE   SOURCE
Pool-0/4  type      volume  -


dd if=/dev/zero of=/dev/zvol/Pool-0/3
dd: failed to open '/dev/zvol/Pool-0/3': No such file or directory

ls /dev/zd0 
/dev/zd0

dd if=/dev/zero of=/dev/zd0
dd: failed to open '/dev/zd0': No such file or directory

udevadm trigger and udevadm settle executed after it does not fix the symlink. Only one solve is export and import Pool again.

When zvol is not used/opened during rename we can see udev events from udevadm monitor:

KERNEL[943.715587] change   /devices/virtual/block/zd0 (block)
KERNEL[943.715599] change   /devices/virtual/block/zd0 (block)
UDEV  [943.719099] change   /devices/virtual/block/zd0 (block)
UDEV  [943.720067] change   /devices/virtual/block/zd0 (block)
UDEV  [943.723341] change   /devices/virtual/block/zd0 (block)
UDEV  [943.726020] change   /devices/virtual/block/zd0 (block)

When zvol is used/opened during rename above udev events are not generated.

It works correctly in 0.6.5.5 and issue was introduced in 0.6.5.6 where some zvol changes have been made.

[behlendorf]

@arturpzol thanks for reporting this. It looks like the issue is cause by this bit of code which prevents the minor renaming for in use volumes, but allow the dataset rename to occur. Let's get @bprotopopov thoughts on this.

[bprotopopov]

Sure I will take a look as soon as I can. As a potential user of this feature, however, I would be concerned with a possibility that this is not a common and well tested workflow/code path in Linux. Typos courtesy of my iPhone On Oct 6, 2017, at 1:01 PM, Brian Behlendorf <[email protected]<mailto:[email protected]>> wrote: @arturpzol<https://github.com/arturpzol> thanks for reporting this. It looks like the issue is cause by this bit<https://github.com/zfsonlinux/zfs/blob/a0430cc5a913406cf471bcafac31c5118d89843f/module/zfs/zvol.c#L2147> of code which prevents the minor renaming for in use volumes, but allow the dataset rename to occur. Let's get @bprotopopov<https://github.com/bprotopopov> thoughts on this. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub<#6729 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/ACX4ucZznYmR_bjFXeTkSu6_UYKT_3FGks5splzjgaJpZM4PwTgT>.

[arturpzol]

@bprotopopov, @behlendorf is this condition here still required as in ef1bdf3 mutex_lock for zvol_state_lock in zvol_ioctl later has been introduced?

I ask regarding to:

Skip opened zvols when renaming minors to avoid modifying
zv_name that might be in use, e.g. in zvol_ioctl().

mentioned here #4346

and

here is a simpler solution we have talked about. I also thought that skipping zvols in use in rename_minors() was a good idea, e.g. in relation to zfs_ioctl() that used zv_name without locking.

mentioned here #3830

BTW: without this condition zvol rename works correctly.

[behlendorf]

@arturpzol It looks to me like now that we're holding the zv_state_lock in zvol_ioctl() and zvol_rename_minors_impl the name is properly protected and it's safe to drop this check.

[bprotopopov]

Yes, I’ve reviewed the code and I can’t see any issues with protection of zvol state remaining after the latest locking rework. One remaining detail is with renaming while suspended that I did not fully look through yet. Typos courtesy of my iPhone On Oct 10, 2017, at 3:24 PM, Brian Behlendorf <[email protected]<mailto:[email protected]>> wrote: @arturpzol<https://github.com/arturpzol> It looks to me like now that we're holding the zv_state_lock in zvol_ioctl() and zvol_rename_minors_impl the name is properly protected and it's safe to drop this check. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub<#6729 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/ACX4ucOrDKLBJOSlSnpdy-40JmhJshZQks5sq8R2gaJpZM4PwTgT>.

[bprotopopov]

I don't feel that I fully understand what is supposed to happen when someone tries to rename a suspended zvol. A zvol can be suspended with or without being open, as I understand it.

If a zvol were to be renamed while suspended, due to asynchronous nature of the zvol_rename_minor_impl(), the zv_name could still contain the old dataset name by the time zvol_resume() was called, and if so, it would seem to me that zvol_resume() would panic in

        if (zv->zv_open_count > 0) {
                VERIFY0(dmu_objset_hold(zv->zv_name, zv, &zv->zv_objset));
                ...

Perhaps zvol_rename_minors() should be synchronous ?

@behlendorf , can you comment ?

[bprotopopov]

@tuxoko maybe you can also comment on the rename/suspend interaction

[behlendorf]

@bprotopopov good catch regarding zvol_resume().

What's curious to me is why we need a dmu_objset_hold() call here at all. When zv_open_count > 0 we know that zvol_first_open has been called and that the zv_objset pointer refers to a valid owned object set. I don't think we need the dmu_objset_hold() and matching dmu_objset_rele() here at all to resume the volume. In which case, there shouldn't be an issue letting the rename run asynchronously to update zv_name.

I'd be interested in @tuxoko's take on this too.

[njvdh]

My intention is to group volumes, in order to be able to snapshot selected volumes at once.
It worked for volumes in a test location, so I decided to rename (move) my production VM' s.
For example: zfs rename STORAGE/W10P STORAGE/VM/W10P

First renamed a volume that was in use by an active VM and noticed the symlinks were not updated.
Then renamed back to the original name and all seemed to be as it was before (tested).

After shutdown of the active VM, I renamed again, but the partition-links were still not updated.

Update: Using the ZFS from standard distribution Ubuntu 16.04 LTS.

[bprotopopov]

@njvdh not sure why you are seeing this; perhaps zvol was still in use when you attempted rename;
try fuser/lsof ?

@behlendorf, looking at the @tuxoko commit 040dab9, it appears that the code segment in zvol_resume() refreshes the zv_objset pointer that could have changed since zvol_suspend() was called. This is done based on the zv_name, which might not be updated in time following rename, and therefore, it is race-prone.
I think there needs to be some sort of interlock between rename and suspend/resume, in order to prevent this from happening, and it is not clear to me if it is there (does not appear to be).

This problem appears to be there regardless of whether we skip referenced zvol on rename or not (the topic of this issue). Perhaps one way to move forward is to address the present issue by removing the check for referenced zvols, and to file another issue that describes the potential concerns vis-a-vis rename and suspend/resume interaction.

[njvdh]

@bprotopopov initially I renamed while the zvol was in use and noticed that the links were not updated.
Then I renamed, shut down the VM that was using it and checked carefully if all was correct again.
I rebooted, checked the links, started the VM, fortunately I introduced no permanent problem.
Then I did the rename again while the zvol was not in use and noticed the links were still not updated.
Using ZFS from Ubuntu 16.04. By the way: the "VM" in STORAGE/VM is a filesystem with VM-images.

[tuxoko]

@bprotopopov
Yes, the zvol_resume and rename does seem quite dangerous. I guess people normally don't do rename while rolback or recv is still going, so we don't see anyone reporting it.

So I look at the filesystem side to code. The correct way seems to get the ds from objset before suspend. And then later use that to find the objset.

[bprotopopov]

thanks for the pointer, @tuxoko, I'll take a look as well

[bbabich]

Looks like it may not just be renaming that triggers it - looking into an issue that looks a bit like this seems to be brought on by creating snapshots or a race condition in 0.7.3.

[stale]

This issue has been automatically marked as "stale" because it has not had any activity for a while. It will be closed in 90 days if no further activity occurs. Thank you for your contributions.