On 10/23/2014 12:56 AM, Lennart Poettering wrote:
On Tue, 07.10.14 14:20, WaLyong Cho (walyong....@samsung.com) wrote:
If systemd-run is called with timer option, then systemd-run call
NewTransientUnit with service unit. And also call StartTransientUnit
with timer unit which has same name with the service. So actually, two
method call is coming and two transient unit is generated. One is
service and the other is timer.
Supported timer options are --after=, --after-boot=, --after-startup=,
--after-active=, --after-inactive=, --calendar=, --accuracy= and
--wake-system. Each option corresponding with OnActiveSec=,
OnBootSec=, OnStartupSec=, OnUnitActiveSec=, OnUnitInactiveSec=,
AccuracySec= and AccuracySec= of timer respectively.
Hmm, I'd really like to stay close to the underlying props here in
naming, and call the options "--on-boot=", "--on-inactive=" and so on
(the -sec suffix we can probably drop).
I'd really tought and worried about naming of the options. At the first
time, I'd made similar named option with timer unit. But I tought the
command line options should be more meaningful or easy to understand. So
I changed like this. But if you say so, I don't want to against to your
opinion. I will change those.
The AccuracySec= and WakeSystem= stuff I think we don't need to cover
with a command line argument of its own, we can cover that with
--property=.
Ah, good, I didn't know that. I will do like that.
+static bool with_timer = false;
+static char *arg_after = NULL;
+static char *arg_after_boot = NULL;
+static char *arg_after_startup = NULL;
+static char *arg_after_active = NULL;
+static char *arg_after_inactive = NULL;
+static char *arg_calendar = NULL;
+static char *arg_accuracy = NULL;
+static bool arg_wake_system = false;
I'd just do one arg_on_timer usec_t variable to store most of the
--on-xyz= values in. Plus one arg_on_calendar to hold the calendar
expression. This should be much simpler than keeping one variable
around for each.
+ case ARG_AFTER:
+ r = parse_sec(optarg, &u);
+ if (r < 0) {
+ log_error("Failed to parse timer value: %s",
optarg);
+ return r;
+ }
+ arg_after = optarg;
+ with_timer = true;
+ break;
This stuff really should not store the string, but the parsed value.
Yeah, a couple of parse_sec doesn't make sense.
How about calendar_spec_from_string? calendar_spec_from_string also be
called twice on run and dbus-timer. I'm not sure the parsed calendar
spec is able to be sent on current StartTransientUnit() interface. But
even if possible, just string is obiously easy to send and receive. How
do you think?
(I'll leave the rest unreviewed for now, please rework this first to
make use of the fourth StartTransientUnit() bus call argument!)
If you reply on above question, then I will send patch again soon.
Thanks,
WaLyong
Thanks,
Lennart
_______________________________________________
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
