| Message ID | 1525942370-17439-2-git-send-email-sbabic@denx.de |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [1/2] handlers: add a dummy handler | expand |
Hi Christian, On 10/05/2018 10:52, Stefano Babic wrote: > Add a dry run option to allow to run SWUpdate first without > any change to Hardware. This can be also used during the build > to check the SWU before testing on the target. > > During a dry-run mode, all handlers are exchanged with the > dummy handler, that skips the image without doing anything. > > Signed-off-by: Stefano Babic <sbabic@denx.de> > --- This should covered your use case with the maintenance window. Coupling the -o and -n option, the whole SWU is parsed and verified without touching the hardware. We could even think about to add a swupdate-native in meta-swupdate and runs it as check before delivering the SWU image. Best regards, Stefano
Hi Stefano, > Add a dry run option to allow to run SWUpdate first without > any change to Hardware. This can be also used during the build > to check the SWU before testing on the target. > > During a dry-run mode, all handlers are exchanged with the > dummy handler, that skips the image without doing anything. > > Signed-off-by: Stefano Babic <sbabic@denx.de> > --- > core/swupdate.c | 7 ++++++- > corelib/installer.c | 29 +++++++++++++++++++++++------ > corelib/stream_interface.c | 4 ++-- > doc/source/swupdate.rst | 2 ++ > include/installer.h | 2 +- > include/swupdate.h | 1 + > 6 files changed, 35 insertions(+), 10 deletions(-) > > diff --git a/core/swupdate.c b/core/swupdate.c > index adc0df1..6228801 100644 > --- a/core/swupdate.c > +++ b/core/swupdate.c > @@ -77,6 +77,7 @@ static struct option long_options[] = { > {"syslog", no_argument, NULL, 'L' }, > {"select", required_argument, NULL, 'e'}, > {"output", required_argument, NULL, 'o'}, > + {"dry-to-run", no_argument, NULL, 'n'}, Hm, isn't this commonly referred to as "dry-run" (without the -to-)? Kind regards, Christian
Hi Stefano, > > Add a dry run option to allow to run SWUpdate first without > > any change to Hardware. This can be also used during the build > > to check the SWU before testing on the target. > > > > During a dry-run mode, all handlers are exchanged with the > > dummy handler, that skips the image without doing anything. > > > > Signed-off-by: Stefano Babic <sbabic@denx.de> > > --- > This should covered your use case with the maintenance window. Coupling > the -o and -n option, the whole SWU is parsed and verified without > touching the hardware. Thanks for this functionality, makes a welcomed CI feature as well! What I do wonder, however, is how to switch this flag while runtime. As it is now, it's global (in the main process and hence all forked processes). This means that there's no way to switch it off and on while runtime, right? Consider the case that I want to do a dry-run from suricatta to test the application of the swu, say to simulate firmware update on "virtual devices", to test the applicability of the firmware update on my fleet of devices, or to save the swu to disk. To do so, the suricatta process must be able to tell SWUpdate core via IPC that it should handle the swu in "dry-run mode" and after the dry-run, SWUpdate core must be switched back to "normal mode", either automatically or again via an IPC call. This cannot be modeled right now or do I miss something here? Of course one can argue whether the dry-run flag should be switchable during runtime at all... What's your take on this? > We could even think about to add a swupdate-native in meta-swupdate and > runs it as check before delivering the SWU image. Yes, this would be very welcomed for image generation within CI which is then tested on real/virtual devices, very nice. Kind regards, Christian P.S. I'm off for extended vacation as of today and may not be able to reply for a few weeks...
On 11/05/2018 09:39, Christian Storm wrote: > Hi Stefano, > >> Add a dry run option to allow to run SWUpdate first without >> any change to Hardware. This can be also used during the build >> to check the SWU before testing on the target. >> >> During a dry-run mode, all handlers are exchanged with the >> dummy handler, that skips the image without doing anything. >> >> Signed-off-by: Stefano Babic <sbabic@denx.de> >> --- >> core/swupdate.c | 7 ++++++- >> corelib/installer.c | 29 +++++++++++++++++++++++------ >> corelib/stream_interface.c | 4 ++-- >> doc/source/swupdate.rst | 2 ++ >> include/installer.h | 2 +- >> include/swupdate.h | 1 + >> 6 files changed, 35 insertions(+), 10 deletions(-) >> >> diff --git a/core/swupdate.c b/core/swupdate.c >> index adc0df1..6228801 100644 >> --- a/core/swupdate.c >> +++ b/core/swupdate.c >> @@ -77,6 +77,7 @@ static struct option long_options[] = { >> {"syslog", no_argument, NULL, 'L' }, >> {"select", required_argument, NULL, 'e'}, >> {"output", required_argument, NULL, 'o'}, >> + {"dry-to-run", no_argument, NULL, 'n'}, > > Hm, isn't this commonly referred to as "dry-run" (without the -to-)? Your're right, I fix in V2. Regards, Stefano
Hi Christian, On 11/05/2018 09:47, Christian Storm wrote: > Hi Stefano, > >>> Add a dry run option to allow to run SWUpdate first without >>> any change to Hardware. This can be also used during the build >>> to check the SWU before testing on the target. >>> >>> During a dry-run mode, all handlers are exchanged with the >>> dummy handler, that skips the image without doing anything. >>> >>> Signed-off-by: Stefano Babic <sbabic@denx.de> >>> --- >> This should covered your use case with the maintenance window. Coupling >> the -o and -n option, the whole SWU is parsed and verified without >> touching the hardware. > > Thanks for this functionality, makes a welcomed CI feature as well! > What I do wonder, however, is how to switch this flag while runtime. > As it is now, it's global (in the main process and hence all forked > processes). This means that there's no way to switch it off and on while > runtime, right? Right, I have not foreseen this. > Consider the case that I want to do a dry-run from > suricatta to test the application of the swu, say to simulate firmware > update on "virtual devices", to test the applicability of the firmware > update on my fleet of devices, or to save the swu to disk. ok > To do so, the suricatta process must be able to tell SWUpdate core via > IPC that it should handle the swu in "dry-run mode" and after the > dry-run, SWUpdate core must be switched back to "normal mode", either > automatically or again via an IPC call. This cannot be modeled right now > or do I miss something here? Right. But I suggest we do with a following up patch. > Of course one can argue whether the dry-run flag should be switchable > during runtime at all... What's your take on this? Right, this was my intention. dry-run is a global option in most programs that has implemented this feature. Let's see what we can do here. I will let the parameter as now because this allows to be used in CI, maybe at the end of a build. I will focus how to switch at runtime. IMHO it should be nice if this is set per install, that is each time an install is started we can configure via the first ipc command if the installation runs in dry-run or in normal mode. If parameter is notpassed, it runs as usual. In fact, if we have a IPC command to switch the dry-run mode, we have a race issue if an installation is started from a different interface, let's say suricatta and webserver. If it is done per install request, the default value is set back at the end. > > >> We could even think about to add a swupdate-native in meta-swupdate and >> runs it as check before delivering the SWU image. > > Yes, this would be very welcomed for image generation within CI which is > then tested on real/virtual devices, very nice. > > > Kind regards, > Christian > > > > P.S. I'm off for extended vacation as of today and may not be able > to reply for a few weeks... Well, then have a nice holiday ! Regards, Stefano
diff --git a/core/swupdate.c b/core/swupdate.c index adc0df1..6228801 100644 --- a/core/swupdate.c +++ b/core/swupdate.c @@ -77,6 +77,7 @@ static struct option long_options[] = { {"syslog", no_argument, NULL, 'L' }, {"select", required_argument, NULL, 'e'}, {"output", required_argument, NULL, 'o'}, + {"dry-to-run", no_argument, NULL, 'n'}, #ifdef CONFIG_SIGNED_IMAGES {"key", required_argument, NULL, 'k'}, #endif @@ -129,6 +130,7 @@ static void usage(char *programname) " -K, --key-aes <key file> : the file contains the symmetric key to be used\n" " to decrypt images\n" #endif + " -n, --dry-to-run : run SWUpdate without installing the software\n" " -o, --output <output file> : saves the incoming stream\n" " -v, --verbose : be verbose, set maximum loglevel\n" #ifdef CONFIG_HW_COMPATIBILITY @@ -550,7 +552,7 @@ int main(int argc, char **argv) #endif memset(main_options, 0, sizeof(main_options)); memset(image_url, 0, sizeof(image_url)); - strcpy(main_options, "vhi:e:l:Lcf:p:o:"); + strcpy(main_options, "vhni:e:l:Lcf:p:o:"); #ifdef CONFIG_MTD strcat(main_options, "b:"); #endif @@ -666,6 +668,9 @@ int main(int argc, char **argv) case 'l': loglevel = strtoul(optarg, NULL, 10); break; + case 'n': + swcfg.globals.dry_run = 1; + break; case 'L': swcfg.globals.syslog_enabled = 1; break; diff --git a/corelib/installer.c b/corelib/installer.c index ec6f180..3d33fe2 100644 --- a/corelib/installer.c +++ b/corelib/installer.c @@ -215,11 +215,18 @@ static int update_bootloader_env(void) return ret; } -int install_single_image(struct img_type *img) +int install_single_image(struct img_type *img, int dry_run) { struct installer_handler *hnd; int ret; + /* + * in case of dry run, replace the handler + * with a dummy doing nothing + */ + if (dry_run) + strcpy(img->type, "dummy"); + hnd = find_handler(img); if (!hnd) { TRACE("Image Type %s not supported", img->type); @@ -255,6 +262,7 @@ int install_images(struct swupdate_cfg *sw, int fdsw, int fromfile) struct filehdr fdh; struct stat buf; const char* TMPDIR = get_tmpdir(); + int dry_run = sw->globals.dry_run; /* Extract all scripts, preinstall scripts must be run now */ const char* tmpdir_scripts = get_tmpdirscripts(); @@ -266,10 +274,12 @@ int install_images(struct swupdate_cfg *sw, int fdsw, int fromfile) } /* Scripts must be run before installing images */ - ret = run_prepost_scripts(&sw->scripts, PREINSTALL); - if (ret) { - ERROR("execute preinstall scripts failed"); - return ret; + if (dry_run) { + ret = run_prepost_scripts(&sw->scripts, PREINSTALL); + if (ret) { + ERROR("execute preinstall scripts failed"); + return ret; + } } /* Update u-boot environment */ @@ -339,7 +349,7 @@ int install_images(struct swupdate_cfg *sw, int fdsw, int fromfile) free_image(img); ret = 0; } else { - ret = install_single_image(img); + ret = install_single_image(img, dry_run); } if (!fromfile) @@ -350,6 +360,13 @@ int install_images(struct swupdate_cfg *sw, int fdsw, int fromfile) } + /* + * Skip scripts in dry-run mode + */ + if (dry_run) { + return ret; + } + ret = run_prepost_scripts(&sw->scripts, POSTINSTALL); if (ret) { ERROR("execute postinstall scripts failed"); diff --git a/corelib/stream_interface.c b/corelib/stream_interface.c index adc6363..267944f 100644 --- a/corelib/stream_interface.c +++ b/corelib/stream_interface.c @@ -238,7 +238,7 @@ static int extract_files(int fd, struct swupdate_cfg *software) && (!strcmp(part->type, "ubipartition")) ) { TRACE("Need to adjust partition %s before streaming %s", part->volname, img->fname); - if (install_single_image(part)) { + if (install_single_image(part, software->globals.dry_run)) { ERROR("Error adjusting partition %s", part->volname); return -1; } @@ -247,7 +247,7 @@ static int extract_files(int fd, struct swupdate_cfg *software) } } img->fdin = fd; - if (install_single_image(img)) { + if (install_single_image(img, software->globals.dry_run)) { ERROR("Error streaming %s", img->fname); return -1; } diff --git a/doc/source/swupdate.rst b/doc/source/swupdate.rst index 6481a20..630108d 100644 --- a/doc/source/swupdate.rst +++ b/doc/source/swupdate.rst @@ -498,6 +498,8 @@ Command line parameters +-------------+----------+--------------------------------------------+ | -i <file> | string | run SWUpdate with a local .swu file | +-------------+----------+--------------------------------------------+ +| -n | - | run SWUpdate in dry-run mode. | ++-------------+----------+--------------------------------------------+ | -o <file> | string | saves the stream (SWU) on a file | +-------------+----------+--------------------------------------------+ | -v | - | activate verbose output | diff --git a/include/installer.h b/include/installer.h index ab83691..9956d7d 100644 --- a/include/installer.h +++ b/include/installer.h @@ -17,7 +17,7 @@ int check_if_required(struct imglist *list, struct filehdr *pfdh, const char *destdir, struct img_type **pimg); int install_images(struct swupdate_cfg *sw, int fdsw, int fromfile); -int install_single_image(struct img_type *img); +int install_single_image(struct img_type *img, int dry_run); int postupdate(struct swupdate_cfg *swcfg, const char *info); void cleanup_files(struct swupdate_cfg *software); diff --git a/include/swupdate.h b/include/swupdate.h index 6f57fab..36bce0c 100644 --- a/include/swupdate.h +++ b/include/swupdate.h @@ -106,6 +106,7 @@ struct swupdate_global_cfg { char mtdblacklist[SWUPDATE_GENERAL_STRING_SIZE]; int loglevel; int syslog_enabled; + int dry_run; char publickeyfname[SWUPDATE_GENERAL_STRING_SIZE]; char aeskeyfname[SWUPDATE_GENERAL_STRING_SIZE]; char postupdatecmd[SWUPDATE_GENERAL_STRING_SIZE];
Add a dry run option to allow to run SWUpdate first without any change to Hardware. This can be also used during the build to check the SWU before testing on the target. During a dry-run mode, all handlers are exchanged with the dummy handler, that skips the image without doing anything. Signed-off-by: Stefano Babic <sbabic@denx.de> --- core/swupdate.c | 7 ++++++- corelib/installer.c | 29 +++++++++++++++++++++++------ corelib/stream_interface.c | 4 ++-- doc/source/swupdate.rst | 2 ++ include/installer.h | 2 +- include/swupdate.h | 1 + 6 files changed, 35 insertions(+), 10 deletions(-)