Code review comment for lp:~darkmuggle-deactivatedaccount/cloud-init/lp-1233698

Revision history for this message
Scott Moser (smoser) wrote :

There is really no reason for:
 dd_cmd = util.which("dd")
 subp(dd_cmd)

That is identical to:
 subp(dd_cmd)

purge_disk doesn't seem that it should be something that is partition table specific.
The end result is that you have an unpartitioned disk.

A "wipe the disk" method might look like this:
 def wipe_disk(devname):
   for f in $devname[0-9]*:
      [ -b "$f" ] && wipefs $f || :

   dd if=/dev/zero of=$devname
   out=$(dd if=/dev/zero of="$target" bs=1024 \
          seek=$(($size-1024)) count=1024 2>&1)
   blockdev --rereadpt $devname
   udevadm settle

Interestingly, as you've found, an attempt to "wipe" a disk and then partition it, and then mkfs might end up in there being an existing filesystem at any of the new partitions (ie, if the old disk format had partitions there).
So really what you'd need to do is:
 erase partition tables (gpt is at end and beginning, so zero the first MB and last MB)
 format disk
 rereadpt && udevadm settle
 'wipefs' all partitions
 mkfs

Doing that would actually mean we do not have to pass '-F' to mkfs.extX (note, that -F is not common to all'mkfs' programs so 'mkfs.$FSTYPE -F' wont necessarily work).

Probably could also get away with just calling 'wipefs --all $dev' before formatting.

I'm not sure really what i htink about changing the builtin defaults to overwrite filesystems on azure. that seems dangerous. better if we coudl identify this one scenario and wipe it.

« Back to merge proposal