-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 09/11/2013 05:51 PM, Brian C. Lane wrote: > I generally like them to be in a separate commit so that they can > be applied by themselves so I can see it fail before applying the > fix.
I suppose it does make it easier to verify they fail first, but on the other hand, it clutters the history a bit. I'm not sure permanently cluttering the history is worth saving a few keystrokes to verify test failure before applying, but I do see your point so if you feel this is important I'll change it. > See commit 4 from the patches I just sent, it preserves the UUID if > it exists, but doesn't make up a new one (which may break things > expecting a certain form). Ahh yes, your version does look better. > Let me see if I understand the problem. The issue is that the disk > isn't right, the gpt backup is in the wrong location and currently > we will sorta fix that by writing to the correct location if > changes are made even if told to ignore it. There are two related, but distinct errors. One problem is that the apparent size of the disk does not agree with what the GPT header says. The other is the location of the backup table. If a disk grows, then the size will be wrong, but the backup location will be correct, relative to the old size, and incorrect relative to the new size of the disk. Because parted checks for the backup location first, and the size second, after growing a disk, it would complain about the backup location, and then also complain about the size. It didn't make much sense to have to fix both and the first message is a bit confusing, so now it will not complain about the backup location since it does agree with the previous size of the disk, and if you choose to fix the size, the backup will then be moved to the correct location as part of the size fix. If you chose not to fix the location ( whether it is wrong entirely or only because of a change in size ), parted would go ahead and write it to the correct location anyhow, without zeroing the previous copy. This is clearly wrong. > I'd lean towards not allowing any operations at all until the > location is fixed. If it isn't at the end it is in the wrong place > and we really have no way to tell why it isn't right. If the apparent disk size is wrong and the user chooses to let it be ( maybe another OS can't see the full disk size and they want to maintain compatibility with it ), then the backup should also be left where it is, at least if it is in the correct location from the perspective of the other OS. As long as the backup lies outside of the usable portion of the disk, not being at the end ( as we see it ) should not cause any harm, and complaining that it isn't at the end as we see it, after they have already said to accept the other OS's view of where the end of the disk is, is counter productive. In the event that the backup is at neither the current or previous end of the disk, I lean towards allowing the user to ignore that and continue unless we have a compelling reason not to. > Unrelated to that, I also notice that the gpt-head-move.py script > isn't taking into account endianness. I just ran into a similar > problem with the gpt-header-munge script. You can't just > pack/unpack from the header without converting from little-endian. I saw that in your patch set today, and yes, since I adapted from gpt-head-move.py, it probably needs the same fix. > I don't have any better ideas yet. I'm just worried all those > changes are going to break something subtle. That's why we have a regression test suite ;) I'm ok with tabling it for the next release, then apply it after and give it some more testing time. > Ends up I just had to do something similar to get filesystem > probing working all the time. See patch 18 in my set. What I did > was keep the kernel 2.6 check and skip the sync on the individual > partitions. I think Hans' point about udev, etc. churn while doing > this is a good one. parted only operates on offsets inside the > parent device anyway so I think it is better to only sync that. I don't think that is enough. Flushing the disk device will clear out any stale data in that cache and force it to be re-read from disk when we probe the fs type, but if the partition cache still has dirty write buffers, the disk is still out of date so re-reading it will still return stale data. As far as udev churn goes, I have another patch that is now in Debian and Ubuntu that I still need to rebase onto upstream parted that refactors the BLKPG code to avoid removing and recreating unmodified partitions. -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.12 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBAgAGBQJSMTGqAAoJEJrBOlT6nu75m7IIALmSTGaMIwwUxzqIgHsHH2Gj aVGeHOvGOFUFTuaSfi8eVnKxV933CkaoDtvMTNpJFXYnziAKUUxV+lB0mYCHMBUo muiVu0cSC+w4AIlbQBf5XhK1sZgiKSpJrgGkrjWeesVeQRVxvbBpo+opai5cYxEv WQDOYju62D9utEzfZdbvaAC/oNlKwLXlekTFwJuYGgwJum07zbox64iK7CqODBUv fablJSIFAYHYjnL9Qz/MnfFkhFxOrQDS+M8QrZZL/FQmTPVnrKTl4mYW0WfORZrn cYfq0DEtaN2XodqzhsV26Z+zsIjrZ0iEt1x1laI912sVbYqAyqe5GSd928O8HHY= =SsuT -----END PGP SIGNATURE-----
