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).
>
> 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=.
>
Rework is almost done. Now I'm testing. But the AccuracySec= and
WakeSystem= options are hard to deal with --property= option. Currently
the --property= option of systemd-run is designed for service or scope
unit. If that option is specified then given string is parsed again by
bus_append_unit_property_assignment(). We can AccuracySec=/WakeSystem=
option to there. But we have to split given properties(by --property
option) according to which unit can have that properties.
So I'd like to keep those two option as separated systemd-run options.
If you agree, I will send a patch soon.
WaLyong
>> +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.
>
> (I'll leave the rest unreviewed for now, please rework this first to
> make use of the fourth StartTransientUnit() bus call argument!)
>
> Thanks,
>
> Lennart
>
_______________________________________________
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
