| Message ID | 1503918326-7716-1-git-send-email-afaustas@gmail.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Stefano Babic |
| Headers | show |
| Series | Description field has been added to the default parser and fixed incorrect update sequence. | expand |
Hi Faustas, please add a valid commit message. If there are more issues as the subject lets think, each issue is solved in a separate patch - thanks ! On 28/08/2017 13:05, Faustas wrote: > diff --git a/include/globals.h b/include/globals.h > index ddb42de..8ac0222 100644 > --- a/include/globals.h > +++ b/include/globals.h > @@ -23,6 +23,7 @@ > #define BANNER "Swupdate v" SWU_VER "\n" > > #define SWUPDATE_GENERAL_STRING_SIZE 256 > +#define SWUPDATE_UPDATE_DESCRIPTION_STRING_SIZE 1024 ??? It is no used at all. > #define MAX_IMAGE_FNAME SWUPDATE_GENERAL_STRING_SIZE > #define MAX_URL SWUPDATE_GENERAL_STRING_SIZE > #define MAX_VOLNAME SWUPDATE_GENERAL_STRING_SIZE > diff --git a/include/swupdate.h b/include/swupdate.h > index bff757e..f65a109 100644 > --- a/include/swupdate.h > +++ b/include/swupdate.h > @@ -114,6 +114,7 @@ struct swupdate_global_cfg { > struct swupdate_cfg { > char name[SWUPDATE_GENERAL_STRING_SIZE]; > char version[SWUPDATE_GENERAL_STRING_SIZE]; > + char description[SWUPDATE_GENERAL_STRING_SIZE]; > char software_set[SWUPDATE_GENERAL_STRING_SIZE]; > char running_mode[SWUPDATE_GENERAL_STRING_SIZE]; > struct hw_type hw; > diff --git a/parser/parser.c b/parser/parser.c > index 178966e..c5526c8 100644 > --- a/parser/parser.c > +++ b/parser/parser.c > @@ -491,10 +491,11 @@ static int parser(parsertype p, void *cfg, struct swupdate_cfg *swcfg) > > /* Now parse the single elements */ > parse_hw_compatibility(p, cfg, swcfg); > - parse_images(p, cfg, swcfg); > + parse_files(p, cfg, swcfg); > parse_scripts(p, cfg, swcfg); > parse_uboot(p, cfg, swcfg); > - parse_files(p, cfg, swcfg); > + parse_images(p, cfg, swcfg); > + This is not correct. First at all, there is no predefined update sequence and then the order in this does not matter. If the order should be changed, this is not the correct place. > > /* > * Move the partitions at the beginning to be processed > @@ -549,6 +550,14 @@ int parse_cfg (struct swupdate_cfg *swcfg, const char *filename) > fprintf(stdout, "Version %s\n", swcfg->version); > } > > + snprintf(node, sizeof(node), "%s.description", > + NODEROOT); > + > + if (config_lookup_string(&cfg, node, &str)) { > + strncpy(swcfg->description, str, sizeof(swcfg->description)); > + fprintf(stdout, "Description %s\n", swcfg->description); > + } > + > ret = parser(p, &cfg, swcfg); > > config_destroy(&cfg); > Best regards, Stefano Babic
Hi Faustas, On 20/09/2017 16:39, Faustas Azuolas Bagdonas wrote: > Hi Stefano, > > I didn't receive the answer from you so I decided to ask one more time. > Could you tell me what is the correct way to implement a predefined > update order? > > For example: > > files: ( > { > filename = "rootfs.tar.gz"; > type = "archive"; > device = "/dev/update_dst"; > filesystem = "ext4"; > path = "/"; > sha256 = > "16d5580efaf683734639194f2e60e7027eb140b2c15f537e2b5712fc164f8b2b"; > }, > { > filename = "file1"; > path = "/"; > device = "/dev/update_dst"; > filesystem = "ext4"; > sha256 = > "8a32d3e7699caf03517c1f008474c946a9d43b64d57db1b50f6e4907be1a6e79" > }, > { > filename = "file2"; > path = "/usr/bin/file2"; > device = "/dev/update_dst"; > filesystem = "ext4"; > sha256 = > "96b5c987273d740b4c3a7ad080c9b38a5607d4d18f9dbab9d908721969a847cc" > }, > { > filename = "file3"; > path = "/opt/file3"; > device = "/dev/update_dst"; > filesystem = "ext4"; > sha256 = > "77596354522ecd5bcf86e7a3fd37f5546b3cde8dbcd5e82fa851769d231ae3a2" > } > ); > > I expect that first of all this will install rootfs.tar.gz and then > file1, file2 and file3 respectively. The order of the entries is not guaranteed - what is guaranteed is that each installation runs in the same way respecting the same order. The order is implementation specific and it is not specified by SWUpdate an order respecting the entries in SWUpdate. It is just guaranteed that: - pre-install scripts run first - images and files (order not guaranteed) - U-Boot environment - post-install script Images (and files) are installed in reverse oder, just because the implementation insert at the top of the list instead of appending. That means you get the order you want by reverting the entries. Anyway, one aspect of SWUpdate is to install all artifacts having no dependencies between them. Of course, a strict order can be reached changing the parser and reordering the lists, starting from the first entry instead of the last one. However, if some artifacts depend on previous, you have a broken build system. Why dont't you generate your rootfilesystem having already file1, file2 and file3 ? > But in this case first of all it > installs file3, then file2 and file1 and finally the rootfs.tar.gz. Yes, entries are inserted in reverse order. > Slightly another case: when I have both "images" and "files" records > then first of all files are installed and then they are overwritten by a > full partition image (which is undesirable). Right - the assumption in SWUpdate is that ther eis no order at all, and just the execution must be reproducible, that means that two identical updates are performed in the same way. As I said, it could be also possible to slightly change the parser to get the same order of the entries in SWUpdate, but I have not found this necessary up now. Best regards, Stefano -- ===================================================================== DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic@denx.de =====================================================================
diff --git a/include/globals.h b/include/globals.h index ddb42de..8ac0222 100644 --- a/include/globals.h +++ b/include/globals.h @@ -23,6 +23,7 @@ #define BANNER "Swupdate v" SWU_VER "\n" #define SWUPDATE_GENERAL_STRING_SIZE 256 +#define SWUPDATE_UPDATE_DESCRIPTION_STRING_SIZE 1024 #define MAX_IMAGE_FNAME SWUPDATE_GENERAL_STRING_SIZE #define MAX_URL SWUPDATE_GENERAL_STRING_SIZE #define MAX_VOLNAME SWUPDATE_GENERAL_STRING_SIZE diff --git a/include/swupdate.h b/include/swupdate.h index bff757e..f65a109 100644 --- a/include/swupdate.h +++ b/include/swupdate.h @@ -114,6 +114,7 @@ struct swupdate_global_cfg { struct swupdate_cfg { char name[SWUPDATE_GENERAL_STRING_SIZE]; char version[SWUPDATE_GENERAL_STRING_SIZE]; + char description[SWUPDATE_GENERAL_STRING_SIZE]; char software_set[SWUPDATE_GENERAL_STRING_SIZE]; char running_mode[SWUPDATE_GENERAL_STRING_SIZE]; struct hw_type hw; diff --git a/parser/parser.c b/parser/parser.c index 178966e..c5526c8 100644 --- a/parser/parser.c +++ b/parser/parser.c @@ -491,10 +491,11 @@ static int parser(parsertype p, void *cfg, struct swupdate_cfg *swcfg) /* Now parse the single elements */ parse_hw_compatibility(p, cfg, swcfg); - parse_images(p, cfg, swcfg); + parse_files(p, cfg, swcfg); parse_scripts(p, cfg, swcfg); parse_uboot(p, cfg, swcfg); - parse_files(p, cfg, swcfg); + parse_images(p, cfg, swcfg); + /* * Move the partitions at the beginning to be processed @@ -549,6 +550,14 @@ int parse_cfg (struct swupdate_cfg *swcfg, const char *filename) fprintf(stdout, "Version %s\n", swcfg->version); } + snprintf(node, sizeof(node), "%s.description", + NODEROOT); + + if (config_lookup_string(&cfg, node, &str)) { + strncpy(swcfg->description, str, sizeof(swcfg->description)); + fprintf(stdout, "Description %s\n", swcfg->description); + } + ret = parser(p, &cfg, swcfg); config_destroy(&cfg);