[PATCH] add setting of the exiting subvolume UUID for fstab

Gene Czarcinski gene at czarc.net
Tue Oct 29 08:56:43 UTC 2013


On 10/28/2013 06:58 PM, David Lehman wrote:
> On Mon, 2013-10-28 at 17:56 -0400, Gene Czarcinski wrote:
>> On 10/28/2013 01:57 PM, David Lehman wrote:
>>> On Sat, 2013-10-26 at 16:13 -0400, Gene Czarcinski wrote:
>>>> Earlier in BTRFSData, the BTRFS volume (the parent of all
>>>> subvolumes) is determined and assigned to "dev".  The value of
>>>> dev.uuid can be used to set the uuid value for the subvolume which,
>>>> in turn, will then be used in fstab.
>>>>
>>>> This patch was tested as an updates img against Fedora 20 Beta TC6.
>>>> The current code works, this patch just makes it work a little better
>>>> by using the UUID.
>>> As I said the last time you posted a patch for this, this is not the
>>> place to fix it. If the subvolumes' UUIDs are not getting set up
>>> correctly during detection of existing storage we should fix it there --
>>> not here.
>>>
>> I guess I do not understand your point.  This is the place in the
>> kickstart code where the mountpoint is set for the existing volume.
> Look where "device" gets set six lines before my recently added line
> that sets the mountpoint. You'll see it gets set by this line:
>
>    device = devicetree.getDeviceByName(self.name)
>
> That is looking up a device in blivet's devicetree. That device has
> already been detected in the system -- all kickstart is doing is
> retrieving it by name.
>
> Since the existing storage has already been scanned, the subvolumes
> should already have their UUIDs assigned just like the main volume does.
> Does that make sense?
Yes, it does make sense.  The problem is that device.uuid is None and 
device.filter.uuid is also None.  What you are pointing out is that 
there may be a flaw in blivet's processing or it may be working as 
someone else intended.  However, since we do have the parent btrfs 
volume in the form of members[0] and can get is uuid with 
members[0].uuid, a "good enough" solution may be to just set this here.

Oops.  I may have found the reason on why we need to do this here.

When fstab is being created [in blivet/__init__.py fstab() ], around 
line 2819 there is the statement:
      devspec = device.fstabSpec
which causes the property around line 676 in blivet/devices.py to be 
executed.  This code returns either self.path or if self.format.uuid 
exists, it returns "UUID=<>"

So, if we want the uuid set, we need to have device.format.uuid set and 
not device.uuid.  That is the way it is designed to work. Therefore, I 
believe that my patch (or a revised version I will resubmit assuming 
that some agreement can be reached) is the correct approach.

I am not sure why blivet does it this way, you will need to ask someone 
who was involved in blivet's design.  A good question to ask is why 
blivet did not set device.format.uuid itself.  It did set 
device.format.mountopts to the correct subvol=xxx value.  Maybe there is 
a bug here that my patch simple compensates for.  I guess I will take a 
look at that code afterall.

BTW, for my last test, I thought I would use a multi-device BTRFS volume 
in a KVM virtual.  I found it interesting that the loop in kickstart for 
"getting a list of all the devices that make up this volume" only got 
the first device for the existing multi-device BTRFS volume.  I guess 
that the "get a list" only applies when a new btrfs volume is being 
created.  Using the same code for two different purposes can be confusing.
>
>> However, as the code now is, when it comes time to actually create the
>> fstab entry, there is no associated UUID value set so the /dev/___ value
>> is used instead.  For BTRFS subvolumes, the UUID value is that of the
>> parent BTRFS-volume.  The BTRFS-volume device was identified  a few
>> lines earlier in this code and the value was assigned to "dev".  If you
>> look a fiew lines after where my patch adds the code, you will see that
>> the sixth paramter of storage.newBTRFS is parents=members which has the
>> BTRFS volume devices list.
>>
>> Now I probably should have coded the right side as "members[0].uuid"
>> rather than "dev.uuid" but, since all devices which make up a BTRFS
>> volume have the same UUID, it really does not matter.
> You could also do this:
>
>   device.format.uuid = device.volume.format.uuid
>
> or this:
>
>   device.format.uuid = device.parents[0].format.uuid
>
> But either of these is missing the point I made earlier.
>
>> This is the point there the parent BTRFS volume is known.  Where else
>> would you have the uuid assigned for existing subvolumes  This is where
>> it is assigned for new subvolumes.  Where else do you have all the right
>> information so readily available?
>>
>> BTW, this IS where existing subvolumes are detected as far as kickstart
>> is concerned!
> No, it isn't. It is where you associate an existing/detected subvolume
> with a mountpoint.
>
>> Gene
>> _______________________________________________
>> anaconda-patches mailing list
>> anaconda-patches at lists.fedorahosted.org
>> https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
>
> _______________________________________________
> anaconda-patches mailing list
> anaconda-patches at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches



More information about the anaconda-patches mailing list