[parted-devel] Queries/doubts regarding linux_swap.c

[David Cantrell]

Debarshi 'Rishi' Ray wrote:
> I have a few queries/doubts regarding the linux-swap code in Parted
> 1.8.0. It would be very helpful if some of you could help me a bit
> regarding this.
> 
> 1. libparted/fs/linux_swap/linux_swap, lines 42-69: The structure of
> the swap header as a union of SwapOldHeader and SwapNewHeader is not
> the same as defined in the vanilla Linux 2.6.18.2 kernel. Ref:
> include/linux/swap.h, lines 52-67. Is SwapOldHeader defined the way it
> is, because of the difficulty in knowing the page size while defining
> the structure or for some other reason? Why were the sws_uuid and
> sws_volume fields left out in SwapNewHeader?

This code was taken from mkswap a while ago.  It doesn't so much matter 
that the sws_uuid and sws_volume are not defined because the size of the 
header is still the same so that swap partitions are compatible across 
Linux revisions.  In swap.h in the kernel, the padding is decreased to 
make room for sws_uuid and sws_volume.

We never use these in libparted, but there's no harm in syncing up the 
definitions to match current standards.  We should not directly include 
swap.h because including kernel headers in userspace code is deemed bad 
by kernel developers at the moment.  This is fine as it gives them more 
control over those headers.  It presents problems for applications like 
parted that do need those things.  The stop gap is to duplicate the 
definition in our code.

> 2. libparted/fs/linux_swap/linux_swap.h, lines 204-247: The function
> swap_open calls swap_alloc, swap_init and ped_geometry read. Now
> swap_init already does make an identical call to ped_geometry_read. If
> swap_init could return the status of ped_geometry_read instead of a
> void, then this second call to ped_geometry_read can be done away
> with. Am I correct?

Well, there in lies a problem.  State is not guaranteed through the run 
of the application.  Someone could have parted running and then go and 
muck with the system behind its back and then come back to parted to do 
stuff.  We need the extra ped_geometry_read call to make sure we deal 
with the system state right then.

This is part of a larger problem in libparted.  It's more about how we 
want to restrict the system when people are working with partitions. 
Obviously you want to allow users maximum system usability, but when you 
start dealing with partition operations, you have problems.  If we can 
establish a clean way to guarantee atomic operations and allow maximum 
system functionality, I think that would be nice.  We should explore 
locking options available to us for parted 2.0.

-- 
David Cantrell
Red Hat / Westford, MA