Re: [patch] this is a pyparted patch for 429185

[Joel Andres Granados <jgranado redhat com>]

Jeremy Katz wrote:
> On Sat, 2008-05-10 at 18:22 +0200, Joel Andres Granados wrote:
> > I'm posting the patch here because I don't know where pyparted discussion list lives :)
> >
> > You can find the patch at https://bugzilla.redhat.com/attachment.cgi?id=305028
> >
> > pyparted needs to search the valid partitions before it returns anything in the py_ped_disk_next_partition function.  With this patch we can now save anaconda tracebacks to usb keys.  I build pyparted but didn't test the patch extensively.
> > Comments are greatly appreciated.
>
> This significantly changes the semantics of the next_partition() API
> call.  While there aren't a lot of cases where you care about the
> non-active partitions, there are a few.  
>
> I think that actually the right fix for the bug is probably something
> along the lines of (completely untested)
> diff --git a/partedUtils.py b/partedUtils.py
> index 573bf4c..22bccb6 100644
> --- a/partedUtils.py
> +++ b/partedUtils.py
> @@ -1362,7 +1362,7 @@ class DiskSet:
>  
>          drives = []
>          for d in isys.removableDriveDict().items():
> -            func = lambda p: not p.get_flag(parted.PARTITION_RAID) and
> not p.get_flag(parted.PARTITION_LVM) and p.fs_type.name in ["ext3",
> "ext2", "fat16", "fat32"]
> +            func = lambda p: p.is_active and not
> p.get_flag(parted.PARTITION_RAID) and not
> p.get_flag(parted.PARTITION_LVM) and p.fs_type.name in ["ext3", "ext2",
> "fat16", "fat32"]
>  
>              disk = self.disks[d[0]]
>              parts = filter_partitions(disk, func)
>
>
> You have to ensure that the partition is an active one before doing any
> of get_flag or fs_type, etc.  Yeah, it's a little ugly, but it's a part
> of the API and until we completely replace it, I think we have to live
> with it
>
> Jeremy
>
> _______________________________________________
> Anaconda-devel-list mailing list
> Anaconda-devel-list redhat com
> https://www.redhat.com/mailman/listinfo/anaconda-devel-list

The argument of waiting for pyparted to be rewritten is a considerable one.  If your going to do the whole thing from scratch, why bother adding new stuff to the current one.  OTOH, checking for the validity of the partition so in the anaconda code make me feel dirty.  The other thing is that fix to anaconda goes in and then might stays there even when the new pyparted is being used.  It might be good to do the anaconda change and have a bug (if someone has a better idea of how to not let this fall through the cracks please tell me) that documents this issue.

btw, where does the new pyparted live?  the patch is done against the latest version present in ssh://git.fedorahosted.org/git/pyparted.git  maybe I can help speed things up and include a patch in the new pyparted. (something like an argument that will give you the "next partition" or the "next sane partition")

thx for the comments.


--
Joel Andres Granados
Red Hat / Brno, Czech Republic