[lvm-devel] 答复： Re: 答复： Re: [PATCH 1/1] added resizing for btrfs

[Lidong Zhong]

 Hi Marcus,


I am sending the patch as the attachment, please take for your reference.
I wrote the patch almost two years ago and it passed my test cases that
all I knew(I am not a btrfs specialist, so the patch probably solid enough).
Eric knows the history. He could do more tests with your cooperation.

Regards,
Lidong
>>> Marcus Müller <mueller at kit.edu> 2017-8-25 下午 16:01 >>>
On 08/24/2017 02:18 PM, Zhen Ren wrote:
> Hi,
>
> Sorry for the delay.
>
>>>> Marcus Müller <mueller at kit.edu> 2017-8-24 pm 15:25 >>>
> Hi Eric,
>
>>> ha! Nearly escaped my memory.
>>> Re: your (justified) objections:
>> 1. lack "check" action in your script
>> In patch [1], it uses "btrfs scrub", which I think you can replace it
>> with "btrfs check".
>>> 1. You're right; `btrfs scrub` just checks the file/metadata checksums
>>> on a live system (and can thus correct /file breakage/), whereas `btrfs
>>> check` checks for structural damage.
>>> I'm not sure `btrfs scrub` is the right thing to do – certainly, it will
>>> detect corrupted blocks; but as far as I can see, these aren't a problem
>>> at all during resizing; they would be moved untouched.
>>> I'm, on the other hand, also not sure we should implement anything here
>>> – other filesystem checks are simply skipped if the filesystem is
>>> already mounted (and mounting is a requirement for resizing). `btrfs
>>> check` would take a long time.
> Yeah, I overlooked this potential problem. IMHO, fsadm should have a option to
> decide if "check" is required after resize.
>
>>> So, either btrfs check or a nop; either is fine with me, what would you
>>> prefer?
> Hmm, the SUSE-specific patch was reviewed by btrfs-tools maintainer David Sterba, and tested
> by btrfs guys. I'd suggest to keep the way to check with "btrfs scrub" as in that patch.
>
> Please send v2 after your changes, and I'll get the v2 patch, test it and then send the respective
> test script for Zdenek review.
>
> Thanks!
> Eric
>
>

Hi Zhen,

I'm certainly not one to withhold a revised patch, but: If Lidong Zhong 
(hi!) got his patch reviewed already from btrfs folks – I might 
personally prefer that

For example, the mount point detection based on the external `findmnt` 
program (didn't know that existed) looks more robust, and precautions 
that I didn't take (like setting LC_ALL for calling `btrfs`) indicate 
that the best-case modification of my patch would actually be his patch. 
I haven't thought of it this way so far.

Do you still want me to modify my patch?

Best regards,
Marcus

--
lvm-devel mailing list
lvm-devel at redhat.com
https://www.redhat.com/mailman/listinfo/lvm-devel


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/lvm-devel/attachments/20170825/36465207/attachment.htm>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fsadm-add-support-for-btrfs.patch
Type: application/octet-stream
Size: 6321 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/lvm-devel/attachments/20170825/36465207/attachment.obj>