bug-parted
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Parted 1.6.12


From: Sven Luther
Subject: Re: Parted 1.6.12
Date: Fri, 3 Sep 2004 09:30:59 +0200
User-agent: Mutt/1.5.6+20040523i

On Fri, Sep 03, 2004 at 04:22:50PM +1000, Andrew Clausen wrote:
> On Thu, Sep 02, 2004 at 09:02:20AM +0200, Sven Luther wrote:
> > I had some discussion with Steve Langasek, one of debian's release manager,
> > about including parted 1.6.12 in sarge. Since parted is ipart of base, and 
> > has
> > thus been frozen since more or less amonth, this cause problem, and we were
> > wondering if your fix could not be backported.
> > 
> > Steve (vorlon on irc) suggested : 
> > 
> >   08:45 < vorlon> svenl: it looks to me like the parted bug could be
> >   backported w/o ABI breakage by hiding the new geom info in the 
> > arch_specific
> >   member.
> > 
> > I have some reservation, given that libparted really has no concept of 
> > hidden
> > data structures, and mostly exports everything, but i suppose that the two 
> > new
> > geom stuff could be added at the end of the structure and still keep 
> > backward
> > compatibility. Not sure though.
> > 
> > Do you have any insight on this question ? 
> 
> I agree that putting stuff in arch_specific is "hiding", and you could
> maintain backward compatability using this trick.  It is conceptually
> very ugly, and I don't want to do this work myself.  It would be painful
> for you Debian people to maintain the Parted package like this
> long-term.

Bah, it will be in stable, so only security fixes will need to be involved.
For the next release we would track the normal development.

> To be honest, I don't see why you can't just recompile packages to use
> 1.6.12.  I doubt there will be any compilation problems.  If there were
> any, they would be minor - things like replacing dev->sectors with
> dev->bios_geom.sectors.  How many packages are we talking about anyway?

Well, there are 4 of them, and rebuilding them is a major undertaking this
late in the release process, but Steve will write more on that.  This is
just sucky timing.

> I think this is a time where you need to look at the code and say:
> these are really trivially changes, and aren't going to break anything.
> I think you should be far more concerned with the possibility of new
> bugs in 1.6.12.

Yeah, ... 

Now, from a conceptual point of view, could you explain a bit more what you
did, i am a bit confused about it, and the more i think of it, the more i feel
the ugliness of the solution you proposed.

As i understand the code, the problem only affects the MBR based partition
tables, and and the guessing involves only DOS/FAT/VFAT partitons (maybe also
NTFS).

I suppose that the guessing is done in the dos/fat/vfat probe code, which has
no information whatsoever on the actual partition table code, which is the
same reason i reimplemented partition table reading in the amiga filesystems
to get the block size for them out of the amiga partition table.

So would a solution not be to reimplement a (basic) reading of the
dos/fat/vfat/whatever filesystems, and getting only the geometry information
of those in the MBR reading code, and then use the geometry heuristic there
too to set the dev geometry. I do something similar in amiga partitions, where
the geometry found on an existing partition table overrides the provided ones.

This way, there would be no compatibility lost, and only some minor
duplication of code, and all in all, it is much more elegant code than the
(bios|hw)_geom variant, where i suppose all partition tables except MBR's use
hw_geom all over, right ? 

And for user code, which did make use of dev->cyl|head|sector, which of the
two geometry should they use ? Should they make checks for the partition type,
and use bios if it is a MBR, and hw in other case ? 

Really sorry to not have participated in this discussion earlier, but i was
overbusy with other stuff and such.

Friendly,

Sven Luther




reply via email to

[Prev in Thread] Current Thread [Next in Thread]