Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Try wipefs and use dd as fallback (issue1327 and related to issue799) #1336

Conversation

jsmeix
Copy link
Member

@jsmeix jsmeix commented Apr 28, 2017

Hereby I implemented basically my proposal
#1327 (comment)
and additionally now dd is used as generic fallback in any case.

I tested it on SLE12 with wipefs in the recovery system
and also without wipefs in the recovery system
(i.e. for me also the dd fallback works).

Details:

With wipefs in the recovery system
I get in diskrestore.sh

# Using wipefs to cleanup '/dev/sda2' before creating filesystem.
wipefs --all --force /dev/sda2 || wipefs --all /dev/sda2 || dd if=/dev/zero of=/dev/sda2 bs=512 count=1 || true

and in the "rear -d -D recover" log file

+ source /usr/share/rear/layout/recreate/default/200_run_script.sh
...
+++ echo -e 'Creating filesystem of type ext4 with mount point / on /dev/sda2.'
+++ wipefs --all --force /dev/sda2
+++ mkfs -t ext4 -b 4096 -i 16377 -U 46d7e8be-7812-49d1-8d24-e25ed0589e94 /dev/sda2

Without wipefs in the recovery system
I get in diskrestore.sh

# Using dd to cleanup the first 512 bytes on '/dev/sda2' before creating filesystem.
dd if=/dev/zero of=/dev/sda2 bs=512 count=1 || true

and in the "rear -d -D recover" log file

+ source /usr/share/rear/layout/recreate/default/200_run_script.sh
...
+++ echo -e 'Creating filesystem of type ext4 with mount point / on /dev/sda2.'
+++ dd if=/dev/zero of=/dev/sda2 bs=512 count=1
1+0 records in
1+0 records out
512 bytes copied, 0.00146513 s, 349 kB/s
+++ mkfs -t ext4 -b 4096 -i 16377 -U 46d7e8be-7812-49d1-8d24-e25ed0589e94 /dev/sda2

@jsmeix jsmeix added bug The code does not do what it is meant to do cleanup enhancement Adaptions and new features labels Apr 28, 2017
@jsmeix jsmeix added this to the ReaR v2.1 milestone Apr 28, 2017
@jsmeix jsmeix self-assigned this Apr 28, 2017
@jsmeix jsmeix requested review from gdha and gozora April 28, 2017 14:07
@jsmeix
Copy link
Member Author

jsmeix commented Apr 28, 2017

@gdha @gozora
I added you as reviewers here mainly FYI.

If there are no furious objections I will "just merge" it soon
because from my point of view it is a noticeable improvement
even if it is not yet the ultimate solution, cf.
#1327 (comment)
versus
#1327 (comment)
and the generic missing functionality
#799

But it follows the traditional Unix philosopy "Worse is better"
that is basically "simplicity outweighs all", see
https://en.wikipedia.org/wiki/Unix_philosophy
and
https://en.wikipedia.org/wiki/Worse_is_better

Copy link
Member

@gozora gozora left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According my brief check $has_wipefs will always be true.
Can't we just completely get rid of it together with all test "$has_wipefs" ?

Copy link
Member

@gozora gozora left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Despite my previous comment I don't have any big objections ;-).

@jsmeix
Copy link
Member Author

jsmeix commented Apr 28, 2017

@gozora
yes, I know, you are right, the current code is not nice.
The 'wipefs*' variables should be renamed to something
like 'cleanup_disk*' and the whole current wipefs/dd code
should be cleaned up - but I have no time for that now.
So for now I only did some kind of "quick addon hack"
to make "rear recover" working a bit better in practice,
see the comments in my code (I mention "Dirty hacks" ;-)

@jsmeix jsmeix merged commit ccb28c7 into rear:master Apr 28, 2017
@jsmeix
Copy link
Member Author

jsmeix commented Apr 28, 2017

I removed the "cleanup" label from this pull request
because it really does not cleanup the code
(but it better cleans up the disk now ;-)

@jsmeix jsmeix deleted the try_wipefs_and_use_dd_as_fallback_issue1327_and_related_to_issue799 branch April 28, 2017 14:43
jsmeix added a commit that referenced this pull request May 4, 2017
…ssue1327_and_pullrequest1336

Cleanup wipefs code, see
#1336 (comment)
which is related to #1327
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug The code does not do what it is meant to do enhancement Adaptions and new features fixed / solved / done
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants