Code review comment for lp:~nick-schutt/lava-dispatcher/nicks-highbank-support

Hello Nick,

I have a few comments about the partition resizing, but don't let them
stop you from going ahead with the merge.

 review approve

> + def resize_rootfs_partition(self, runner):
> + partno = '2'
> + start = None
> +
> + runner.run('parted -s /dev/sda print',
> + response='\s+%s\s+([0-9.]+.B)\s+\S+\s+\S+\s+primary\s+(\S+)' % partno,
> + wait_prompt=False)

This regex looks a little scary to me, but I think we already have
scarier ones. In general, I am worried that small changes in the output
format might break us in the future, but maybe I am overthinking this.

parted has a -m option (standing for machine-readable output), which
produces output in a parsing-friendly way:

$ sudo parted -m -s /dev/sda print
BYT;
/dev/sda:750GB:scsi:512:4096:msdos:ATA Hitachi HTS54757;
1:1049kB:256MB:255MB:ext2::boot;
2:257MB:750GB:750GB:::;
5:257MB:750GB:750GB:::;

I noted that this output does not have the partition type information,
which is needed for the resize command below.

Is the partition to be resized always a primary one? Does it have to be?
Is it safe to just assume it is?

> + if runner.match_id != 0:
> + msg = "Unable to determine rootfs partition"
> + logging.warning(msg)
> + else:
> + start = runner.match.group(1)
> + parttype = runner.match.group(2)
> +
> + if parttype == 'ext2' or parttype == 'ext3' or parttype == 'ext4':
> + runner.run('parted -s /dev/sda rm %s' % partno)
> + runner.run('parted -s /dev/sda mkpart primary %s 100%%' % start)
> + runner.run('resize2fs -f /dev/sda%s' % partno)
> + elif parttpe == 'brtfs':
> + logging.warning("resize of btrfs partition not supported")
> + else:
> + logging.warning("unknown partition type for resize: %s" % parttype)

--
Antonio Terceiro
Software Engineer - Linaro
http://www.linaro.org