bug#15299: [PATCH 1/9] parted: fix EOF and ctrl-c handling

[Phillip Susi]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi Brian, it's been a while and I still haven't heard back from you.
Could you provide some more comments so I can try to get your concerns
addressed and get these applied?  I also have had time to follow up on
some of these, see comments inline.

On 08/13/2013 11:46 PM, Phillip Susi wrote:
> On 08/08/2013 08:45 PM, Brian C. Lane wrote:
>> Commit messages need to follow the normal parted form. I'd also
>> like to see tests as separate commits.
> 
> Can you be a bit more specific about the form?  As far as I know
> this is the usual form that Jim had me use.  He also stressed that
> new feature and test for it go hand in hand, and logically they are
> two parts of the same conceptual change, so it makes sense to keep
> them in the same commit.
> 
>> 4/9 - Nak. I have a more general patch for this -- not
>> everything inserts DMRAID in there.
> 
> Could you be a bit more specific?
> 
>> 5/9 - Shouldn't it use disk->dev->sector_size instead of
>> hard-coded 512/1024? The logic seems a bit messy, using an if
>> block instead of the == 1 ? 512 : 1024 may clean it up a little.
>> If it is already set to 512 (or really ->sector_size) we don't
>> need to walk the other partitions. One place where a tab snuck
>> in. Looks pretty ok other than that.
> 
> I'm not sure what you mean about walking the other partitions.  I 
> suppose it should use ->sector size.  I'll have to check the
> kernel sources again to see how they behave with non 512 byte
> sectors.

I checked the kernel and yes, it does round up to the sector size for
> 512b sectors, so I'll fix this.

>> 6/9 - Nak. I'm not sure if this is the right way to handle
>> things.
> 
> Could you be more specific?
> 
>> 7/9 - Nak. I really don't like the giant pile of changes for a
>> problem that is only in the gpt code.
> 
> Do you have another suggestion?  I don't see another way of fixing
> the bug, and the constant re-reading of the partition table seems
> a senseless waste of time to begin with, so it kills two birds with
> one stone.
> 
>> 8/9 - Nak. It looks to me like removing some of these checks is
>> going to allow user (ui or library) checks that should fail on
>> new data to pass. I'm ok with letting us run with these errors so
>> we can fix them, but you shouldn't be able to enter bad numbers
>> after the warnings.
> 
> I thought that the ui did its own checks and stops you before it
> gets to the library but I'll take a look at that again.

I just tested it and trying to create an overlapping partition does
still throw the message that it can't be done, this is the closest we
can manage, is that ok?  So parted won't let you create a bad table.
I don't have a good way of testing it but I'm pretty sure that the
library will warn another caller about it and they will have to choose
to ignore the exception to proceed with creating a bad table.  I think
that's OK.

>> 9/9 - Nak. I need more details here. The reasoning in the
>> original commit seems sound to me, and I'd be more worried about
>> busy devices than the filesystem not matching (I'm not sure we
>> should even be in the fs detection business anymore).
> 
> The reasoning was sound, it was just based on a falsehood: the
> kernel does indeed still maintain two separate caches for the two
> devices, so when you write a fs superblock to the partition device
> then try to read it via the whole disk device, you don't see the
> superblock unless you flush the caches.  See:
> 
> http://lists.alioth.debian.org/pipermail/parted-devel/2012-December/004289.html
>
> 
> 
> I never heard back from Hans about why he put that in, but removing
> it fixed the problem, and I couldn't find any attempt to resolve
> the cache aliasing problem in the code, or heard of anyone talking
> about it on linux-mm.
> 
> 
> 

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.12 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQEcBAEBAgAGBQJSK1R+AAoJEJrBOlT6nu75lGAH/2SpocdikrSKNkl3pqm1Xyjp
qjneMGMSsZwKOJAEV6CDUOIXQW57SeYH8krEw8zRR+6hSEZcT/LE+vLU4OcHRZhM
E7k0EEzEEZIN41jB2YjTlNp8pD7SZEViILpCFoM6VK3abwjfW8mHfYp/RB7hzFjr
2/8CUIxQ5GnuvWeNarOoczAUiD93NQNXKfZkrJJRqVV4z5NlW2PW3UWjj+fCtyh+
Tr9BpRW/483PikXwfiWsSo4m5RdoXiaGpxCP28dadgjPVCfXP6PxXS6jek/G5HV+
fVKNuK3i5AxZZRKeVK0QuZlj/V7pHEqWMFNK3bk5W6L15jvCMRzT4IdZ+iYcavI=
=iiBa
-----END PGP SIGNATURE-----