[systemd-devel] [PATCH] shared/install: when unit contains only Also=, report 'indirect'

2014-11-07 Thread Jan Synacek 
Continuation of 
http://lists.freedesktop.org/archives/systemd-devel/2014-November/025041.html.

Jan Synacek (1):
  shared/install: when unit contains only Also=, report 'indirect'

 man/systemctl.xml |  5 +
 src/shared/install.c  | 45 +++--
 src/shared/install.h  |  2 ++
 src/systemctl/systemctl.c |  7 +++
 4 files changed, 41 insertions(+), 18 deletions(-)

-- 
1.9.3

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] shared/install: when unit contains only Also=, report 'indirect'

2014-11-07 Thread Jan Synacek 
If a unit contains only Also=, with no Alias= or WantedBy=, it shouldn't
be reported as static. New 'indirect' status shall be introduced.

https://bugzilla.redhat.com/show_bug.cgi?id=864298
---
 man/systemctl.xml |  5 +
 src/shared/install.c  | 45 +++--
 src/shared/install.h  |  2 ++
 src/systemctl/systemctl.c |  7 +++
 4 files changed, 41 insertions(+), 18 deletions(-)

diff --git a/man/systemctl.xml b/man/systemctl.xml
index 7cbaa6c..fa85d0b 100644
--- a/man/systemctl.xml
+++ b/man/systemctl.xml
@@ -1013,6 +1013,11 @@ kobject-uevent 1 systemd-udevd-kernel.socket 
systemd-udevd.service
 entry0/entry
   /row
   row
+entryliteralindirect/literal/entry
+entryUnit's status is determined indirectly by another 
unit(s) specified in literalAlso=/literal in [Install] section/entry
+entry0/entry
+  /row
+  row
 entryliteraldisabled/literal/entry
 entryUnit is not enabled/entry
 entry1/entry
diff --git a/src/shared/install.c b/src/shared/install.c
index cab93e8..83d1093 100644
--- a/src/shared/install.c
+++ b/src/shared/install.c
@@ -840,6 +840,7 @@ static void install_info_free(InstallInfo *i) {
 strv_free(i-aliases);
 strv_free(i-wanted_by);
 strv_free(i-required_by);
+strv_free(i-also);
 free(i-default_instance);
 free(i);
 }
@@ -948,6 +949,7 @@ static int config_parse_also(
 size_t l;
 const char *word, *state;
 InstallContext *c = data;
+InstallInfo *i = userdata;
 
 assert(filename);
 assert(lvalue);
@@ -964,6 +966,10 @@ static int config_parse_also(
 r = install_info_add(c, n, NULL);
 if (r  0)
 return r;
+
+r = strv_extend(i-also, n);
+if (r  0)
+return r;
 }
 if (!isempty(state))
 log_syntax(unit, LOG_ERR, filename, line, EINVAL,
@@ -1043,7 +1049,8 @@ static int unit_file_load(
 const char *path,
 const char *root_dir,
 bool allow_symlink,
-bool load) {
+bool load,
+char ***also) {
 
 const ConfigTableItem items[] = {
 { Install, Alias,   config_parse_strv, 
0, info-aliases   },
@@ -1087,6 +1094,9 @@ static int unit_file_load(
 if (r  0)
 return r;
 
+if (also  !strv_isempty(info-also))
+*also = strv_copy(info-also);
+
 return
 (int) strv_length(info-aliases) +
 (int) strv_length(info-wanted_by) +
@@ -1099,7 +1109,8 @@ static int unit_file_search(
 LookupPaths *paths,
 const char *root_dir,
 bool allow_symlink,
-bool load) {
+bool load,
+char ***also) {
 
 char **p;
 int r;
@@ -1109,7 +1120,7 @@ static int unit_file_search(
 assert(paths);
 
 if (info-path)
-return unit_file_load(c, info, info-path, root_dir, 
allow_symlink, load);
+return unit_file_load(c, info, info-path, root_dir, 
allow_symlink, load, also);
 
 assert(info-name);
 
@@ -1120,7 +1131,7 @@ static int unit_file_search(
 if (!path)
 return -ENOMEM;
 
-r = unit_file_load(c, info, path, root_dir, allow_symlink, 
load);
+r = unit_file_load(c, info, path, root_dir, allow_symlink, 
load, also);
 if (r = 0) {
 info-path = path;
 path = NULL;
@@ -1149,7 +1160,7 @@ static int unit_file_search(
 if (!path)
 return -ENOMEM;
 
-r = unit_file_load(c, info, path, root_dir, 
allow_symlink, load);
+r = unit_file_load(c, info, path, root_dir, 
allow_symlink, load, also);
 if (r = 0) {
 info-path = path;
 path = NULL;
@@ -1167,7 +1178,8 @@ static int unit_file_can_install(
 LookupPaths *paths,
 const char *root_dir,
 const char *name,
-bool allow_symlink) {
+bool allow_symlink,
+char ***also) {
 
 _cleanup_(install_context_done) InstallContext c = {};
 InstallInfo *i;
@@ -1182,7 +1194,7 @@ static int unit_file_can_install(
 
 assert_se(i = ordered_hashmap_first(c.will_install));
 
-r = unit_file_search(c, i, paths, root_dir, allow_symlink, true);
+r = unit_file_search(c, i, paths, root_dir, allow_symlink,

Re: [systemd-devel] [PATCH] shared/install: when unit contains only Also=, report 'indirect'

2014-11-07 Thread Lennart Poettering 
On Fri, 07.11.14 15:18, Jan Synacek (jsyna...@redhat.com) wrote:

  }
  if (!isempty(state))
  log_syntax(unit, LOG_ERR, filename, line, EINVAL,
 @@ -1043,7 +1049,8 @@ static int unit_file_load(
  const char *path,
  const char *root_dir,
  bool allow_symlink,
 -bool load) {
 +bool load,
 +char ***also) {

Hmm, do we really want to return the full list here? I don't think any
caller really is interested in that, or am I wrong? Wouldn't a bool*
suffice here that tells us if also is empty, that we fill in? 

I mean, I think the inner calls should parse the whole strv, i see no
problem with that, I just don't think we really need to pass the whole
thing all the way up...

  const ConfigTableItem items[] = {
  { Install, Alias,   config_parse_strv,   
   0, info-aliases   },
 @@ -1087,6 +1094,9 @@ static int unit_file_load(
  if (r  0)
  return r;
  
 +if (also  !strv_isempty(info-also))
 +*also = strv_copy(info-also);
 +

If the argument would just be a bool* we wouldn't have to do the
expensive strv_copy() here... (which is missing an OOM check btw...)

Otherwise looks great!

Lennart

-- 
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] shared/install: when unit contains only Also=, report 'indirect'

2014-11-07 Thread Jan Synacek 
Lennart Poettering lenn...@poettering.net writes:
 On Fri, 07.11.14 15:18, Jan Synacek (jsyna...@redhat.com) wrote:

  }
  if (!isempty(state))
  log_syntax(unit, LOG_ERR, filename, line, EINVAL,
 @@ -1043,7 +1049,8 @@ static int unit_file_load(
  const char *path,
  const char *root_dir,
  bool allow_symlink,
 -bool load) {
 +bool load,
 +char ***also) {

 Hmm, do we really want to return the full list here? I don't think any
 caller really is interested in that, or am I wrong? Wouldn't a bool*
 suffice here that tells us if also is empty, that we fill in? 

No, it's not necessary. I'm not sure why I wrote it that way. Let's
blame it on friday...

 I mean, I think the inner calls should parse the whole strv, i see no
 problem with that, I just don't think we really need to pass the whole
 thing all the way up...

  const ConfigTableItem items[] = {
  { Install, Alias,   config_parse_strv,  
0, info-aliases   },
 @@ -1087,6 +1094,9 @@ static int unit_file_load(
  if (r  0)
  return r;
  
 +if (also  !strv_isempty(info-also))
 +*also = strv_copy(info-also);
 +

 If the argument would just be a bool* we wouldn't have to do the
 expensive strv_copy() here... (which is missing an OOM check btw...)

 Otherwise looks great!

 Lennart

Next version incoming!

Cheers,
-- 
Jan Synacek
Software Engineer, Red Hat


signature.asc
Description: PGP signature
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel