Re: svn commit: r294329 - in head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs: . sys

[Shawn Webb]

On Tue, Jan 19, 2016 at 05:00:25PM +, Alan Somers wrote:
> Author: asomers
> Date: Tue Jan 19 17:00:25 2016
> New Revision: 294329
> URL: https://svnweb.freebsd.org/changeset/base/294329
> 
> Log:
>   Disallow zvol-backed ZFS pools
>   
>   Using zvols as backing devices for ZFS pools is fraught with panics and
>   deadlocks. For example, attempting to online a missing device in the
>   presence of a zvol can cause a panic when vdev_geom tastes the zvol.  Better
>   to completely disable vdev_geom from ever opening a zvol. The solution
>   relies on setting a thread-local variable during vdev_geom_open, and
>   returning EOPNOTSUPP during zvol_open if that thread-local variable is set.
>   
>   Remove the check for MUTEX_HELD(_state_lock) in zvol_open. Its intent
>   was to prevent a recursive mutex acquisition panic. However, the new check
>   for the thread-local variable also fixes that problem.
>   
>   Also, fix a panic in vdev_geom_taste_orphan. For an unknown reason, this
>   function was set to panic. But it can occur that a device disappears during
>   tasting, and it causes no problems to ignore this departure.
>   
>   Reviewed by:delphij
>   MFC after:  1 week
>   Relnotes:   yes
>   Sponsored by:   Spectra Logic Corp
>   Differential Revision:  https://reviews.freebsd.org/D4986

I've just been bit by this pretty hard. I have a bhyve VM that's backed
by a zvol. The VM is running root-on-zfs. I wrote some experimental code
that's now preventing the VM from booting (kernel panic due to userland
change). Since I can't import the pool, I have no way of fixing the
problem.

I'm probably just going to go revert this commit locally on my box so I
can get some work done.

Thanks,

-- 
Shawn Webb
HardenedBSD

GPG Key ID:  0x6A84658F52456EEE
GPG Key Fingerprint: 2ABA B6BD EF6A F486 BE89  3D9E 6A84 658F 5245 6EEE


signature.asc
Description: PGP signature

Re: svn commit: r294329 - in head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs: . sys

[Julian Elischer]

On 24/01/2016 10:13 PM, Shawn Webb wrote:

On Tue, Jan 19, 2016 at 05:00:25PM +, Alan Somers wrote:

Author: asomers
Date: Tue Jan 19 17:00:25 2016
New Revision: 294329
URL: https://svnweb.freebsd.org/changeset/base/294329

Log:
   Disallow zvol-backed ZFS pools
   
   Using zvols as backing devices for ZFS pools is fraught with panics and

   deadlocks. For example, attempting to online a missing device in the
   presence of a zvol can cause a panic when vdev_geom tastes the zvol.  Better
   to completely disable vdev_geom from ever opening a zvol. The solution
   relies on setting a thread-local variable during vdev_geom_open, and
   returning EOPNOTSUPP during zvol_open if that thread-local variable is set.
   
   Remove the check for MUTEX_HELD(_state_lock) in zvol_open. Its intent

   was to prevent a recursive mutex acquisition panic. However, the new check
   for the thread-local variable also fixes that problem.
   
   Also, fix a panic in vdev_geom_taste_orphan. For an unknown reason, this

   function was set to panic. But it can occur that a device disappears during
   tasting, and it causes no problems to ignore this departure.
   
   Reviewed by:	delphij

   MFC after:   1 week
   Relnotes:yes
   Sponsored by:Spectra Logic Corp
   Differential Revision:   https://reviews.freebsd.org/D4986

I've just been bit by this pretty hard. I have a bhyve VM that's backed
by a zvol. The VM is running root-on-zfs. I wrote some experimental code
that's now preventing the VM from booting (kernel panic due to userland
change). Since I can't import the pool, I have no way of fixing the
problem.

I'm probably just going to go revert this commit locally on my box so I
can get some work done.

I use an md device to do this in bhyve but I was considering using a zvol.
Populating it in the host system and then presenting it to the VM as a 
disk.

maybe it could be enabled with a sysctl?  Is the problem always present?


Thanks,



___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r294329 - in head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs: . sys

[Slawa Olhovchenkov]

On Thu, Jan 21, 2016 at 01:34:57AM +0200, Andriy Gapon wrote:

> On 20/01/2016 22:03, Alan Somers wrote:
> > On Wed, Jan 20, 2016 at 2:20 AM, Andriy Gapon  wrote:
> >> On 19/01/2016 19:20, Alan Somers wrote:
> >>> The thing is, it never really worked in the first place.  Panics and
> >>> deadlocks are so frequent that I don't think the feature was usable
> >>> for anybody.
> >>
> >> The feature is perfectly usable for me.  I have never run into the 
> >> problems that
> >> you describe.  Why not fix the real bugs that you've run into?
> > 
> > Spectra Logic and iXSystems both experienced many problems with this.
> > The worst is a deadlock that can be triggered simply by pulling a
> > drive from a redundant pool when there exists a zvol anywhere in the
> > system (see https://reviews.freebsd.org/D4998 for a quick way to
> > reproduce).  Fixing it correctly would likely require far more time
> > than I have available.  I just want the bugs to go away.  See that
> > same code review for a change to make the feature optional.
> 
> I think that we all want all the bugs to go way.  One way to remove bugs is to
> remove (disable) code that contains bugs.  That way the perfect bug-free
> software is clearly achievable :-)  Unfortunately, that technique is not 
> always
> welcomed.
> 
> P.S.
> I think that the real problem here is that a method of a geom must never drop
> topology_lock.  In other words, the GEOM management code (like g_xxx() stuff 
> in
> geom_subr.c) expects that a topology can not change underneath it.  But
> zvol_geom_access() clearly breaks that contract.

May be same cause problem with swap on zvol (don't test on latest
-stable)?
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r294329 - in head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs: . sys

[Cy Schubert]

In message <20160121113813.gg37...@zxy.spb.ru>, Slawa Olhovchenkov writes:
> On Thu, Jan 21, 2016 at 01:34:57AM +0200, Andriy Gapon wrote:
> 
> > On 20/01/2016 22:03, Alan Somers wrote:
> > > On Wed, Jan 20, 2016 at 2:20 AM, Andriy Gapon  wrote:
> > >> On 19/01/2016 19:20, Alan Somers wrote:
> > >>> The thing is, it never really worked in the first place.  Panics and
> > >>> deadlocks are so frequent that I don't think the feature was usable
> > >>> for anybody.
> > >>
> > >> The feature is perfectly usable for me.  I have never run into the probl
> ems that
> > >> you describe.  Why not fix the real bugs that you've run into?
> > > 
> > > Spectra Logic and iXSystems both experienced many problems with this.
> > > The worst is a deadlock that can be triggered simply by pulling a
> > > drive from a redundant pool when there exists a zvol anywhere in the
> > > system (see https://reviews.freebsd.org/D4998 for a quick way to
> > > reproduce).  Fixing it correctly would likely require far more time
> > > than I have available.  I just want the bugs to go away.  See that
> > > same code review for a change to make the feature optional.
> > 
> > I think that we all want all the bugs to go way.  One way to remove bugs is
>  to
> > remove (disable) code that contains bugs.  That way the perfect bug-free
> > software is clearly achievable :-)  Unfortunately, that technique is not al
> ways
> > welcomed.
> > 
> > P.S.
> > I think that the real problem here is that a method of a geom must never dr
> op
> > topology_lock.  In other words, the GEOM management code (like g_xxx() stuf
> f in
> > geom_subr.c) expects that a topology can not change underneath it.  But
> > zvol_geom_access() clearly breaks that contract.
> 
> May be same cause problem with swap on zvol (don't test on latest
> -stable)?

Not related but should be disabled by default through sysctl is vdev on 
USB. OK for a laptop but not a server. 100% hang on shutdown/reboot.


-- 
Cheers,
Cy Schubert  or 
FreeBSD UNIX:     Web:  http://www.FreeBSD.org

The need of the many outweighs the greed of the few.


___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r294329 - in head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs: . sys

[alan somers]

On Thu, Jan 21, 2016 at 4:38 AM, Slawa Olhovchenkov  wrote:
> On Thu, Jan 21, 2016 at 01:34:57AM +0200, Andriy Gapon wrote:
>
>> On 20/01/2016 22:03, Alan Somers wrote:
>> > On Wed, Jan 20, 2016 at 2:20 AM, Andriy Gapon  wrote:
>> >> On 19/01/2016 19:20, Alan Somers wrote:
>> >>> The thing is, it never really worked in the first place.  Panics and
>> >>> deadlocks are so frequent that I don't think the feature was usable
>> >>> for anybody.
>> >>
>> >> The feature is perfectly usable for me.  I have never run into the 
>> >> problems that
>> >> you describe.  Why not fix the real bugs that you've run into?
>> >
>> > Spectra Logic and iXSystems both experienced many problems with this.
>> > The worst is a deadlock that can be triggered simply by pulling a
>> > drive from a redundant pool when there exists a zvol anywhere in the
>> > system (see https://reviews.freebsd.org/D4998 for a quick way to
>> > reproduce).  Fixing it correctly would likely require far more time
>> > than I have available.  I just want the bugs to go away.  See that
>> > same code review for a change to make the feature optional.
>>
>> I think that we all want all the bugs to go way.  One way to remove bugs is 
>> to
>> remove (disable) code that contains bugs.  That way the perfect bug-free
>> software is clearly achievable :-)  Unfortunately, that technique is not 
>> always
>> welcomed.
>>
>> P.S.
>> I think that the real problem here is that a method of a geom must never drop
>> topology_lock.  In other words, the GEOM management code (like g_xxx() stuff 
>> in
>> geom_subr.c) expects that a topology can not change underneath it.  But
>> zvol_geom_access() clearly breaks that contract.
>
> May be same cause problem with swap on zvol (don't test on latest
> -stable)?

I'm not familiar with that problem.  Is there a PR?
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r294329 - in head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs: . sys

[Slawa Olhovchenkov]

On Thu, Jan 21, 2016 at 08:03:25AM -0700, alan somers wrote:

> On Thu, Jan 21, 2016 at 4:38 AM, Slawa Olhovchenkov  wrote:
> > On Thu, Jan 21, 2016 at 01:34:57AM +0200, Andriy Gapon wrote:
> >
> >> On 20/01/2016 22:03, Alan Somers wrote:
> >> > On Wed, Jan 20, 2016 at 2:20 AM, Andriy Gapon  wrote:
> >> >> On 19/01/2016 19:20, Alan Somers wrote:
> >> >>> The thing is, it never really worked in the first place.  Panics and
> >> >>> deadlocks are so frequent that I don't think the feature was usable
> >> >>> for anybody.
> >> >>
> >> >> The feature is perfectly usable for me.  I have never run into the 
> >> >> problems that
> >> >> you describe.  Why not fix the real bugs that you've run into?
> >> >
> >> > Spectra Logic and iXSystems both experienced many problems with this.
> >> > The worst is a deadlock that can be triggered simply by pulling a
> >> > drive from a redundant pool when there exists a zvol anywhere in the
> >> > system (see https://reviews.freebsd.org/D4998 for a quick way to
> >> > reproduce).  Fixing it correctly would likely require far more time
> >> > than I have available.  I just want the bugs to go away.  See that
> >> > same code review for a change to make the feature optional.
> >>
> >> I think that we all want all the bugs to go way.  One way to remove bugs 
> >> is to
> >> remove (disable) code that contains bugs.  That way the perfect bug-free
> >> software is clearly achievable :-)  Unfortunately, that technique is not 
> >> always
> >> welcomed.
> >>
> >> P.S.
> >> I think that the real problem here is that a method of a geom must never 
> >> drop
> >> topology_lock.  In other words, the GEOM management code (like g_xxx() 
> >> stuff in
> >> geom_subr.c) expects that a topology can not change underneath it.  But
> >> zvol_geom_access() clearly breaks that contract.
> >
> > May be same cause problem with swap on zvol (don't test on latest
> > -stable)?
> 
> I'm not familiar with that problem.  Is there a PR?

I am find PR 199189.
My expirense slightly different: VirtualBox VM with test install (384M
RAM) hang, not crashed, just infinite wait somewhere. Tested on 10.1.
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r294329 - in head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs: . sys

[Alan Somers]

On Wed, Jan 20, 2016 at 4:09 PM, Andriy Gapon  wrote:
> On 20/01/2016 22:04, Alan Somers wrote:
>> On Wed, Jan 20, 2016 at 2:20 AM, Andriy Gapon  wrote:
>>> On 19/01/2016 19:00, Alan Somers wrote:
 Author: asomers
 Date: Tue Jan 19 17:00:25 2016
 New Revision: 294329
 URL: https://svnweb.freebsd.org/changeset/base/294329

 Log:
   Disallow zvol-backed ZFS pools
>>>
>>> What, again?...
>>>
>>
>> What do you mean, "again"?
>>
>
> zpools on zvols were disabled once in the past, took a while to re-enable the
> support.

Now I see the commit of which you speak.  It seems that zpools on
zvols were removed due to precisely the same problems that I'm
experiencing, then reenabled without comment when pjd imported zpool
version 28.  But the underlying issue remains.  What do you think
about my change in https://reviews.freebsd.org/D4998?  Disable the
feature by default so that those of us who aren't using it can operate
deadlock-free, but allow it to be enabled for people like Kurt Lidl
with unusual configurations.

-Alan
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r294329 - in head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs: . sys

[Alan Somers]

On Wed, Jan 20, 2016 at 2:20 AM, Andriy Gapon  wrote:
> On 19/01/2016 19:20, Alan Somers wrote:
>> The thing is, it never really worked in the first place.  Panics and
>> deadlocks are so frequent that I don't think the feature was usable
>> for anybody.
>
> The feature is perfectly usable for me.  I have never run into the problems 
> that
> you describe.  Why not fix the real bugs that you've run into?
>
> --
> Andriy Gapon

Spectra Logic and iXSystems both experienced many problems with this.
The worst is a deadlock that can be triggered simply by pulling a
drive from a redundant pool when there exists a zvol anywhere in the
system (see https://reviews.freebsd.org/D4998 for a quick way to
reproduce).  Fixing it correctly would likely require far more time
than I have available.  I just want the bugs to go away.  See that
same code review for a change to make the feature optional.

-Alan
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r294329 - in head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs: . sys

[Alan Somers]

On Wed, Jan 20, 2016 at 2:20 AM, Andriy Gapon  wrote:
> On 19/01/2016 19:00, Alan Somers wrote:
>> Author: asomers
>> Date: Tue Jan 19 17:00:25 2016
>> New Revision: 294329
>> URL: https://svnweb.freebsd.org/changeset/base/294329
>>
>> Log:
>>   Disallow zvol-backed ZFS pools
>
> What, again?...
>

What do you mean, "again"?
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r294329 - in head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs: . sys

[Andriy Gapon]

On 20/01/2016 22:04, Alan Somers wrote:
> On Wed, Jan 20, 2016 at 2:20 AM, Andriy Gapon  wrote:
>> On 19/01/2016 19:00, Alan Somers wrote:
>>> Author: asomers
>>> Date: Tue Jan 19 17:00:25 2016
>>> New Revision: 294329
>>> URL: https://svnweb.freebsd.org/changeset/base/294329
>>>
>>> Log:
>>>   Disallow zvol-backed ZFS pools
>>
>> What, again?...
>>
> 
> What do you mean, "again"?
> 

zpools on zvols were disabled once in the past, took a while to re-enable the
support.

-- 
Andriy Gapon
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r294329 - in head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs: . sys

[Andriy Gapon]

On 20/01/2016 22:03, Alan Somers wrote:
> On Wed, Jan 20, 2016 at 2:20 AM, Andriy Gapon  wrote:
>> On 19/01/2016 19:20, Alan Somers wrote:
>>> The thing is, it never really worked in the first place.  Panics and
>>> deadlocks are so frequent that I don't think the feature was usable
>>> for anybody.
>>
>> The feature is perfectly usable for me.  I have never run into the problems 
>> that
>> you describe.  Why not fix the real bugs that you've run into?
> 
> Spectra Logic and iXSystems both experienced many problems with this.
> The worst is a deadlock that can be triggered simply by pulling a
> drive from a redundant pool when there exists a zvol anywhere in the
> system (see https://reviews.freebsd.org/D4998 for a quick way to
> reproduce).  Fixing it correctly would likely require far more time
> than I have available.  I just want the bugs to go away.  See that
> same code review for a change to make the feature optional.

I think that we all want all the bugs to go way.  One way to remove bugs is to
remove (disable) code that contains bugs.  That way the perfect bug-free
software is clearly achievable :-)  Unfortunately, that technique is not always
welcomed.

P.S.
I think that the real problem here is that a method of a geom must never drop
topology_lock.  In other words, the GEOM management code (like g_xxx() stuff in
geom_subr.c) expects that a topology can not change underneath it.  But
zvol_geom_access() clearly breaks that contract.

-- 
Andriy Gapon
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r294329 - in head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs: . sys

[Andriy Gapon]

On 19/01/2016 19:00, Alan Somers wrote:
> Author: asomers
> Date: Tue Jan 19 17:00:25 2016
> New Revision: 294329
> URL: https://svnweb.freebsd.org/changeset/base/294329
> 
> Log:
>   Disallow zvol-backed ZFS pools

What, again?...

>   Using zvols as backing devices for ZFS pools is fraught with panics and
>   deadlocks. For example, attempting to online a missing device in the
>   presence of a zvol can cause a panic when vdev_geom tastes the zvol.  Better
>   to completely disable vdev_geom from ever opening a zvol. The solution
>   relies on setting a thread-local variable during vdev_geom_open, and
>   returning EOPNOTSUPP during zvol_open if that thread-local variable is set.
>   
>   Remove the check for MUTEX_HELD(_state_lock) in zvol_open. Its intent
>   was to prevent a recursive mutex acquisition panic. However, the new check
>   for the thread-local variable also fixes that problem.
>   
>   Also, fix a panic in vdev_geom_taste_orphan. For an unknown reason, this
>   function was set to panic. But it can occur that a device disappears during
>   tasting, and it causes no problems to ignore this departure.
>   
>   Reviewed by:delphij
>   MFC after:  1 week
>   Relnotes:   yes
>   Sponsored by:   Spectra Logic Corp
>   Differential Revision:  https://reviews.freebsd.org/D4986
> 



-- 
Andriy Gapon
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r294329 - in head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs: . sys

[Andriy Gapon]

On 19/01/2016 19:20, Alan Somers wrote:
> The thing is, it never really worked in the first place.  Panics and
> deadlocks are so frequent that I don't think the feature was usable
> for anybody.

The feature is perfectly usable for me.  I have never run into the problems that
you describe.  Why not fix the real bugs that you've run into?

-- 
Andriy Gapon
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r294329 - in head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs: . sys

[Nikolai Lifanov]

On 01/19/16 16:02, Nikolai Lifanov wrote:
> On 01/19/16 15:52, Kurt Lidl wrote:
>> On 1/19/16 1:55 PM, Alan Somers wrote:
>>> On Tue, Jan 19, 2016 at 10:00 AM, Alan Somers 
>>> wrote:
 Author: asomers
 Date: Tue Jan 19 17:00:25 2016
 New Revision: 294329
 URL: https://svnweb.freebsd.org/changeset/base/294329

 Log:
Disallow zvol-backed ZFS pools

Using zvols as backing devices for ZFS pools is fraught with
 panics and
deadlocks. For example, attempting to online a missing device in the
presence of a zvol can cause a panic when vdev_geom tastes the
 zvol.  Better
to completely disable vdev_geom from ever opening a zvol. The
 solution
relies on setting a thread-local variable during vdev_geom_open, and
returning EOPNOTSUPP during zvol_open if that thread-local
 variable is set.

Remove the check for MUTEX_HELD(_state_lock) in zvol_open.
 Its intent
was to prevent a recursive mutex acquisition panic. However, the
 new check
for the thread-local variable also fixes that problem.

Also, fix a panic in vdev_geom_taste_orphan. For an unknown
 reason, this
function was set to panic. But it can occur that a device
 disappears during
tasting, and it causes no problems to ignore this departure.

Reviewed by:  delphij
MFC after:1 week
Relnotes: yes
Sponsored by: Spectra Logic Corp
Differential Revision:https://reviews.freebsd.org/D4986

 Modified:
head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/vdev_impl.h
head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c
head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ioctl.c
head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zvol.c

 Modified:
 head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/vdev_impl.h
>>>
>>> Due to popular demand, I will conditionalize this behavior on a
>>> sysctl, and I won't MFC it.  The sysctl must default to off (ZFS on
>>> zvols not allowed) because having the ability to put pools on zvols
>>> can cause panics even for users who aren't using it.
>>
>> Thank you!
>>
>>> And let me clear up some confusion:
>>>
>>> 1) Having the ability to put a zpool on a zvol can cause panics and
>>> deadlocks, even if that ability is unused.
>>> 2) Putting a zpool atop a zvol causes unnecessary performance problems
>>> because there are two layers of COW involved, with all their software
>>> complexities.  This also applies to putting a zpool atop files on a
>>> ZFS filesystem.
>>> 3) A VM guest putting a zpool on its virtual disk, where the VM host
>>> backs that virtual disk with a zvol, will work fine.  That's the ideal
>>> use case for zvols.
>>> 3b) Using ZFS on both host and guest isn't ideal for performance, as
>>> described in item 2.  That's why I prefer to use UFS for VM guests.
>>
>> The patch as is does very much break the way some people do operations
>> on zvols.  My script that does virtual machine cloning via snapshots
>> of zvols containing zpools is currently broken due to this. (I upgraded
>> one of my dev hosts right after your commit, to verify the broken
>> behavior.)
>>
>> In my script, I boot an auto-install .iso into bhyve:
>>
>> bhyve -c 2 -m ${vmmem} -H -A -I -g 0 \
>> -s 0:0,hostbridge \
>> -s 1,lpc -l com1,stdio \
>> -s 2:0,virtio-net,${template_tap} \
>> -s 3:0,ahci-hd,"${zvol}" \
>> -s 4:0,ahci-cd,"${isofile}" \
>> ${vmname} || \
>> echo "trapped error exit from bhyve: $?"
>>
>> So, yes, the zpool gets created by the client VM.  Then on
>> the hypervisor host, the script imports that zpool and renames it,
>> so that I can have different pool names for all the client VMs.
>> This step now fails:
>>
>> + zpool import -R /virt/base -d /dev/zvol/zdata sys base
>> cannot import 'sys' as 'base': no such pool or dataset
>> Destroy and re-create the pool from
>> a backup source.
>>
>> I import the clients' zpools after the zpools on them has
>> been renamed, so the hypervisor host can manipulate the
>> files directly.  It only disturbs a small amount of the
>> disk blocks on each of the snapshots of the zvol to rename
>> the zpools.
>>
>> In this way, I can instantiate ~30 virtual machines from
>> a custom install.iso image in less than 3 minutes.  And
>> the bulk of that time is doing the installation from the
>> custom install.iso into the first virtual machine.  The
>> cloning of the zvols, and manipulation of the resulting
>> filesystems is very fast.
>>
> 
> Can't you just set volmode=dev and use zfs clone?
> 

Never mind, you want different pool names to manipulate files directly.

>> -Kurt
>>
>>
>>
>> ___
>> svn-src-head@freebsd.org 

Re: svn commit: r294329 - in head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs: . sys

[Kurt Lidl]

On 1/19/16 1:55 PM, Alan Somers wrote:

On Tue, Jan 19, 2016 at 10:00 AM, Alan Somers  wrote:

Author: asomers
Date: Tue Jan 19 17:00:25 2016
New Revision: 294329
URL: https://svnweb.freebsd.org/changeset/base/294329

Log:
   Disallow zvol-backed ZFS pools

   Using zvols as backing devices for ZFS pools is fraught with panics and
   deadlocks. For example, attempting to online a missing device in the
   presence of a zvol can cause a panic when vdev_geom tastes the zvol.  Better
   to completely disable vdev_geom from ever opening a zvol. The solution
   relies on setting a thread-local variable during vdev_geom_open, and
   returning EOPNOTSUPP during zvol_open if that thread-local variable is set.

   Remove the check for MUTEX_HELD(_state_lock) in zvol_open. Its intent
   was to prevent a recursive mutex acquisition panic. However, the new check
   for the thread-local variable also fixes that problem.

   Also, fix a panic in vdev_geom_taste_orphan. For an unknown reason, this
   function was set to panic. But it can occur that a device disappears during
   tasting, and it causes no problems to ignore this departure.

   Reviewed by:  delphij
   MFC after:1 week
   Relnotes: yes
   Sponsored by: Spectra Logic Corp
   Differential Revision:https://reviews.freebsd.org/D4986

Modified:
   head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/vdev_impl.h
   head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c
   head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ioctl.c
   head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zvol.c

Modified: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/vdev_impl.h


Due to popular demand, I will conditionalize this behavior on a
sysctl, and I won't MFC it.  The sysctl must default to off (ZFS on
zvols not allowed) because having the ability to put pools on zvols
can cause panics even for users who aren't using it.


Thank you!


And let me clear up some confusion:

1) Having the ability to put a zpool on a zvol can cause panics and
deadlocks, even if that ability is unused.
2) Putting a zpool atop a zvol causes unnecessary performance problems
because there are two layers of COW involved, with all their software
complexities.  This also applies to putting a zpool atop files on a
ZFS filesystem.
3) A VM guest putting a zpool on its virtual disk, where the VM host
backs that virtual disk with a zvol, will work fine.  That's the ideal
use case for zvols.
3b) Using ZFS on both host and guest isn't ideal for performance, as
described in item 2.  That's why I prefer to use UFS for VM guests.


The patch as is does very much break the way some people do operations
on zvols.  My script that does virtual machine cloning via snapshots
of zvols containing zpools is currently broken due to this. (I upgraded
one of my dev hosts right after your commit, to verify the broken
behavior.)

In my script, I boot an auto-install .iso into bhyve:

bhyve -c 2 -m ${vmmem} -H -A -I -g 0 \
-s 0:0,hostbridge \
-s 1,lpc -l com1,stdio \
-s 2:0,virtio-net,${template_tap} \
-s 3:0,ahci-hd,"${zvol}" \
-s 4:0,ahci-cd,"${isofile}" \
${vmname} || \
echo "trapped error exit from bhyve: $?"

So, yes, the zpool gets created by the client VM.  Then on
the hypervisor host, the script imports that zpool and renames it,
so that I can have different pool names for all the client VMs.
This step now fails:

+ zpool import -R /virt/base -d /dev/zvol/zdata sys base
cannot import 'sys' as 'base': no such pool or dataset
Destroy and re-create the pool from
a backup source.

I import the clients' zpools after the zpools on them has
been renamed, so the hypervisor host can manipulate the
files directly.  It only disturbs a small amount of the
disk blocks on each of the snapshots of the zvol to rename
the zpools.

In this way, I can instantiate ~30 virtual machines from
a custom install.iso image in less than 3 minutes.  And
the bulk of that time is doing the installation from the
custom install.iso into the first virtual machine.  The
cloning of the zvols, and manipulation of the resulting
filesystems is very fast.

-Kurt



___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r294329 - in head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs: . sys

[Nikolai Lifanov]

On 01/19/16 15:52, Kurt Lidl wrote:
> On 1/19/16 1:55 PM, Alan Somers wrote:
>> On Tue, Jan 19, 2016 at 10:00 AM, Alan Somers 
>> wrote:
>>> Author: asomers
>>> Date: Tue Jan 19 17:00:25 2016
>>> New Revision: 294329
>>> URL: https://svnweb.freebsd.org/changeset/base/294329
>>>
>>> Log:
>>>Disallow zvol-backed ZFS pools
>>>
>>>Using zvols as backing devices for ZFS pools is fraught with
>>> panics and
>>>deadlocks. For example, attempting to online a missing device in the
>>>presence of a zvol can cause a panic when vdev_geom tastes the
>>> zvol.  Better
>>>to completely disable vdev_geom from ever opening a zvol. The
>>> solution
>>>relies on setting a thread-local variable during vdev_geom_open, and
>>>returning EOPNOTSUPP during zvol_open if that thread-local
>>> variable is set.
>>>
>>>Remove the check for MUTEX_HELD(_state_lock) in zvol_open.
>>> Its intent
>>>was to prevent a recursive mutex acquisition panic. However, the
>>> new check
>>>for the thread-local variable also fixes that problem.
>>>
>>>Also, fix a panic in vdev_geom_taste_orphan. For an unknown
>>> reason, this
>>>function was set to panic. But it can occur that a device
>>> disappears during
>>>tasting, and it causes no problems to ignore this departure.
>>>
>>>Reviewed by:  delphij
>>>MFC after:1 week
>>>Relnotes: yes
>>>Sponsored by: Spectra Logic Corp
>>>Differential Revision:https://reviews.freebsd.org/D4986
>>>
>>> Modified:
>>>head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/vdev_impl.h
>>>head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c
>>>head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ioctl.c
>>>head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zvol.c
>>>
>>> Modified:
>>> head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/vdev_impl.h
>>
>> Due to popular demand, I will conditionalize this behavior on a
>> sysctl, and I won't MFC it.  The sysctl must default to off (ZFS on
>> zvols not allowed) because having the ability to put pools on zvols
>> can cause panics even for users who aren't using it.
> 
> Thank you!
> 
>> And let me clear up some confusion:
>>
>> 1) Having the ability to put a zpool on a zvol can cause panics and
>> deadlocks, even if that ability is unused.
>> 2) Putting a zpool atop a zvol causes unnecessary performance problems
>> because there are two layers of COW involved, with all their software
>> complexities.  This also applies to putting a zpool atop files on a
>> ZFS filesystem.
>> 3) A VM guest putting a zpool on its virtual disk, where the VM host
>> backs that virtual disk with a zvol, will work fine.  That's the ideal
>> use case for zvols.
>> 3b) Using ZFS on both host and guest isn't ideal for performance, as
>> described in item 2.  That's why I prefer to use UFS for VM guests.
> 
> The patch as is does very much break the way some people do operations
> on zvols.  My script that does virtual machine cloning via snapshots
> of zvols containing zpools is currently broken due to this. (I upgraded
> one of my dev hosts right after your commit, to verify the broken
> behavior.)
> 
> In my script, I boot an auto-install .iso into bhyve:
> 
> bhyve -c 2 -m ${vmmem} -H -A -I -g 0 \
> -s 0:0,hostbridge \
> -s 1,lpc -l com1,stdio \
> -s 2:0,virtio-net,${template_tap} \
> -s 3:0,ahci-hd,"${zvol}" \
> -s 4:0,ahci-cd,"${isofile}" \
> ${vmname} || \
> echo "trapped error exit from bhyve: $?"
> 
> So, yes, the zpool gets created by the client VM.  Then on
> the hypervisor host, the script imports that zpool and renames it,
> so that I can have different pool names for all the client VMs.
> This step now fails:
> 
> + zpool import -R /virt/base -d /dev/zvol/zdata sys base
> cannot import 'sys' as 'base': no such pool or dataset
> Destroy and re-create the pool from
> a backup source.
> 
> I import the clients' zpools after the zpools on them has
> been renamed, so the hypervisor host can manipulate the
> files directly.  It only disturbs a small amount of the
> disk blocks on each of the snapshots of the zvol to rename
> the zpools.
> 
> In this way, I can instantiate ~30 virtual machines from
> a custom install.iso image in less than 3 minutes.  And
> the bulk of that time is doing the installation from the
> custom install.iso into the first virtual machine.  The
> cloning of the zvols, and manipulation of the resulting
> filesystems is very fast.
> 

Can't you just set volmode=dev and use zfs clone?

> -Kurt
> 
> 
> 
> ___
> svn-src-head@freebsd.org mailing list
> https://lists.freebsd.org/mailman/listinfo/svn-src-head
> To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

___
svn-src-head@freebsd.org mailing list

Re: svn commit: r294329 - in head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs: . sys

[Bryan Drewery]

On 1/19/2016 10:07 AM, Shawn Webb wrote:
> On Tue, Jan 19, 2016 at 10:20:38AM -0700, Alan Somers wrote:
>> On Tue, Jan 19, 2016 at 10:08 AM, Kurt Lidl  wrote:
>>> Removing the ability to run different zpools on top of a zvol is
>>> a significant reduction in functionality of the entire system, and a huge
>>> violation of the POLA.
>> The thing is, it never really worked in the first place.  Panics and
>> deadlocks are so frequent that I don't think the feature was usable
>> for anybody.
> I actively use this every day and will be making even more use of it in
> the very near future.

Why?

-- 
Regards,
Bryan Drewery



signature.asc
Description: OpenPGP digital signature

Re: svn commit: r294329 - in head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs: . sys

[Shawn Webb]

On Tue, Jan 19, 2016 at 10:20:38AM -0700, Alan Somers wrote:
> On Tue, Jan 19, 2016 at 10:08 AM, Kurt Lidl  wrote:
> > Removing the ability to run different zpools on top of a zvol is
> > a significant reduction in functionality of the entire system, and a huge
> > violation of the POLA.
> 
> The thing is, it never really worked in the first place.  Panics and
> deadlocks are so frequent that I don't think the feature was usable
> for anybody.

I actively use this every day and will be making even more use of it in
the very near future. I haven't had a single kernel panic. I have had
deadlocks at shutdown, but since it's a laptop and I'm sitting right in
front of it, it's not a big deal.

I'd say fix the underlying problem, don't prevent people from getting
work done.

> 
> >
> > At the very least, can you not add a sysctl that allows the
> > dangerous behavior (even if it defaults to off)?  Myself
> > and certainly others rely on having being able to use a zpool
> > installed into a zvol for hosting bhyve virtual machines.
> 
> Your use case should be unaffected.  The guest has a different ZFS
> instance than the host, so it should work just fine.  Please let me
> know if you have problems.

-- 
Shawn Webb
HardenedBSD

GPG Key ID:  0x6A84658F52456EEE
GPG Key Fingerprint: 2ABA B6BD EF6A F486 BE89  3D9E 6A84 658F 5245 6EEE


signature.asc
Description: PGP signature

Re: svn commit: r294329 - in head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs: . sys

[Alan Somers]

On Tue, Jan 19, 2016 at 10:00 AM, Alan Somers  wrote:
> Author: asomers
> Date: Tue Jan 19 17:00:25 2016
> New Revision: 294329
> URL: https://svnweb.freebsd.org/changeset/base/294329
>
> Log:
>   Disallow zvol-backed ZFS pools
>
>   Using zvols as backing devices for ZFS pools is fraught with panics and
>   deadlocks. For example, attempting to online a missing device in the
>   presence of a zvol can cause a panic when vdev_geom tastes the zvol.  Better
>   to completely disable vdev_geom from ever opening a zvol. The solution
>   relies on setting a thread-local variable during vdev_geom_open, and
>   returning EOPNOTSUPP during zvol_open if that thread-local variable is set.
>
>   Remove the check for MUTEX_HELD(_state_lock) in zvol_open. Its intent
>   was to prevent a recursive mutex acquisition panic. However, the new check
>   for the thread-local variable also fixes that problem.
>
>   Also, fix a panic in vdev_geom_taste_orphan. For an unknown reason, this
>   function was set to panic. But it can occur that a device disappears during
>   tasting, and it causes no problems to ignore this departure.
>
>   Reviewed by:  delphij
>   MFC after:1 week
>   Relnotes: yes
>   Sponsored by: Spectra Logic Corp
>   Differential Revision:https://reviews.freebsd.org/D4986
>
> Modified:
>   head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/vdev_impl.h
>   head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c
>   head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ioctl.c
>   head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zvol.c
>
> Modified: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/vdev_impl.h

Due to popular demand, I will conditionalize this behavior on a
sysctl, and I won't MFC it.  The sysctl must default to off (ZFS on
zvols not allowed) because having the ability to put pools on zvols
can cause panics even for users who aren't using it.  And let me clear
up some confusion:

1) Having the ability to put a zpool on a zvol can cause panics and
deadlocks, even if that ability is unused.
2) Putting a zpool atop a zvol causes unnecessary performance problems
because there are two layers of COW involved, with all their software
complexities.  This also applies to putting a zpool atop files on a
ZFS filesystem.
3) A VM guest putting a zpool on its virtual disk, where the VM host
backs that virtual disk with a zvol, will work fine.  That's the ideal
use case for zvols.
3b) Using ZFS on both host and guest isn't ideal for performance, as
described in item 2.  That's why I prefer to use UFS for VM guests.
4) Using UFS on a zvol as Stefen Esser described works fine.  I'm not
aware of any performance problems associated with mixing UFS and ZFS.
Perhaps Stefan was referring to duplication between the ARC and UFS's
vnode cache.  The same duplication would be present in a ZFS on top of
zvol scenario.

-Alan
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r294329 - in head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs: . sys

[Kurt Lidl]

Removing the ability to run different zpools on top of a zvol is
a significant reduction in functionality of the entire system, and a 
huge violation of the POLA.


At the very least, can you not add a sysctl that allows the
dangerous behavior (even if it defaults to off)?  Myself
and certainly others rely on having being able to use a zpool
installed into a zvol for hosting bhyve virtual machines.

I've managed to deadlock the system when experimenting with this,
but for the normal course of operations, it works just fine.

-Kurt

On 1/19/16 12:00 PM, Alan Somers wrote:

Author: asomers
Date: Tue Jan 19 17:00:25 2016
New Revision: 294329
URL: https://svnweb.freebsd.org/changeset/base/294329

Log:
   Disallow zvol-backed ZFS pools

   Using zvols as backing devices for ZFS pools is fraught with panics and
   deadlocks. For example, attempting to online a missing device in the
   presence of a zvol can cause a panic when vdev_geom tastes the zvol.  Better
   to completely disable vdev_geom from ever opening a zvol. The solution
   relies on setting a thread-local variable during vdev_geom_open, and
   returning EOPNOTSUPP during zvol_open if that thread-local variable is set.

   Remove the check for MUTEX_HELD(_state_lock) in zvol_open. Its intent
   was to prevent a recursive mutex acquisition panic. However, the new check
   for the thread-local variable also fixes that problem.

   Also, fix a panic in vdev_geom_taste_orphan. For an unknown reason, this
   function was set to panic. But it can occur that a device disappears during
   tasting, and it causes no problems to ignore this departure.

   Reviewed by: delphij
   MFC after:   1 week
   Relnotes:yes
   Sponsored by:Spectra Logic Corp
   Differential Revision:   https://reviews.freebsd.org/D4986

Modified:
   head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/vdev_impl.h
   head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c
   head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ioctl.c
   head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zvol.c


[diff truncated]


___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r294329 - in head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs: . sys

[Alan Somers]

On Tue, Jan 19, 2016 at 10:08 AM, Kurt Lidl  wrote:
> Removing the ability to run different zpools on top of a zvol is
> a significant reduction in functionality of the entire system, and a huge
> violation of the POLA.

The thing is, it never really worked in the first place.  Panics and
deadlocks are so frequent that I don't think the feature was usable
for anybody.

>
> At the very least, can you not add a sysctl that allows the
> dangerous behavior (even if it defaults to off)?  Myself
> and certainly others rely on having being able to use a zpool
> installed into a zvol for hosting bhyve virtual machines.

Your use case should be unaffected.  The guest has a different ZFS
instance than the host, so it should work just fine.  Please let me
know if you have problems.

-Alan

>
> I've managed to deadlock the system when experimenting with this,
> but for the normal course of operations, it works just fine.
>
> -Kurt
>
>
> On 1/19/16 12:00 PM, Alan Somers wrote:
>>
>> Author: asomers
>> Date: Tue Jan 19 17:00:25 2016
>> New Revision: 294329
>> URL: https://svnweb.freebsd.org/changeset/base/294329
>>
>> Log:
>>Disallow zvol-backed ZFS pools
>>
>>Using zvols as backing devices for ZFS pools is fraught with panics and
>>deadlocks. For example, attempting to online a missing device in the
>>presence of a zvol can cause a panic when vdev_geom tastes the zvol.
>> Better
>>to completely disable vdev_geom from ever opening a zvol. The solution
>>relies on setting a thread-local variable during vdev_geom_open, and
>>returning EOPNOTSUPP during zvol_open if that thread-local variable is
>> set.
>>
>>Remove the check for MUTEX_HELD(_state_lock) in zvol_open. Its
>> intent
>>was to prevent a recursive mutex acquisition panic. However, the new
>> check
>>for the thread-local variable also fixes that problem.
>>
>>Also, fix a panic in vdev_geom_taste_orphan. For an unknown reason,
>> this
>>function was set to panic. But it can occur that a device disappears
>> during
>>tasting, and it causes no problems to ignore this departure.
>>
>>Reviewed by: delphij
>>MFC after:   1 week
>>Relnotes:yes
>>Sponsored by:Spectra Logic Corp
>>Differential Revision:   https://reviews.freebsd.org/D4986
>>
>> Modified:
>>head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/vdev_impl.h
>>head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c
>>head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ioctl.c
>>head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zvol.c
>
>
> [diff truncated]
>
>
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"