Re: [systemd-devel] [PATCH] shared/install: avoid prematurely rejecting missing units

2014-10-31 Thread Dave Reisner 
On Fri, Oct 31, 2014 at 04:04:53AM +0100, Zbigniew Jędrzejewski-Szmek wrote:
 On Thu, Oct 30, 2014 at 08:28:14PM -0400, Dave Reisner wrote:
  f7101b7368df copied some logic to prevent enabling masked units, but
  also added a check which causes attempts to enable templated units to
  fail. Since we know the logic beyond this check will properly handle
  units which truly do not exist, we can rely on the unit file state
  comparison to suffice for expressing the intent of f7101b7368df.
  
  ref: https://bugs.archlinux.org/task/42616
  ---
  This seems to me like the right thing to do, but I'm not so familiar with
  this code...
 
 I verified that your fix works. Can you add a comment in the code which
 explains why state is not checked though? It should help with future
 modifications.
 
 Zbyszek
 

Thanks! Added a comment and pushed.

  
   src/shared/install.c | 5 -
   1 file changed, 5 deletions(-)
  
  diff --git a/src/shared/install.c b/src/shared/install.c
  index 035b44c..3ad5362 100644
  --- a/src/shared/install.c
  +++ b/src/shared/install.c
  @@ -1621,11 +1621,6 @@ int unit_file_enable(
   UnitFileState state;
   
   state = unit_file_get_state(scope, root_dir, *i);
  -if (state  0) {
  -log_error(Failed to get unit file state for %s: 
  %s, *i, strerror(-state));
  -return state;
  -}
  -
   if (state == UNIT_FILE_MASKED || state == 
  UNIT_FILE_MASKED_RUNTIME) {
   log_error(Failed to enable unit: Unit %s is 
  masked, *i);
   return -ENOTSUP;
  -- 
  2.1.3
  ___
  systemd-devel mailing list
  systemd-devel@lists.freedesktop.org
  http://lists.freedesktop.org/mailman/listinfo/systemd-devel
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] shared/install: avoid prematurely rejecting missing units

2014-10-30 Thread Dave Reisner 
f7101b7368df copied some logic to prevent enabling masked units, but
also added a check which causes attempts to enable templated units to
fail. Since we know the logic beyond this check will properly handle
units which truly do not exist, we can rely on the unit file state
comparison to suffice for expressing the intent of f7101b7368df.

ref: https://bugs.archlinux.org/task/42616
---
This seems to me like the right thing to do, but I'm not so familiar with
this code...

 src/shared/install.c | 5 -
 1 file changed, 5 deletions(-)

diff --git a/src/shared/install.c b/src/shared/install.c
index 035b44c..3ad5362 100644
--- a/src/shared/install.c
+++ b/src/shared/install.c
@@ -1621,11 +1621,6 @@ int unit_file_enable(
 UnitFileState state;
 
 state = unit_file_get_state(scope, root_dir, *i);
-if (state  0) {
-log_error(Failed to get unit file state for %s: %s, 
*i, strerror(-state));
-return state;
-}
-
 if (state == UNIT_FILE_MASKED || state == 
UNIT_FILE_MASKED_RUNTIME) {
 log_error(Failed to enable unit: Unit %s is masked, 
*i);
 return -ENOTSUP;
-- 
2.1.3
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] shared/install: avoid prematurely rejecting missing units

2014-10-30 Thread Zbigniew Jędrzejewski-Szmek 
On Thu, Oct 30, 2014 at 08:28:14PM -0400, Dave Reisner wrote:
 f7101b7368df copied some logic to prevent enabling masked units, but
 also added a check which causes attempts to enable templated units to
 fail. Since we know the logic beyond this check will properly handle
 units which truly do not exist, we can rely on the unit file state
 comparison to suffice for expressing the intent of f7101b7368df.
 
 ref: https://bugs.archlinux.org/task/42616
 ---
 This seems to me like the right thing to do, but I'm not so familiar with
 this code...

I verified that your fix works. Can you add a comment in the code which
explains why state is not checked though? It should help with future
modifications.

Zbyszek

 
  src/shared/install.c | 5 -
  1 file changed, 5 deletions(-)
 
 diff --git a/src/shared/install.c b/src/shared/install.c
 index 035b44c..3ad5362 100644
 --- a/src/shared/install.c
 +++ b/src/shared/install.c
 @@ -1621,11 +1621,6 @@ int unit_file_enable(
  UnitFileState state;
  
  state = unit_file_get_state(scope, root_dir, *i);
 -if (state  0) {
 -log_error(Failed to get unit file state for %s: 
 %s, *i, strerror(-state));
 -return state;
 -}
 -
  if (state == UNIT_FILE_MASKED || state == 
 UNIT_FILE_MASKED_RUNTIME) {
  log_error(Failed to enable unit: Unit %s is 
 masked, *i);
  return -ENOTSUP;
 -- 
 2.1.3
 ___
 systemd-devel mailing list
 systemd-devel@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/systemd-devel
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel