Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Changes in "util-linux" (blkid) -> LABEL vs. LABEL_FATBOOT #30

Open
flipsa opened this issue Aug 26, 2019 · 10 comments
Open

Changes in "util-linux" (blkid) -> LABEL vs. LABEL_FATBOOT #30

flipsa opened this issue Aug 26, 2019 · 10 comments

Comments

@flipsa
Copy link

flipsa commented Aug 26, 2019

Hey @natevw

I just ran into a problem with software that relies on fatfs. See this Xen Orchestra issue.

I am not sure if you are aware of it, but util-linux > 2.33-rc1 now uses "LABEL_FATBOOT" instead of "LABEL" for labels written to the boot sector of a disk. Is it possible to change this in fatfs, or maybe set both LABEL and LABEL_FATBOOT (to the same value)?

@flipsa flipsa changed the title Changes in "util-linux" (blkid) -> LABEL is now LABEL_FATBOOT Changes in "util-linux" (blkid) -> LABEL vs. LABEL_FATBOOT Aug 26, 2019
@natevw
Copy link
Owner

natevw commented Sep 4, 2019

Hi @flipsa, thanks for this report though I must admit I'm a bit puzzled. The only FAT-related field might be the VolLab but I don't recall that is even being exposed by this library and generally this just works on an existing (pre-formatted) partition. I wonder if it's another component that needs to handle the volume's partition table setup differently?

@flipsa
Copy link
Author

flipsa commented Sep 4, 2019

Hey @natevw

Maybe I misunderstood how fatfs works, or how it's being used by Xen Orchestra, but after skimming their code I was under the assumption that they write the label into VolLab. I'm not too familiar with the XO codebase, but I think this is the relevant piece of code:
https://github.com/vatesfr/xen-orchestra/blob/master/packages/xo-server/src/fatfs-buffer.js

@natevw
Copy link
Owner

natevw commented Sep 4, 2019

Yes, it looks like they do use one of this libraries structs directly to format the volume from xo-server itself, and set the VolLab in the boot header.

The summary at https://lkml.org/lkml/2017/10/4/512 is helpful and clarifies that the boot structure value is usually not used. I'm assuming that would be the LABEL_FATBOOT version you mention.

In order to set the other entry which I presume is the LABEL variable, it looks like XO would need to also initialize a special file entry with a "Volume Label" flag attribute set when they format the volume.

This library doesn't usually do anything with this kind of entry except to ignore it when iterating through through a directory listing. However, studying the file entry handling code it looks like it might be possible to set the FREG (octal 0100000) permissions mode bit on a file to set that special Volume Label flag. I haven't tested this though.

@natevw
Copy link
Owner

natevw commented Sep 4, 2019

Actually, correction:

The FREG flag fatfs/struts.js corresponds to S_IFREG in <sys/stat.h>. That is, it indicates that the entry is a "regular" file and so it is usually already set for files.

From the public interface of fatfs it looks like this flag is made inaccessible via the public interface because fchmod whitelists out the mode argument passed to it and preserves the special directory/entry flags. The most straightforward way might be for the XO code to create this special directory entry itself using the [internal] fatfs struct definitions which it is already importing from to initialize the volume boot record.

@julien-f
Copy link

@natevw Hey I'm looking into it but I have to say that I don't see how to do it, would it possible for you to give me a small example? 🙂

@natevw
Copy link
Owner

natevw commented Oct 15, 2019

@julien-f Sorry been busy with other projects and I have to admit I'm myself rusty on the details of this library.

Basically you would need to do something like what dir.addFile would do, creating a file in the root directory chain but with a special bit set. The change is that after the current call to _updateEntry you would tweak the mode, something like:

_updateEntry(vol, mainEntry, {firstCluster:fileCluster, size:0, ctime:true,_touch:true});
mainEntry.Attr.volume_id = true;
// … proceed …

Or more simply, create a file in the root directory of the volume using the normal API and then do what dir.updateEntry would do when newStats.mode = 0 [I think]. The ultimate goal in either case is to trigger a true value for entry.Attr.volume_id in/after _updateEntry so that the special bit gets recorded when the entry is serialized to disk.

Does that help a bit?

Ultimately, since iirc the original XO code where this is getting used is formatting a brand new volume, much more assumptions could be made and it might be possible to sort of "hardcode" all the offsets/entries a bit more since this would be the only directory entry to worry about at that point.

This is unnecessarily complicated because the public API of fatfs prevents this in its attempt to be compatible with the real node.js filesystem API. Specifically fs.fchmod always sets the FREG bit on regular files, whereas for the special volume name "file" it would need to stay cleared. (fs.open seems not to apply mode at all which is probably a bug…)

@natevw
Copy link
Owner

natevw commented Oct 15, 2019

Taking a step back:

I suspect it would be helpful to add a public "format volume" helper method that does everything needed here? That doesn't totally solve the general case (i.e. wouldn't allow changing the label later, etc.) but might help XO be less dependent on the gory internals here.

This would be a top-level API, sibling to the current fatfs.createFileSystem which for clarity creates an instance for accessing an existing filesystem. Something like fatfs.formatVolume taking whatever FAT parameters would be appropriate including a volume label.

Specifically, this would move the work started at https://github.com/vatesfr/xen-orchestra/blob/ad58f6a14795feebe3caf9b1db5f05b593586f6d/packages/xo-server/src/fatfs-buffer.js#L27 into a public method on this library. But make it a bit more generic: using a volume driver rather than initializing own buffer, and un-hardcoding configuration stuff using on driver.sectorSize+driver.numSectors instead. Maybe letting user control e.g. FAT12/16/32. And of course the big issue here of storing the volume label as that special file entry…

@julien-f
Copy link

Sorry been busy with other projects and I have to admit I'm myself rusty on the details of this library.

No problem, I'm also rather busy, I'll take a look at your answer in the coming weeks when I have more time, thank you so much! 🙂

@andrzejressel
Copy link

andrzejressel commented Jun 13, 2020

So yesterday and today I was researching this and I've discovered few interesting things:

I've managed to add hacky support by replacing

    if ('firstCluster' in newStats) {
        entry.FstClusLO = newStats.firstCluster & 0xFFFF;
        entry.FstClusHI = newStats.firstCluster >>> 16;
        entry._firstCluster = newStats.firstCluster;
    }

with

    entry.Attr.volume_id = true
    
    if ('firstCluster' in newStats) {
        entry.FstClusLO = newStats.firstCluster & 0xFFFF;
        entry.FstClusHI = newStats.firstCluster >>> 16;
        entry._firstCluster = newStats.firstCluster;
    }

    entry.FstClusLO = 0
    entry.FstClusHI = 0

in dir.js

UPDATE1:
I see there is one issue however: label is in uppercase and I think cloud-init requires is to be lowercase

UPDATE2:
Fixed:

replace

var _snInvalid = /[^A-Z0-9$%'-_@~`!(){}^#&.]/g;
name = name.toUpperCase().replace(/ /g, '').replace(/^\.+/, '');

with

var _snInvalid = /[^A-Za-z0-9$%'-_@~`!(){}^#&.]/g;
name = name.replace(/ /g, '').replace(/^\.+/, '');

in helpers.js

@andrzejressel
Copy link

andrzejressel commented Jun 13, 2020

So I see some issues with supporting lowercase characters. From what I understand names by default must be uppercase and lowercase letters can be used only(?) in VFAT file names. Unfortunatelly util-linux does not support reading label from them: https://github.com/karelzak/util-linux/blob/f0ca7e80d7a171701d0d04a3eae22d97f15d0683/libblkid/src/superblocks/vfat.c#L162

Lower/Upper case is not an issue, because cloud-init works with both

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants