← Back to team overview

cloud-init-dev team mailing list archive

Re: [Merge] lp:~daniel-thewatkins/cloud-init/lp1460715 into lp:cloud-init

 

some comments inline.
other thing.. it seems like it would make sense to partition assuming sectors of 4096 bytes. just to ensure alignment or to round all partition sizes to 1MB or even 4MB.

if the user has just given us such course grained data as percentage, it seems we have some wiggle room to work with and should try to do same things.

for reference it seems that /sys/block/sda/queue/hw_sector_size has sector sizes. i'm not sure that i'd bother caring though wether there was 512 or 4096 there. I'm thinking just easiest to never partition in smaller units than 4096.


Diff comments:

> === modified file 'cloudinit/config/cc_disk_setup.py'
> --- cloudinit/config/cc_disk_setup.py	2015-07-22 19:14:33 +0000
> +++ cloudinit/config/cc_disk_setup.py	2015-10-19 14:33:26 +0000
> @@ -343,13 +345,16 @@
>  
>  
>  def get_mbr_hdd_size(device):
> -    size_cmd = [SFDISK_CMD, '--show-size', device]
> -    size = None
> +    size_cmd = [SFDISK_CMD, '-l', device]

use --list rather than '-l'

>      try:
> -        size, _err = util.subp(size_cmd)
> +        output, _ = util.subp(size_cmd)
>      except Exception as e:
>          raise Exception("Failed to get %s size\n%s" % (device, e))
>  
> +    match = re.search(r'(?P<sector_count>\d+) sectors', output)
> +    if match is None:
> +        raise Exception("Failed to get %s size\n%s" % (device, e))
> +    size = match.groupdict()['sector_count']
>      return int(size.strip())

you can get size in sectors easier than parsing sfdisk output by either
  blockdev --getsz /dev/sda
or
  $ cat /sys/block/sda/size 

both in 512 size sectors.

http://stackoverflow.com/questions/2773604/query-size-of-block-device-file-in-python
http://unix.stackexchange.com/questions/52215/determine-the-size-of-a-block-device

>  
>  


-- 
https://code.launchpad.net/~daniel-thewatkins/cloud-init/lp1460715/+merge/274897
Your team cloud init development team is requested to review the proposed merge of lp:~daniel-thewatkins/cloud-init/lp1460715 into lp:cloud-init.


References