diff mbox series

[RFC,userspace,5/5] semodule: add command-line option to detect module changes

Message ID 20220113143935.195125-6-omosnace@redhat.com (mailing list archive)
State Superseded
Headers show
Series Allow rebuilding policy store only if there were external changes to modules | expand

Commit Message

Ondrej Mosnacek Jan. 13, 2022, 2:39 p.m. UTC
Add a new command-line option "--smart" (for the lack of a better
name...) to control the newly introduced check_ext_changes libsemanage
flag.

For example, running `semodule -B --smart` will ensure that any
externally added/removed modules (e.g. by an RPM transaction) are
reflected in the compiled policy, while skipping the most expensive part
of the rebuild if no module change was deteceted since the last
libsemanage transaction.

Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
---
 policycoreutils/semodule/semodule.c | 23 ++++++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)

Comments

Christian Göttsche Jan. 15, 2022, 5:38 p.m. UTC | #1
On Thu, 13 Jan 2022 at 15:39, Ondrej Mosnacek <omosnace@redhat.com> wrote:
>
> Add a new command-line option "--smart" (for the lack of a better
> name...) to control the newly introduced check_ext_changes libsemanage
> flag.
>
> For example, running `semodule -B --smart` will ensure that any
> externally added/removed modules (e.g. by an RPM transaction) are
> reflected in the compiled policy, while skipping the most expensive part
> of the rebuild if no module change was deteceted since the last
> libsemanage transaction.
>
> Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
> ---
>  policycoreutils/semodule/semodule.c | 23 ++++++++++++++++++++---
>  1 file changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/policycoreutils/semodule/semodule.c b/policycoreutils/semodule/semodule.c
> index a5b71fc4..638edb39 100644
> --- a/policycoreutils/semodule/semodule.c
> +++ b/policycoreutils/semodule/semodule.c
> @@ -47,6 +47,7 @@ static int verbose;
>  static int reload;
>  static int no_reload;
>  static int build;
> +static int check_ext_changes;
>  static int disable_dontaudit;
>  static int preserve_tunables;
>  static int ignore_module_cache;
> @@ -149,6 +150,8 @@ static void usage(char *progname)
>         printf("  -c, --cil extract module as cil. This only affects module extraction.\n");
>         printf("  -H, --hll extract module as hll. This only affects module extraction.\n");
>         printf("  -m, --checksum   print module checksum (SHA256).\n");
> +       printf("      --smart      force policy rebuild if module content changed since\n"
> +              "                   last rebuild (based on checksum)\n");

Some other naming suggestions:

incremental
on-update
on-change
changed-only
updated-only

Also maybe describe with `force policy rebuild only if ...`, cause
otherwise one might think without --smart modules are never rebuild.

>  }
>
>  /* Sets the global mode variable to new_mode, but only if no other
> @@ -180,6 +183,7 @@ static void set_mode(enum client_modes new_mode, char *arg)
>  static void parse_command_line(int argc, char **argv)
>  {
>         static struct option opts[] = {
> +               {"smart", 0, NULL, '\0'},
>                 {"store", required_argument, NULL, 's'},
>                 {"base", required_argument, NULL, 'b'},
>                 {"help", 0, NULL, 'h'},
> @@ -207,15 +211,26 @@ static void parse_command_line(int argc, char **argv)
>         };
>         int extract_selected = 0;
>         int cil_hll_set = 0;
> -       int i;
> +       int i, longind;
>         verbose = 0;
>         reload = 0;
>         no_reload = 0;
> +       check_ext_changes = 0;
>         priority = 400;
>         while ((i =
> -               getopt_long(argc, argv, "s:b:hi:l::vr:u:RnNBDCPX:e:d:p:S:E:cHm", opts,
> -                           NULL)) != -1) {
> +               getopt_long(argc, argv, "s:b:hi:l::vr:u:RnNBDCPX:e:d:p:S:E:cHm",
> +                           opts, &longind)) != -1) {
>                 switch (i) {
> +               case '\0':
> +                       switch(longind) {
> +                       case 0: /* --smart */
> +                               check_ext_changes = 1;
> +                               break;
> +                       default:
> +                               usage(argv[0]);
> +                               exit(1);
> +                       }
> +                       break;
>                 case 'b':
>                         fprintf(stderr, "The --base option is deprecated. Use --install instead.\n");
>                         set_mode(INSTALL_M, optarg);
> @@ -813,6 +828,8 @@ cleanup_disable:
>                         semanage_set_reload(sh, 0);
>                 if (build)
>                         semanage_set_rebuild(sh, 1);
> +               if (check_ext_changes)
> +                       semanage_set_check_ext_changes(sh, 1);
>                 if (disable_dontaudit)
>                         semanage_set_disable_dontaudit(sh, 1);
>                 else if (build)
> --
> 2.34.1
>
James Carter Jan. 20, 2022, 10:10 p.m. UTC | #2
On Sun, Jan 16, 2022 at 11:07 AM Christian Göttsche
<cgzones@googlemail.com> wrote:
>
> On Thu, 13 Jan 2022 at 15:39, Ondrej Mosnacek <omosnace@redhat.com> wrote:
> >
> > Add a new command-line option "--smart" (for the lack of a better
> > name...) to control the newly introduced check_ext_changes libsemanage
> > flag.
> >
> > For example, running `semodule -B --smart` will ensure that any
> > externally added/removed modules (e.g. by an RPM transaction) are
> > reflected in the compiled policy, while skipping the most expensive part
> > of the rebuild if no module change was deteceted since the last
> > libsemanage transaction.
> >
> > Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
> > ---
> >  policycoreutils/semodule/semodule.c | 23 ++++++++++++++++++++---
> >  1 file changed, 20 insertions(+), 3 deletions(-)
> >
> > diff --git a/policycoreutils/semodule/semodule.c b/policycoreutils/semodule/semodule.c
> > index a5b71fc4..638edb39 100644
> > --- a/policycoreutils/semodule/semodule.c
> > +++ b/policycoreutils/semodule/semodule.c
> > @@ -47,6 +47,7 @@ static int verbose;
> >  static int reload;
> >  static int no_reload;
> >  static int build;
> > +static int check_ext_changes;
> >  static int disable_dontaudit;
> >  static int preserve_tunables;
> >  static int ignore_module_cache;
> > @@ -149,6 +150,8 @@ static void usage(char *progname)
> >         printf("  -c, --cil extract module as cil. This only affects module extraction.\n");
> >         printf("  -H, --hll extract module as hll. This only affects module extraction.\n");
> >         printf("  -m, --checksum   print module checksum (SHA256).\n");
> > +       printf("      --smart      force policy rebuild if module content changed since\n"
> > +              "                   last rebuild (based on checksum)\n");
>
> Some other naming suggestions:
>
> incremental
> on-update
> on-change
> changed-only
> updated-only
>
> Also maybe describe with `force policy rebuild only if ...`, cause
> otherwise one might think without --smart modules are never rebuild.
>

I was going to suggest "--if-required" or "--if-needed". I think that
"incremental" or "on-change" would be ok as well.

Jim

> >  }
> >
> >  /* Sets the global mode variable to new_mode, but only if no other
> > @@ -180,6 +183,7 @@ static void set_mode(enum client_modes new_mode, char *arg)
> >  static void parse_command_line(int argc, char **argv)
> >  {
> >         static struct option opts[] = {
> > +               {"smart", 0, NULL, '\0'},
> >                 {"store", required_argument, NULL, 's'},
> >                 {"base", required_argument, NULL, 'b'},
> >                 {"help", 0, NULL, 'h'},
> > @@ -207,15 +211,26 @@ static void parse_command_line(int argc, char **argv)
> >         };
> >         int extract_selected = 0;
> >         int cil_hll_set = 0;
> > -       int i;
> > +       int i, longind;
> >         verbose = 0;
> >         reload = 0;
> >         no_reload = 0;
> > +       check_ext_changes = 0;
> >         priority = 400;
> >         while ((i =
> > -               getopt_long(argc, argv, "s:b:hi:l::vr:u:RnNBDCPX:e:d:p:S:E:cHm", opts,
> > -                           NULL)) != -1) {
> > +               getopt_long(argc, argv, "s:b:hi:l::vr:u:RnNBDCPX:e:d:p:S:E:cHm",
> > +                           opts, &longind)) != -1) {
> >                 switch (i) {
> > +               case '\0':
> > +                       switch(longind) {
> > +                       case 0: /* --smart */
> > +                               check_ext_changes = 1;
> > +                               break;
> > +                       default:
> > +                               usage(argv[0]);
> > +                               exit(1);
> > +                       }
> > +                       break;
> >                 case 'b':
> >                         fprintf(stderr, "The --base option is deprecated. Use --install instead.\n");
> >                         set_mode(INSTALL_M, optarg);
> > @@ -813,6 +828,8 @@ cleanup_disable:
> >                         semanage_set_reload(sh, 0);
> >                 if (build)
> >                         semanage_set_rebuild(sh, 1);
> > +               if (check_ext_changes)
> > +                       semanage_set_check_ext_changes(sh, 1);
> >                 if (disable_dontaudit)
> >                         semanage_set_disable_dontaudit(sh, 1);
> >                 else if (build)
> > --
> > 2.34.1
> >
James Carter Jan. 20, 2022, 10:12 p.m. UTC | #3
On Thu, Jan 13, 2022 at 6:36 PM Ondrej Mosnacek <omosnace@redhat.com> wrote:
>
> Add a new command-line option "--smart" (for the lack of a better
> name...) to control the newly introduced check_ext_changes libsemanage
> flag.
>
> For example, running `semodule -B --smart` will ensure that any
> externally added/removed modules (e.g. by an RPM transaction) are
> reflected in the compiled policy, while skipping the most expensive part
> of the rebuild if no module change was deteceted since the last
> libsemanage transaction.
>

I tested "semodule -B" followed by "semodule -B --smart" and "semodule
-DB" followed by "semodule -DB --smart" and both worked as expected.

Thanks,
Jim


> Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
> ---
>  policycoreutils/semodule/semodule.c | 23 ++++++++++++++++++++---
>  1 file changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/policycoreutils/semodule/semodule.c b/policycoreutils/semodule/semodule.c
> index a5b71fc4..638edb39 100644
> --- a/policycoreutils/semodule/semodule.c
> +++ b/policycoreutils/semodule/semodule.c
> @@ -47,6 +47,7 @@ static int verbose;
>  static int reload;
>  static int no_reload;
>  static int build;
> +static int check_ext_changes;
>  static int disable_dontaudit;
>  static int preserve_tunables;
>  static int ignore_module_cache;
> @@ -149,6 +150,8 @@ static void usage(char *progname)
>         printf("  -c, --cil extract module as cil. This only affects module extraction.\n");
>         printf("  -H, --hll extract module as hll. This only affects module extraction.\n");
>         printf("  -m, --checksum   print module checksum (SHA256).\n");
> +       printf("      --smart      force policy rebuild if module content changed since\n"
> +              "                   last rebuild (based on checksum)\n");
>  }
>
>  /* Sets the global mode variable to new_mode, but only if no other
> @@ -180,6 +183,7 @@ static void set_mode(enum client_modes new_mode, char *arg)
>  static void parse_command_line(int argc, char **argv)
>  {
>         static struct option opts[] = {
> +               {"smart", 0, NULL, '\0'},
>                 {"store", required_argument, NULL, 's'},
>                 {"base", required_argument, NULL, 'b'},
>                 {"help", 0, NULL, 'h'},
> @@ -207,15 +211,26 @@ static void parse_command_line(int argc, char **argv)
>         };
>         int extract_selected = 0;
>         int cil_hll_set = 0;
> -       int i;
> +       int i, longind;
>         verbose = 0;
>         reload = 0;
>         no_reload = 0;
> +       check_ext_changes = 0;
>         priority = 400;
>         while ((i =
> -               getopt_long(argc, argv, "s:b:hi:l::vr:u:RnNBDCPX:e:d:p:S:E:cHm", opts,
> -                           NULL)) != -1) {
> +               getopt_long(argc, argv, "s:b:hi:l::vr:u:RnNBDCPX:e:d:p:S:E:cHm",
> +                           opts, &longind)) != -1) {
>                 switch (i) {
> +               case '\0':
> +                       switch(longind) {
> +                       case 0: /* --smart */
> +                               check_ext_changes = 1;
> +                               break;
> +                       default:
> +                               usage(argv[0]);
> +                               exit(1);
> +                       }
> +                       break;
>                 case 'b':
>                         fprintf(stderr, "The --base option is deprecated. Use --install instead.\n");
>                         set_mode(INSTALL_M, optarg);
> @@ -813,6 +828,8 @@ cleanup_disable:
>                         semanage_set_reload(sh, 0);
>                 if (build)
>                         semanage_set_rebuild(sh, 1);
> +               if (check_ext_changes)
> +                       semanage_set_check_ext_changes(sh, 1);
>                 if (disable_dontaudit)
>                         semanage_set_disable_dontaudit(sh, 1);
>                 else if (build)
> --
> 2.34.1
>
diff mbox series

Patch

diff --git a/policycoreutils/semodule/semodule.c b/policycoreutils/semodule/semodule.c
index a5b71fc4..638edb39 100644
--- a/policycoreutils/semodule/semodule.c
+++ b/policycoreutils/semodule/semodule.c
@@ -47,6 +47,7 @@  static int verbose;
 static int reload;
 static int no_reload;
 static int build;
+static int check_ext_changes;
 static int disable_dontaudit;
 static int preserve_tunables;
 static int ignore_module_cache;
@@ -149,6 +150,8 @@  static void usage(char *progname)
 	printf("  -c, --cil extract module as cil. This only affects module extraction.\n");
 	printf("  -H, --hll extract module as hll. This only affects module extraction.\n");
 	printf("  -m, --checksum   print module checksum (SHA256).\n");
+	printf("      --smart      force policy rebuild if module content changed since\n"
+	       "                   last rebuild (based on checksum)\n");
 }
 
 /* Sets the global mode variable to new_mode, but only if no other
@@ -180,6 +183,7 @@  static void set_mode(enum client_modes new_mode, char *arg)
 static void parse_command_line(int argc, char **argv)
 {
 	static struct option opts[] = {
+		{"smart", 0, NULL, '\0'},
 		{"store", required_argument, NULL, 's'},
 		{"base", required_argument, NULL, 'b'},
 		{"help", 0, NULL, 'h'},
@@ -207,15 +211,26 @@  static void parse_command_line(int argc, char **argv)
 	};
 	int extract_selected = 0;
 	int cil_hll_set = 0;
-	int i;
+	int i, longind;
 	verbose = 0;
 	reload = 0;
 	no_reload = 0;
+	check_ext_changes = 0;
 	priority = 400;
 	while ((i =
-		getopt_long(argc, argv, "s:b:hi:l::vr:u:RnNBDCPX:e:d:p:S:E:cHm", opts,
-			    NULL)) != -1) {
+		getopt_long(argc, argv, "s:b:hi:l::vr:u:RnNBDCPX:e:d:p:S:E:cHm",
+			    opts, &longind)) != -1) {
 		switch (i) {
+		case '\0':
+			switch(longind) {
+			case 0: /* --smart */
+				check_ext_changes = 1;
+				break;
+			default:
+				usage(argv[0]);
+				exit(1);
+			}
+			break;
 		case 'b':
 			fprintf(stderr, "The --base option is deprecated. Use --install instead.\n");
 			set_mode(INSTALL_M, optarg);
@@ -813,6 +828,8 @@  cleanup_disable:
 			semanage_set_reload(sh, 0);
 		if (build)
 			semanage_set_rebuild(sh, 1);
+		if (check_ext_changes)
+			semanage_set_check_ext_changes(sh, 1);
 		if (disable_dontaudit)
 			semanage_set_disable_dontaudit(sh, 1);
 		else if (build)