Merge lp://staging/~psusi/ubuntu/natty/parted/fix-dmraid into lp://staging/ubuntu/natty/parted

Proposed by Phillip Susi
Status: Merged
Merged at revision: 89
Proposed branch: lp://staging/~psusi/ubuntu/natty/parted/fix-dmraid
Merge into: lp://staging/ubuntu/natty/parted
Diff against target: 256 lines (+134/-18)
9 files modified
debian/changelog (+17/-0)
debian/control (+1/-0)
debian/patches/dasd-sync.patch (+1/-1)
debian/patches/dm_p_separator.patch (+33/-0)
debian/patches/dmraid.patch (+3/-14)
debian/patches/fix-head-size-assertion.patch (+74/-0)
debian/patches/series (+2/-0)
debian/patches/tiny-disk-constraint.patch (+1/-1)
debian/patches/udevadm-settle.patch (+2/-2)
To merge this branch: bzr merge lp://staging/~psusi/ubuntu/natty/parted/fix-dmraid
Reviewer Review Type Date Requested Status
Colin Watson (community) Approve
Phillip Susi (community) Needs Resubmitting
Review via email: mp+52285@code.staging.launchpad.net

Description of the change

  * Fix probe_partition_for_geom() to fail gracefully rather
    than abort with PED_ASSERT(). (LP: #545911)
  * Add dm_p_separator.patch: Device mapper type should not
    automatically mean add 'p' before the partition number.
    Fall back to adding it only if the previous character is
    a digit. This compilies with kpartx behavior.
  * Refresh dmraid.patch to apply with dm_p_separator.patch

To post a comment you must log in.
Revision history for this message
Colin Watson (cjwatson) wrote :

On Sat, Mar 05, 2011 at 05:44:10AM -0000, Phillip Susi wrote:
> + a digit. This compilies with kpartx behavior.

Typo: "complies".

> === added file 'debian/patches/dm_p_separator.patch'
> --- debian/patches/dm_p_separator.patch 1970-01-01 00:00:00 +0000
> +++ debian/patches/dm_p_separator.patch 2011-03-05 05:43:54 +0000
> @@ -0,0 +1,30 @@
> +Device mapper type should not automatically mean add 'p'
> +before the partition number. Fall back to adding it only
> +if the previous character is a digit. This compilies
> +with kpartx behavior.

The comments I made on your dmraid branch about DEP-3 patch headers
apply here too.

> +@@ -2735,7 +2734,11 @@
> +
> + dev_name = dm_task_get_name (task);
> +
> +- if (asprintf (&vol_name, "%sp%d", dev_name, part->num) == -1)
> ++ if (isdigit(dev_name[strlen(dev_name)-1]))
> ++ {

Please use consistent formatting within files/functions. The local
style is to put a space before open parentheses and around arithmetic
operators, and to cuddle open braces onto the same line as ifs, so:

  if (isdigit (dev_name[strlen (dev_name) - 1])) {

> === modified file 'debian/patches/dmraid.patch'
> --- debian/patches/dmraid.patch 2011-02-02 10:11:46 +0000
> +++ debian/patches/dmraid.patch 2011-03-05 05:43:54 +0000
> @@ -5,10 +5,10 @@
> nodes created needlessly, and make sure that partition nodes for DM-RAID
> devices are not probed.
>
> -Index: b/libparted/arch/linux.c
> +Index: parted/libparted/arch/linux.c
> ===================================================================
> ---- a/libparted/arch/linux.c
> -+++ b/libparted/arch/linux.c
> +--- parted.orig/libparted/arch/linux.c 2011-03-04 20:27:35.611473999 -0500
> ++++ parted/libparted/arch/linux.c 2011-03-04 20:28:46.431473991 -0500

Could you use 'quilt refresh -pab' here to avoid these deltas, since
that's how the previous patches were constructed?

> === modified file 'debian/patches/series'
> --- debian/patches/series 2010-12-24 17:00:39 +0000
> +++ debian/patches/series 2011-03-05 05:43:54 +0000
> @@ -26,3 +26,4 @@
>
> # Symbols for this ABI (amd64 as reference)
> update-abi-symbols.patch
> +dm_p_separator.patch

Please make sure that new patches go above the terminating
update-abi-symbols.patch. You should just be able to edit
debian/patches/series and move this line up to the end of the "Ubuntu
additions" section.

> === modified file 'libparted/labels/dos.c'
> --- libparted/labels/dos.c 2010-08-05 21:06:19 +0000
> +++ libparted/labels/dos.c 2011-03-05 05:43:54 +0000

Please put this in a quilt patch in the "Ubuntu additions" section
rather than applying it directly.

I don't see any substantive problems with this branch, so, like the
dmraid branch, it can be merged once you've fixed these cosmetic
problems. Thanks!

 review needs-fixing

review: Needs Fixing
89. By Phillip Susi

Add fix-head-size-assertion.patch:
Fix probe_partition_for_geom() to fail gracefully rather
than abort with PED_ASSERT(). (LP: #545911)

Revision history for this message
Phillip Susi (psusi) wrote :

How about dem apples?

review: Needs Resubmitting
90. By Phillip Susi

* Add dm_p_separator.patch: Device mapper type should not
  automatically mean add 'p' before the partition number.
  Fall back to adding it only if the previous character is
  a digit. This complies with kpartx behavior and linux
  behavior "since the dawn of time".
* Refresh dasd-sync.patch, dmraid.patch, tiny-disk-constraint.patch,
  udevadm-settle.patch.
* debian/control: Breaks versions of dmraid not
  patched to follow the same 'p' behavior.

Revision history for this message
Colin Watson (cjwatson) wrote :

Thanks, this looks much better now.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches

to all changes: