Manage generic attributes

[guillaumerems]

As the attributes are not used by default in littlefs but it is intended to be used depending on the implementation.

In my project I use the attributes to store the modification date of an entry.

Here is my modification I have made on lfs to implement it:

static int lfs_dir_update(lfs_t *lfs, lfs_dir_t *dir,
        const lfs_entry_t *entry,
                          const void *attribute, const void *name) {
    return lfs_dir_commit(lfs, dir, (struct lfs_region[]){
            {entry->off, sizeof(entry->d), &entry->d, sizeof(entry->d)},
            {entry->off+sizeof(entry->d), entry->d.alen, attribute, entry->d.alen},
            {entry->off+sizeof(entry->d)+entry->d.alen, entry->d.nlen, name, entry->d.nlen}
        }, name ? 3 : attribute ? 2 : 1);
}

static int lfs_dir_append(lfs_t *lfs, lfs_dir_t *dir,
        lfs_entry_t *entry, const void *attribute, const void *name) {
    // check if we fit, if top bit is set we do not and move on
    while (true) {
        if (dir->d.size + lfs_entry_size(entry) <= lfs->cfg->block_size) {
            entry->off = dir->d.size - 4;
            return lfs_dir_commit(lfs, dir, (struct lfs_region[]){
                    {entry->off, 0, &entry->d, sizeof(entry->d)},
                    {entry->off, 0, attribute, entry->d.alen},
                    {entry->off, 0, name, entry->d.nlen}
                }, 3);
        }

        // we need to allocate a new dir block
        if (!(0x80000000 & dir->d.size)) {
            lfs_dir_t newdir;
            int err = lfs_dir_alloc(lfs, &newdir);
            if (err) {
                return err;
            }

            newdir.d.tail[0] = dir->d.tail[0];
            newdir.d.tail[1] = dir->d.tail[1];
            entry->off = newdir.d.size - 4;
            err = lfs_dir_commit(lfs, &newdir, (struct lfs_region[]){
		            {entry->off, 0, &entry->d, sizeof(entry->d)},
		            {entry->off, 0, attribute, entry->d.alen},
		            {entry->off, 0, name, entry->d.nlen}
                }, 3);
            if (err) {
                return err;
            }

            dir->d.size |= 0x80000000;
            dir->d.tail[0] = newdir.pair[0];
            dir->d.tail[1] = newdir.pair[1];
            return lfs_dir_commit(lfs, dir, NULL, 0);
        }

        int err = lfs_dir_fetch(lfs, dir, dir->d.tail);
        if (err) {
            return err;
        }
    }
}

In lfs_mkdir:

...
	uint8_t attribute[ATTRIBUTE_LENGTH];
    entry.d.type = LFS_TYPE_DIR;
    entry.d.elen = sizeof(entry.d) - 4;
	entry.d.alen = lfs_dir_compute_attribute(attribute);
	entry.d.nlen = strlen(path);
    entry.d.u.dir[0] = dir.pair[0];
    entry.d.u.dir[1] = dir.pair[1];

    cwd.d.tail[0] = dir.pair[0];
    cwd.d.tail[1] = dir.pair[1];

    err = lfs_dir_append(lfs, &cwd, &entry, attribute, path);
...

In lfs_dir_read

...
	if(entry.d.alen > 0){
		lfs_dir_extract_attribute(lfs, dir, &entry, info);
	}
...

In lfs_file_open

...
    if (err == LFS_ERR_NOENT) {
        if (!(flags & LFS_O_CREAT)) {
            return LFS_ERR_NOENT;
        }

	    uint8_t attribute[ATTRIBUTE_LENGTH];

        // create entry to remember name
        entry.d.type = LFS_TYPE_REG;
        entry.d.elen = sizeof(entry.d) - 4;
        entry.d.alen = lfs_dir_compute_attribute(attribute);
        entry.d.nlen = strlen(path);
        entry.d.u.file.head = 0xffffffff;
        entry.d.u.file.size = 0;
	    err = lfs_dir_append(lfs, &cwd, &entry, attribute, path);
	    if (err) {
		    return err;
	    }
    } else if (entry.d.type == LFS_TYPE_DIR) {
        return LFS_ERR_ISDIR;
    } else if (flags & LFS_O_EXCL) {
        return LFS_ERR_EXIST;
    }
...

In lfs_file_sync

...
	    uint8_t attribute[ATTRIBUTE_LENGTH];
        entry.d.u.file.head = file->head;
        entry.d.u.file.size = file->size;
		entry.d.alen = lfs_dir_compute_attribute(attribute);
        err = lfs_dir_update(lfs, &cwd, &entry, attribute, NULL);
...

And then the 2 function to generate and extract the attributes

static uint8_t lfs_dir_compute_attribute(uint8_t *buf) {
	uint32_t datetime = now(NULL);
	buf[0] = LFS_ATTRIBUTE_TIME;
	buf[1] = datetime & 0xFF;
	buf[2] = (datetime >> 8) & 0xFF;
	buf[3] = (datetime >> 16) & 0xFF;
	buf[4] = (datetime >> 24) & 0xFF;

	return ATTRIBUTE_LENGTH;
}
static int lfs_dir_extract_attribute(lfs_t *lfs, lfs_dir_t *dir, lfs_entry_t *entry, struct lfs_info *info) {
	uint8_t *attribute = malloc(entry->d.alen);
	ASSERT(attribute);
	if (attribute) {
		int err = lfs_bd_read(lfs, dir->pair[0],
		                      entry->off + 4 + entry->d.elen,
		                      attribute, entry->d.alen);
		if (err) {
			free(attribute);
			return err;
		}

		if (entry->d.alen >= ATTRIBUTE_LENGTH) {
			if (attribute[0] == LFS_ATTRIBUTE_TIME) {
				info->time =
						attribute[1] + attribute[2] * 256 + attribute[3] * 256 * 256 + attribute[4] * 256 * 256 * 256;
			}
		}

		free(attribute);
	}

	return 0;
}

What do you think to include this in littlefs by default, to manage the attributes and let the developer to implement its custom attributes ?

[geky]

This is very cool! So this adds the modification time as an attribute to the info structure? I wonder if it would be possible to make the API more generic for different types of attributes.

One concern is if this adds to the code size. It'd be interesting to see if it's possible to add attributes without changing the existing functions, since that would allow those functions to be completely gced at compile time if they aren't used.

[guillaumerems]

Exactely with this modification I have the last modification time for each entry, which was mandatory for me.

In order to get a generic API, It is exactely what I tried to do when I modified the littlefs lib
I had to modify lfs_dir_update and lfs_dir_append because I had no solution to write the attributes before the name at the right place, but it is possible to set attribute at NULL if you do not have any.
I wanted to put the 2 new functions with __weak attribute so to let the developer implement them but I need lfs_bd_read which is static so we have to find a better way ;)

[sn00pster]

Would love to see some more examples/documentation on how to implement these attributes.

Timestamp would be very nice for a file.

[guillaumerems]

Here is an example: #31
;)

[sn00pster]

:D

I've seen your example I was just wondering if there's a documented example of exactly what/why the changes are! Because, well, lazy ;)

[guillaumerems]

These changes implement the attributes planned in littlefs but were not implemented and force to 0 by default.
So this example show how to put an attribute with a timestamp but it could be whatever else.

[geky]

Sorry I haven't had time yet to add actual support for custom attributes. I'm still impressed you managed to get the timestamp attributes working with the implementation as is.

Instead of adding attributes specific to timestamps, what if we added generic functions that could be used for any type of attribute? (roughly based on getxattr and friends).

// note, attributes types are limited to 8-bits, 
int lfs_attr_get(lfs_t *lfs, const char *path, uint8_t type, void *buffer, size_t size);
int lfs_attr_set(lfs_t *lfs, const char *path, uint8_t type, const void *buffer, size_t size);
int lfs_attr_remove(lfs_t *lfs, const char *path, uint8_t type);

For example, a system level wrapper could add attributes as needed (sorry for my pseduocode):

#define SYSTEM_ATTR_TIME 0x10

int system_file_open(const char *path) {
    lfs_file_open(&lfs, &file, path, O_RDWR);

    struct system_time current_time = get_time();
    lfs_attr_set(&lfs, path, SYSTEM_ATTR_TIME, &current_time, sizeof(current_time));
}

int system_file_stat(const char *path, struct system_info *sysinfo) {
    struct lfs_info info;
    lfs_stat(path, &info);
    copy_to_system_info(&info, sysinfo);
    lfs_attr_get(&lfs, path, SYSTEM_ATTR_TIME, &sysinfo.create_time, sizeof(sysinfo.create_time));
}

Could these work as a replacement for building timestamps into littlefs? What do you think?

[sn00pster]

I like! The easier it is to add/retrieve these attributes the better from my point of view.

[loboris]

@geky

Any news about implemening generic attributes ?

[geky]

Hi, sorry I've been silent for a while.

I didn't want to make too much noise until I actually had something to show, and I still don't quite yet.

Adding custom attribute has been surprisingly tricky because right now the littlefs driver doesn't currently support resizing metadata entries. This required some fundamental changes to the internals of littlefs, so it's required a decent bit of work.

I've been working on adding resizable entries on these two branches as a part of adding inline files:
https://github.com/geky/littlefs/tree/inline-files
https://github.com/geky/littlefs/tree/resizable-entries

After getting resizable entries in, custom attributes should come in shortly after.

[loboris]

Thank you, I hope it will be ready soon.

In the meantime, I have implemented littlefs with atributes support (for timestamp) based on guillaumerems's PR in in my MicroPython for ESP32. It works great, I've had no issues so far.

[guillaumerems]

@loboris good to hear ;)

[geky]

Finally got around to implementing custom attributes. Unfortunately hit an interesting problem.

The attr get/set/remove functions I outlined above work well until you care about atomic operations (prototype here). If you call setattr with a timestamp and then close the file, if power is lost you might get only the timestamp update.

For timestamps this might not be a big deal, but if custom attributes are used for, say, encryption, the user might lose their file.

littlefs could buffer setattr calls, and write them out atomically, but this would require more RAM...

---

So here's what I'm thinking. littlefs provides an lfs_file_setattrs function that takes an array of attributes which all get written out with the file update.

The tricky part here is that it's up to the user to keep this memory backing these attributes allocated until the file is closed.

Here's what the API might look like:

// Custom attribute structure
struct lfs_attr {
    // Type of attribute, provided by user and used to identify attribute
    uint8_t type;

    // Pointer to buffer containing the attribute
    void *data;

    // Size of attribute in bytes, limited to LFS_ATTRS_MAX
    lfs_size_t size;
};

// Get custom attributes attached to a file
//
// Attributes are looked up based on the type id. If the stored attribute is
// smaller than the buffer, it is padded with zeros. It the stored attribute
// is larger than the buffer, LFS_ERR_RANGE is returned.
//
// Returns a negative error code on failure.
int lfs_file_getattrs(lfs_t *lfs, lfs_file_t *file, struct lfs_attr *attrs, int count);

// Set custom attributes on a file
//
// The array of attributes will be used to update the attributes stored on
// disk based on their type id. Unspecified attributes are left unmodified.
//
// Note: Attributes are not actually written out until a call to lfs_file_sync
// or lfs_file_close and must be allocated until the file is closed or
// lfs_file_setattrs is called with a count of zero.
//
// Returns a negative error code on failure.
int lfs_file_setattrs(lfs_t *lfs, lfs_file_t *file, struct lfs_attr *attrs, int count);

And here's what it could look like to add timestamps with your own file type:

typedef struct time_file {
    lfs_file_t file;
    uint32_t timestamp;
    struct lfs_attr attrs[1];
} time_file_t;

int time_file_open(lfs_t *lfs, time_file_t *file, char *path, int flags) {
    lfs_file_open(lfs, &file->file, path, flags);

    file->attrs[0].type = LFS_ATTR_TIMESTAMP;
    file->attrs[0].data = &file->timestamp;
    file->attrs[0].size = sizeof(file->timestamp);
    lfs_file_getattrs(lfs, &file->file, file->attrs, 1); // load attrs from disk
    lfs_file_setattrs(lfs, &file->file, file->attrs, 1); // setup file attrs to be used when file is written
}

int time_file_write(lfs_t *lfs, struct time_file *file, void *buffer, size_t size) {
    file->timestamp = now(NULL);
    lfs_file_write(file->lfs, &file->file, buffer, size);
}

There should probably also be standalone lfs_getattrs and lfs_setattrs functions using just a path:

uint32_t timestamp;
lfs_getattrs(&lfs, "/hi", (struct lfs_attr[]){
    {LFS_ATTR_TIMESTAMP, &timestamp, sizeof(timestamp)}}, 1);

It's a bit complicated, but gets the job done.

What do you guys think?

[geky]

It still needs a lot of work (mostly testing, I wouldn't try it out yet). But here's an pr for the lfs_setattrs/lfs_getattrs functions: #48

[guillaumerems]

Thanks @geky I will test it
I hope the migration from my version already in the filesystem will not be too hard :(

[geky]

Oh, I hadn't thought about that...

Hold on, right now it will probably break. I should add a check that ignores malformed attributes. (I added a length to for each attribute). Would it work if your existing attributes are just ignored?

[guillaumerems]

Of course we have to manage incorrect attributes.

I read quickly the code and it looks good, I will try to run it in the next days.
Quick question, does it follow the example in the SPEC or not ? I think you added the size of each attributes right ?

[geky]

Yeah, my bad. Thanks for pointing it out.

Unfortunately this does break what's documented in the SPEC.md. Documenting the attribute region early was a mistake on my part.

Not storing the length on each attribute will make it difficult in the future for two systems to share a filesystem if they have different attributes. (for example mounting an embedded device on a PC).

Do you think it's not worth breaking the SPEC? I don't know how many people are using custom attributes at the moment.

At least it's easy to make the system ignore the old attributes. Though you may need to change the id you're using to identify the timestamp.

[guillaumerems]

No problem I will handle it, it is not critical for the moment.
I prefer to do it right now and get a clean solution for the future

[husigeza]

Hello!
Do you have any wild guesses when the official solution is going to be released ? Both versions seems good, however for future compatibility if the official one will be released soon, then it would worth the time to wait for it. Thanks!

[geky]

Hi @husigeza, sorry for the delay on this. At the moment I would guess it's roughly a month out.

#48 is nearly ready to go, but it required a large change to the internal implementation that increased the complexity and code size. I'm currently looking into an alternative design that may improve this (it's a mess, but currently the prototype is over here). Unfortunately it's taken longer than I had hoped to see if it's a feasible solution.

That's the main reason for the stagnation. I'm trying to avoid merging new code until I'm sure it's the best path forward. Sorry for any inconveniences this is causing.

[geky]

Ok! Sorry about the delay on this, but now #85 is looking close to merging. It's a big change, so it's probably going to take another week at least before it's ready to go in.

It also comes as a breaking change for various reasons. At least for you @guillaumerems this isn't that much worse than breaking the SPEC. Since you took the time to follow it, the least I can do is make it as bad an experience for everyone else as for you.

I am hoping to make this jump as smooth as it can be. I'm going to create an upgrade function before bringing #85 in. Though this may have a significant code cost.

---

There were also a few changes to the API if you all have any feedback on them:

1. Normal 1-attribute access functions:

   // Get a custom attribute
   //
   // Custom attributes are uniquely identified by an 8-bit type and limited
   // to LFS_ATTR_MAX bytes. When read, if the stored attribute is smaller than
   // the buffer, it will be padded with zeros. If the stored attribute is larger,
   // then it will be silently truncated.
   //
   // Returns the size of the attribute, or a negative error code on failure.
   // Note, the returned size is the size of the attribute on disk, irrespective
   // of the size of the buffer. This can be used to dynamically allocate a buffer
   // or check for existance.
   lfs_ssize_t lfs_getattr(lfs_t *lfs, const char *path,
           uint8_t type, void *buffer, lfs_size_t size);
   
   // Set custom attributes
   //
   // Custom attributes are uniquely identified by an 8-bit type and limited
   // to LFS_ATTR_MAX bytes. If an attribute is not found, it will be
   // implicitly created, and setting the size of an attribute to zero deletes
   // the attribute.
   //
   // Returns a negative error code on failure.
   int lfs_setattr(lfs_t *lfs, const char *path,
           uint8_t type, const void *buffer, lfs_size_t size);

2. Atomic updates are now through a parameter in the optional config struct added in Added possibility to open multiple files with LFS_NO_MALLOC enabled #58. This better aligns with the idea that you can add your own custom attributes to a custom file structure and littlefs will take care of fetching them from disk:

   // Optional configuration provided during lfs_file_opencfg
   struct lfs_file_config {
       // Optional, linked list of custom attributes related to the file. If the
       // file is opened with read access, the attributes will be read from
       // during the open call. If the file is opened with write access, the
       // attributes will be written to disk every file sync or close. This
       // write occurs atomically with update to the file's contents.
       //
       // Custom attributes are uniquely identified by an 8-bit type and limited
       // to LFS_ATTR_MAX bytes. When read, if the stored attribute is smaller
       // than the buffer, it will be padded with zeros. If the stored attribute
       // is larger, then it will be silently truncated. If the attribute is not
       // found, it will be created implicitly.
       struct lfs_attr *attrs;
   };

   // Custom attribute structure
   struct lfs_attr {
       // 8-bit type of attribute, provided by user and used to
       // identify the attribute
       uint8_t type;
   
       // Pointer to buffer containing the attribute
       void *buffer;
   
       // Size of attribute in bytes, limited to LFS_ATTR_MAX
       lfs_size_t size;
   
       // Pointer to next attribute in linked list
       struct lfs_attr *next;
   };

[guillaumerems]

Thanks @geky on the hard job you have done,
The major issue for me will be the upgrade from what I have done to the official next release :) But I will take care on this

Do you expect next littlefs (release 2) retro-compatible with current version without breaking the low-level existing storage ?

[geky]

Do you expect next littlefs (release 2) retro-compatible with current version without breaking the low-level existing storage ?

Unfortunately no, #85 (v2) is going to break disk compatibility. This is a big downside, but it allows us to fix several limitations of the initial design. Some of the goal is to get these changes in while littlefs is in its infancy, and hopefully v2 will be able to last much longer than v1.

One thing I have planned is to create an "upgrade" function before the release. Because this update only changes the metadata, it should be possible to upgrade in-place without additional storage. However, this will require both versions to be compiled into the codebase to upgrade.

[geky]

Ok, it's gone through a few iterations, but custom attributes are now available on master as a part of v2 (#85).

Here's how you might create timestamps with a custom file wrapper:

#define ATTR_TIMESTAMP 0x74

typedef struct time_file {
    lfs_file_t file;
    uint32_t timestamp;
    struct lfs_attr attrs[1];
    struct lfs_file_config cfg;
} time_file_t;

int time_file_open(lfs_t *lfs, time_file_t *file, char *path, int flags) {
    // set up description of timestamp attribute
    file->attrs[0].type = ATTR_TIMESTAMP;
    file->attrs[0].buffer = &file->timestamp;
    file->attrs[0].size = sizeof(&file->timestamp);

    // set up config to indicate file has custom attributes
    memset(&file->cfg, 0, sizeof(file->cfg));
    file->cfg->attrs = &file->attrs;
    file->cfg->attr_count = 1;

    // attributes will be automatically populated during open call
    return lfs_file_opencfg(lfs, &file->file, path, flags, &file->cfg);
}

int time_file_write(lfs_t *lfs, struct time_file *file, void *buffer, size_t size) {
    // update timestamp
    file->timestamp = now(NULL);
    lfs_file_write(file->lfs, &file->file, buffer, size);

    // note that like file data, custom attributes are not actually written to disk until file sync or file close
}

Let me know any questions/concerns anyone has. And i'd be happy to hear if anyone uses this successfully.

[mjs513]

Hi all
Know this is an old issue and incorporation of attributes is completed but does any have an example sketch using the custom file wrapper?

Thanks
Mike