Code review comment for lp:~yuningdodo/ubuntu/trusty/util-linux/util-linux.backport-wipefs-partition-table-erasing-support

Revision history for this message
Yu Ning (yuningdodo) wrote :

Thanks for the review.

I checked the ubuntu branch, lp:ubuntu/util-linux, bug #1046665 was firstly fixed in rev90 by retrying the operation several times. Later in rev110 the code was entirely merged with debian experimental, and the previous retry was now implemented by calling blkid_do_probe() multiple times until it returns false. What I proposed to do for the trusty branch is similar, first retire the previous retrying logic then introduce the new implementation, so I believe it will not regress bug #1046665. About libblkid, here is a summary of what have been changed:

* new flag: BLKID_PARTS_MAGIC.
* new public method blkid_probe_wipe().
* declaration of blkid_probe_set_magic() is moved to libblkid/src/blkid.h from libblkid/src/superblocks/superblocks.h, so it will become a public method.

Besides these API changes, there are also some internal changes. I think they won't break the API backward compatibility of libblkid.

About the picked patches, I can't say I have a deep enough understanding about the patches, actually I picked them based on the test results of this simple testcase:

dd if=/dev/zero of=data bs=1M count=4
parted data --script -- mklabel msdos
parted data --script -- mkpart primary fat16 0 -0
wipefs -a data # it's expected to output "2 bytes were erased at offset 0x000001fe (dos): 55 aa"

With the first two patches it's not enough to erase the "55aa" signature. In fact the key patch is the last one, 44765fdd841fb1369cf68f360131ed076f3a2771, the other 4 patches are just its dependences. If we want to only pick the key patch, we may have to modify it a lot and causes future conflicts when other patches are being backported.

« Back to merge proposal