Skip to content

Use parted (If available) to partition disk, else fallback to sfdisk.#191

Closed
shishir-a412ed wants to merge 1 commit intoprojectatomic:masterfrom
shishir-a412ed:use_parted
Closed

Use parted (If available) to partition disk, else fallback to sfdisk.#191
shishir-a412ed wants to merge 1 commit intoprojectatomic:masterfrom
shishir-a412ed:use_parted

Conversation

@shishir-a412ed
Copy link
Copy Markdown

Signed-off-by: Shishir Mahajan shishir.mahajan@redhat.com

@shishir-a412ed
Copy link
Copy Markdown
Author

ping @rhvgoyal

Comment thread docker-storage-setup.sh Outdated
local dev="$1" size

create_partition_sfdisk(){
local dev="$1" size
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

no spaces before local.

Comment thread docker-storage-setup.sh Outdated

create_partition_parted(){
local dev="$1"
parted $dev --script mklabel gpt mkpart primary 0% 100%
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

add 2 spaces before start of line.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

sfdisk creates dos type partition table by default. I think we should do same for parted. At some point of time we want to switch to GPT but this is not the patch to do so.

We might experience surprises when we move to GPT. So use mklabel dos

@shishir-a412ed shishir-a412ed force-pushed the use_parted branch 2 times, most recently from d1a09d6 to 44a5559 Compare January 26, 2017 17:13
@shishir-a412ed
Copy link
Copy Markdown
Author

@rhvgoyal PTAL.

Comment thread docker-storage-setup.sh Outdated

create_partition_parted(){
local dev="$1"
parted $dev --script mklabel dos mkpart primary 0% 100%
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

it should be mklabel msdos and not mklabel dos

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Where is it documented that parted accepts start and end value in %. Can you point me to it

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

May be we should use 0 as start value which means start at 0 MB and use -1s as end value to tell parted to create partition till end of disk.

I think it will still do the right alignment and create partition beginning at 2048 sector.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I was following your comment for mklabel dos :)
#191 (comment)

I found the mkpart primary 0% 100% syntax on stack overflow. I tested it and it works. We can also do 0 and -1, that should work too.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I see in man page that one can specify unit and % is one of the supported units. so may be parted is parsing 0% and 100% properly.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I have changed mklabel from dos to msdos based on this documentation:
https://www.gnu.org/software/parted/manual/html_node/mklabel.html

Comment thread docker-storage-setup.sh Outdated

create_partition_parted(){
local dev="$1"
parted $dev --script mklabel msdos mkpart primary 0% 100%
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

sfdisk sets this partition type to lvm. For the sake of keeping it same,, lets do the same in parted too. Something like.

parted $dev --script mklabel msdos mkpart primary 0% 100% set 1 lvm

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I forgot the keyword on at the end. So it should be.

arted $dev --script mklabel msdos mkpart primary 0% 100% set 1 lvm on

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

parted $dev --script mklabel msdos mkpart primary 0% 100% set 1 lvm on

Signed-off-by: Shishir Mahajan <shishir.mahajan@redhat.com>
@rhvgoyal
Copy link
Copy Markdown
Collaborator

LGTM

@rhvgoyal
Copy link
Copy Markdown
Collaborator

@rh-atomic-bot r+ 2da4cfa

@rh-atomic-bot
Copy link
Copy Markdown

⌛ Testing commit 2da4cfa with merge 5e1f47b...

@rh-atomic-bot
Copy link
Copy Markdown

☀️ Test successful - status-redhatci
Approved by: rhvgoyal
Pushing 5e1f47b to master...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants