Re: [systemd-devel] [RFC] [PATCH] core: avoid duplicate unit property set
On 12/09/2014 12:10 AM, Lennart Poettering wrote: > On Mon, 08.12.14 15:32, WaLyong Cho (walyong@samsung.com) wrote: > >> Hi, >> >> First, I'd like to ask unit property should be applied immediately when >> systemctl set-property is called? >> If yes, after systemctl set-property, why we can see the message for >> daemon-reload. I think that should be re-loaded automatically. >> If no, some of unit properties are set immediatly. >> >> See below: >> # systemctl set-property dbus.service CPUShares=1200 >> # systemctl status dbus.service >> * dbus.service - D-Bus System Message Bus >>Loaded: loaded (/usr/lib/systemd/system/dbus.service; enabled; vendor >> preset: disabled) >>Active: active (running) since Mon 2014-12-08 14:59:12 KST; 1min 30s ago >> Docs: man:dbus-daemon(1) >> Main PID: 31 (dbus-daemon) >>CGroup: /machine.slice/machine-container.scope/system.slice/dbus.service >>`-31 /usr/bin/dbus-daemon --system --address=systemd: >> --nofork --nopidfile --systemd-activation >> >> Dec 08 14:59:12 container systemd[1]: Starting D-Bus System Message Bus... >> Dec 08 14:59:12 container systemd[1]: Started D-Bus System Message Bus. >> Dec 08 14:59:12 container dbus[31]: [system] Successfully activated >> service 'org.freedesktop.systemd1' >> >> Warning: Unit file changed on disk, 'systemctl daemon-reload' recommended. >> >> systemctl status saied "systemctl daemon-reload" is needed. It sound >> something is changed and to apply that daemon-reload is needed. >> But I can notice that property already applied. > > Hmm, yeah, that message shouldn't be shown. Are you sure it was > triggered by the set-property call though? Maybe it was there already before? See this log: [root@container ~]# systemctl status dbus.service * dbus.service - D-Bus System Message Bus Loaded: loaded (/usr/lib/systemd/system/dbus.service; enabled; vendor preset: disabled) Active: active (running) since Tue 2014-12-09 00:22:50 KST; 10s ago Docs: man:dbus-daemon(1) Main PID: 32 (dbus-daemon) CGroup: /machine.slice/machine-container.scope/system.slice/dbus.service `-32 /usr/bin/dbus-daemon --system --address=systemd: --nofork --nopidfile --systemd-activation Dec 09 00:22:50 container systemd[1]: Starting D-Bus System Message Bus... Dec 09 00:22:50 container systemd[1]: Started D-Bus System Message Bus. Dec 09 00:22:50 container dbus[32]: [system] Successfully activated service 'org.freedesktop.systemd1' [root@container ~]# [root@container ~]# systemctl set-property dbus.service CPUShares=777 [root@container ~]# [root@container ~]# systemctl status dbus.service * dbus.service - D-Bus System Message Bus Loaded: loaded (/usr/lib/systemd/system/dbus.service; enabled; vendor preset: disabled) Active: active (running) since Tue 2014-12-09 00:22:50 KST; 29s ago Docs: man:dbus-daemon(1) Main PID: 32 (dbus-daemon) CGroup: /machine.slice/machine-container.scope/system.slice/dbus.service `-32 /usr/bin/dbus-daemon --system --address=systemd: --nofork --nopidfile --systemd-activation Dec 09 00:22:50 container systemd[1]: Starting D-Bus System Message Bus... Dec 09 00:22:50 container systemd[1]: Started D-Bus System Message Bus. Dec 09 00:22:50 container dbus[32]: [system] Successfully activated service 'org.freedesktop.systemd1' Warning: Unit file changed on disk, 'systemctl daemon-reload' recommended. [root@container ~]# [root@container ~]# cat /sys/fs/cgroup/cpu/machine.slice/machine-container.scope/system.slice/dbus.service/cpu.shares 777 set-property is applied immediately well. I will try to prepare patch for this. And if the property set should be applied immediately then it seems there are duplicate property set by unit_load_dropin(). Should we skip unit_load_dropin() if the unit is transient? WaLyong > > Lennart > ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [RFC] [PATCH] core: avoid duplicate unit property set
On Mon, 08.12.14 15:32, WaLyong Cho (walyong@samsung.com) wrote: > Hi, > > First, I'd like to ask unit property should be applied immediately when > systemctl set-property is called? > If yes, after systemctl set-property, why we can see the message for > daemon-reload. I think that should be re-loaded automatically. > If no, some of unit properties are set immediatly. > > See below: > # systemctl set-property dbus.service CPUShares=1200 > # systemctl status dbus.service > * dbus.service - D-Bus System Message Bus >Loaded: loaded (/usr/lib/systemd/system/dbus.service; enabled; vendor > preset: disabled) >Active: active (running) since Mon 2014-12-08 14:59:12 KST; 1min 30s ago > Docs: man:dbus-daemon(1) > Main PID: 31 (dbus-daemon) >CGroup: /machine.slice/machine-container.scope/system.slice/dbus.service >`-31 /usr/bin/dbus-daemon --system --address=systemd: > --nofork --nopidfile --systemd-activation > > Dec 08 14:59:12 container systemd[1]: Starting D-Bus System Message Bus... > Dec 08 14:59:12 container systemd[1]: Started D-Bus System Message Bus. > Dec 08 14:59:12 container dbus[31]: [system] Successfully activated > service 'org.freedesktop.systemd1' > > Warning: Unit file changed on disk, 'systemctl daemon-reload' recommended. > > systemctl status saied "systemctl daemon-reload" is needed. It sound > something is changed and to apply that daemon-reload is needed. > But I can notice that property already applied. Hmm, yeah, that message shouldn't be shown. Are you sure it was triggered by the set-property call though? Maybe it was there already before? 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] [RFC] [PATCH] core: avoid duplicate unit property set
On Mon, 08.12.14 15:12, WaLyong Cho (walyong@samsung.com) wrote: > Currently, unit property set apis set unit property and also make a > dropin files in each dbus-xyz.c. And the dropin will set its > properties again in unit_load(). > So don't need to set property immediatly. That will be set next > unit_load(). Just write dropin files only. Well, we there are two kinds of options currently: a) the ones you can change any time, using "systemctl set-property". Primarily these are cgroup options to change resource controls during runtime. When "systemctl set-property" is invoked, then the changes should be instantly applied. b) The ones you can only change when creating a transient unit, which cannot be changed later on with "systemctl set-property". Now, both kinds of properties can be set when creating transient units, but only the properties of type (a) can be altered after creating the units using "systemctl set-property". Now, unit_load() is only invoked when we create units, or on "systemctl daemon-reload". We write the drop-in snippets for these cases. However, we also want to allow the properties of type (a) to be effective *without* actually waiting for unit_load(), they should apply instantly, hence we apply them also directly, without unit_load(). Hope this makes sense? 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] [RFC] [PATCH] core: avoid duplicate unit property set
Hi,
First, I'd like to ask unit property should be applied immediately when
systemctl set-property is called?
If yes, after systemctl set-property, why we can see the message for
daemon-reload. I think that should be re-loaded automatically.
If no, some of unit properties are set immediatly.
See below:
# systemctl set-property dbus.service CPUShares=1200
# systemctl status dbus.service
* dbus.service - D-Bus System Message Bus
Loaded: loaded (/usr/lib/systemd/system/dbus.service; enabled; vendor
preset: disabled)
Active: active (running) since Mon 2014-12-08 14:59:12 KST; 1min 30s ago
Docs: man:dbus-daemon(1)
Main PID: 31 (dbus-daemon)
CGroup: /machine.slice/machine-container.scope/system.slice/dbus.service
`-31 /usr/bin/dbus-daemon --system --address=systemd:
--nofork --nopidfile --systemd-activation
Dec 08 14:59:12 container systemd[1]: Starting D-Bus System Message Bus...
Dec 08 14:59:12 container systemd[1]: Started D-Bus System Message Bus.
Dec 08 14:59:12 container dbus[31]: [system] Successfully activated
service 'org.freedesktop.systemd1'
Warning: Unit file changed on disk, 'systemctl daemon-reload' recommended.
systemctl status saied "systemctl daemon-reload" is needed. It sound
something is changed and to apply that daemon-reload is needed.
But I can notice that property already applied.
# systemctl show dbus.service | grep CPUShares
CPUShares=1200
StartupCPUShares=18446744073709551615
This immediate property set make duplicate operation when systemd-run is
called. The first will be set by transient property set in method call
handler. The second will be set by unit_load_dropin() the last actual load.
What it the most proper way?
WaLyong
On 12/08/2014 03:12 PM, WaLyong Cho wrote:
> Currently, unit property set apis set unit property and also make a
> dropin files in each dbus-xyz.c. And the dropin will set its
> properties again in unit_load().
> So don't need to set property immediatly. That will be set next
> unit_load(). Just write dropin files only.
> ---
> src/core/dbus-cgroup.c | 325
> ++-
> src/core/dbus-cgroup.h | 2 +-
> src/core/dbus-execute.c | 77 +++
> src/core/dbus-execute.h | 2 +-
> src/core/dbus-kill.c | 26 ++--
> src/core/dbus-kill.h | 2 +-
> src/core/dbus-mount.c| 8 +-
> src/core/dbus-scope.c| 6 +-
> src/core/dbus-service.c | 77 +--
> src/core/dbus-slice.c| 6 +-
> src/core/dbus-socket.c | 6 +-
> src/core/dbus-swap.c | 6 +-
> src/core/dbus-unit.c | 12 +-
> src/core/load-fragment.c | 61 ++---
> 14 files changed, 227 insertions(+), 389 deletions(-)
>
> diff --git a/src/core/dbus-cgroup.c b/src/core/dbus-cgroup.c
> index db99834..ffbd8d5 100644
> --- a/src/core/dbus-cgroup.c
> +++ b/src/core/dbus-cgroup.c
> @@ -24,6 +24,7 @@
> #include "cgroup-util.h"
> #include "cgroup.h"
> #include "dbus-cgroup.h"
> +#include "strv.h"
>
> static BUS_DEFINE_PROPERTY_GET_ENUM(property_get_cgroup_device_policy,
> cgroup_device_policy, CGroupDevicePolicy);
>
> @@ -173,16 +174,15 @@ const sd_bus_vtable bus_cgroup_vtable[] = {
>
> static int bus_cgroup_set_transient_property(
> Unit *u,
> -CGroupContext *c,
> const char *name,
> sd_bus_message *message,
> UnitSetPropertiesMode mode,
> sd_bus_error *error) {
>
> +
> int r;
>
> assert(u);
> -assert(c);
> assert(name);
> assert(message);
>
> @@ -194,8 +194,9 @@ static int bus_cgroup_set_transient_property(
> return r;
>
> if (mode != UNIT_CHECK) {
> -c->delegate = b;
> -unit_write_drop_in_private(u, mode, name, b ?
> "Delegate=yes" : "Delegate=no");
> +r = unit_write_drop_in_private(u, mode, name, b ?
> "Delegate=yes" : "Delegate=no");
> +if (r < 0)
> +return r;
> }
>
> return 1;
> @@ -206,7 +207,6 @@ static int bus_cgroup_set_transient_property(
>
> int bus_cgroup_set_property(
> Unit *u,
> -CGroupContext *c,
> const char *name,
> sd_bus_message *message,
> UnitSetPropertiesMode mode,
> @@ -215,7 +215,6 @@ int bus_cgroup_set_property(
> int r;
>
> assert(u);
> -assert(c);
> assert(name);
> assert(message);
>
> @@ -227,9 +226,9 @@ int bus_cgroup_set_property(
> return r;
>
> if (mode != UNIT_CHECK) {
> -c->cpu_accounting = b;
> -u->cgroup_realized_mask &= ~CGROUP_CPUACCT;
> -unit_write_drop_in_private(u, mode, name, b ?
> "CPUAccounting=yes" : "CPUAccounting=no");
> +
[systemd-devel] [RFC] [PATCH] core: avoid duplicate unit property set
Currently, unit property set apis set unit property and also make a
dropin files in each dbus-xyz.c. And the dropin will set its
properties again in unit_load().
So don't need to set property immediatly. That will be set next
unit_load(). Just write dropin files only.
---
src/core/dbus-cgroup.c | 325 ++-
src/core/dbus-cgroup.h | 2 +-
src/core/dbus-execute.c | 77 +++
src/core/dbus-execute.h | 2 +-
src/core/dbus-kill.c | 26 ++--
src/core/dbus-kill.h | 2 +-
src/core/dbus-mount.c| 8 +-
src/core/dbus-scope.c| 6 +-
src/core/dbus-service.c | 77 +--
src/core/dbus-slice.c| 6 +-
src/core/dbus-socket.c | 6 +-
src/core/dbus-swap.c | 6 +-
src/core/dbus-unit.c | 12 +-
src/core/load-fragment.c | 61 ++---
14 files changed, 227 insertions(+), 389 deletions(-)
diff --git a/src/core/dbus-cgroup.c b/src/core/dbus-cgroup.c
index db99834..ffbd8d5 100644
--- a/src/core/dbus-cgroup.c
+++ b/src/core/dbus-cgroup.c
@@ -24,6 +24,7 @@
#include "cgroup-util.h"
#include "cgroup.h"
#include "dbus-cgroup.h"
+#include "strv.h"
static BUS_DEFINE_PROPERTY_GET_ENUM(property_get_cgroup_device_policy,
cgroup_device_policy, CGroupDevicePolicy);
@@ -173,16 +174,15 @@ const sd_bus_vtable bus_cgroup_vtable[] = {
static int bus_cgroup_set_transient_property(
Unit *u,
-CGroupContext *c,
const char *name,
sd_bus_message *message,
UnitSetPropertiesMode mode,
sd_bus_error *error) {
+
int r;
assert(u);
-assert(c);
assert(name);
assert(message);
@@ -194,8 +194,9 @@ static int bus_cgroup_set_transient_property(
return r;
if (mode != UNIT_CHECK) {
-c->delegate = b;
-unit_write_drop_in_private(u, mode, name, b ?
"Delegate=yes" : "Delegate=no");
+r = unit_write_drop_in_private(u, mode, name, b ?
"Delegate=yes" : "Delegate=no");
+if (r < 0)
+return r;
}
return 1;
@@ -206,7 +207,6 @@ static int bus_cgroup_set_transient_property(
int bus_cgroup_set_property(
Unit *u,
-CGroupContext *c,
const char *name,
sd_bus_message *message,
UnitSetPropertiesMode mode,
@@ -215,7 +215,6 @@ int bus_cgroup_set_property(
int r;
assert(u);
-assert(c);
assert(name);
assert(message);
@@ -227,9 +226,9 @@ int bus_cgroup_set_property(
return r;
if (mode != UNIT_CHECK) {
-c->cpu_accounting = b;
-u->cgroup_realized_mask &= ~CGROUP_CPUACCT;
-unit_write_drop_in_private(u, mode, name, b ?
"CPUAccounting=yes" : "CPUAccounting=no");
+r = unit_write_drop_in_private(u, mode, name, b ?
"CPUAccounting=yes" : "CPUAccounting=no");
+if (r < 0)
+return r;
}
return 1;
@@ -251,9 +250,9 @@ int bus_cgroup_set_property(
}
if (mode != UNIT_CHECK) {
-c->cpu_shares = ul;
-u->cgroup_realized_mask &= ~CGROUP_CPU;
-unit_write_drop_in_private_format(u, mode, name,
"CPUShares=%lu", ul);
+r = unit_write_drop_in_private_format(u, mode, name,
"CPUShares=%lu", ul);
+if (r < 0)
+return r;
}
return 1;
@@ -275,9 +274,9 @@ int bus_cgroup_set_property(
}
if (mode != UNIT_CHECK) {
-c->startup_cpu_shares = ul;
-u->cgroup_realized_mask &= ~CGROUP_CPU;
-unit_write_drop_in_private_format(u, mode, name,
"StartupCPUShares=%lu", ul);
+r = unit_write_drop_in_private_format(u, mode, name,
"StartupCPUShares=%lu", ul);
+if (r < 0)
+return r;
}
return 1;
@@ -293,9 +292,9 @@ int bus_cgroup_set_property(
return sd_bus_error_set_errnof(error, EINVAL,
"CPUQuotaPerSecUSec value out of range");
if (mode != UNIT_CHECK) {
-c->cpu_quota_per_sec_usec = u64;
-u->cgroup_realized_mask &= ~CGROUP_CPU;
-unit_write_drop_in_private_format(u, mode, "CPUQuota",
"CPUQuota=%0.f%%", (double) (c->cpu_quota_per_sec_usec / 1));
+r = unit_write_drop_in_private_format(u, mode,
"CPUQuota", "CPUQuota=%0.f%%", (do
