| Message ID | 20191022211136.151818-1-tolga.hosgor@siemens.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [v5] Add Zstd compression support | expand |
Hi, Am 22.10.19 um 23:11 schrieb Hosgor, Tolga (IOT DS EU TR MTS): > Zstd compression support can now be enabled with the CONFIG_ZSTD build > configuration. > > This commit extends the 'compressed' field in the sw-description, > allowing to specify 'compressed = "zstd"' or 'compressed = "zlib". The > old method of specfying the boolean 'compressed = true' will default to > using the 'zlib' format. > > Signed-off-by: Hosgor, Tolga (IOT DS EU TR MTS) <tolga.hosgor@siemens.com> > --- > The difference of this from v4 is that it does not introduce the value > 'compression = "none"' and a documentation correction about unknown > strings causing an error. > > Kconfig | 8 ++ > Makefile.deps | 4 + > Makefile.flags | 4 + > core/cpio_utils.c | 158 ++++++++++++++++++++++++++-------- > corelib/lua_interface.c | 30 ++++++- > doc/source/roadmap.rst | 9 +- > doc/source/sw-description.rst | 11 ++- > doc/source/swupdate.rst | 2 +- > include/swupdate.h | 7 ++ > parser/parse_external.c | 19 +++- > parser/parser.c | 16 +++- > 11 files changed, 218 insertions(+), 50 deletions(-) > [snip] > diff --git a/corelib/lua_interface.c b/corelib/lua_interface.c > index 5a66db9..0c865f2 100644 > --- a/corelib/lua_interface.c > +++ b/corelib/lua_interface.c > @@ -34,6 +34,12 @@ extern const char EMBEDDED_LUA_SRC_END[]; > lua_settable(L, -3); \ > } while (0) > > +#define LUA_PUSH_IMG_STRING_VALUE(img, attr, value) do { \ > + lua_pushstring(L, attr); \ > + lua_pushstring(L, value); \ > + lua_settable(L, -3); \ > +} while (0) > + > #define LUA_PUSH_IMG_BOOL(img, attr, field) do { \ > lua_pushstring(L, attr); \ > lua_pushboolean(L, img->field); \ > @@ -241,6 +247,16 @@ static void lua_string_to_img(struct img_type *img, const char *key, > const char offset[] = "offset"; > char seek_str[MAX_SEEK_STRING_SIZE]; > > + if (!strcmp(key, "compressed")) { > + if (!strcmp(value, "zlib")) { > + img->compressed = COMPRESSED_ZLIB; > + } else if (!strcmp(value, "zstd")) { > + img->compressed = COMPRESSED_ZSTD; > + } else { > + ERROR("compressed argument: '%s' invalid, defaulting to 'false'", value); > + img->compressed = false; > + } > + } What happen if the value is true? > if (!strcmp(key, "name")) { > strncpy(img->id.name, value, > sizeof(img->id.name)); > @@ -317,7 +333,6 @@ static void lua_number_to_img(struct img_type *img, const char *key, > img->size = (long long)val; > if (!strcmp(key, "checksum")) > img->checksum = (unsigned int)val; > - > } > > #ifdef CONFIG_HANDLER_IN_LUA > @@ -461,7 +476,6 @@ static void update_table(lua_State* L, struct img_type *img) > LUA_PUSH_IMG_STRING(img, "data", type_data); > LUA_PUSH_IMG_STRING(img, "filesystem", filesystem); > > - LUA_PUSH_IMG_BOOL(img, "compressed", compressed); > LUA_PUSH_IMG_BOOL(img, "installed_directly", install_directly); > LUA_PUSH_IMG_BOOL(img, "install_if_different", id.install_if_different); > LUA_PUSH_IMG_BOOL(img, "install_if_higher", id.install_if_higher); > @@ -473,6 +487,18 @@ static void update_table(lua_State* L, struct img_type *img) > LUA_PUSH_IMG_NUMBER(img, "size", size); > LUA_PUSH_IMG_NUMBER(img, "checksum", checksum); > > + switch (img->compressed) { > + case COMPRESSED_ZLIB: > + LUA_PUSH_IMG_STRING_VALUE(img, "compressed", "zlib"); > + break; > + case COMPRESSED_ZSTD: > + LUA_PUSH_IMG_STRING_VALUE(img, "compressed", "zstd"); > + break; > + default: > + LUA_PUSH_IMG_BOOL(img, "compressed", compressed); > + break; > + } > + Does we really need a backward compatibility for the lua scripts? Maybe we could use "nil" instead of a Boolean value to be backward compatible: https://www.lua.org/pil/2.2.html This means "compressed" could have on of the following values: "zlib" "zstd" nil A condition of "compressed" will return true for "zlib" and "zstd" and false for nil. This means we could make the Boolean value of "compressed" obsolete and keep a backward compatibility by interpret the true value as "zlib". > lua_pushstring(L, "properties"); > lua_newtable (L); > LIST_FOREACH(property, &img->properties, next) { > diff --git a/doc/source/roadmap.rst b/doc/source/roadmap.rst > index bf0a25e..bb9d546 100644 > --- a/doc/source/roadmap.rst > +++ b/doc/source/roadmap.rst > @@ -33,10 +33,11 @@ and not only UBI volumes, and add further features as restoring configuration da > Support for further compressors > =============================== > > -SWUpdate supports image compressed with zlib. This is a compromise between compression rate > -and speed to decompress the single artifact. To reduce bandwidth or for big images, a stronger > -compressor could help. Adding a new compressor must be careful done because it changes the > -core of handling an image. > +SWUpdate supports image compressed with following formats: zlib, zstd. This is > +a compromise between compression rate and speed to decompress the single artifact. > +To reduce bandwidth or for big images, a stronger compressor could help. > +Adding a new compressor must be careful done because it changes the core of > +handling an image. > > System Update > ============= > diff --git a/doc/source/sw-description.rst b/doc/source/sw-description.rst > index 70efa30..e1d1709 100644 > --- a/doc/source/sw-description.rst > +++ b/doc/source/sw-description.rst > @@ -1242,9 +1242,14 @@ There are 4 main sections inside sw-description: > | | | scripts | regitsters itself. | > | | | | Example: "ubivol", "raw", "rawfile", | > +-------------+----------+------------+---------------------------------------+ > - | compressed | bool | images | flag to indicate that "filename" is | > - | | | files | zlib-compressed and must be | > - | | | | decompressed before being installed | > + | compressed | string | images | string to indicate the "filename" is | > + | | or bool | files | compressed and must be decompressed | > + | | | | before being installed. the value | > + | | | | denotes the compression type. | > + | | | | currently supported values are "zlib" | > + | | | | and "zstd" . if the bool form is used | > + | | | | , then the compression type is | > + | | | | assumed to be "zlib". | > +-------------+----------+------------+---------------------------------------+ > | installed-\ | bool | images | flag to indicate that image is | > | directly | | | streamed into the target without any | > diff --git a/doc/source/swupdate.rst b/doc/source/swupdate.rst > index 8cf911b..6c7a98e 100644 > --- a/doc/source/swupdate.rst > +++ b/doc/source/swupdate.rst > @@ -56,7 +56,7 @@ General Overview > is saved and restored after repartitioning with all data > it contains, to maintain user's data. > > -- support for compressed images, using the zlib library. > +- support for compressed images, using the zlib and zstd library. > tarball (tgz file) are supported. > > - support for partitioned USB-pen or unpartitioned (mainly > diff --git a/include/swupdate.h b/include/swupdate.h > index b28504c..8cd1574 100644 > --- a/include/swupdate.h > +++ b/include/swupdate.h > @@ -41,6 +41,13 @@ enum { > INSTALL_FROM_STREAM > }; > > +enum { > + COMPRESSED_FALSE, > + COMPRESSED_TRUE, > + COMPRESSED_ZLIB, > + COMPRESSED_ZSTD, > +}; > + Do we really need the COMPRESSED_TRUE value? > struct sw_version { > char name[SWUPDATE_GENERAL_STRING_SIZE]; > char version[SWUPDATE_GENERAL_STRING_SIZE]; [snip] Regards Stefan
Hello, On Thu, 2019-10-24 at 10:18 +0200, Stefan Herbrechtsmeier wrote: > Hi, > > Am 22.10.19 um 23:11 schrieb Hosgor, Tolga (IOT DS EU TR MTS): > > Zstd compression support can now be enabled with the CONFIG_ZSTD > > build > > configuration. > > > > This commit extends the 'compressed' field in the sw-description, > > allowing to specify 'compressed = "zstd"' or 'compressed = "zlib". > > The > > old method of specfying the boolean 'compressed = true' will > > default to > > using the 'zlib' format. > > > > Signed-off-by: Hosgor, Tolga (IOT DS EU TR MTS) < > > tolga.hosgor@siemens.com> > > --- > > The difference of this from v4 is that it does not introduce the > > value > > 'compression = "none"' and a documentation correction about unknown > > strings causing an error. > > > > Kconfig | 8 ++ > > Makefile.deps | 4 + > > Makefile.flags | 4 + > > core/cpio_utils.c | 158 ++++++++++++++++++++++++++--- > > ----- > > corelib/lua_interface.c | 30 ++++++- > > doc/source/roadmap.rst | 9 +- > > doc/source/sw-description.rst | 11 ++- > > doc/source/swupdate.rst | 2 +- > > include/swupdate.h | 7 ++ > > parser/parse_external.c | 19 +++- > > parser/parser.c | 16 +++- > > 11 files changed, 218 insertions(+), 50 deletions(-) > > > > [snip] > > > diff --git a/corelib/lua_interface.c b/corelib/lua_interface.c > > index 5a66db9..0c865f2 100644 > > --- a/corelib/lua_interface.c > > +++ b/corelib/lua_interface.c > > @@ -34,6 +34,12 @@ extern const char EMBEDDED_LUA_SRC_END[]; > > lua_settable(L, -3); \ > > } while (0) > > > > +#define LUA_PUSH_IMG_STRING_VALUE(img, attr, value) do { \ > > + lua_pushstring(L, attr); \ > > + lua_pushstring(L, value); \ > > + lua_settable(L, -3); \ > > +} while (0) > > + > > #define LUA_PUSH_IMG_BOOL(img, attr, field) do { \ > > lua_pushstring(L, attr); \ > > lua_pushboolean(L, img->field); \ > > @@ -241,6 +247,16 @@ static void lua_string_to_img(struct img_type > > *img, const char *key, > > const char offset[] = "offset"; > > char seek_str[MAX_SEEK_STRING_SIZE]; > > > > + if (!strcmp(key, "compressed")) { > > + if (!strcmp(value, "zlib")) { > > + img->compressed = COMPRESSED_ZLIB; > > + } else if (!strcmp(value, "zstd")) { > > + img->compressed = COMPRESSED_ZSTD; > > + } else { > > + ERROR("compressed argument: '%s' invalid, > > defaulting to 'false'", value); > > + img->compressed = false; > > + } > > + } > > What happen if the value is true? Boolean cases for the lua is still handled in `lua_bool_to_img`. I kept it as it is. > > > if (!strcmp(key, "name")) { > > strncpy(img->id.name, value, > > sizeof(img->id.name)); > > @@ -317,7 +333,6 @@ static void lua_number_to_img(struct img_type > > *img, const char *key, > > img->size = (long long)val; > > if (!strcmp(key, "checksum")) > > img->checksum = (unsigned int)val; > > - > > } > > > > #ifdef CONFIG_HANDLER_IN_LUA > > @@ -461,7 +476,6 @@ static void update_table(lua_State* L, struct > > img_type *img) > > LUA_PUSH_IMG_STRING(img, "data", type_data); > > LUA_PUSH_IMG_STRING(img, "filesystem", filesystem); > > > > - LUA_PUSH_IMG_BOOL(img, "compressed", compressed); > > LUA_PUSH_IMG_BOOL(img, "installed_directly", > > install_directly); > > LUA_PUSH_IMG_BOOL(img, "install_if_different", > > id.install_if_different); > > LUA_PUSH_IMG_BOOL(img, "install_if_higher", > > id.install_if_higher); > > @@ -473,6 +487,18 @@ static void update_table(lua_State* L, struct > > img_type *img) > > LUA_PUSH_IMG_NUMBER(img, "size", size); > > LUA_PUSH_IMG_NUMBER(img, "checksum", checksum); > > > > + switch (img->compressed) { > > + case COMPRESSED_ZLIB: > > + LUA_PUSH_IMG_STRING_VALUE(img, > > "compressed", "zlib"); > > + break; > > + case COMPRESSED_ZSTD: > > + LUA_PUSH_IMG_STRING_VALUE(img, > > "compressed", "zstd"); > > + break; > > + default: > > + LUA_PUSH_IMG_BOOL(img, "compressed", > > compressed); > > + break; > > + } > > + > > Does we really need a backward compatibility for the lua scripts? I don't know. Could lose the boolean form altogether if that is acceptable. It would have a maintenance cost to users to update. > > Maybe we could use "nil" instead of a Boolean value to be backward > compatible: > https://www.lua.org/pil/2.2.html > > This means "compressed" could have on of the following values: > "zlib" > "zstd" > nil > > A condition of "compressed" will return true for "zlib" and "zstd" > and > false for nil. I was going for that at first. That would work for `if compressed` but one could have written `if compressed == true`. Same goes for `false`. If we drop the boolean form, then `if compressed == true` will be invalid anyway. > > This means we could make the Boolean value of "compressed" obsolete > and > keep a backward compatibility by interpret the true value as "zlib". It may create confusion. The variable would read as a string in Lua scripts although it was a boolean. I would rather get rid of the boolean form altogether. > > > > lua_pushstring(L, "properties"); > > lua_newtable (L); > > LIST_FOREACH(property, &img->properties, next) { > > diff --git a/doc/source/roadmap.rst b/doc/source/roadmap.rst > > index bf0a25e..bb9d546 100644 > > --- a/doc/source/roadmap.rst > > +++ b/doc/source/roadmap.rst > > @@ -33,10 +33,11 @@ and not only UBI volumes, and add further > > features as restoring configuration da > > Support for further compressors > > =============================== > > > > -SWUpdate supports image compressed with zlib. This is a compromise > > between compression rate > > -and speed to decompress the single artifact. To reduce bandwidth > > or for big images, a stronger > > -compressor could help. Adding a new compressor must be careful > > done because it changes the > > -core of handling an image. > > +SWUpdate supports image compressed with following formats: zlib, > > zstd. This is > > +a compromise between compression rate and speed to decompress the > > single artifact. > > +To reduce bandwidth or for big images, a stronger compressor could > > help. > > +Adding a new compressor must be careful done because it changes > > the core of > > +handling an image. > > > > System Update > > ============= > > diff --git a/doc/source/sw-description.rst b/doc/source/sw- > > description.rst > > index 70efa30..e1d1709 100644 > > --- a/doc/source/sw-description.rst > > +++ b/doc/source/sw-description.rst > > @@ -1242,9 +1242,14 @@ There are 4 main sections inside sw- > > description: > > | | | scripts | regitsters > > itself. | > > | | | | Example: "ubivol", > > "raw", "rawfile", | > > +-------------+----------+------------+----------------------- > > ----------------+ > > - | compressed | bool | images | flag to indicate that > > "filename" is | > > - | | | files | zlib-compressed and > > must be | > > - | | | | decompressed before > > being installed | > > + | compressed | string | images | string to indicate the > > "filename" is | > > + | | or bool | files | compressed and must be > > decompressed | > > + | | | | before being installed. > > the value | > > + | | | | denotes the compression > > type. | > > + | | | | currently supported > > values are "zlib" | > > + | | | | and "zstd" . if the > > bool form is used | > > + | | | | , then the compression > > type is | > > + | | | | assumed to be > > "zlib". | > > +-------------+----------+------------+----------------------- > > ----------------+ > > | installed-\ | bool | images | flag to indicate that > > image is | > > | directly | | | streamed into the > > target without any | > > diff --git a/doc/source/swupdate.rst b/doc/source/swupdate.rst > > index 8cf911b..6c7a98e 100644 > > --- a/doc/source/swupdate.rst > > +++ b/doc/source/swupdate.rst > > @@ -56,7 +56,7 @@ General Overview > > is saved and restored after repartitioning with all data > > it contains, to maintain user's data. > > > > -- support for compressed images, using the zlib library. > > +- support for compressed images, using the zlib and zstd library. > > tarball (tgz file) are supported. > > > > - support for partitioned USB-pen or unpartitioned (mainly > > diff --git a/include/swupdate.h b/include/swupdate.h > > index b28504c..8cd1574 100644 > > --- a/include/swupdate.h > > +++ b/include/swupdate.h > > @@ -41,6 +41,13 @@ enum { > > INSTALL_FROM_STREAM > > }; > > > > +enum { > > + COMPRESSED_FALSE, > > + COMPRESSED_TRUE, > > + COMPRESSED_ZLIB, > > + COMPRESSED_ZSTD, > > +}; > > + > > Do we really need the COMPRESSED_TRUE value? It is used to distinguish between 'compressed = true' and 'compressed = "zlib"' in `update_table` function so that `if compressed == true` does not break due to the 'compressed = true' variable getting converted to "zstd". > > > struct sw_version { > > char name[SWUPDATE_GENERAL_STRING_SIZE]; > > char version[SWUPDATE_GENERAL_STRING_SIZE]; > > [snip] > > Regards > Stefan > Best Regards, Tolga.
Hi, Am 24.10.19 um 15:51 schrieb Tolga HOŞGÖR: > Hello, > > On Thu, 2019-10-24 at 10:18 +0200, Stefan Herbrechtsmeier wrote: >> Hi, >> >> Am 22.10.19 um 23:11 schrieb Hosgor, Tolga (IOT DS EU TR MTS): >>> Zstd compression support can now be enabled with the CONFIG_ZSTD >>> build >>> configuration. >>> >>> This commit extends the 'compressed' field in the sw-description, >>> allowing to specify 'compressed = "zstd"' or 'compressed = "zlib". >>> The >>> old method of specfying the boolean 'compressed = true' will >>> default to >>> using the 'zlib' format. >>> >>> Signed-off-by: Hosgor, Tolga (IOT DS EU TR MTS) < >>> tolga.hosgor@siemens.com> >>> --- >>> The difference of this from v4 is that it does not introduce the >>> value >>> 'compression = "none"' and a documentation correction about unknown >>> strings causing an error. >>> >>> Kconfig | 8 ++ >>> Makefile.deps | 4 + >>> Makefile.flags | 4 + >>> core/cpio_utils.c | 158 ++++++++++++++++++++++++++--- >>> ----- >>> corelib/lua_interface.c | 30 ++++++- >>> doc/source/roadmap.rst | 9 +- >>> doc/source/sw-description.rst | 11 ++- >>> doc/source/swupdate.rst | 2 +- >>> include/swupdate.h | 7 ++ >>> parser/parse_external.c | 19 +++- >>> parser/parser.c | 16 +++- >>> 11 files changed, 218 insertions(+), 50 deletions(-) >>> >> >> [snip] >> >>> diff --git a/corelib/lua_interface.c b/corelib/lua_interface.c >>> index 5a66db9..0c865f2 100644 >>> --- a/corelib/lua_interface.c >>> +++ b/corelib/lua_interface.c >>> @@ -34,6 +34,12 @@ extern const char EMBEDDED_LUA_SRC_END[]; >>> lua_settable(L, -3); \ >>> } while (0) >>> >>> +#define LUA_PUSH_IMG_STRING_VALUE(img, attr, value) do { \ >>> + lua_pushstring(L, attr); \ >>> + lua_pushstring(L, value); \ >>> + lua_settable(L, -3); \ >>> +} while (0) >>> + >>> #define LUA_PUSH_IMG_BOOL(img, attr, field) do { \ >>> lua_pushstring(L, attr); \ >>> lua_pushboolean(L, img->field); \ >>> @@ -241,6 +247,16 @@ static void lua_string_to_img(struct img_type >>> *img, const char *key, >>> const char offset[] = "offset"; >>> char seek_str[MAX_SEEK_STRING_SIZE]; >>> >>> + if (!strcmp(key, "compressed")) { >>> + if (!strcmp(value, "zlib")) { >>> + img->compressed = COMPRESSED_ZLIB; >>> + } else if (!strcmp(value, "zstd")) { >>> + img->compressed = COMPRESSED_ZSTD; >>> + } else { >>> + ERROR("compressed argument: '%s' invalid, >>> defaulting to 'false'", value); >>> + img->compressed = false; >>> + } >>> + } >> >> What happen if the value is true? > Boolean cases for the lua is still handled in `lua_bool_to_img`. I kept > it as it is. >> >>> if (!strcmp(key, "name")) { >>> strncpy(img->id.name, value, >>> sizeof(img->id.name)); >>> @@ -317,7 +333,6 @@ static void lua_number_to_img(struct img_type >>> *img, const char *key, >>> img->size = (long long)val; >>> if (!strcmp(key, "checksum")) >>> img->checksum = (unsigned int)val; >>> - >>> } >>> >>> #ifdef CONFIG_HANDLER_IN_LUA >>> @@ -461,7 +476,6 @@ static void update_table(lua_State* L, struct >>> img_type *img) >>> LUA_PUSH_IMG_STRING(img, "data", type_data); >>> LUA_PUSH_IMG_STRING(img, "filesystem", filesystem); >>> >>> - LUA_PUSH_IMG_BOOL(img, "compressed", compressed); >>> LUA_PUSH_IMG_BOOL(img, "installed_directly", >>> install_directly); >>> LUA_PUSH_IMG_BOOL(img, "install_if_different", >>> id.install_if_different); >>> LUA_PUSH_IMG_BOOL(img, "install_if_higher", >>> id.install_if_higher); >>> @@ -473,6 +487,18 @@ static void update_table(lua_State* L, struct >>> img_type *img) >>> LUA_PUSH_IMG_NUMBER(img, "size", size); >>> LUA_PUSH_IMG_NUMBER(img, "checksum", checksum); >>> >>> + switch (img->compressed) { >>> + case COMPRESSED_ZLIB: >>> + LUA_PUSH_IMG_STRING_VALUE(img, >>> "compressed", "zlib"); >>> + break; >>> + case COMPRESSED_ZSTD: >>> + LUA_PUSH_IMG_STRING_VALUE(img, >>> "compressed", "zstd"); >>> + break; >>> + default: >>> + LUA_PUSH_IMG_BOOL(img, "compressed", >>> compressed); >>> + break; >>> + } >>> + >> >> Does we really need a backward compatibility for the lua scripts? > I don't know. Could lose the boolean form altogether if that is > acceptable. It would have a maintenance cost to users to update. > >> >> Maybe we could use "nil" instead of a Boolean value to be backward >> compatible: >> https://www.lua.org/pil/2.2.html >> >> This means "compressed" could have on of the following values: >> "zlib" >> "zstd" >> nil >> >> A condition of "compressed" will return true for "zlib" and "zstd" >> and >> false for nil. > I was going for that at first. That would work for `if compressed` but > one could have written `if compressed == true`. Same goes for `false`. > If we drop the boolean form, then `if compressed == true` will be > invalid anyway. This is bad. > >> >> This means we could make the Boolean value of "compressed" obsolete >> and >> keep a backward compatibility by interpret the true value as "zlib". > It may create confusion. The variable would read as a string in Lua > scripts although it was a boolean. I would rather get rid of the > boolean form altogether. >> >> >>> lua_pushstring(L, "properties"); >>> lua_newtable (L); >>> LIST_FOREACH(property, &img->properties, next) { >>> diff --git a/doc/source/roadmap.rst b/doc/source/roadmap.rst >>> index bf0a25e..bb9d546 100644 >>> --- a/doc/source/roadmap.rst >>> +++ b/doc/source/roadmap.rst >>> @@ -33,10 +33,11 @@ and not only UBI volumes, and add further >>> features as restoring configuration da >>> Support for further compressors >>> =============================== >>> >>> -SWUpdate supports image compressed with zlib. This is a compromise >>> between compression rate >>> -and speed to decompress the single artifact. To reduce bandwidth >>> or for big images, a stronger >>> -compressor could help. Adding a new compressor must be careful >>> done because it changes the >>> -core of handling an image. >>> +SWUpdate supports image compressed with following formats: zlib, >>> zstd. This is >>> +a compromise between compression rate and speed to decompress the >>> single artifact. >>> +To reduce bandwidth or for big images, a stronger compressor could >>> help. >>> +Adding a new compressor must be careful done because it changes >>> the core of >>> +handling an image. >>> >>> System Update >>> ============= >>> diff --git a/doc/source/sw-description.rst b/doc/source/sw- >>> description.rst >>> index 70efa30..e1d1709 100644 >>> --- a/doc/source/sw-description.rst >>> +++ b/doc/source/sw-description.rst >>> @@ -1242,9 +1242,14 @@ There are 4 main sections inside sw- >>> description: >>> | | | scripts | regitsters >>> itself. | >>> | | | | Example: "ubivol", >>> "raw", "rawfile", | >>> +-------------+----------+------------+----------------------- >>> ----------------+ >>> - | compressed | bool | images | flag to indicate that >>> "filename" is | >>> - | | | files | zlib-compressed and >>> must be | >>> - | | | | decompressed before >>> being installed | >>> + | compressed | string | images | string to indicate the >>> "filename" is | >>> + | | or bool | files | compressed and must be >>> decompressed | >>> + | | | | before being installed. >>> the value | >>> + | | | | denotes the compression >>> type. | >>> + | | | | currently supported >>> values are "zlib" | >>> + | | | | and "zstd" . if the >>> bool form is used | >>> + | | | | , then the compression >>> type is | >>> + | | | | assumed to be >>> "zlib". | >>> +-------------+----------+------------+----------------------- >>> ----------------+ >>> | installed-\ | bool | images | flag to indicate that >>> image is | >>> | directly | | | streamed into the >>> target without any | >>> diff --git a/doc/source/swupdate.rst b/doc/source/swupdate.rst >>> index 8cf911b..6c7a98e 100644 >>> --- a/doc/source/swupdate.rst >>> +++ b/doc/source/swupdate.rst >>> @@ -56,7 +56,7 @@ General Overview >>> is saved and restored after repartitioning with all data >>> it contains, to maintain user's data. >>> >>> -- support for compressed images, using the zlib library. >>> +- support for compressed images, using the zlib and zstd library. >>> tarball (tgz file) are supported. >>> >>> - support for partitioned USB-pen or unpartitioned (mainly >>> diff --git a/include/swupdate.h b/include/swupdate.h >>> index b28504c..8cd1574 100644 >>> --- a/include/swupdate.h >>> +++ b/include/swupdate.h >>> @@ -41,6 +41,13 @@ enum { >>> INSTALL_FROM_STREAM >>> }; >>> >>> +enum { >>> + COMPRESSED_FALSE, >>> + COMPRESSED_TRUE, >>> + COMPRESSED_ZLIB, >>> + COMPRESSED_ZSTD, >>> +}; >>> + >> >> Do we really need the COMPRESSED_TRUE value? > It is used to distinguish between 'compressed = true' and 'compressed = > "zlib"' in `update_table` function so that `if compressed == true` does > not break due to the 'compressed = true' variable getting converted to > "zstd". You are right. I thought we could remove the Boolean type and keep backward compatibility but it isn't possible. I don't like a property with two possible types. Maybe we should mark the Boolean deprecated and print a warning. Alternative we could add compression and deprecated compressed. Regards Stefan
On Thu, 2019-10-24 at 16:12 +0200, Stefan Herbrechtsmeier wrote: > Hi, > > Am 24.10.19 um 15:51 schrieb Tolga HOŞGÖR: > > Hello, > > > > On Thu, 2019-10-24 at 10:18 +0200, Stefan Herbrechtsmeier wrote: > > > Hi, > > > > > > Am 22.10.19 um 23:11 schrieb Hosgor, Tolga (IOT DS EU TR MTS): > > > > Zstd compression support can now be enabled with the > > > > CONFIG_ZSTD > > > > build > > > > configuration. > > > > > > > > This commit extends the 'compressed' field in the sw- > > > > description, > > > > allowing to specify 'compressed = "zstd"' or 'compressed = > > > > "zlib". > > > > The > > > > old method of specfying the boolean 'compressed = true' will > > > > default to > > > > using the 'zlib' format. > > > > > > > > Signed-off-by: Hosgor, Tolga (IOT DS EU TR MTS) < > > > > tolga.hosgor@siemens.com> > > > > --- > > > > The difference of this from v4 is that it does not introduce > > > > the > > > > value > > > > 'compression = "none"' and a documentation correction about > > > > unknown > > > > strings causing an error. > > > > > > > > Kconfig | 8 ++ > > > > Makefile.deps | 4 + > > > > Makefile.flags | 4 + > > > > core/cpio_utils.c | 158 > > > > ++++++++++++++++++++++++++--- > > > > ----- > > > > corelib/lua_interface.c | 30 ++++++- > > > > doc/source/roadmap.rst | 9 +- > > > > doc/source/sw-description.rst | 11 ++- > > > > doc/source/swupdate.rst | 2 +- > > > > include/swupdate.h | 7 ++ > > > > parser/parse_external.c | 19 +++- > > > > parser/parser.c | 16 +++- > > > > 11 files changed, 218 insertions(+), 50 deletions(-) > > > > > > > > > > [snip] > > > > > > > diff --git a/corelib/lua_interface.c b/corelib/lua_interface.c > > > > index 5a66db9..0c865f2 100644 > > > > --- a/corelib/lua_interface.c > > > > +++ b/corelib/lua_interface.c > > > > @@ -34,6 +34,12 @@ extern const char EMBEDDED_LUA_SRC_END[]; > > > > lua_settable(L, -3); \ > > > > } while (0) > > > > > > > > +#define LUA_PUSH_IMG_STRING_VALUE(img, attr, value) do { \ > > > > + lua_pushstring(L, attr); \ > > > > + lua_pushstring(L, value); \ > > > > + lua_settable(L, -3); \ > > > > +} while (0) > > > > + > > > > #define LUA_PUSH_IMG_BOOL(img, attr, field) do { \ > > > > lua_pushstring(L, attr); \ > > > > lua_pushboolean(L, img->field); \ > > > > @@ -241,6 +247,16 @@ static void lua_string_to_img(struct > > > > img_type > > > > *img, const char *key, > > > > const char offset[] = "offset"; > > > > char seek_str[MAX_SEEK_STRING_SIZE]; > > > > > > > > + if (!strcmp(key, "compressed")) { > > > > + if (!strcmp(value, "zlib")) { > > > > + img->compressed = COMPRESSED_ZLIB; > > > > + } else if (!strcmp(value, "zstd")) { > > > > + img->compressed = COMPRESSED_ZSTD; > > > > + } else { > > > > + ERROR("compressed argument: '%s' > > > > invalid, > > > > defaulting to 'false'", value); > > > > + img->compressed = false; > > > > + } > > > > + } > > > > > > What happen if the value is true? > > Boolean cases for the lua is still handled in `lua_bool_to_img`. I > > kept > > it as it is. > > > > if (!strcmp(key, "name")) { > > > > strncpy(img->id.name, value, > > > > sizeof(img->id.name)); > > > > @@ -317,7 +333,6 @@ static void lua_number_to_img(struct > > > > img_type > > > > *img, const char *key, > > > > img->size = (long long)val; > > > > if (!strcmp(key, "checksum")) > > > > img->checksum = (unsigned int)val; > > > > - > > > > } > > > > > > > > #ifdef CONFIG_HANDLER_IN_LUA > > > > @@ -461,7 +476,6 @@ static void update_table(lua_State* L, > > > > struct > > > > img_type *img) > > > > LUA_PUSH_IMG_STRING(img, "data", type_data); > > > > LUA_PUSH_IMG_STRING(img, "filesystem", > > > > filesystem); > > > > > > > > - LUA_PUSH_IMG_BOOL(img, "compressed", > > > > compressed); > > > > LUA_PUSH_IMG_BOOL(img, "installed_directly", > > > > install_directly); > > > > LUA_PUSH_IMG_BOOL(img, "install_if_different", > > > > id.install_if_different); > > > > LUA_PUSH_IMG_BOOL(img, "install_if_higher", > > > > id.install_if_higher); > > > > @@ -473,6 +487,18 @@ static void update_table(lua_State* L, > > > > struct > > > > img_type *img) > > > > LUA_PUSH_IMG_NUMBER(img, "size", size); > > > > LUA_PUSH_IMG_NUMBER(img, "checksum", checksum); > > > > > > > > + switch (img->compressed) { > > > > + case COMPRESSED_ZLIB: > > > > + LUA_PUSH_IMG_STRING_VALUE(img, > > > > "compressed", "zlib"); > > > > + break; > > > > + case COMPRESSED_ZSTD: > > > > + LUA_PUSH_IMG_STRING_VALUE(img, > > > > "compressed", "zstd"); > > > > + break; > > > > + default: > > > > + LUA_PUSH_IMG_BOOL(img, > > > > "compressed", > > > > compressed); > > > > + break; > > > > + } > > > > + > > > > > > Does we really need a backward compatibility for the lua scripts? > > I don't know. Could lose the boolean form altogether if that is > > acceptable. It would have a maintenance cost to users to update. > > > > > Maybe we could use "nil" instead of a Boolean value to be > > > backward > > > compatible: > > > https://www.lua.org/pil/2.2.html > > > > > > This means "compressed" could have on of the following values: > > > "zlib" > > > "zstd" > > > nil > > > > > > A condition of "compressed" will return true for "zlib" and > > > "zstd" > > > and > > > false for nil. > > I was going for that at first. That would work for `if compressed` > > but > > one could have written `if compressed == true`. Same goes for > > `false`. > > If we drop the boolean form, then `if compressed == true` will be > > invalid anyway. > > This is bad. > > > > This means we could make the Boolean value of "compressed" > > > obsolete > > > and > > > keep a backward compatibility by interpret the true value as > > > "zlib". > > It may create confusion. The variable would read as a string in Lua > > scripts although it was a boolean. I would rather get rid of the > > boolean form altogether. > > > > > > > lua_pushstring(L, "properties"); > > > > lua_newtable (L); > > > > LIST_FOREACH(property, &img->properties, next) > > > > { > > > > diff --git a/doc/source/roadmap.rst b/doc/source/roadmap.rst > > > > index bf0a25e..bb9d546 100644 > > > > --- a/doc/source/roadmap.rst > > > > +++ b/doc/source/roadmap.rst > > > > @@ -33,10 +33,11 @@ and not only UBI volumes, and add further > > > > features as restoring configuration da > > > > Support for further compressors > > > > =============================== > > > > > > > > -SWUpdate supports image compressed with zlib. This is a > > > > compromise > > > > between compression rate > > > > -and speed to decompress the single artifact. To reduce > > > > bandwidth > > > > or for big images, a stronger > > > > -compressor could help. Adding a new compressor must be careful > > > > done because it changes the > > > > -core of handling an image. > > > > +SWUpdate supports image compressed with following formats: > > > > zlib, > > > > zstd. This is > > > > +a compromise between compression rate and speed to decompress > > > > the > > > > single artifact. > > > > +To reduce bandwidth or for big images, a stronger compressor > > > > could > > > > help. > > > > +Adding a new compressor must be careful done because it > > > > changes > > > > the core of > > > > +handling an image. > > > > > > > > System Update > > > > ============= > > > > diff --git a/doc/source/sw-description.rst b/doc/source/sw- > > > > description.rst > > > > index 70efa30..e1d1709 100644 > > > > --- a/doc/source/sw-description.rst > > > > +++ b/doc/source/sw-description.rst > > > > @@ -1242,9 +1242,14 @@ There are 4 main sections inside sw- > > > > description: > > > > | | | scripts | regitsters > > > > itself. | > > > > | | | | Example: > > > > "ubivol", > > > > "raw", "rawfile", | > > > > +-------------+----------+------------+---------------- > > > > ------- > > > > ----------------+ > > > > - | compressed | bool | images | flag to indicate > > > > that > > > > "filename" is | > > > > - | | | files | zlib-compressed and > > > > must be | > > > > - | | | | decompressed before > > > > being installed | > > > > + | compressed | string | images | string to indicate > > > > the > > > > "filename" is | > > > > + | | or bool | files | compressed and must > > > > be > > > > decompressed | > > > > + | | | | before being > > > > installed. > > > > the value | > > > > + | | | | denotes the > > > > compression > > > > type. | > > > > + | | | | currently supported > > > > values are "zlib" | > > > > + | | | | and "zstd" . if the > > > > bool form is used | > > > > + | | | | , then the > > > > compression > > > > type is | > > > > + | | | | assumed to be > > > > "zlib". | > > > > +-------------+----------+------------+---------------- > > > > ------- > > > > ----------------+ > > > > | installed-\ | bool | images | flag to indicate > > > > that > > > > image is | > > > > | directly | | | streamed into the > > > > target without any | > > > > diff --git a/doc/source/swupdate.rst b/doc/source/swupdate.rst > > > > index 8cf911b..6c7a98e 100644 > > > > --- a/doc/source/swupdate.rst > > > > +++ b/doc/source/swupdate.rst > > > > @@ -56,7 +56,7 @@ General Overview > > > > is saved and restored after repartitioning with all data > > > > it contains, to maintain user's data. > > > > > > > > -- support for compressed images, using the zlib library. > > > > +- support for compressed images, using the zlib and zstd > > > > library. > > > > tarball (tgz file) are supported. > > > > > > > > - support for partitioned USB-pen or unpartitioned (mainly > > > > diff --git a/include/swupdate.h b/include/swupdate.h > > > > index b28504c..8cd1574 100644 > > > > --- a/include/swupdate.h > > > > +++ b/include/swupdate.h > > > > @@ -41,6 +41,13 @@ enum { > > > > INSTALL_FROM_STREAM > > > > }; > > > > > > > > +enum { > > > > + COMPRESSED_FALSE, > > > > + COMPRESSED_TRUE, > > > > + COMPRESSED_ZLIB, > > > > + COMPRESSED_ZSTD, > > > > +}; > > > > + > > > > > > Do we really need the COMPRESSED_TRUE value? > > It is used to distinguish between 'compressed = true' and > > 'compressed = > > "zlib"' in `update_table` function so that `if compressed == true` > > does > > not break due to the 'compressed = true' variable getting converted > > to > > "zstd". > > You are right. I thought we could remove the Boolean type and keep > backward compatibility but it isn't possible. I don't like a > property > with two possible types. Maybe we should mark the Boolean deprecated > and > print a warning. Alternative we could add compression and deprecated > compressed. I have added "deprecated" statements as v6 and v7. I don't know about introducing "compression". It's a valid idea. Positive impacts: * cleaner code maybe * cleaner docu Negative impacts: * not really I guess > > Regards > Stefan
diff --git a/Kconfig b/Kconfig index 70907c9..6f1e7ad 100644 --- a/Kconfig +++ b/Kconfig @@ -61,6 +61,10 @@ config HAVE_ZLIB bool option env="HAVE_ZLIB" +config HAVE_ZSTD + bool + option env="HAVE_ZSTD" + config HAVE_LIBSSL bool option env="HAVE_LIBSSL" @@ -434,5 +438,9 @@ config GUNZIP depends on HAVE_ZLIB default y +config ZSTD + bool "Zstd compression support" + depends on HAVE_ZSTD + source parser/Config.in source handlers/Config.in diff --git a/Makefile.deps b/Makefile.deps index 512d4b8..5e2bd2f 100644 --- a/Makefile.deps +++ b/Makefile.deps @@ -38,6 +38,10 @@ ifeq ($(HAVE_ZLIB),) export HAVE_ZLIB = y endif +ifeq ($(HAVE_ZSTD),) +export HAVE_ZSTD = y +endif + ifeq ($(HAVE_LIBUBOOTENV),) export HAVE_LIBUBOOTENV = y endif diff --git a/Makefile.flags b/Makefile.flags index b880d32..614d772 100644 --- a/Makefile.flags +++ b/Makefile.flags @@ -162,6 +162,10 @@ ifeq ($(CONFIG_GUNZIP),y) LDLIBS += z endif +ifeq ($(CONFIG_ZSTD),y) +LDLIBS += zstd +endif + ifeq ($(CONFIG_RDIFFHANDLER),y) LDLIBS += rsync endif diff --git a/core/cpio_utils.c b/core/cpio_utils.c index 1b6cb14..5910c69 100644 --- a/core/cpio_utils.c +++ b/core/cpio_utils.c @@ -13,6 +13,9 @@ #ifdef CONFIG_GUNZIP #include <zlib.h> #endif +#ifdef CONFIG_ZSTD +#include <zstd.h> +#endif #include "generated/autoconf.h" #include "cpiohdr.h" @@ -233,22 +236,29 @@ static int decrypt_step(void *state, void *buffer, size_t size) return 0; } -#ifdef CONFIG_GUNZIP +#if defined(CONFIG_GUNZIP) || defined(CONFIG_ZSTD) +typedef int (*DecompressStep)(void *state, void *buffer, size_t size); -struct GunzipState -{ +struct DecompressState { PipelineStep upstream_step; void *upstream_state; + void *impl_state; + uint8_t input[BUFF_SIZE]; + bool eof; +}; +#endif +#ifdef CONFIG_GUNZIP + +struct GunzipState { z_stream strm; bool initialized; - uint8_t input[BUFF_SIZE]; - bool eof; }; static int gunzip_step(void *state, void *buffer, size_t size) { - struct GunzipState *s = (struct GunzipState *)state; + struct DecompressState *ds = (struct DecompressState *)state; + struct GunzipState *s = (struct GunzipState *)ds->impl_state; int ret; int outlen = 0; @@ -256,23 +266,23 @@ static int gunzip_step(void *state, void *buffer, size_t size) s->strm.avail_out = size; while (outlen == 0) { if (s->strm.avail_in == 0) { - ret = s->upstream_step(s->upstream_state, s->input, sizeof s->input); + ret = ds->upstream_step(ds->upstream_state, ds->input, sizeof ds->input); if (ret < 0) { return ret; } else if (ret == 0) { - s->eof = true; + ds->eof = true; } s->strm.avail_in = ret; - s->strm.next_in = s->input; + s->strm.next_in = ds->input; } - if (s->eof) { + if (ds->eof) { break; } ret = inflate(&s->strm, Z_NO_FLUSH); outlen = size - s->strm.avail_out; if (ret == Z_STREAM_END) { - s->eof = true; + ds->eof = true; break; } if (ret != Z_OK && ret != Z_BUF_ERROR) { @@ -285,6 +295,52 @@ static int gunzip_step(void *state, void *buffer, size_t size) #endif +#ifdef CONFIG_ZSTD + +struct ZstdState { + ZSTD_DStream* dctx; + ZSTD_inBuffer input_view; +}; + +static int zstd_step(void* state, void* buffer, size_t size) +{ + struct DecompressState *ds = (struct DecompressState *)state; + struct ZstdState *s = (struct ZstdState *)ds->impl_state; + size_t decompress_ret; + int ret; + ZSTD_outBuffer output = { buffer, size, 0 }; + + do { + if (s->input_view.pos == s->input_view.size) { + ret = ds->upstream_step(ds->upstream_state, ds->input, sizeof ds->input); + if (ret < 0) { + return ret; + } else if (ret == 0) { + ds->eof = true; + } + s->input_view.size = ret; + s->input_view.pos = 0; + } + + do { + decompress_ret = ZSTD_decompressStream(s->dctx, &output, &s->input_view); + + if (ZSTD_isError(decompress_ret)) { + ERROR("ZSTD_decompressStream failed (returned %zu)", decompress_ret); + return -1; + } + + if (output.pos == output.size) { + break; + } + } while (s->input_view.pos < s->input_view.size); + } while (output.pos == 0 && !ds->eof); + + return output.pos; +} + +#endif + int copyfile(int fdin, void *out, unsigned int nbytes, unsigned long *offs, unsigned long long seek, int skip_file, int __attribute__ ((__unused__)) compressed, uint32_t *checksum, unsigned char *hash, int encrypted, writeimage callback) @@ -314,17 +370,29 @@ int copyfile(int fdin, void *out, unsigned int nbytes, unsigned long *offs, unsi .outlen = 0, .eof = false }; +#if defined(CONFIG_GUNZIP) || defined(CONFIG_ZSTD) + struct DecompressState decompress_state = { + .upstream_step = NULL, .upstream_state = NULL, + .impl_state = NULL + }; + + DecompressStep decompress_step = NULL; #ifdef CONFIG_GUNZIP struct GunzipState gunzip_state = { - .upstream_step = NULL, .upstream_state = NULL, .strm = { .zalloc = Z_NULL, .zfree = Z_NULL, .opaque = Z_NULL, .avail_in = 0, .next_in = Z_NULL, .avail_out = 0, .next_out = Z_NULL }, .initialized = false, - .eof = false }; +#endif +#ifdef CONFIG_ZSTD + struct ZstdState zstd_state = { + .dctx = NULL, + .input_view = { NULL, 0, 0 }, + }; +#endif #endif PipelineStep step = NULL; @@ -357,21 +425,38 @@ int copyfile(int fdin, void *out, unsigned int nbytes, unsigned long *offs, unsi if (compressed) { #ifdef CONFIG_GUNZIP - /* - * 16 + MAX_WBITS means that Zlib should expect and decode a - * gzip header. - */ - if (inflateInit2(&gunzip_state.strm, 16 + MAX_WBITS) != Z_OK) { - ERROR("inflateInit2 failed"); - ret = -EFAULT; + if (compressed == COMPRESSED_ZLIB || compressed == COMPRESSED_TRUE) { + /* + * 16 + MAX_WBITS means that Zlib should expect and decode a + * gzip header. + */ + if (inflateInit2(&gunzip_state.strm, 16 + MAX_WBITS) != Z_OK) { + ERROR("inflateInit2 failed"); + ret = -EFAULT; + goto copyfile_exit; + } + gunzip_state.initialized = true; + decompress_step = &gunzip_step; + decompress_state.impl_state = &gunzip_state; + } else +#endif +#ifdef CONFIG_ZSTD + if (compressed == COMPRESSED_ZSTD) { + if ((zstd_state.dctx = ZSTD_createDStream()) == NULL) { + ERROR("ZSTD_createDStream failed"); + ret = -EFAULT; + goto copyfile_exit; + } + zstd_state.input_view.src = decompress_state.input; + decompress_step = &zstd_step; + decompress_state.impl_state = &zstd_state; + } else +#endif + { + TRACE("Requested decompression method (%d) is not configured!", compressed); + ret = -EINVAL; goto copyfile_exit; } - gunzip_state.initialized = true; -#else - TRACE("Request decompressing, but CONFIG_GUNZIP not set !"); - ret = -EINVAL; - goto copyfile_exit; -#endif } if (seek) { @@ -384,19 +469,19 @@ int copyfile(int fdin, void *out, unsigned int nbytes, unsigned long *offs, unsi } } -#ifdef CONFIG_GUNZIP +#if defined(CONFIG_GUNZIP) || defined(CONFIG_ZSTD) if (compressed) { if (encrypted) { decrypt_state.upstream_step = &input_step; decrypt_state.upstream_state = &input_state; - gunzip_state.upstream_step = &decrypt_step; - gunzip_state.upstream_state = &decrypt_state; + decompress_state.upstream_step = &decrypt_step; + decompress_state.upstream_state = &decrypt_state; } else { - gunzip_state.upstream_step = &input_step; - gunzip_state.upstream_state = &input_state; + decompress_state.upstream_step = &input_step; + decompress_state.upstream_state = &input_state; } - step = &gunzip_step; - state = &gunzip_state; + step = decompress_step; + state = &decompress_state; } else { #endif if (encrypted) { @@ -408,7 +493,7 @@ int copyfile(int fdin, void *out, unsigned int nbytes, unsigned long *offs, unsi step = &input_step; state = &input_state; } -#ifdef CONFIG_GUNZIP +#if defined(CONFIG_GUNZIP) || defined(CONFIG_ZSTD) } #endif @@ -487,6 +572,11 @@ copyfile_exit: inflateEnd(&gunzip_state.strm); } #endif +#ifdef CONFIG_ZSTD + if (zstd_state.dctx != NULL) { + ZSTD_freeDStream(zstd_state.dctx); + } +#endif return ret; } diff --git a/corelib/lua_interface.c b/corelib/lua_interface.c index 5a66db9..0c865f2 100644 --- a/corelib/lua_interface.c +++ b/corelib/lua_interface.c @@ -34,6 +34,12 @@ extern const char EMBEDDED_LUA_SRC_END[]; lua_settable(L, -3); \ } while (0) +#define LUA_PUSH_IMG_STRING_VALUE(img, attr, value) do { \ + lua_pushstring(L, attr); \ + lua_pushstring(L, value); \ + lua_settable(L, -3); \ +} while (0) + #define LUA_PUSH_IMG_BOOL(img, attr, field) do { \ lua_pushstring(L, attr); \ lua_pushboolean(L, img->field); \ @@ -241,6 +247,16 @@ static void lua_string_to_img(struct img_type *img, const char *key, const char offset[] = "offset"; char seek_str[MAX_SEEK_STRING_SIZE]; + if (!strcmp(key, "compressed")) { + if (!strcmp(value, "zlib")) { + img->compressed = COMPRESSED_ZLIB; + } else if (!strcmp(value, "zstd")) { + img->compressed = COMPRESSED_ZSTD; + } else { + ERROR("compressed argument: '%s' invalid, defaulting to 'false'", value); + img->compressed = false; + } + } if (!strcmp(key, "name")) { strncpy(img->id.name, value, sizeof(img->id.name)); @@ -317,7 +333,6 @@ static void lua_number_to_img(struct img_type *img, const char *key, img->size = (long long)val; if (!strcmp(key, "checksum")) img->checksum = (unsigned int)val; - } #ifdef CONFIG_HANDLER_IN_LUA @@ -461,7 +476,6 @@ static void update_table(lua_State* L, struct img_type *img) LUA_PUSH_IMG_STRING(img, "data", type_data); LUA_PUSH_IMG_STRING(img, "filesystem", filesystem); - LUA_PUSH_IMG_BOOL(img, "compressed", compressed); LUA_PUSH_IMG_BOOL(img, "installed_directly", install_directly); LUA_PUSH_IMG_BOOL(img, "install_if_different", id.install_if_different); LUA_PUSH_IMG_BOOL(img, "install_if_higher", id.install_if_higher); @@ -473,6 +487,18 @@ static void update_table(lua_State* L, struct img_type *img) LUA_PUSH_IMG_NUMBER(img, "size", size); LUA_PUSH_IMG_NUMBER(img, "checksum", checksum); + switch (img->compressed) { + case COMPRESSED_ZLIB: + LUA_PUSH_IMG_STRING_VALUE(img, "compressed", "zlib"); + break; + case COMPRESSED_ZSTD: + LUA_PUSH_IMG_STRING_VALUE(img, "compressed", "zstd"); + break; + default: + LUA_PUSH_IMG_BOOL(img, "compressed", compressed); + break; + } + lua_pushstring(L, "properties"); lua_newtable (L); LIST_FOREACH(property, &img->properties, next) { diff --git a/doc/source/roadmap.rst b/doc/source/roadmap.rst index bf0a25e..bb9d546 100644 --- a/doc/source/roadmap.rst +++ b/doc/source/roadmap.rst @@ -33,10 +33,11 @@ and not only UBI volumes, and add further features as restoring configuration da Support for further compressors =============================== -SWUpdate supports image compressed with zlib. This is a compromise between compression rate -and speed to decompress the single artifact. To reduce bandwidth or for big images, a stronger -compressor could help. Adding a new compressor must be careful done because it changes the -core of handling an image. +SWUpdate supports image compressed with following formats: zlib, zstd. This is +a compromise between compression rate and speed to decompress the single artifact. +To reduce bandwidth or for big images, a stronger compressor could help. +Adding a new compressor must be careful done because it changes the core of +handling an image. System Update ============= diff --git a/doc/source/sw-description.rst b/doc/source/sw-description.rst index 70efa30..e1d1709 100644 --- a/doc/source/sw-description.rst +++ b/doc/source/sw-description.rst @@ -1242,9 +1242,14 @@ There are 4 main sections inside sw-description: | | | scripts | regitsters itself. | | | | | Example: "ubivol", "raw", "rawfile", | +-------------+----------+------------+---------------------------------------+ - | compressed | bool | images | flag to indicate that "filename" is | - | | | files | zlib-compressed and must be | - | | | | decompressed before being installed | + | compressed | string | images | string to indicate the "filename" is | + | | or bool | files | compressed and must be decompressed | + | | | | before being installed. the value | + | | | | denotes the compression type. | + | | | | currently supported values are "zlib" | + | | | | and "zstd" . if the bool form is used | + | | | | , then the compression type is | + | | | | assumed to be "zlib". | +-------------+----------+------------+---------------------------------------+ | installed-\ | bool | images | flag to indicate that image is | | directly | | | streamed into the target without any | diff --git a/doc/source/swupdate.rst b/doc/source/swupdate.rst index 8cf911b..6c7a98e 100644 --- a/doc/source/swupdate.rst +++ b/doc/source/swupdate.rst @@ -56,7 +56,7 @@ General Overview is saved and restored after repartitioning with all data it contains, to maintain user's data. -- support for compressed images, using the zlib library. +- support for compressed images, using the zlib and zstd library. tarball (tgz file) are supported. - support for partitioned USB-pen or unpartitioned (mainly diff --git a/include/swupdate.h b/include/swupdate.h index b28504c..8cd1574 100644 --- a/include/swupdate.h +++ b/include/swupdate.h @@ -41,6 +41,13 @@ enum { INSTALL_FROM_STREAM }; +enum { + COMPRESSED_FALSE, + COMPRESSED_TRUE, + COMPRESSED_ZLIB, + COMPRESSED_ZSTD, +}; + struct sw_version { char name[SWUPDATE_GENERAL_STRING_SIZE]; char version[SWUPDATE_GENERAL_STRING_SIZE]; diff --git a/parser/parse_external.c b/parser/parse_external.c index 39b4300..ef6c74c 100644 --- a/parser/parse_external.c +++ b/parser/parse_external.c @@ -83,8 +83,19 @@ static void sw_append_stream(struct img_type *img, const char *key, ascii_to_hash(img->sha256, value); if (!strcmp(key, "encrypted")) img->is_encrypted = 1; - if (!strcmp(key, "compressed")) - img->compressed = 1; + if (!strcmp(key, "compressed")) { + if (value != NULL) { + if (!strcmp(value, "zlib")) { + img->compressed = COMPRESSED_ZLIB; + } else if (!strcmp(value, "zstd")) { + img->compressed = COMPRESSED_ZSTD; + } else { + img->compressed = COMPRESSED_TRUE; + } + } else { + img->compressed = COMPRESSED_TRUE; + } + } if (!strcmp(key, "installed-directly")) img->install_directly = 1; if (!strcmp(key, "install-if-different")) @@ -113,7 +124,7 @@ int parse_external(struct swupdate_cfg *software, const char *filename) if (ret) { LUAstackDump(L); ERROR("ERROR preparing Parser in Lua %d", ret); - + return 1; } @@ -153,7 +164,7 @@ int parse_external(struct swupdate_cfg *software, const char *filename) if (lua_type(L, -1) == LUA_TTABLE) { lua_pushnil(L); - image = (struct img_type *)calloc(1, sizeof(struct img_type)); + image = (struct img_type *)calloc(1, sizeof(struct img_type)); if (!image) { ERROR( "No memory: malloc failed"); return -ENOMEM; diff --git a/parser/parser.c b/parser/parser.c index 3cb3c97..67a56f7 100644 --- a/parser/parser.c +++ b/parser/parser.c @@ -139,7 +139,7 @@ static bool get_common_fields(parsertype p, void *cfg, struct swupdate_cfg *swcf ERROR("Missing version in configuration file"); return false; } - + GET_FIELD_STRING(p, setting, NULL, swcfg->version); TRACE("Version %s", swcfg->version); @@ -263,6 +263,7 @@ static int run_embscript(parsertype p, void *elem, struct img_type *img, static int parse_common_attributes(parsertype p, void *elem, struct img_type *image) { char seek_str[MAX_SEEK_STRING_SIZE]; + const char* compressed; /* * GET_FIELD_STRING does not touch the passed string if it is not @@ -290,7 +291,18 @@ static int parse_common_attributes(parsertype p, void *elem, struct img_type *im return -1; } - get_field(p, elem, "compressed", &image->compressed); + if ((compressed = get_field_string(p, elem, "compressed")) != NULL) { + if (!strcmp(compressed, "zlib")) { + image->compressed = COMPRESSED_ZLIB; + } else if (!strcmp(compressed, "zstd")) { + image->compressed = COMPRESSED_ZSTD; + } else { + ERROR("compressed argument: '%s' unknown", compressed); + return -1; + } + } else { + get_field(p, elem, "compressed", &image->compressed); + } get_field(p, elem, "installed-directly", &image->install_directly); get_field(p, elem, "preserve-attributes", &image->preserve_attributes); get_field(p, elem, "install-if-different", &image->id.install_if_different);
Zstd compression support can now be enabled with the CONFIG_ZSTD build configuration. This commit extends the 'compressed' field in the sw-description, allowing to specify 'compressed = "zstd"' or 'compressed = "zlib". The old method of specfying the boolean 'compressed = true' will default to using the 'zlib' format. Signed-off-by: Hosgor, Tolga (IOT DS EU TR MTS) <tolga.hosgor@siemens.com> --- The difference of this from v4 is that it does not introduce the value 'compression = "none"' and a documentation correction about unknown strings causing an error. Kconfig | 8 ++ Makefile.deps | 4 + Makefile.flags | 4 + core/cpio_utils.c | 158 ++++++++++++++++++++++++++-------- corelib/lua_interface.c | 30 ++++++- doc/source/roadmap.rst | 9 +- doc/source/sw-description.rst | 11 ++- doc/source/swupdate.rst | 2 +- include/swupdate.h | 7 ++ parser/parse_external.c | 19 +++- parser/parser.c | 16 +++- 11 files changed, 218 insertions(+), 50 deletions(-)