A safer patch that takes into account the -T flag would be:

[vdev_label_read_config()]
...
} else if ((spa->spa_extreme_rewind || label_txg <= txg) && label_txg > 
best_txg) {

spa_extreme_rewind is set to true when -X or -T is provided.

I’m currently working on a large patch for ZFS on Illumos that is aiming to 
improve the pool recovery process.
It will allow rewinding across configuration changes and import pools with 
missing devices.
It also adds much more descriptive debug messages on import failure.

It’s pending review right now at Delphix.

Pavel

> On Apr 22, 2016, at 7:59 PM, Josh Paetzel <j...@tcbug.org> wrote:
> 
> I can see how using that patch unconditionally is a bad idea. What I'm 
> talking about is using the patch conditionally if you specify the -T flag.
> 
> To turn the question around, what would be the point of requiring -F flags if 
> it's guaranteed not to work without them?
> 
> Thanks,
> 
> Josh Paetzel
> 
> On Apr 22, 2016, at 6:54 PM, ilove zfs <ilove...@icloud.com 
> <mailto:ilove...@icloud.com>> wrote:
> 
>> I'd suggest you ask the author: 
>> https://github.com/zfsonlinux/zfs/issues/2452#issuecomment-61020758 
>> <https://github.com/zfsonlinux/zfs/issues/2452#issuecomment-61020758>
>> 
>> On Apr 22, 2016, at 04:49 PM, Josh Paetzel <j...@tcbug.org 
>> <mailto:j...@tcbug.org>> wrote:
>> 
>>> If -T doesn't work now at all, how would making the patch conditional on 
>>> using -T be unsafe?
>>> 
>>> It's an inherently unsafe operation to begin with. 
>>> 
>>> Thanks,
>>> 
>>> Josh Paetzel
>>> 
>>> On Apr 22, 2016, at 5:58 PM, ilove zfs <ilove...@icloud.com 
>>> <mailto:ilove...@icloud.com>> wrote:
>>> 
>>>> No. As is said in the issue containing the patch, that would be quite 
>>>> unsafe. At minimum it would require a -F or two or three.
>>>> 
>>>> On Apr 22, 2016, at 03:56 PM, Josh Paetzel <jpaet...@fastmail.net 
>>>> <mailto:jpaet...@fastmail.net>> wrote:
>>>> 
>>>>> I just tested it.
>>>>>  
>>>>> 
>>>>> 
>>>>> root@10:/home/jpaetzel <> # zpool create test md0
>>>>> root@10:/home/jpaetzel <> # touch /test/hello
>>>>> root@10:/home/jpaetzel <> # zpool export test
>>>>> root@10:/home/jpaetzel <> # zdb -vuuue test
>>>>> 
>>>>> 
>>>>> 
>>>>> 
>>>>> Uberblock:
>>>>> magic = 0000000000bab10c
>>>>> version = 5000
>>>>> txg = 10
>>>>> 
>>>>> 
>>>>> ....
>>>>> 
>>>>> 
>>>>> 
>>>>> root@10:/home/jpaetzel <> # zpool import -T 6 test
>>>>> root@10:/home/jpaetzel <> # ls /test
>>>>> hello
>>>>> root@10:/home/jpaetzel <> # zpool export test
>>>>> root@10:/home/jpaetzel <> # zpool import -T 5 test
>>>>> root@10:/home/jpaetzel <> # ls /test 
>>>>> root@10:/home/jpaetzel <> #
>>>>>  
>>>>> So the real question is would it be ok to make the change conditional on 
>>>>> the zpool import -T flag.
>>>>> 
>>>>> 
>>>>>  
>>>>>  
>>>>> On Fri, Apr 22, 2016, at 04:12 PM, ilove zfs wrote:
>>>>>> FWIW, that patch never worked for me.
>>>>>>  
>>>>>> On Apr 22, 2016, at 02:01 PM, Josh Paetzel <jpaet...@fastmail.net 
>>>>>> <mailto:jpaet...@fastmail.net>> wrote:
>>>>>>  
>>>>>>> Alright, well that helped me.
>>>>>>> 
>>>>>>> I'll test that patch now, then see if I can make it conditional on the
>>>>>>> -T flag to import.
>>>>>>> 
>>>>>>> On Fri, Apr 22, 2016, at 01:53 PM, Turbo Fredriksson wrote:
>>>>>>>> On Apr 22, 2016, at 6:03 PM, Turbo Fredriksson wrote:
>>>>>>>> 
>>>>>>>> > This is (was?) broken on ZoL as well. Haven't tested it in a year
>>>>>>>> > or so, but it WAS broken at least…
>>>>>>>> 
>>>>>>>> 
>>>>>>>> Just found the ZoL ticket, and it seems to be still a problem
>>>>>>>> (posted Dec 4, 2013, so it's an OOOOLD issue).
>>>>>>>> 
>>>>>>>> https://github.com/zfsonlinux/zfs/issues/1926 
>>>>>>>> <https://github.com/zfsonlinux/zfs/issues/1926>
>>>>>>>> --
>>>>>>>> Imagine you're an idiot and then imagine you're in
>>>>>>>> the government. Oh, sorry. Now I'm repeating myself
>>>>>>>> Mark Twain
>>>>>>>> 
>>>>>>> 
>>>>>>> 
>>>>>>> -- 
>>>>>>> 
>>>>>>> Thanks,
>>>>>>> 
>>>>>>> Josh Paetzel
>>>>>>> 
>>>>>>> 
>>>>>>> http://www.listbox.com <http://www.listbox.com/>
>>>>>  
>>>>> --
>>>>>  
>>>>> Thanks,
>>>>>  
>>>>> Josh Paetzel
>>>>>  
>>>>>  
> 
> openzfs-developer | Archives 
> <https://www.listbox.com/member/archive/274414/=now>  
> <https://www.listbox.com/member/archive/rss/274414/28098372-2227bd0b> | 
> Modify <https://www.listbox.com/member/?&;> Your Subscription        
> <http://www.listbox.com/>



-------------------------------------------
openzfs-developer
Archives: https://www.listbox.com/member/archive/274414/=now
RSS Feed: https://www.listbox.com/member/archive/rss/274414/28015062-cce53afa
Modify Your Subscription: 
https://www.listbox.com/member/?member_id=28015062&id_secret=28015062-f966d51c
Powered by Listbox: http://www.listbox.com

Reply via email to