Re: [systemd-devel] How to get used to systemd vs init

[Ronny Chevalier]

On Wed, Jun 24, 2015 at 8:53 AM, Chad  wrote:
> Killermoehre,
> Thank you for your time and reply.
>
> I intend to do exactly that when I start using systemd (I am still using
> init.d at the moment). In fact I have already suggested that very thing on
> the fail2ban mailing list so that can add it to the tree and no custom rule
> is needed. To my knowlage there is not a built in/standard way to tie
> init.d/iptables to init.d/fail2ban.
>
> The test for the chains existence is still needed in case the chain is
> removed by other means (like a manual delete from the cli).
> I have found that I can trust nothing and that I should check/test
> everything :) when I think something is impossible or so unlikely that it
> will "never happen to me" it inevitably is a problem at the worst possible
> moment. I bet some of you know what I mean.
>
> ^C
>

I think the best for you will be to read "The systemd for
Administrators Blog Series", which is a section at
http://www.freedesktop.org/wiki/Software/systemd/

It explains for example how to convert a sysv init script to a systemd
service file, what are the changes regarding configuration files with
systemd, how to improve the security of your services, analyze the
startup,... It will help for the transition from sysv to systemd.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] How to get used to systemd vs init

[Ronny Chevalier]

On Wed, Jun 24, 2015 at 1:37 AM, Chad  wrote:
> Oh, wait this is the reverse of what I want/need (systemd-sysv-generator
> goes from init.d to systemd, I need from systemd to init.d).
> I have a nagios script that runs something like:
> /etc/init.d/httpd status
> It then reads the output and makes sure httpd is running, if not it takes
> action depending on the service.
> I use that method for tons of services.
> I don't want to have to re-write the modules to use:
> systemctl status httpd
> If I did that then I will not be able to rsync the scripts/configs around
> and would have to maintain 2 versions of the code.
> I was wondering if there was an easy way to create a /etc/init.d/httpd
> script that called something like this inside:
> #!/bin/bash
> systemctl $1 $0
> I know it is not that simple ($0 for example is the full path
> /etc/init.d/httpd not just the httpd), which is why I am hoping there is a
> tool for this.
>

If you just want to know if a service is active you can use:

systemctl is-active httpd

If $? equals 0 then the service is active, else it is not :)

If you make your script use this I don't see why you would have to
maintain multiple versions, if your intention is to use systemd
everywhere.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] How to get used to systemd vs init

[Ronny Chevalier]

On Wed, Jun 24, 2015 at 1:07 AM, Chad  wrote:
> Mr. Chevalier,
> Thank you for your time and reply.
>
> On 6/23/2015 1:30 PM, Ronny Chevalier wrote:
>>
>> On Tue, Jun 23, 2015 at 9:45 PM, Chad  wrote:
>>>
>>> I am sure this is the wrong place to send this e-mail, but I could not
>>> find
>>> another place to send it.
>>
>> Hi,
>>
>> It is the good place :)
>
> Great, thanks.
>
>>> I want to learn and use systemd, but have run into a few problems on my
>>> way.
>>> Please don't see this as an attack on systemd, I want to learn something
>>> new, but change is hard.
>>>
>>> I am an old school kind of sysadmin and I am planning on moving from
>>> CentOS
>>> 6 to CentOS 7, but I am having trouble with systemd. I am hoping you know
>>> some shortcuts/tricks to help me learn the new way.
>>>
>>> #
>>> 1. I can't spell. With init I don't have to know how to spell things
>>> because
>>> I have tab complete. I use tab complete for almost every command I type.
>>> For
>>> example:
>>> /e gets -> /etc/
>>> /etc/in  gets -> /etc/init
>>> /etc/init.  gets -> /etc/init.d/
>>> /etc/init.d/ht /etc/init.d/httpd
>>> /etc/init.d/httpd restart
>>> So I entered 19 characters and got 25 with tab complete.
>>>
>>> The new systemd way would be to type (23 total characters, no tab
>>> complete):
>>> systemctl restart httpd
>>> Maybe I could tab complete systemctl, but I don't currently have a CentOS
>>> 7
>>> system to test on.
>>>
>>> The real issue is that I have to know (in the above example) that it is
>>> httpd not http.
>>> With so many systems, distros, and services it is hard to remember every
>>> service name exactly (and some names are very long). For example ntpd has
>>> a
>>> d, but nfs does not.
>>> Tab completion fixes this issue for me.
>>>
>>> How can I use tab completion with systemd?
>>
>> If you use either bash or zsh, systemd provides shell completion for them.
>>
>> You could do something like:
>>
>> systemctl start htt
>> systemctl st
>>
>> or else, and it will complete it.
>
> I use bash. This is a cool trick that systemd has over init.d. I know not
> all programs can do that shell completion, for example /etc/init.d/httpd
> res does not work (I try it all the time out of tab completion habit!).
>>
>>
>>>
>>> #
>>> 2. How to find all possible services:
>>>
>>> The init way:
>>> ls -l /etc/init/d
>>>
>>> The systemd way:
>>> ls -l /lib/systemd/system/*.service /etc/systemd/system/*.service
>>>
>>> This seems WAY harder and I have to remember 2 locations instead of 1.
>>
>> There is:
>>
>> systemctl list-unit-files
>>
>> It lists all the units installed on your system. In systemd a unit is
>> a configuration file that can describe a service, a mount point, a
>> device,... So a service is a subtype of unit, see "man 5 systemd.unit"
>> for more information.
>>
>> So if you want to only display the services, you just have to specify the
>> type
>>
>> systemctl list-unit-files --type=service
>
> Ok, so that is a lot more to remember than ls -l /etc/init.d, but I can
> learn it.
>>
>>
>>> #
>>> 3. List all services and their start levels:
>>>
>> In systemd world, start levels equivalent are the targets (see man 5
>> systemd.target). A target is a synchronisation point between multiple
>> units. For example, there is sysinit.target which is the
>> synchronization point for early boot services. This way a unit can ask
>> to be started only after a specific target, for example.
>>
>>> The init way (all services):
>>> chkconfig --list
>>>
>>> The init way (only active services. I use this a lot):
>>> chkconfig --list | grep :on
>>>
>>> The systemd way (all services):
>>> systemctl list-unit-files --type=service
>>>
>>> The systemd way (only active services, I don't know how to do this).
>>> systemctl ???
>>>
>> With systemctl you can provide a filter according to the current state
>> of a unit. If you want to list all the active service, you can do:
>>
>> systemctl --state=active --type=service list-units
>
> Ok, again more to type and remember, but

Re: [systemd-devel] How to get used to systemd vs init

[Ronny Chevalier]

On Tue, Jun 23, 2015 at 10:16 PM, Chad  wrote:
> On 6/23/2015 1:01 PM, Reindl Harald wrote:
>>
>>
>>
>> Am 23.06.2015 um 21:45 schrieb Chad:
>>>
>>> The new systemd way would be to type (23 total characters, no tab
>>> complete):
>>> systemctl restart httpd
>>> Maybe I could tab complete systemctl, but I don't currently have a
>>> CentOS 7 system to test on.
>>
>>
>> maybe you should just install CentOS inside a VM and test it
>>
>> [root@srv-rhsoft:~]$ systemctl restart h
>> halt-local.service   haveged.service home.mount
>> hostapd-guest.service
>> httpd-lounge-worker.service  hybrid-sleep.target
>> halt.target  hibernate.target
>> hostapd-guest-interface.service  hostapd.service httpd.service
>>
>>
>>> The real issue is that I have to know (in the above example) that it is
>>> httpd not http.
>>> With so many systems, distros, and services it is hard to remember every
>>> service name exactly (and some names are very long). For example ntpd
>>> has a d, but nfs does not.
>>> Tab completion fixes this issue for me.
>>>
>>> How can I use tab completion with systemd?
>>
>>
>> as like for any other software - hit the TAB key
>>
>>> #
>>> 2. How to find all possible services:
>>>
>>> The init way:
>>> ls -l /etc/init/d
>>>
>>> The systemd way:
>>> ls -l /lib/systemd/system/*.service /etc/systemd/system/*.service
>>>
>>> This seems WAY harder and I have to remember 2 locations instead of 1
>>
>>
>> nobody but you installs systemd-units in /etc/ and so you have only one
>> location AND customized ones - with sysvinit you
>> had no way to override /etc/init.d/httpd without doing the work after each
>> update again
>>
>
> Harald,
> Thank you for your reply and time.
>
> I will make some time at some point to install CentOS 7 again, I just don't
> have one installed right now.
>
> #1 I did not know you could tab complete that way! i.e. as part of a command
> argument, not just as part of a path.
> Guess I learned something new after 20 years :)
>
> #2 I can see the advantages of having a local override just like /usr/bin
> has /usr/local/bin.
> Out of curiosity is there a reason the team did not follow the local pattern
> with something like: /lib/systemd/local/system?
> It is easy enough to create an alias on systems I use often, it will just
> take time to learn/memorize the new paths, I am so used to /etc/init.d.
>

See http://www.freedesktop.org/wiki/Software/systemd/TheCaseForTheUsrMerge/
for why /lib/ is not used anymore.

Anyway, /usr is considered to be vendor specific (what your distro
provides you) and you should not modify it. /etc, in the other hand,
is entirely for you. If you want to override a unit configuration, you
do not modify /usr/lib/systemd/system/foobar.service but you put your
version in /etc/systemd/system/foobar.service. Then you reload the
units via systemctl daemon-reload. systemctl also provides a simple
tool to do this:

systemctl edit --full foobar.service

It will copy the unit in /etc and open your editor. When you save and
quit, it will automatically run systemctl daemon-reload. You can also
only modify just one directive of a unit file by using:

systemctl edit foobar.service

It will create /etc/systemd/system/foobar.service.d/override.conf and
open your editor. In this file you just have to put the
lines/directives that you want to modify from the original one,
without having to edit the entire unit.

> ^C
>
> ___
> 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

Re: [systemd-devel] How to get used to systemd vs init

[Ronny Chevalier]

On Tue, Jun 23, 2015 at 9:45 PM, Chad  wrote:
> I am sure this is the wrong place to send this e-mail, but I could not find
> another place to send it.

Hi,

It is the good place :)

> I want to learn and use systemd, but have run into a few problems on my way.
> Please don't see this as an attack on systemd, I want to learn something
> new, but change is hard.
>
> I am an old school kind of sysadmin and I am planning on moving from CentOS
> 6 to CentOS 7, but I am having trouble with systemd. I am hoping you know
> some shortcuts/tricks to help me learn the new way.
>
> #
> 1. I can't spell. With init I don't have to know how to spell things because
> I have tab complete. I use tab complete for almost every command I type. For
> example:
> /e gets -> /etc/
> /etc/in  gets -> /etc/init
> /etc/init.  gets -> /etc/init.d/
> /etc/init.d/ht /etc/init.d/httpd
> /etc/init.d/httpd restart
> So I entered 19 characters and got 25 with tab complete.
>
> The new systemd way would be to type (23 total characters, no tab complete):
> systemctl restart httpd
> Maybe I could tab complete systemctl, but I don't currently have a CentOS 7
> system to test on.
>
> The real issue is that I have to know (in the above example) that it is
> httpd not http.
> With so many systems, distros, and services it is hard to remember every
> service name exactly (and some names are very long). For example ntpd has a
> d, but nfs does not.
> Tab completion fixes this issue for me.
>
> How can I use tab completion with systemd?

If you use either bash or zsh, systemd provides shell completion for them.

You could do something like:

systemctl start htt
systemctl st

or else, and it will complete it.

>
>
> #
> 2. How to find all possible services:
>
> The init way:
> ls -l /etc/init/d
>
> The systemd way:
> ls -l /lib/systemd/system/*.service /etc/systemd/system/*.service
>
> This seems WAY harder and I have to remember 2 locations instead of 1.

There is:

systemctl list-unit-files

It lists all the units installed on your system. In systemd a unit is
a configuration file that can describe a service, a mount point, a
device,... So a service is a subtype of unit, see "man 5 systemd.unit"
for more information.

So if you want to only display the services, you just have to specify the type

systemctl list-unit-files --type=service

>
> #
> 3. List all services and their start levels:
>

In systemd world, start levels equivalent are the targets (see man 5
systemd.target). A target is a synchronisation point between multiple
units. For example, there is sysinit.target which is the
synchronization point for early boot services. This way a unit can ask
to be started only after a specific target, for example.

> The init way (all services):
> chkconfig --list
>
> The init way (only active services. I use this a lot):
> chkconfig --list | grep :on
>
> The systemd way (all services):
> systemctl list-unit-files --type=service
>
> The systemd way (only active services, I don't know how to do this).
> systemctl ???
>

With systemctl you can provide a filter according to the current state
of a unit. If you want to list all the active service, you can do:

systemctl --state=active --type=service list-units

>
> #
> 4. What about the many programs that rely on /etc/init.d/
> status/start/stop/restart
> I have many services that are monitored by nagios or cron jobs (like
> logrotate) that rely on /etc/init.d/ status/start/stop/restart.
> I don't want to change them because right now they work on every server and
> I don't want to have to maintain 2 versions of the code or hunt them all
> down.

There is systemd-sysv-generator which creates wrapper .service for
sysv scripts automatically at boot. But you need to specify additional
headers if you want to use ordering. See man systemd-sysv-generator.

> Is there some trick/3rd party script to create /etc/init.d wrappers/scripts
> to make all the services work with the old path?
> Something like:
> ln -s /lib/systemd/system/.service /etc/init.d/
> Or maybe a shell script like:
> service=`basename "$0"`
> systemctl $1 $service
>
> So I would like to move forward with systemd (and will eventually have to if
> I want modern/supported OSs), but systemd seems harder to deal with and will
> break a lot of my existing scripts/cronjobs/monitors.
>
>
> Thank you all for your work on FOSS, you are making the world a better
> place!!
>
> --
>
> ^C
> Chad Columbus
> 20 years of application development and sysadmin
> Currently maintaining about 30 CentOS 6 servers.
> Have maintained over 1,000 linux servers over the years.
>
> ___
> 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

Re: [systemd-devel] [ANNOUNCE] Git development moved to github

[Ronny Chevalier]

On Fri, Jun 12, 2015 at 5:30 AM, Filipe Brandenburger
 wrote:
> On Thu, Jun 11, 2015 at 12:31 PM, Ronny Chevalier
>  wrote:
>> On Thu, Jun 11, 2015 at 6:31 PM, Filipe Brandenburger 
>>  wrote:
>>> Another downside of adding comments to the commits is that e-mail
>>> notifications are not sent for them (I just noticed that while lurking
>>> on #164, I got e-mails for the main thread but not for Lennart's
>>> comments on commit 5f33680.)
>>
>> Yes you need to specify for each PR you are interested in that you
>> want to receive mail notifications for the PR... (I think it's the
>> subscribe button at the bottom)
>
> Sorry I should have explained myself better...

No I understood, but what I meant is that in addition of "watching", I
think you have to subscribe to a particular PR to receive mail
notifications for the comments on the commits, or maybe I'm wrong and
there is no way to get mail notifications for this, which is weird I
think...

>
> I "watch" systemd/systemd as a whole, so I get all notifications
> without having to ask for them individually...
>
> On #164, I *did* get an e-mail for @zonque's comment ("Also, you
> forgot to add the new files to Makefile.am and po/LINGUAS...") but I
> did *not* get e-mails for @poettering's comments on commit 5f33680
> ("Hmm, can you please change the commit msg to say this is the catalog
> translation? ...") and the replies on that thread (@s8321414 replied
> "@poettering How can I do this using git?" etc.)
>
> I think that's one more symptom of the fact that, for GitHub, the
> commit itself doesn't directly belong to the PR, and so does not
> belong to the project either...
>
> The e-mail notifications are not really a great big deal (still, they
> are annoying), but I think it's just one more sign that adding
> comments to the commits will end up causing trouble in the future...
>
> Cheers,
> Filipe
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [ANNOUNCE] Git development moved to github

[Ronny Chevalier]

On Thu, Jun 11, 2015 at 6:31 PM, Filipe Brandenburger
 wrote:
> On Wed, Jun 10, 2015 at 9:52 AM, Lennart Poettering
>  wrote:
>> On Wed, 10.06.15 08:25, Filipe Brandenburger (filbran...@google.com) wrote:
>>> On Wed, Jun 10, 2015 at 6:31 AM, Alban Crequy  wrote:
>>> > FWIW it only loses the comments if people comment on individual
>>> > commits instead of commenting on the "Files changed" tab of a PR. I
>>> > usually comment in this way on purpose instead of commenting on
>>> > commits, so that the history of comments are kept in the PR, even
>>> > after rebase (it might be folded if the chunk of the patch is not
>>> > there anymore, but the comment is still in the PR). If you really want
>>> > to comment on an individual commit (but I don't recommend it), you can
>>> > include the reference of the PR in your comment (#42), then github
>>> > will keep your comment attached to the PR.
>>>
>>> Ah that makes sense!
>>>
>>> Indeed as I explained I like to look at the individual commits, so
>>> that would explain why my comments would get lost as a new version is
>>> pushed...
>>>
>>> > I think it is fine as it is as long as people comment in the "Files
>>> > changed" tab.
>>>
>>> Lennart, do you think setting that rule is better than the "one PR per
>>> version of patchset"?
>>
>> No. We should review commits, not diffs. We also should review commit
>> msgs. (see other mail)
>
> Another downside of adding comments to the commits is that e-mail
> notifications are not sent for them (I just noticed that while lurking
> on #164, I got e-mails for the main thread but not for Lennart's
> comments on commit 5f33680.)

Yes you need to specify for each PR you are interested in that you
want to receive mail notifications for the PR... (I think it's the
subscribe button at the bottom)

>
> I think adding comments on the "Files changed" would work on cases such as:
>
> 1) The PR contains only a single commit, in which case the diff in
> "Files changed" will match the commit itself. (You still need to look
> at the commit description, but even if you do it from the "Commits"
> tab you can't really add any line comments directly to it anyways.)
>
> 2) If the commits change disjoint sets of files (you could check that
> first, and then review the code in the "Files changed" tab.)
>
> I think the exception is when a PR is both introducing new code and
> later changing it in a follow up commit but I guess that's not really
> too frequent (though I'm clearly guilty of it on #44.)
>
> Can we try to add comments to "Files changed"? Not asking not to look
> at the commits, yes looking at the commits is important! It's just
> that I think if we could have the e-mail notifications for the line
> comments, make sure they are kept in the same thread and be able to
> keep multiple versions of a patchset around in the same PR (instead of
> the wonky PR linking) I think that would be a huge win... We can
> always fall back to opening a new PR and closing the old one, but I'd
> prefer if that was the exception and not the rule...
>
> It really sounds like what you really want is Gerrit... I think
> gerrithub.io (which I haven't tried personally) might be what bridges
> these two worlds... Makes it easy for the submitters to send you
> commits, makes it easy for the reviewers to adopt the new code, tracks
> pending requests.
>
> Cheers!
> Filipe
> ___
> 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

Re: [systemd-devel] Static test coverage

[Ronny Chevalier]

On Thu, Jun 11, 2015 at 11:07 AM, Daniel Mack  wrote:
> Hi,
>
> Now that we're using Semaphore CI for building all pull requests and
> pushes to the master branch, I've set up a second VM instance to also
> use their service for static code analysis on a nightly base.
>
> We've had the systemd project registered with Coverity for a while, and
> so far, new builds were manually uploaded once in a while by Philippe de
> Swert (thanks for that!). This is now done automatically every night.
> The results can be seen here:
>
>   https://scan.coverity.com/projects/350/
>
> While at it, I also taught the build bot to use LLVM's scan-build, and
> sync the output with a new repository:
>
>   https://github.com/systemd/systemd-build-scan
>
> The patches are pushed to the 'gh-pages' branch, hence the HTML files
> are published here:
>
>   https://systemd.github.io/systemd-build-scan/
>
> Unfortunately, scan-build does not seem to understand the _cleanup_*
> variable annotations, so it currently reports lots of false-positive
> memory leaks.
>
>
> Hope this helps getting those collections of possible issues more
> exposure. If you want me to add more automated static testing, please
> let me know.

Hi,

Maybe it could be useful to add automatic code coverage also?

>
>
> Daniel
> ___
> 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

Re: [systemd-devel] Is SystemCallFilter working for you?

[Ronny Chevalier]

On Tue, Jun 9, 2015 at 1:00 PM, Martin Pitt  wrote:
> Hello all,
>
> I was about to (re-)enable seccomp support in our systemd packages,
> and to write an integration test for it. However, it seems that this
> currently does not seem to work at all.
>
> config.h has HAVE_SECCOMP==1, and systemctl --version shows +SECCOMP,
> kernel has CONFIG_SECCOMP=y, CONFIG_HAVE_ARCH_SECCOMP_FILTER=y, and
> CONFIG_SECCOMP_FILTER=y, and I'm running on x86-64, so that all seems
> fine.
>
> But if I have a unit like
>
> | [Unit]
> | Description=seccomp test
> |
> | [Service]
> | ExecStart=/bin/cat /etc/machine-id
> | SystemCallFilter=access
>
> (which really ought to fail) it just succeeds. Also, running
> ./test-execute as root fails in test_exec_systemcallfilter():
>
> | exec-systemcallfilter-failing.service
> |   UMask: 0022
> |   WorkingDirectory: /home/martin
> |   RootDirectory: /
> |   NonBlocking: no
> |   PrivateTmp: no
> |   PrivateNetwork: no
> |   PrivateDevices: no
> |   ProtectHome: no
> |   ProtectSystem: no
> |   IgnoreSIGPIPE: yes
> |   StandardInput: null
> |   StandardOutput: inherit
> |   StandardError: inherit
> | This should not be seen
> |   PID: 16439
> |   Start Timestamp: Tue 2015-06-09 12:56:51 CEST
> |   Exit Timestamp: Tue 2015-06-09 12:56:51 CEST
> |   Exit Code: exited
> |   Exit Status: 0
> | Assertion 'service->main_exec_status.status == status_expected' failed at 
> src/test/test-execute.c:57, function check(). Aborting.
>
> This is with libseccomp 2.2.1, I tested kernel 3.19 and 4.0. Is that
> working for anyone else? In particular, could you check if you have
> HAVE_SECCOMP and test-execute succeeds (as root) for you?
>

Hi,

It works for me. I tested on my machine with Linux 4.0.5 (archlinux)
and libseccomp 2.2.0 and test-execute passed.

But by looking at your output, there is something weird, you should
have a line like this for this test:
SystemCallFilter: exit exit_group rt_sigreturn ioperm execve

Just after StandardError: inherit and just before PID: 16439.

Because in exec_context_dump() it prints the SystemCallFilter line if
it isn't empty. Since test-execute launched the seccomp tests,
HAVE_SECCOMP is set, but it seems that syscall_filter == NULL in your
case?

> Thanks,
>
> Martin
> --
> Martin Pitt| http://www.piware.de
> Ubuntu Developer (www.ubuntu.com)  | Debian Developer  (www.debian.org)
> ___
> 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

Re: [systemd-devel] [ANNOUNCE] Git development moved to github

[Ronny Chevalier]

On Tue, Jun 2, 2015 at 1:06 PM, David Herrmann  wrote:
> Hi
>
> On Mon, Jun 1, 2015 at 8:12 PM, David Herrmann  wrote:
>> Hi
>>
>> As of today we've disabled git-push to fd.o. The official development
>> git repository is now at github [1]. The old repository will still be
>> back-synced, but we had to disable push-access to avoid getting
>> out-of-sync with github.
>>
>> In recent months, keeping up with the mailing-list has become more and
>> more cumbersome, with many of us missing mails or unable to keep up
>> with the traffic. To make sure all community requests and patches will
>> get handled in time, we're now trying out the github infrastructure.
>> We encourage everyone in the development community to switch over now,
>> even though the old fd.o infrastructure will still be maintained.
>> Distributions are free to wait until the next release announcement
>> before updating anything.
>>
>> If github does not work out, we will see what else we can try out. But
>> lets give it at least a try.
>
> Short update trying to answer all the questions:
>
> Our preferred way to send future patches is "the github way". This
> means sending pull-requests to the github repo. Furthermore, all
> feature patches should go through pull-requests and should get
> reviewed pre-commit. This applies to everyone. Exceptions are
> non-controversial patches like typos and obvious bug-fixes.
> The exact 'rules' on when to merge a pull-request need to be figured
> out once we get going. Ideas welcome! Until then, just apply common
> sense. Push-access can be granted to contributors like before.
> However, given that we want a pre-commit review model, it will not
> make much of a difference which person eventually merges the patches.
> We still highly appreciate the effort spent by many commiters to
> review and apply trivial changes up to critical bugfixes. This worked
> well and we want to keep this model, but avoid it for any feature
> development.
>
> The mailing-list will still be used for non-code related discussions,
> and I think (?) patches from new contributors on the ML might still be
> handled as before. But I guess this is mostly limited to trivial
> patches. Bigger patchsets should really go through github to avoid
> them getting lost on mailing-lists.
> Regarding the bug-tracker, I honestly don't know what the plan is. I
> think the plan is to stick to everything github provides us, to make
> sure we don't spread our tools across multiple hosts. However, I
> personally would prefer to discuss this in the community and see what
> issues come up. Anyone?
>
> The reason behind this move is that our current post-commit model
> places a high burden on anyone doing a release. It really does not
> scale and requires often more than a month to review everything. It is
> hard to distribute the workload as the infrastructure doesn't provide
> any help here. The result could be seen with the several hiccups
> during the 220 release.
> Furthermore, we want to avoid miscommunications on bigger feature
> patches that might not make it into upstream. With a pre-commit
> review, we hope to settle discussions before any code makes it into
> git, and save everyone the hassle of reverting patches which maybe
> other projects already relied on.
>
> Regarding the final github address: David Strauss kindly offered the
> 'systemd' user to us. Hence, we hope to move the repository to
> github.com/systemd/systemd this week. Sorry for the confusion, I hope
> we can settle all this this week.
>
> Finally, please speak up if there are any issues. I will do my best to
> address them. We want to tryout github to reduce the burden on the
> maintainers, but to also improve the interactions with outside
> contributions. Feedback is welcome! And to everyone not happy with
> that move, we'd appreciate if you could still give it a try. Lets see
> how it works out!

Will the systemd-commits ML still receive mails when commits are pushed?

It was easy to do post-review of the commits pushed on the repo with
this ML (or maybe there is a github feature that sends mail
notifications when there is commits pushed that I missed).

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

Re: [systemd-devel] [ANNOUNCE] Git development moved to github

[Ronny Chevalier]

On Mon, Jun 1, 2015 at 8:12 PM, David Herrmann  wrote:
> Hi
>
> As of today we've disabled git-push to fd.o. The official development
> git repository is now at github [1]. The old repository will still be
> back-synced, but we had to disable push-access to avoid getting
> out-of-sync with github.
>
> In recent months, keeping up with the mailing-list has become more and
> more cumbersome, with many of us missing mails or unable to keep up
> with the traffic. To make sure all community requests and patches will
> get handled in time, we're now trying out the github infrastructure.
> We encourage everyone in the development community to switch over now,
> even though the old fd.o infrastructure will still be maintained.
> Distributions are free to wait until the next release announcement
> before updating anything.
>
> If github does not work out, we will see what else we can try out. But
> lets give it at least a try.

About applying patches, do we still rebase on top of master, or do we
start to merge pull requests from the github interface?

>
> Thanks
> David
>
> [1] https://github.com/systemd-devs/systemd
> ___
> 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

Re: [systemd-devel] [PATCH 0/9] Allow \: escapes in nspawn command line

[Ronny Chevalier]

On Thu, May 28, 2015 at 2:02 PM, Richard Maw
 wrote:
> File paths may contain : characters, and the nspawn command-line argument
> parser had no way of being able to tell which were part of the paths, or used
> to separate paths.
>
> This is fixable by introducing an escaping mechanism.
>
> The --overlay option had a related problem, as it would still fail if the 
> paths
> contained : characters or , characters, as they are used as path separators 
> for
> the lowerdir union, and as the option separator.
>
> In this case overlayfs has an escaping mechanism, so we just needed to escape
> the paths correctly.
>
>
> This re-uses the existing escaping and parsing logic as much as possible.
> I couldn't find anything that was an exact fit, but unquote_first_word was the
> closest, so I extended the flags a bit and added a variant that accepted a set
> of separator characters, rather than always using whitespace.
>
> I've been running these patches on my system for a day, and tested that I 
> could
> nspawn a system using the changed arguments, but I couldn't get the test suite
> to run, so I may have missed something.

Hi,

Apart from the test suite there is also all the unit tests, it would
be great if you could add unit tests for unescape_first_word and
strv_split_escaped functions (src/test/test-unit.c and
src/test/test-strv.c).

Thanks

>
> Richard Maw (9):
>   util: Add unescape_first_word()
>   nspawn: Allow : characters in --tmpfs path
>   man: Document \: escapes in nspawn's --tmpfs option
>   strv: Add strv_split_escaped
>   nspawn: Allow : characters in nspawn --bind paths
>   man: Document \: escapes in nspawn's --bind option
>   nspawn: escape paths in overlay mount options
>   nspawn: Allow : characters in overlay paths
>   man: Document \: escapes in nspawn's --overlay option
>
>  man/systemd-nspawn.xml |  13 +-
>  src/nspawn/nspawn.c| 119 
> ++---
>  src/shared/strv.c  |   8 +++-
>  src/shared/strv.h  |   1 +
>  src/shared/util.c  |  35 ++-
>  src/shared/util.h  |   7 ++-
>  6 files changed, 142 insertions(+), 41 deletions(-)
>
> --
> 1.9.1
>
> ___
> 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

Re: [systemd-devel] [systemd-commits] 4 commits - src/core src/libsystemd src/systemctl src/udev

[Ronny Chevalier]

On Wed, Apr 15, 2015 at 3:51 AM, Zbigniew Jędrzejewski-Szmek
 wrote:
>  src/core/selinux-access.c |   31 -
>  src/libsystemd/sd-device/device-private.h |2 -
>  src/systemctl/systemctl.c |   11 +
>  src/udev/udev-builtin-usb_id.c|   36 
> +++---
>  4 files changed, 45 insertions(+), 35 deletions(-)
>
> New commits:
> commit 17af49f24812a6dd1b3f0732e33ea5dae9e32b29
> Author: Zbigniew Jędrzejewski-Szmek 
> Date:   Mon Feb 23 20:06:00 2015 -0500
>
> selinux: use different log priorites for log messages
>
> When selinux calls our callback with a log message, it specifies the
> type as AVC or INFO/WARNING/ERROR. The question is how to map this to
> audit types and/or log priorities. SELINUX_AVC maps to AUDIT_USER_AVC
> reasonably, but for the other messages we have no idea, hence we use
> AUDIT_USER_AVC for everything. When not using audit logging, we can
> map those selinux levels to LOG_INFO/WARNING/ERROR etc.
>
> Also update comment which was not valid anymore in light of journald
> sucking in audit logs, and was actually wrong from the beginning —
> libselinux uses the callback for everything, not just avcs.
>
> This stemmed out of https://bugzilla.redhat.com/show_bug.cgi?id=1195330,
> but does not solve it.
>
> diff --git a/src/core/selinux-access.c b/src/core/selinux-access.c
> index a8c9a4b..7058b78 100644
> --- a/src/core/selinux-access.c
> +++ b/src/core/selinux-access.c
> @@ -80,17 +80,33 @@ static int audit_callback(
>  return 0;
>  }
>
> +static int callback_type_to_priority(int type) {
> +switch(type) {
> +case SELINUX_ERROR:   return LOG_ERR;
> +case SELINUX_WARNING: return LOG_WARNING;
> +case SELINUX_INFO:return LOG_INFO;
> +case SELINUX_AVC:
> +default:  return LOG_NOTICE;
> +}
> +}
> +
>  /*
> -   Any time an access gets denied this callback will be called
> -   code copied from dbus. If audit is turned on the messages will go as
> -   user_avc's into the /var/log/audit/audit.log, otherwise they will be
> -   sent to syslog.
> +   libselinux uses this callback when access gets denied or other
> +   events happen. If audit is turned on, messages will be reported
> +   using audit netlink, otherwise they will be logged using the usual
> +   channels.
> +
> +   Code copied from dbus and modified.
>  */
>  _printf_(2, 3) static int log_callback(int type, const char *fmt, ...) {
>  va_list ap;
>
>  #ifdef HAVE_AUDIT
> -if (get_audit_fd() >= 0) {
> +int fd;
> +
> +fd = get_audit_fd();
> +
> +if (fd >= 0) {
>  _cleanup_free_ char *buf = NULL;
>  int r;
>
> @@ -99,14 +115,15 @@ _printf_(2, 3) static int log_callback(int type, const 
> char *fmt, ...) {
>  va_end(ap);
>
>  if (r >= 0) {
> -audit_log_user_avc_message(get_audit_fd(), 
> AUDIT_USER_AVC, buf, NULL, NULL, NULL, 0);
> +audit_log_user_avc_message(fd, AUDIT_USER_AVC, buf, 
> NULL, NULL, NULL, 0);
>  return 0;
>  }
>  }
>  #endif
>
>  va_start(ap, fmt);
> -log_internalv(LOG_AUTH | LOG_INFO, 0, __FILE__, __LINE__, 
> __FUNCTION__, fmt, ap);
> +log_internalv(LOG_AUTH | callback_type_to_priority(type),
> +  0, __FILE__, __LINE__, __FUNCTION__, fmt, ap);
>  va_end(ap);
>
>  return 0;
>
> commit 40acc203c043fd419f3c045dc6f116c3a28411d8
> Author: Zbigniew Jędrzejewski-Szmek 
> Date:   Tue Apr 14 20:47:20 2015 -0500
>
> systemctl: avoid bumping NOFILE rlimit unless needed
>
> We actually only use the journal when showing status. Move setrlimit call
> so it is only called for status.
>
> https://bugzilla.redhat.com/show_bug.cgi?id=1184712
>
> diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c
> index 75d709d..4e702fb 100644
> --- a/src/systemctl/systemctl.c
> +++ b/src/systemctl/systemctl.c
> @@ -4466,6 +4466,12 @@ static int show(sd_bus *bus, char **args) {
>  if (show_properties)
>  pager_open_if_enabled();
>
> +if (show_status)
> +/* Increase max number of open files to 16K if we can, we
> + * might needs this when browsing journal files, which might
> + * be split up into many files. */
> +setrlimit_closest(RLIMIT_NOFILE, &RLIMIT_MAKE_CONST(16384));
> +
>  /* If no argument is specified inspect the manager itself */
>
>  if (show_properties && strv_length(args) <= 1)
> @@ -7164,11 +7170,6 @@ found:
>  }
>  }
>
> -/* Increase max number of open files to 16K if we can, we
> - * might needs this when browsing journal files, which might
> - * be split up into many files. */
> - 

Re: [systemd-devel] [PATCH] po: update French translation

[Ronny Chevalier]

On Wed, Apr 8, 2015 at 12:40 AM, Sylvain Plantefève
 wrote:
> Add strings introduced by 5bdf22430e367799dfa66c724144b624c5479518
> ---
>  po/fr.po | 20 +---
>  1 file changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/po/fr.po b/po/fr.po
> index f26451f..69862fb 100644
> --- a/po/fr.po
> +++ b/po/fr.po
> @@ -7,7 +7,7 @@ msgid ""
>  msgstr ""
>  "Project-Id-Version: systemd\n"
>  "Report-Msgid-Bugs-To: \n"
> -"POT-Creation-Date: 2015-03-09 18:30+0100\n"
> +"POT-Creation-Date: 2015-04-07 20:05+0200\n"
>  "PO-Revision-Date: 2014-12-28 13:04+0100\n"
>  "Last-Translator: Sylvain Plantefève \n"
>  "Language-Team: French\n"
> @@ -438,6 +438,20 @@ msgstr ""
>  "Authentification requise pour verrouiller ou déverrouiller des sessions "
>  "actives."
>
> +#: ../src/login/org.freedesktop.login1.policy.in.h:53
> +msgid "Allow indication to the firmware to boot to setup interface"
> +msgstr ""
> +"Permet d'indiquer au micrologiciel de démarrer sur l'interface de "
> +"configuration"
> +
> +#: ../src/login/org.freedesktop.login1.policy.in.h:54
> +msgid ""
> +"Authentication is required to indicate to the firmware to boot to setup "
> +"interface."
> +msgstr ""
> +"Authentification requise pour indiquer au micrologiciel de démarrer sur "
> +"l'interface de configuration."
> +
>  #: ../src/machine/org.freedesktop.machine1.policy.in.h:1
>  msgid "Log into a local container"
>  msgstr "Connexion dans un conteneur local"
> @@ -512,13 +526,13 @@ msgstr ""
>  "Authentification requise pour activer ou désactiver la synchronisation de "
>  "l'heure avec le réseau."
>
> -#: ../src/fsckd/fsckd.c:186
> +#: ../src/fsckd/fsckd.c:297
>  msgid "Press Ctrl+C to cancel all filesystem checks in progress"
>  msgstr ""
>  "Appuyez sur Ctrl+C pour annuler toutes vérifications en cours du système de 
> "
>  "fichiers"
>
> -#: ../src/fsckd/fsckd.c:227
> +#: ../src/fsckd/fsckd.c:343
>  #, c-format
>  msgid "Checking in progress on %d disk (%3.1f%% complete)"
>  msgid_plural "Checking in progress on %d disks (%3.1f%% complete)"
> --

Applied.

Thanks

> 2.1.0
>
> ___
> 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

Re: [systemd-devel] [PATCH] po: update French translation

[Ronny Chevalier]

Hi Sylvain,

On Tue, Apr 7, 2015 at 8:26 PM, Sylvain Plantefève
 wrote:
> Add strings introduced by 5bdf22430e367799dfa66c724144b624c5479518
> ---
>  po/fr.po | 18 +++---
>  1 file changed, 15 insertions(+), 3 deletions(-)
>
> diff --git a/po/fr.po b/po/fr.po
> index f26451f..3111c49 100644
> --- a/po/fr.po
> +++ b/po/fr.po
> @@ -7,7 +7,7 @@ msgid ""
>  msgstr ""
>  "Project-Id-Version: systemd\n"
>  "Report-Msgid-Bugs-To: \n"
> -"POT-Creation-Date: 2015-03-09 18:30+0100\n"
> +"POT-Creation-Date: 2015-04-07 20:05+0200\n"
>  "PO-Revision-Date: 2014-12-28 13:04+0100\n"
>  "Last-Translator: Sylvain Plantefève \n"
>  "Language-Team: French\n"
> @@ -438,6 +438,18 @@ msgstr ""
>  "Authentification requise pour verrouiller ou déverrouiller des sessions "
>  "actives."
>
> +#: ../src/login/org.freedesktop.login1.policy.in.h:53
> +msgid "Allow indication to the firmware to boot to setup interface"
> +msgstr "Indiquer au micrologiciel de démarrer sur l'interface de 
> configuration"
> +

I think you meant:

"Permet d'indiquer au micrologiciel [...]"

Right?

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

Re: [systemd-devel] systemctl edit TODO item

[Ronny Chevalier]

2015-03-18 13:39 GMT+01:00 Daniele Nicolodi :
> On 18/03/15 10:09, Zbigniew Jędrzejewski-Szmek wrote:
>> On Wed, Mar 18, 2015 at 12:13:39PM +0100, Ronny Chevalier wrote:
>>> Hi,
>>>
>>> About the TODO item of systemctl edit:
>>> "Upon editor exit, lines with one # are removed, lines with two # are
>>> left with one #, etc."
>>>
>>> I don't really agree, because if you have some comments in your
>>> drop-in file, you will have to remember every time you edit it to
>>> prepend one # to each one of your comments if you don't want them to
>>> be removed.
>> The idea was to prepend all existing comments with an extra hash.
>> Then lines with a single hash can be used without conflicting with
>> existing comments and the old comments can be restored exactly to their
>> previous form after editing is done.
>
> Wouldn't it be easy to prefix the lines to remove with something else
> than a hash or anything else that can be valid content for the file that
> is being edited, or that generally is much less likely to be present in
> the original file?
>
> For example mercurial uses the "HG: " prefix for lines that are removed
> from the commit message template file.

Using # makes editors highlight the lines as comments without having
to deal with a new systemd specific format for comments.

With your suggestion, when editing a file we will not see the specific
lines prefixed as comments.

I think keeping the # as prefix is better here.

>
> Cheers,
> Daniele
>
> ___
> 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

Re: [systemd-devel] systemctl edit TODO item

[Ronny Chevalier]

2015-03-18 10:09 GMT+01:00 Zbigniew Jędrzejewski-Szmek :
> On Wed, Mar 18, 2015 at 12:13:39PM +0100, Ronny Chevalier wrote:
>> Hi,
>>
>> About the TODO item of systemctl edit:
>> "Upon editor exit, lines with one # are removed, lines with two # are
>> left with one #, etc."
>>
>> I don't really agree, because if you have some comments in your
>> drop-in file, you will have to remember every time you edit it to
>> prepend one # to each one of your comments if you don't want them to
>> be removed.
> The idea was to prepend all existing comments with an extra hash.
> Then lines with a single hash can be used without conflicting with
> existing comments and the old comments can be restored exactly to their
> previous form after editing is done.
>
> existing
> blah
> # comment
>
> during edit
> blah
> ## comment
> # edit-intstructions and whatnot
>
> after edit
> blah
> # comment
>

Ok, I understand, it makes more sense now.

Thanks!

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

[systemd-devel] systemctl edit TODO item

[Ronny Chevalier]

Hi,

About the TODO item of systemctl edit:
"Upon editor exit, lines with one # are removed, lines with two # are
left with one #, etc."

I don't really agree, because if you have some comments in your
drop-in file, you will have to remember every time you edit it to
prepend one # to each one of your comments if you don't want them to
be removed.

I think it would be better:
- to remove lines that starts with two # and keep the ones with one #,
- or to remove each line that starts with one # and keep those with two #.

And before the editor is launch, we add a comment explaining this.
This way we would not lose useful comments by mistake.

What do you think?

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

Re: [systemd-devel] [PATCH 6/6] refactor in_addr_to_string()

[Ronny Chevalier]

2015-03-11 16:13 GMT+01:00 Shawn Landden :
> ---

Hi,

>  src/resolve/resolved-dns-rr.c |  6 ++
>  src/shared/in-addr-util.c | 32 +++-
>  2 files changed, 13 insertions(+), 25 deletions(-)
>
> diff --git a/src/resolve/resolved-dns-rr.c b/src/resolve/resolved-dns-rr.c
> index 78d9e4a..a73ccd7 100644
> --- a/src/resolve/resolved-dns-rr.c
> +++ b/src/resolve/resolved-dns-rr.c
> @@ -527,13 +527,11 @@ int dns_resource_record_to_string(const 
> DnsResourceRecord *rr, char **ret) {
>  break;
>
>  case DNS_TYPE_A: {
> -_cleanup_free_ char *x = NULL;
> -
> -r = in_addr_to_string(AF_INET, (const union in_addr_union*) 
> &rr->a.in_addr, &x);
> +r = in_addr_to_string(AF_INET, (const union in_addr_union*) 
> &rr->a.in_addr, &t);
>  if (r < 0)
>  return r;
>
> -s = strjoin(k, " ", x, NULL);
> +s = strjoin(k, " ", t, NULL);
>  if (!s)
>  return -ENOMEM;
>  break;
> diff --git a/src/shared/in-addr-util.c b/src/shared/in-addr-util.c
> index b7532a6..f23b343 100644
> --- a/src/shared/in-addr-util.c
> +++ b/src/shared/in-addr-util.c
> @@ -22,6 +22,7 @@
>  #include 
>
>  #include "in-addr-util.h"
> +#include "socket-util.h"
>
>  int in_addr_is_null(int family, const union in_addr_union *u) {
>  assert(u);
> @@ -179,31 +180,20 @@ int in_addr_prefix_next(int family, union in_addr_union 
> *u, unsigned prefixlen)
>  }
>
>  int in_addr_to_string(int family, const union in_addr_union *u, char **ret) {
> -char *x;
> -size_t l;
> +union sockaddr_union sa;
>
> -assert(u);
> -assert(ret);
> +sa.sa.sa_family = family;
>
> -if (family == AF_INET)
> -l = INET_ADDRSTRLEN;
> -else if (family == AF_INET6)
> -l = INET6_ADDRSTRLEN;
> -else
> -return -EAFNOSUPPORT;
> +if (sa.sa.sa_family == AF_INET) {
> +sa.in.sin_addr = u->in;
>
> -x = new(char, l);
> -if (!x)
> -return -ENOMEM;
> +return sockaddr_pretty(&sa.sa, (socklen_t)sizeof(sa.in),  
> false, false, ret);
> +} else if (family == AF_INET6) {
> +sa.in6.sin6_addr = u->in6;
>
> -errno = 0;
> -if (!inet_ntop(family, u, x, l)) {
> -free(x);
> -return errno ? -errno : -EINVAL;
> -}
> -
> -*ret = x;
> -return 0;
> +return sockaddr_pretty(&sa.sa, (socklen_t)sizeof(sa.in6), 
> false, false, ret);
> +} else
> +return -EAFNOSUPPORT;
>  }
>

I don't see the benefit of refactoring this way.

sockaddr_pretty internally uses inet_ntop for AF_INET6 according to
the parameters you gave it, and for AF_INET it will gave us exactly
the same output without using inet_ntop. So in the end it would be the
same output, but it will add some overhead from sockaddr_pretty.

Could you elaborate?


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

Re: [systemd-devel] [PATCH] path-lookup: use secure_getenv()

[Ronny Chevalier]

2015-03-16 18:31 GMT+01:00 David Herrmann :
> Hi
>
> On Sun, Mar 15, 2015 at 12:36 PM, Ronny Chevalier
>  wrote:
>> 2015-03-15 3:27 GMT+01:00 Shawn Landden :
>>> All these except user_data_home_dir() are certainly vectors for
>>> arbitrary code execution. These should use secure_getenv()
>>> ---
>>
>> Hi,
>>
>> I don't see why secure_getenv() is appropriate here? These functions
>> are never used in the libraries systemd provides, they are mostly used
>> by systemctl and the dbus manager. Can you provide more details?
>
> You're right, but on the other hand secure_getenv() is usually
> sufficient (we don't use setuid() nor fs-caps). So secure_getenv()
> wouldn't hurt.

I think it would hurt in a SELinux environment. Because if the
AT_SECURE flag is set, secure_getenv will return NULL and tools like
systemctl will fail for certain tasks.

> But I don't really care..
>
> Thanks
> David
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] fix compiler warning

[Ronny Chevalier]

2015-03-14 17:54 GMT+01:00 Shawn Landden :
> On Sat, Mar 14, 2015 at 6:31 AM, Ronny Chevalier 
> wrote:
>>
>> 2015-03-11 4:42 GMT+01:00 Shawn Landden :
>> > warning: pointer/integer type mismatch in conditional expression
>> > ---
>> >  src/shared/socket-util.c | 4 ++--
>> >  1 file changed, 2 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/src/shared/socket-util.c b/src/shared/socket-util.c
>> > index 5820279..73e1177 100644
>> > --- a/src/shared/socket-util.c
>> > +++ b/src/shared/socket-util.c
>> > @@ -475,8 +475,8 @@ int sockaddr_port(const struct sockaddr *_sa) {
>> >  return -EAFNOSUPPORT;
>> >
>> >  return ntohs(sa->sa.sa_family == AF_INET6 ?
>> > -   sa->in6.sin6_port :
>> > -   sa->in.sin_port);
>> > +   (uint16_t)sa->in6.sin6_port :
>> > +   (uint16_t)sa->in.sin_port);
>> >  }
>> >
>>
>> Hi,
>>
>> I don't see any warning, and the man (man netinet_in.h) says that
>> sin_port and sin6_port are of type in_port_t which is equivalent to
>> uint16_t.
>
>
> in_port_t and in6_port_t. If the type returned by a ternary operator is not
> identical gcc-5 warns, even though they are both backed by uint16_t and thus
> there is no violation.

netinet/in.h does not provide in6_port_t according to the manpage. I
even check with the glibc and on master there is no mention of
in6_port_t.
Maybe you are using another libc?

>>
>>
>> Maybe I'm missing something?
>>
>> Ronny
>> ___
>> systemd-devel mailing list
>> systemd-devel@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/systemd-devel
>
>
>
>
> --
> Shawn Landden
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] path-lookup: use secure_getenv()

[Ronny Chevalier]

2015-03-15 3:27 GMT+01:00 Shawn Landden :
> All these except user_data_home_dir() are certainly vectors for
> arbitrary code execution. These should use secure_getenv()
> ---

Hi,

I don't see why secure_getenv() is appropriate here? These functions
are never used in the libraries systemd provides, they are mostly used
by systemctl and the dbus manager. Can you provide more details?

>  src/shared/path-lookup.c | 20 ++--
>  1 file changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/src/shared/path-lookup.c b/src/shared/path-lookup.c
> index fbf46cd..0fb86df 100644
> --- a/src/shared/path-lookup.c
> +++ b/src/shared/path-lookup.c
> @@ -33,7 +33,7 @@ int user_config_home(char **config_home) {
>  const char *e;
>  char *r;
>
> -e = getenv("XDG_CONFIG_HOME");
> +e = secure_getenv("XDG_CONFIG_HOME");
>  if (e) {
>  r = strappend(e, "/systemd/user");
>  if (!r)
> @@ -44,7 +44,7 @@ int user_config_home(char **config_home) {
>  } else {
>  const char *home;
>
> -home = getenv("HOME");
> +home = secure_getenv("HOME");
>  if (home) {
>  r = strappend(home, "/.config/systemd/user");
>  if (!r)
> @@ -62,7 +62,7 @@ int user_runtime_dir(char **runtime_dir) {
>  const char *e;
>  char *r;
>
> -e = getenv("XDG_RUNTIME_DIR");
> +e = secure_getenv("XDG_RUNTIME_DIR");
>  if (e) {
>  r = strappend(e, "/systemd/user");
>  if (!r)
> @@ -83,13 +83,13 @@ static int user_data_home_dir(char **dir, const char 
> *suffix) {
>   * suggests because we assume that that is a link to
>   * /etc/systemd/ anyway. */
>
> -e = getenv("XDG_DATA_HOME");
> +e = secure_getenv("XDG_DATA_HOME");
>  if (e)
>  res = strappend(e, suffix);
>  else {
>  const char *home;
>
> -home = getenv("HOME");
> +home = secure_getenv("HOME");
>  if (home)
>  res = strjoin(home, "/.local/share", suffix, NULL);
>  else
> @@ -146,7 +146,7 @@ static char** user_dirs(
>  if (user_runtime_dir(&runtime_dir) < 0)
>  return NULL;
>
> -e = getenv("XDG_CONFIG_DIRS");
> +e = secure_getenv("XDG_CONFIG_DIRS");
>  if (e) {
>  config_dirs = strv_split(e, ":");
>  if (!config_dirs)
> @@ -157,7 +157,7 @@ static char** user_dirs(
>  if (r < 0)
>  return NULL;
>
> -e = getenv("XDG_DATA_DIRS");
> +e = secure_getenv("XDG_DATA_DIRS");
>  if (e)
>  data_dirs = strv_split(e, ":");
>  else
> @@ -248,7 +248,7 @@ int lookup_paths_init(
>
>  /* First priority is whatever has been passed to us via env
>   * vars */
> -e = getenv("SYSTEMD_UNIT_PATH");
> +e = secure_getenv("SYSTEMD_UNIT_PATH");
>  if (e) {
>  if (endswith(e, ":")) {
>  e = strndupa(e, strlen(e) - 1);
> @@ -340,7 +340,7 @@ int lookup_paths_init(
>  #ifdef HAVE_SYSV_COMPAT
>  /* /etc/init.d/ compatibility does not matter to users */
>
> -e = getenv("SYSTEMD_SYSVINIT_PATH");
> +e = secure_getenv("SYSTEMD_SYSVINIT_PATH");
>  if (e) {
>  p->sysvinit_path = path_split_and_make_absolute(e);
>  if (!p->sysvinit_path)
> @@ -358,7 +358,7 @@ int lookup_paths_init(
>  return -ENOMEM;
>  }
>
> -e = getenv("SYSTEMD_SYSVRCND_PATH");
> +e = secure_getenv("SYSTEMD_SYSVRCND_PATH");
>  if (e) {
>  p->sysvrcnd_path = path_split_and_make_absolute(e);
>  if (!p->sysvrcnd_path)
> --
> 2.2.1.209.g41e5f3a
>
> ___
> 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

Re: [systemd-devel] [PATCH 1/6] fix strict aliasing issues in src/udev/udev-ctrl.c

[Ronny Chevalier]

2015-03-11 16:13 GMT+01:00 Shawn Landden :
> it is ironic that
> "The only purpose of this structure is to cast the structure pointer
> passed in addr in order to avoid compiler warnings.  See EXAMPLE below."
> from bind(2)
> ---
>  src/udev/udev-ctrl.c | 19 ++-
>  1 file changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/src/udev/udev-ctrl.c b/src/udev/udev-ctrl.c
> index c0c5981..61d3c5b 100644
> --- a/src/udev/udev-ctrl.c
> +++ b/src/udev/udev-ctrl.c
> @@ -18,6 +18,7 @@
>  #include 
>  #include 
>
> +#include "socket-util.h"
>  #include "udev.h"
>
>  /* wire protocol magic must match */
> @@ -55,7 +56,7 @@ struct udev_ctrl {
>  int refcount;
>  struct udev *udev;
>  int sock;
> -struct sockaddr_un saddr;
> +union sockaddr_union saddr;
>  socklen_t addrlen;
>  bool bound;
>  bool cleanup_socket;
> @@ -94,9 +95,9 @@ struct udev_ctrl *udev_ctrl_new_from_fd(struct udev *udev, 
> int fd) {
>  if (r < 0)
>  log_warning_errno(errno, "could not set SO_PASSCRED: %m");
>
> -uctrl->saddr.sun_family = AF_LOCAL;
> -strscpy(uctrl->saddr.sun_path, sizeof(uctrl->saddr.sun_path), 
> "/run/udev/control");
> -uctrl->addrlen = offsetof(struct sockaddr_un, sun_path) + 
> strlen(uctrl->saddr.sun_path);
> +uctrl->saddr.un.sun_family = AF_LOCAL;
> +strscpy(uctrl->saddr.un.sun_path, sizeof(uctrl->saddr.un.sun_path), 
> "/run/udev/control");
> +uctrl->addrlen = offsetof(struct sockaddr_un, sun_path) + 
> strlen(uctrl->saddr.un.sun_path);
>  return uctrl;
>  }
>
> @@ -108,10 +109,10 @@ int udev_ctrl_enable_receiving(struct udev_ctrl *uctrl) 
> {
>  int err;
>
>  if (!uctrl->bound) {
> -err = bind(uctrl->sock, (struct sockaddr *)&uctrl->saddr, 
> uctrl->addrlen);
> +err = bind(uctrl->sock, &uctrl->saddr.sa, uctrl->addrlen);
>  if (err < 0 && errno == EADDRINUSE) {
> -unlink(uctrl->saddr.sun_path);
> -err = bind(uctrl->sock, (struct sockaddr 
> *)&uctrl->saddr, uctrl->addrlen);
> +unlink(uctrl->saddr.un.sun_path);
> +err = bind(uctrl->sock, &uctrl->saddr.sa, 
> uctrl->addrlen);
>  }
>
>  if (err < 0) {
> @@ -160,7 +161,7 @@ int udev_ctrl_cleanup(struct udev_ctrl *uctrl) {
>  if (uctrl == NULL)
>  return 0;
>  if (uctrl->cleanup_socket)
> -unlink(uctrl->saddr.sun_path);
> +unlink(uctrl->saddr.un.sun_path);
>  return 0;
>  }
>
> @@ -249,7 +250,7 @@ static int ctrl_send(struct udev_ctrl *uctrl, enum 
> udev_ctrl_msg_type type, int
>  ctrl_msg_wire.intval = intval;
>
>  if (!uctrl->connected) {
> -if (connect(uctrl->sock, (struct sockaddr *)&uctrl->saddr, 
> uctrl->addrlen) < 0) {
> +if (connect(uctrl->sock, &uctrl->saddr.sa, uctrl->addrlen) < 
> 0) {
>  err = -errno;
>  goto out;
>  }
> --
> 2.2.1.209.g41e5f3a

Applied, thanks!

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

Re: [systemd-devel] [PATCH] fix strict aliasing issue in src/libsystemd-network/sd-dhcp-client.c

[Ronny Chevalier]

2015-03-11 4:45 GMT+01:00 Shawn Landden :
> ---
>  src/libsystemd-network/sd-dhcp-client.c | 11 +++
>  1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/src/libsystemd-network/sd-dhcp-client.c 
> b/src/libsystemd-network/sd-dhcp-client.c
> index 4224e01..a477ccc 100644
> --- a/src/libsystemd-network/sd-dhcp-client.c
> +++ b/src/libsystemd-network/sd-dhcp-client.c
> @@ -1469,7 +1469,7 @@ static int client_receive_message_udp(sd_event_source 
> *s, int fd,
>  _cleanup_free_ DHCPMessage *message = NULL;
>  int buflen = 0, len, r;
>  const struct ether_addr zero_mac = { { 0, 0, 0, 0, 0, 0 } };
> -const struct ether_addr *expected_chaddr = NULL;
> +bool expect_chaddr;
>  uint8_t expected_hlen = 0;
>
>  assert(s);
> @@ -1514,11 +1514,11 @@ static int client_receive_message_udp(sd_event_source 
> *s, int fd,
>
>  if (client->arp_type == ARPHRD_ETHER) {
>  expected_hlen = ETH_ALEN;
> -expected_chaddr = (const struct ether_addr *) 
> &client->mac_addr;
> +expect_chaddr = true;
>  } else {
> /* Non-ethernet links expect zero chaddr */
> expected_hlen = 0;
> -   expected_chaddr = &zero_mac;
> +   expect_chaddr = false;
>  }
>
>  if (message->hlen != expected_hlen) {
> @@ -1526,7 +1526,10 @@ static int client_receive_message_udp(sd_event_source 
> *s, int fd,
>  return 0;
>  }
>
> -if (memcmp(&message->chaddr[0], expected_chaddr, ETH_ALEN)) {
> +if (memcmp(&message->chaddr[0], expect_chaddr ?
> +  (void *)&client->mac_addr :
> +  (void *)&zero_mac,
> +ETH_ALEN)) {
>  log_dhcp_client(client, "received chaddr does not match "
>  "expected: ignoring");
>  return 0;
> --
> 2.2.1.209.g41e5f3a
>

Applied, thanks!

(I fixed the commit message to be more inline with the others and shorter)

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

Re: [systemd-devel] [PATCH] fix compiler warning

[Ronny Chevalier]

2015-03-11 4:42 GMT+01:00 Shawn Landden :
> warning: pointer/integer type mismatch in conditional expression
> ---
>  src/shared/socket-util.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/src/shared/socket-util.c b/src/shared/socket-util.c
> index 5820279..73e1177 100644
> --- a/src/shared/socket-util.c
> +++ b/src/shared/socket-util.c
> @@ -475,8 +475,8 @@ int sockaddr_port(const struct sockaddr *_sa) {
>  return -EAFNOSUPPORT;
>
>  return ntohs(sa->sa.sa_family == AF_INET6 ?
> -   sa->in6.sin6_port :
> -   sa->in.sin_port);
> +   (uint16_t)sa->in6.sin6_port :
> +   (uint16_t)sa->in.sin_port);
>  }
>

Hi,

I don't see any warning, and the man (man netinet_in.h) says that
sin_port and sin6_port are of type in_port_t which is equivalent to
uint16_t.

Maybe I'm missing something?

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

Re: [systemd-devel] [PATCH] add REMOTE_ADDR and REMOTE_PORT for Accept=yes

[Ronny Chevalier]

2015-03-10 12:41 GMT+01:00 Shawn Landden :
> ---
>  TODO |  2 -
>  man/systemd.socket.xml   |  7 ++-
>  src/core/service.c   | 41 -
>  src/libsystemd/sd-resolve/test-resolve.c |  2 +-
>  src/shared/socket-util.c | 76 
> +++-
>  src/shared/socket-util.h |  4 +-
>  src/timesync/timesyncd-server.h  |  2 +-
>  7 files changed, 106 insertions(+), 28 deletions(-)
>
> diff --git a/TODO b/TODO
> index ae32388..780084a 100644
> --- a/TODO
> +++ b/TODO
> @@ -164,8 +164,6 @@ Features:
>  * as soon as we have kdbus, and sender timestamps, revisit coalescing 
> multiple parallel daemon reloads:
>
> http://lists.freedesktop.org/archives/systemd-devel/2014-December/025862.html
>
> -* set $REMOTE_IP (or $REMOTE_ADDR/$REMOTE_PORT) environment variable when 
> doing per-connection socket activation. use format introduced by xinetd or 
> CGI for this
> -
>  * the install state probably shouldn't get confused by generated units, 
> think dbus1/kdbus compat!
>
>  * in systemctl list-unit-files: show the install value the presets would 
> suggest for a service in a third column
> diff --git a/man/systemd.socket.xml b/man/systemd.socket.xml
> index 3938345..6808179 100644
> --- a/man/systemd.socket.xml
> +++ b/man/systemd.socket.xml
> @@ -357,7 +357,12 @@
>  daemons designed for usage with
>  
> inetd8
>  to work unmodified with systemd socket
> -activation.
> +activation.
> +
> +For IPv4 and IPv6 connections the 
> REMOTE_ADDR
> +environment variable will contain the remote IP, and 
> REMOTE_PORT
> +will contain the remote port. This is the same as the format used by 
> CGI.
> +For SOCK_RAW the port is the IP protocol.
>
>
>
> diff --git a/src/core/service.c b/src/core/service.c
> index cc4ea19..bcfce96 100644
> --- a/src/core/service.c
> +++ b/src/core/service.c
> @@ -1095,7 +1095,7 @@ static int service_spawn(
>  if (r < 0)
>  goto fail;
>
> -our_env = new0(char*, 4);
> +our_env = new0(char*, 6);
>  if (!our_env) {
>  r = -ENOMEM;
>  goto fail;
> @@ -1119,6 +1119,45 @@ static int service_spawn(
>  goto fail;
>  }
>
> +if (UNIT_DEREF(s->accept_socket)) {
> +union sockaddr_union sa;
> +socklen_t salen = sizeof(sa);
> +
> +r = getpeername(s->socket_fd, &sa.sa, &salen);
> +if (r < 0) {
> +r = -errno;
> +goto fail;
> +}
> +
> +if (IN_SET(sa.sa.sa_family, AF_INET, AF_INET6)) {
> +_cleanup_free_ char *addr = NULL;
> +char *t;
> +int port;
> +
> +r = sockaddr_pretty(&sa.sa, salen, true, false, 
> &addr);
> +if (r < 0)
> +goto fail;
> +
> +t = strappend("REMOTE_ADDR=", addr);
> +if (!t) {
> +r = -ENOMEM;
> +goto fail;
> +}
> +our_env[n_env++] = t;
> +
> +port = sockaddr_port(&sa.sa);
> +if (port < 0) {
> +r = port;
> +goto fail;
> +}
> +
> +if (asprintf((our_env + n_env++), "REMOTE_PORT=%u", 
> port) < 0) {
> +r = -ENOMEM;
> +goto fail;
> +}
> +}
> +}
> +
>  final_env = strv_env_merge(2, UNIT(s)->manager->environment, 
> our_env, NULL);
>  if (!final_env) {
>  r = -ENOMEM;
> diff --git a/src/libsystemd/sd-resolve/test-resolve.c 
> b/src/libsystemd/sd-resolve/test-resolve.c
> index 3187ce9..354a407 100644
> --- a/src/libsystemd/sd-resolve/test-resolve.c
> +++ b/src/libsystemd/sd-resolve/test-resolve.c
> @@ -46,7 +46,7 @@ static int getaddrinfo_handler(sd_resolve_query *q, int 
> ret, const struct addrin
>  for (i = ai; i; i = i->ai_next) {
>  _cleanup_free_ char *addr = NULL;
>
> -assert_se(sockaddr_pretty(i->ai_addr, i->ai_addrlen, false, 
> &addr) == 0);
> +assert_se(sockaddr_pretty(i->ai_addr, i->ai_addrlen, false, 
> true, &addr) == 0);
>  puts(addr);
>  }
>
> diff --git a/src/shared/socket-util.c b/src/shared/socket-util.c
> index 74d90fa..0d87cb1 100644
> --- a/src/shared/socket-util.c
> +++ b/src/shared/socket-util.c
> @@ -297,7 +297,7 @@ int socket_address_print(const SocketAddress *a, char 
> **ret) {
>  return 0;
>   

Re: [systemd-devel] [PATCH 1/3] Allow systemd-tmpfiles to set the file/directory attributes

[Ronny Chevalier]

2015-03-10 0:17 GMT+01:00 Goffredo Baroncelli :
> On 2015-03-08 15:00, Ronny Chevalier wrote:
>> 2015-03-08 12:48 GMT+01:00 Goffredo Baroncelli :
>>> From: Goffredo Baroncelli 
>>>
>>
>> Hi,
>>
>>> Allow systemd-tmpfiles to set the file/directory attributes, like chattr(1)
>>> does. Two more commands are added: 'H' and 'h' to set the attributes,
>>> recursively and not.
>>>
>>> Signed-off-by: Goffredo Baroncelli 
>>
>> No Signed-off-by in systemd.
>>
>>> ---
>>>  src/tmpfiles/tmpfiles.c | 155 
>>> 
>>>  1 file changed, 155 insertions(+)
>>>
>>> diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c
>>> index c948d4d..cb77047 100644
>>> --- a/src/tmpfiles/tmpfiles.c
>>> +++ b/src/tmpfiles/tmpfiles.c
>>> @@ -40,6 +40,7 @@
>>>  #include 
>>>  #include 
>>>  #include 
>>> +#include 
>>>
>>>  #include "log.h"
>>>  #include "util.h"
>>> @@ -90,6 +91,8 @@ typedef enum ItemType {
>>>  ADJUST_MODE = 'm', /* legacy, 'z' is identical to this */
>>>  RELABEL_PATH = 'z',
>>>  RECURSIVE_RELABEL_PATH = 'Z',
>>> +SET_ATTRIB = 'h',
>>> +RECURSIVE_SET_ATTRIB = 'H',
>>>  } ItemType;
>>>
>>>  typedef struct Item {
>>> @@ -108,12 +111,15 @@ typedef struct Item {
>>>  usec_t age;
>>>
>>>  dev_t major_minor;
>>> +int attrib_value;
>>> +int attrib_mask;
>>>
>>>  bool uid_set:1;
>>>  bool gid_set:1;
>>>  bool mode_set:1;
>>>  bool age_set:1;
>>>  bool mask_perms:1;
>>> +bool attrib_set:1;
>>>
>>>  bool keep_first_level:1;
>>>
>>> @@ -762,6 +768,130 @@ static int path_set_acls(Item *item, const char 
>>> *path) {
>>>  return 0;
>>>  }
>>>
>>> +static int get_attrib_from_arg(Item *item) {
>>> +struct attributes_list_t { int value; char ch; } ;
>>> +static struct attributes_list_t attributes[] = {
>>> +{ FS_NOATIME_FL, 'A' },   /* do not update atime */
>>> +{ FS_SYNC_FL, 'S' },  /* Synchronous updates */
>>> +{ FS_DIRSYNC_FL, 'D' },   /* dirsync behaviour 
>>> (directories only) */
>>> +{ FS_APPEND_FL, 'a' },/* writes to file may 
>>> only append */
>>> +{ FS_COMPR_FL, 'c' }, /* Compress file */
>>> +{ FS_NODUMP_FL, 'd' },/* do not dump file */
>>> +{ FS_EXTENT_FL, 'e'}, /* Top of directory 
>>> hierarchies*/
>>> +{ FS_IMMUTABLE_FL, 'i' }, /* Immutable file */
>>> +{ FS_JOURNAL_DATA_FL, 'j' }, /* Reserved for ext3 
>>> */
>>> +{ FS_SECRM_FL, 's' }, /* Secure deletion */
>>> +{ FS_UNRM_FL, 'u' },  /* Undelete */
>>> +{ FS_NOTAIL_FL, 't' },/* file tail should not 
>>> be merged */
>>> +{ FS_TOPDIR_FL, 'T' },/* Top of directory 
>>> hierarchies*/
>>> +{ FS_NOCOW_FL, 'C' }, /* Do not cow file */
>>> +{ 0, 0 }
>>> +};
>>> +char *p = item->argument;
>>> +enum { MODE_ADD, MODE_DEL, MODE_SET } mode = MODE_ADD;
>>> +int value=0, mask=0;
>>> +
>>> +if (!p) {
>>> +log_error("\"%s\": setting ATTR need an argument", 
>>> item->path);
>>> +return -1;
>>
>> Please use errno style error code. In this case -EINVAL is appropriate.
> ok
>>
>>> +}
>>> +
>>> +if (*p == '+') {
>>> +mode = MODE_ADD;
>>> +p++;
>>> +} else if (*p == '-') {
>>> +mode = MODE_DEL;
>>> +p++;
>>> +} else  if (*p == '=&

Re: [systemd-devel] [PATCH] add REMOTE_ADDR and REMOTE_PORT for Accept=yes

[Ronny Chevalier]

2015-03-09 23:09 GMT+01:00 Shawn Landden :
> On Mon, Mar 9, 2015 at 1:18 PM, Lennart Poettering 
> wrote:
>>
>> On Mon, 09.03.15 13:09, Shawn Landden (sh...@churchofgit.com) wrote:
>>
>> > +if (UNIT_DEREF(s->accept_socket)) {
>> > +union sockaddr_union sa;
>> > +socklen_t salen = sizeof(sa);
>> > +
>> > +r = getpeername(s->socket_fd, &sa.sa, &salen);
>> > +if (r < 0) {
>> > +r = -errno;
>> > +goto fail;
>> > +}
>> > +
>> > +if (sa.sa.sa_family == AF_INET ||
>> > +sa.sa.sa_family == AF_INET6) {
>> > +_cleanup_free_ char *addr = NULL;
>> > +uint16_t port = (uint16_t)sockaddr_port(&sa);
>>
>> We try to avoid invoking functions in the same lines as we declare
>> variables.
>>
>> Also, even though this cannot fail in this case, I think it would be
>> nicer, to make port an int, and check for < 0 and return an error in
>> that case...
>>
>> > +
>> > +r = sockaddr_pretty(&sa.sa, salen, true, false,
>> > &addr);
>> > +if (r < 0)
>> > +goto fail;
>> > +
>> > +if (!(our_env[n_env++] =
>> > strappend("REMOTE_ADDR=", addr))) {
>>
>> In newer code we try to avoid making assignments and doing if checks
>> in the same line.
>>
> Taking this out would be pretty gruesome because of use of the
> post-increment operator.

Maybe by using a temporary variable like this:

char *env_str;

[...]

env_str = strappend("REMOTE_ADDR", addr);
if (!env_str) {
r = -ENOMEM;
goto fail;
}
our_env[n_env++] = env_str;

And the same with the other one ?

>>
>> >
>> > -int sockaddr_pretty(const struct sockaddr *_sa, socklen_t salen, bool
>> > translate_ipv6, char **ret) {
>> > +int sockaddr_port(const union sockaddr_union *sa) {
>> > +assert_return(sa->sa.sa_family == AF_INET6 ||
>> > +  sa->sa.sa_family == AF_INET,
>> > +  -ENOTSUP);
>>
>> assert_return() is a macro to use only for cases where there's a clear
>> programming error of the caller. But I am pretty sure that this is not
>> the case here. If it gets called on an non-AF_INET/AF_INET6 socket
>> then this should fail without logging.
>>
>> Also I think an error code of EAFNOSUPPORT would be more appropriate.
>>
>> Hmm, and I think this should probably take an actual "struct
>> sockaddr", rather than a union sockaddr_union...
>>
>> > @@ -475,42 +485,40 @@ int sockaddr_pretty(const struct sockaddr *_sa,
>> > socklen_t salen, bool translate_
>> >
>> >  switch (sa->sa.sa_family) {
>> >
>> > -case AF_INET: {
>> > -uint32_t a;
>> > -
>> > -a = ntohl(sa->in.sin_addr.s_addr);
>> > -
>> > -if (asprintf(&p,
>> > - "%u.%u.%u.%u:%u",
>> > - a >> 24, (a >> 16) & 0xFF, (a >> 8) &
>> > 0xFF, a & 0xFF,
>> > - ntohs(sa->in.sin_port)) < 0)
>> > -return -ENOMEM;
>> > -
>> > -break;
>> > -}
>> > -
>> > +case AF_INET:
>> >  case AF_INET6: {
>> > -static const unsigned char ipv4_prefix[] = {
>> > -0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0xFF, 0xFF
>> > -};
>> > -
>> > -if (translate_ipv6 && memcmp(&sa->in6.sin6_addr,
>> > ipv4_prefix, sizeof(ipv4_prefix)) == 0) {
>> > -const uint8_t *a =
>> > sa->in6.sin6_addr.s6_addr+12;
>> > +char a[MAX(INET6_ADDRSTRLEN, INET_ADDRSTRLEN)];
>> > +const char *addr;
>> > +bool ipv4_mapped = false;
>> > +
>> > +if (inet_ntop(sa->sa.sa_family,
>> > +  /* this field of the API is kinda
>> > braindead,
>> > +   * should take head of struct so it can
>> > be passed the union...*/
>> > +  sa->sa.sa_family == AF_INET6 ?
>> > +&sa->in6.sin6_addr :
>> > +&sa->in.sin_addr,
>> > +  a, sizeof(a)) == NULL)
>> > +  return -ENOMEM;
>> > +
>> > +/* glibc inet_ntop() presents v4-mapped addresses in
>> > :::a.b.c.d form */
>> > +if (translate_ipv6 && sa->sa.sa_family == AF_INET6 &&
>> > strchr(a, '.')) {
>> > +ipv4_mapped = true;
>> > +addr = strempty(startswith(a, ":::"));
>>
>> I think it would be a lot nicer to check the raw IP prefix as before,
>> and only then format things, then first formatting and then looking at
>> the string again.
>>
>> We shouldn't bind us too closely to the precise formatting. I mean, if
>> one day libc decides to format the thing as ipv6 again, or

Re: [systemd-devel] [PATCH] po: update French translation

[Ronny Chevalier]

2015-03-09 18:36 GMT+01:00 Sylvain Plantefève :
> Add strings for importd, following 587fec427c80b6c34dcf1d7570f891fcb652a7c5

Hi,

Patch applied, thanks!

Ronny


> ---
>  po/fr.po | 18 ++
>  1 file changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/po/fr.po b/po/fr.po
> index b3b17ce..f26451f 100644
> --- a/po/fr.po
> +++ b/po/fr.po
> @@ -7,7 +7,7 @@ msgid ""
>  msgstr ""
>  "Project-Id-Version: systemd\n"
>  "Report-Msgid-Bugs-To: \n"
> -"POT-Creation-Date: 2015-03-06 21:16+0100\n"
> +"POT-Creation-Date: 2015-03-09 18:30+0100\n"
>  "PO-Revision-Date: 2014-12-28 13:04+0100\n"
>  "Last-Translator: Sylvain Plantefève \n"
>  "Language-Team: French\n"
> @@ -103,14 +103,24 @@ msgstr "Importer une image de machine virtuelle (VM) ou 
> de conteneur"
>  #: ../src/import/org.freedesktop.import1.policy.in.h:2
>  msgid "Authentication is required to import a VM or container image"
>  msgstr ""
> -"Authentification requise pour importer une image de machine virtuelle "
> -"(VM) ou de conteneur."
> +"Authentification requise pour importer une image de machine virtuelle (VM) "
> +"ou de conteneur."
>
>  #: ../src/import/org.freedesktop.import1.policy.in.h:3
> +msgid "Export a VM or container image"
> +msgstr "Exporter une image de machine virtuelle (VM) ou de conteneur"
> +
> +#: ../src/import/org.freedesktop.import1.policy.in.h:4
> +msgid "Authentication is required to export a VM or container image"
> +msgstr ""
> +"Authentification requise pour exporter une image de machine virtuelle (VM) "
> +"ou de conteneur."
> +
> +#: ../src/import/org.freedesktop.import1.policy.in.h:5
>  msgid "Download a VM or container image"
>  msgstr "Télécharger une image de machine virtuelle (VM) ou de conteneur"
>
> -#: ../src/import/org.freedesktop.import1.policy.in.h:4
> +#: ../src/import/org.freedesktop.import1.policy.in.h:6
>  msgid "Authentication is required to download a VM or container image"
>  msgstr ""
>  "Authentification requise pour télécharger une image de machine virtuelle "
> --
> 2.1.4
>
> ___
> 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

Re: [systemd-devel] [PATCH] tmpfiles: port to unquote_many_words()

[Ronny Chevalier]

2015-03-09 18:51 GMT+01:00 daurnimator :

Hi,

> ---
>  TODO|  2 --
>  man/tmpfiles.d.xml  |  2 ++
>  src/tmpfiles/tmpfiles.c | 24 ++--
>  3 files changed, 12 insertions(+), 16 deletions(-)
>
> diff --git a/TODO b/TODO
> index 60efaaf..4d5e2b6 100644
> --- a/TODO
> +++ b/TODO
> @@ -226,8 +226,6 @@ Features:
>
>  * exponential backoff in timesyncd and resolved when we cannot reach a server
>
> -* tmpfiles: port to unquote_many_words(), similar to sysusers
> -
>  * unquote_many_words() should probably be used by a lot of code that
>currently uses FOREACH_WORD and friends. For example, most conf
>parsing callbacks should use it.
> diff --git a/man/tmpfiles.d.xml b/man/tmpfiles.d.xml
> index 4bd0fcf..c1c4a44 100644
> --- a/man/tmpfiles.d.xml
> +++ b/man/tmpfiles.d.xml
> @@ -118,6 +118,8 @@
>  d/run/user   0755 root root 10d -
>  L/tmp/foobar ----   /dev/null
>
> +Fields can contain C-style escapes
> +
>  
>Type
>
> diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c
> index 652fe5f..e6ee5b8 100644
> --- a/src/tmpfiles/tmpfiles.c
> +++ b/src/tmpfiles/tmpfiles.c
> @@ -1502,27 +1502,30 @@ static int parse_line(const char *fname, unsigned 
> line, const char *buffer) {
>  {}
>  };
>
> -_cleanup_free_ char *action = NULL, *mode = NULL, *user = NULL, 
> *group = NULL, *age = NULL, *path = NULL;
> +_cleanup_free_ char *action = NULL, *mode = NULL, *user = NULL, 
> *group = NULL, *age = NULL, *path = NULL, *argument = NULL;
>  _cleanup_(item_free_contents) Item i = {};
>  ItemArray *existing;
>  Hashmap *h;
> -int r, c = -1, pos;
> +int r, pos;
>  bool force = false, boot = false;
>
>  assert(fname);
>  assert(line >= 1);
>  assert(buffer);
>
> -r = sscanf(buffer,
> -   "%ms %ms %ms %ms %ms %ms %n",
> +r = unquote_many_words(&buffer,
> &action,
> &path,
> &mode,
> &user,
> &group,
> &age,
> -   &c);
> -if (r < 2) {
> +   &argument,
> +   NULL);
> +if (r < 0) {
> +log_error_errno(r, "[%s:%u] Failed to parse line: %m", 
> fname, line);
> +return r;

You can directly use:
return log_error_errno(r, ...);

> +} else if (r < 2) {
>  log_error("[%s:%u] Syntax error.", fname, line);
>  return -EIO;
>  }
> @@ -1559,14 +1562,7 @@ static int parse_line(const char *fname, unsigned 
> line, const char *buffer) {
>  return r;
>  }
>
> -if (c >= 0)  {
> -c += strspn(buffer+c, WHITESPACE);
> -if (buffer[c] != 0 && (buffer[c] != '-' || buffer[c+1] != 
> 0)) {
> -i.argument = unquote(buffer+c, "\"");
> -if (!i.argument)
> -return log_oom();
> -}
> -}
> +i.argument = argument;
>
>  switch (i.type) {
>
> --
> 2.3.1
>
> ___
> 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

Re: [systemd-devel] [PATCH 2/3] Update the man page of tmpfiles.d(5), to document the new h/H command.

[Ronny Chevalier]

2015-03-08 12:48 GMT+01:00 Goffredo Baroncelli :
> From: Goffredo Baroncelli 
>
> Update the man page of tmpfiles.d(5), to document the new h/H command.
>
> Signed-off-by: Goffredo Baroncelli 

No Signed-off-by.

Also, why not merge the 3 commits in one ? I don't see why separating
the man page update of the new feature in another commit is useful ?

> ---
>  man/tmpfiles.d.xml | 32 
>  1 file changed, 32 insertions(+)
>
> diff --git a/man/tmpfiles.d.xml b/man/tmpfiles.d.xml
> index 8815bf9..f9074dd 100644
> --- a/man/tmpfiles.d.xml
> +++ b/man/tmpfiles.d.xml
> @@ -303,6 +303,37 @@
>  
>
>  
> +  h
> +  Set file/directory attributes. Lines of this type
> +  accept shell-style globs in place of normal path names.
> +
> +  The format of agrument field is 
> [+-=][aAcCdDeijsStTu]

the argument*

> +  
> +
> +  The prefix + causes the
> +  attribute(s) to be added; - causes the
> +  attribute(s) to be removed; =
> +  causes the attributes to set exactly as the following 
> letters.
> +  The letters 'aAcCdDeijsStTu' select the new
> +  attributes for the files, see
> +  chattr
> +  1 for further information.
> +  
> +  Passing only = as argument,
> +  reset all the file attributes.
> +
> +  
> +
> +
> +
> +  H
> +  Recursively set file/directory attributes Lines

A . is missing before Lines.

> +  of this type accept shell-style globs in place of normal
> +  path names.
> +  
> +
> +
> +
>a
>a+
>Set POSIX ACLs (access control lists). If
> @@ -529,6 +560,7 @@
> project='man-pages'>setfattr1,
> project='man-pages'>setfacl1,
> project='man-pages'>getfacl1
> +   project='man-pages'>chattr1
>  
>
>
> --
> 2.1.4
>
> ___
> 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

Re: [systemd-devel] [PATCH 1/3] Allow systemd-tmpfiles to set the file/directory attributes

[Ronny Chevalier]

2015-03-08 12:48 GMT+01:00 Goffredo Baroncelli :
> From: Goffredo Baroncelli 
>

Hi,

> Allow systemd-tmpfiles to set the file/directory attributes, like chattr(1)
> does. Two more commands are added: 'H' and 'h' to set the attributes,
> recursively and not.
>
> Signed-off-by: Goffredo Baroncelli 

No Signed-off-by in systemd.

> ---
>  src/tmpfiles/tmpfiles.c | 155 
> 
>  1 file changed, 155 insertions(+)
>
> diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c
> index c948d4d..cb77047 100644
> --- a/src/tmpfiles/tmpfiles.c
> +++ b/src/tmpfiles/tmpfiles.c
> @@ -40,6 +40,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>
>  #include "log.h"
>  #include "util.h"
> @@ -90,6 +91,8 @@ typedef enum ItemType {
>  ADJUST_MODE = 'm', /* legacy, 'z' is identical to this */
>  RELABEL_PATH = 'z',
>  RECURSIVE_RELABEL_PATH = 'Z',
> +SET_ATTRIB = 'h',
> +RECURSIVE_SET_ATTRIB = 'H',
>  } ItemType;
>
>  typedef struct Item {
> @@ -108,12 +111,15 @@ typedef struct Item {
>  usec_t age;
>
>  dev_t major_minor;
> +int attrib_value;
> +int attrib_mask;
>
>  bool uid_set:1;
>  bool gid_set:1;
>  bool mode_set:1;
>  bool age_set:1;
>  bool mask_perms:1;
> +bool attrib_set:1;
>
>  bool keep_first_level:1;
>
> @@ -762,6 +768,130 @@ static int path_set_acls(Item *item, const char *path) {
>  return 0;
>  }
>
> +static int get_attrib_from_arg(Item *item) {
> +struct attributes_list_t { int value; char ch; } ;
> +static struct attributes_list_t attributes[] = {
> +{ FS_NOATIME_FL, 'A' },   /* do not update atime */
> +{ FS_SYNC_FL, 'S' },  /* Synchronous updates */
> +{ FS_DIRSYNC_FL, 'D' },   /* dirsync behaviour 
> (directories only) */
> +{ FS_APPEND_FL, 'a' },/* writes to file may only 
> append */
> +{ FS_COMPR_FL, 'c' }, /* Compress file */
> +{ FS_NODUMP_FL, 'd' },/* do not dump file */
> +{ FS_EXTENT_FL, 'e'}, /* Top of directory 
> hierarchies*/
> +{ FS_IMMUTABLE_FL, 'i' }, /* Immutable file */
> +{ FS_JOURNAL_DATA_FL, 'j' }, /* Reserved for ext3 */
> +{ FS_SECRM_FL, 's' }, /* Secure deletion */
> +{ FS_UNRM_FL, 'u' },  /* Undelete */
> +{ FS_NOTAIL_FL, 't' },/* file tail should not be 
> merged */
> +{ FS_TOPDIR_FL, 'T' },/* Top of directory 
> hierarchies*/
> +{ FS_NOCOW_FL, 'C' }, /* Do not cow file */
> +{ 0, 0 }
> +};
> +char *p = item->argument;
> +enum { MODE_ADD, MODE_DEL, MODE_SET } mode = MODE_ADD;
> +int value=0, mask=0;
> +
> +if (!p) {
> +log_error("\"%s\": setting ATTR need an argument", 
> item->path);
> +return -1;

Please use errno style error code. In this case -EINVAL is appropriate.

> +}
> +
> +if (*p == '+') {
> +mode = MODE_ADD;
> +p++;
> +} else if (*p == '-') {
> +mode = MODE_DEL;
> +p++;
> +} else  if (*p == '=') {
> +mode = MODE_SET;
> +p++;
> +}
> +
> +if (!*p && mode != MODE_SET) {
> +log_error("\"%s\": setting ATTR: argument is empty", 
> item->path);
> +return -4;

Same here.

> +}
> +for ( ; *p ; p++ ) {
> +int i;
> +for ( i = 0; attributes[i].ch && attributes[i].ch != *p 
> ;i++);
> +if (!attributes[i].ch) {
> +log_error("\"%s\": setting ATTR: unknown attr '%c'",
> +item->path, *p);
> +return -2;

Same here.

> +}
> +if (mode == MODE_ADD || mode == MODE_SET)
> +value |= attributes[i].value;
> +else
> +value &= ~attributes[i].value;
> +mask |= attributes[i].value;
> +}
> +
> +if (mode == MODE_SET) {
> +int i;
> +for ( i = 0; attributes[i].ch;i++)
> +mask |= attributes[i].value;
> +}
> +
> +assert(mask);
> +
> +item->attrib_mask |= mask;
> +item->attrib_value &= ~mask;
> +item->attrib_value |= value;
> +item->attrib_set = true;
> +
> +

Useless newline.

> +return 0;
> +
> +}
> +
> +static int path_set_attrib(Item *item, const char *path) {
> +int fd, r, f;
> +struct stat st;
> +
> +/* do nothing */
> +if (item->attrib_mask == 0 || !i

Re: [systemd-devel] [PATCH] po: update French translation

[Ronny Chevalier]

Le 19 février 2015 23:31, Sylvain Plantefève
 a écrit :
> ---
>  po/fr.po | 76 
> +---
>  1 file changed, 68 insertions(+), 8 deletions(-)
>
> diff --git a/po/fr.po b/po/fr.po
> index 8e44e0c..58a0b85 100644
> --- a/po/fr.po
> +++ b/po/fr.po
> @@ -7,7 +7,7 @@ msgid ""
>  msgstr ""
>  "Project-Id-Version: systemd\n"
>  "Report-Msgid-Bugs-To: \n"
> -"POT-Creation-Date: 2015-01-22 16:03+0100\n"
> +"POT-Creation-Date: 2015-02-18 19:48+0100\n"
>  "PO-Revision-Date: 2014-12-28 13:04+0100\n"
>  "Last-Translator: Sylvain Plantefève \n"
>  "Language-Team: French\n"
> @@ -27,11 +27,11 @@ msgid ""
>  msgstr "Authentification requise pour renvoyer la phrase secrète au système."
>
>  #: ../src/core/org.freedesktop.systemd1.policy.in.in.h:3
> -msgid "Manage system services or units"
> +msgid "Manage system services or other units"
>  msgstr "Gérer les services système ou les unités"
>
>  #: ../src/core/org.freedesktop.systemd1.policy.in.in.h:4
> -msgid "Authentication is required to manage system services or units."
> +msgid "Authentication is required to manage system services or other units."
>  msgstr ""
>  "Authentification requise pour gérer les services système ou les unités."
>
> @@ -46,10 +46,24 @@ msgstr ""
>  "unités."
>
>  #: ../src/core/org.freedesktop.systemd1.policy.in.in.h:7
> +msgid "Set or unset system and service manager environment variables"
> +msgstr ""
> +"Définir ou supprimer des variables d'environnement du système ou du "
> +"gestionnaire de services"
> +
> +#: ../src/core/org.freedesktop.systemd1.policy.in.in.h:8
> +msgid ""
> +"Authentication is required to set or unset system and service manager "
> +"environment variables."
> +msgstr ""
> +"Authentification requise pour définir ou supprimer des variables "
> +"d'environnement du système ou du gestionnaire de services."
> +
> +#: ../src/core/org.freedesktop.systemd1.policy.in.in.h:9
>  msgid "Reload the systemd state"
>  msgstr "Recharger l'état de systemd"
>
> -#: ../src/core/org.freedesktop.systemd1.policy.in.in.h:8
> +#: ../src/core/org.freedesktop.systemd1.policy.in.in.h:10
>  msgid "Authentication is required to reload the systemd state."
>  msgstr "Authentification requise pour recharger l'état de systemd"
>
> @@ -89,8 +103,8 @@ msgstr "Télécharger une image de machine virtuelle (VM) ou 
> de conteneur"
>  #: ../src/import/org.freedesktop.import1.policy.in.h:2
>  msgid "Authentication is required to download a VM or container image"
>  msgstr ""
> -"Authentification requise pour télécharger une image de "
> -"machine virtuelle (VM) ou de conteneur."
> +"Authentification requise pour télécharger une image de machine virtuelle "
> +"(VM) ou de conteneur."
>
>  #: ../src/locale/org.freedesktop.locale1.policy.in.h:1
>  msgid "Set system locale"
> @@ -383,15 +397,59 @@ msgstr ""
>  "Authentification requise pour mettre le système en hibernation alors qu'une 
> "
>  "application a demandé de l'empêcher."
>
> +#: ../src/login/org.freedesktop.login1.policy.in.h:49
> +msgid "Manager active sessions, users and seats"
> +msgstr "Gérer les sessions actives, les utilisateurs et les postes (seats)"
> +
> +#: ../src/login/org.freedesktop.login1.policy.in.h:50
> +msgid ""
> +"Authentication is required for managing active sessions, users and seats."
> +msgstr ""
> +"Authentification requise pour gérer les sessions actives, les utilisateurs "
> +"et les postes (seats)."
> +
> +#: ../src/login/org.freedesktop.login1.policy.in.h:51
> +msgid "Lock or unlock active sessions"
> +msgstr "Verrouiller ou déverrouiller des sessions actives"
> +
> +#: ../src/login/org.freedesktop.login1.policy.in.h:52
> +msgid "Authentication is required for locking or unlocking active sessions."
> +msgstr ""
> +"Authentification requise pour verrouiller ou déverrouiller des sessions "
> +"actives."
> +
>  #: ../src/machine/org.freedesktop.machine1.policy.in.h:1
>  msgid "Log into a local container"
>  msgstr "Connexion dans un conteneur local"
>
>  #: ../src/machine/org.freedesktop.machine1.policy.in.h:2
> -msgid "Authentication is required to log into a local container"
> +msgid "Authentication is required to log into a local container."
>  msgstr ""
>  "Authentification requise pour permettre la connexion dans un conteneur 
> local."
>
> +#: ../src/machine/org.freedesktop.machine1.policy.in.h:3
> +msgid "Manage local virtual machines and containers"
> +msgstr "Gérer les machines virtuelles (VM) et conteneurs locaux"
> +
> +#: ../src/machine/org.freedesktop.machine1.policy.in.h:4
> +msgid ""
> +"Authentication is required to manage local virtual machines and containers."
> +msgstr ""
> +"Authentification requise pour gérer les machines virtuelles (VM) et les "
> +"conteneurs locaux."
> +
> +#: ../src/machine/org.freedesktop.machine1.policy.in.h:5
> +msgid "Manage local virtual machine and container images"
> +msgstr "Gérer les images locales de machines virtuelles (VM) et de 
> conteneurs"
> +
> +#: ../src/machine/org.

Re: [systemd-devel] [PATCH] po: update French translation

[Ronny Chevalier]

Hi,

Le 19 février 2015 20:47, Sylvain Plantefève
 a écrit :
> ---
>  po/fr.po | 76 
> +---
>  1 file changed, 68 insertions(+), 8 deletions(-)
>
> diff --git a/po/fr.po b/po/fr.po
> index 8e44e0c..e0257cb 100644
> --- a/po/fr.po
> +++ b/po/fr.po
> @@ -7,7 +7,7 @@ msgid ""
>  msgstr ""
>  "Project-Id-Version: systemd\n"
>  "Report-Msgid-Bugs-To: \n"
> -"POT-Creation-Date: 2015-01-22 16:03+0100\n"
> +"POT-Creation-Date: 2015-02-18 19:48+0100\n"
>  "PO-Revision-Date: 2014-12-28 13:04+0100\n"
>  "Last-Translator: Sylvain Plantefève \n"
>  "Language-Team: French\n"
> @@ -27,11 +27,11 @@ msgid ""
>  msgstr "Authentification requise pour renvoyer la phrase secrète au système."
>
>  #: ../src/core/org.freedesktop.systemd1.policy.in.in.h:3
> -msgid "Manage system services or units"
> +msgid "Manage system services or other units"
>  msgstr "Gérer les services système ou les unités"
>
>  #: ../src/core/org.freedesktop.systemd1.policy.in.in.h:4
> -msgid "Authentication is required to manage system services or units."
> +msgid "Authentication is required to manage system services or other units."
>  msgstr ""
>  "Authentification requise pour gérer les services système ou les unités."
>
> @@ -46,10 +46,24 @@ msgstr ""
>  "unités."
>
>  #: ../src/core/org.freedesktop.systemd1.policy.in.in.h:7
> +msgid "Set or unset system and service manager environment variables"
> +msgstr ""
> +"Définir ou supprimer des variables d'environnement du système ou du "
> +"gestionnaire de services"
> +
> +#: ../src/core/org.freedesktop.systemd1.policy.in.in.h:8
> +msgid ""
> +"Authentication is required to set or unset system and service manager "
> +"environment variables."
> +msgstr ""
> +"Authentification requise pour définir ou supprimer des variables "
> +"d'environnement du système ou du gestionnaire de services."
> +
> +#: ../src/core/org.freedesktop.systemd1.policy.in.in.h:9
>  msgid "Reload the systemd state"
>  msgstr "Recharger l'état de systemd"
>
> -#: ../src/core/org.freedesktop.systemd1.policy.in.in.h:8
> +#: ../src/core/org.freedesktop.systemd1.policy.in.in.h:10
>  msgid "Authentication is required to reload the systemd state."
>  msgstr "Authentification requise pour recharger l'état de systemd"
>
> @@ -89,8 +103,8 @@ msgstr "Télécharger une image de machine virtuelle (VM) ou 
> de conteneur"
>  #: ../src/import/org.freedesktop.import1.policy.in.h:2
>  msgid "Authentication is required to download a VM or container image"
>  msgstr ""
> -"Authentification requise pour télécharger une image de "
> -"machine virtuelle (VM) ou de conteneur."
> +"Authentification requise pour télécharger une image de machine virtuelle "
> +"(VM) ou de conteneur."
>
>  #: ../src/locale/org.freedesktop.locale1.policy.in.h:1
>  msgid "Set system locale"
> @@ -383,15 +397,59 @@ msgstr ""
>  "Authentification requise pour mettre le système en hibernation alors qu'une 
> "
>  "application a demandé de l'empêcher."
>
> +#: ../src/login/org.freedesktop.login1.policy.in.h:49
> +msgid "Manager active sessions, users and seats"
> +msgstr "Gérer les sessions actives, les utilisateurs et les postes (seats)"
> +
> +#: ../src/login/org.freedesktop.login1.policy.in.h:50
> +msgid ""
> +"Authentication is required for managing active sessions, users and seats."
> +msgstr ""
> +"Authentification requise pour gérer les sessions actives, les utilisateurs "
> +"et les postes (seats)."
> +
> +#: ../src/login/org.freedesktop.login1.policy.in.h:51
> +msgid "Lock or unlock active sessions"
> +msgstr "Verrouiller ou déverrouiller des sessions actives"
> +
> +#: ../src/login/org.freedesktop.login1.policy.in.h:52
> +msgid "Authentication is required for locking or unlocking active sessions."
> +msgstr ""
> +"Authentification requise pour verrouiller ou déverrouiller des sessions "
> +"actives."
> +
>  #: ../src/machine/org.freedesktop.machine1.policy.in.h:1
>  msgid "Log into a local container"
>  msgstr "Connexion dans un conteneur local"
>
>  #: ../src/machine/org.freedesktop.machine1.policy.in.h:2
> -msgid "Authentication is required to log into a local container"
> +msgid "Authentication is required to log into a local container."
>  msgstr ""
>  "Authentification requise pour permettre la connexion dans un conteneur 
> local."
>
> +#: ../src/machine/org.freedesktop.machine1.policy.in.h:3
> +msgid "Manage local virtual machines and containers"
> +msgstr "Gérer les machines virtuelles (VM) et conteneurs locaux"
> +
> +#: ../src/machine/org.freedesktop.machine1.policy.in.h:4
> +msgid ""
> +"Authentication is required to manage local virtual machines and containers."
> +msgstr ""
> +"Authentification requise pour gérer les machines virtuelles (VM) et les "
> +"conteneurs locaux."
> +
> +#: ../src/machine/org.freedesktop.machine1.policy.in.h:5
> +msgid "Manage local virtual machine and container images"
> +msgstr "Gérer les images locales de machines virtuelles (VM) et de 
> conteneurs"
> +
> +#: ../src/machine

Re: [systemd-devel] Removing unnecessary includes

[Ronny Chevalier]

2015-02-07 14:05 GMT+01:00 Daniele Nicolodi :
> On 07/02/15 10:29, Thomas H.P. Andersen wrote:
>> I am looking at ways to automatically trim the unnecessary includes.
>> One way to do it is a script[1] which simply tests if the compile
>> still works after removing each include one at a time. It does this in
>> reverse order for all includes in the .c files. Using -Werror we catch
>> any new warnings too.
>
> Hello Thomas,
>
> this approach is not correct: in this way each source file would not be
> required to include the headers included by other files included before.
> For example, if header file "a.h" includes "shared.h" and implementation
> file requires the definitions of "a.h" and "shared.h", only the first
> dependency would be detected by this method.
>
> However, it is good practice to include all the required header files,
> whether those are already included by others or not.
>

Hi,

I agree with Daniele. If you want to include the proper headers in
each file maybe you can use include-what-you-use [0], but this is a
rather recent project with lots of issues that will force you to do a
lots of manual review.

[0] https://code.google.com/p/include-what-you-use/

> Cheers,
> Daniele
>
> ___
> 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

Re: [systemd-devel] [PATCH 0/2] Update french translations

[Ronny Chevalier]

2015-01-22 21:51 GMT+01:00 Sylvain Plantefève :
> Hi folks,

Hi,

>
> These pathes update the french translations of both systemd itself (fr.po) 
> and the catalog, to follow the changes
> from 3d7415f43f0fe6a821d7bc4a341ba371e8a30ef3 and 
> 2057124e7910c4cab7e53d26e0c3749d326ae2bb respectively.
>
> Best regards,
>
>  Sylvain Plantefève
>
>
>
> Sylvain Plantefève (2):
>   catalog: update french translation
>   po: update french translation
>

Both applied.

Thanks.

>  catalog/systemd.fr.catalog |   8 +-
>  po/fr.po   | 191 
> -
>  2 files changed, 106 insertions(+), 93 deletions(-)
>
> --
> 2.1.0
>
> ___
> 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

Re: [systemd-devel] [PATCH] units: move machines.target to dist_systemunit_DATA

[Ronny Chevalier]

2014-12-30 0:22 GMT+01:00 Filipe Brandenburger :
> Move units/machines.target from nodist_systemunit_DATA to 
> dist_systemunit_DATA,
> since it's not a generated file. Otherwise, `make clean` would remove the
> committed copy of the file.
>
> Tested that `./autogen.sh c` will not remove it and that `make distcheck` 
> works
> after this fix.
> ---

Hi,

Another patch has been applied fixing the same issue:
699b7227a2faeb9818e90e90837794aef18f9bef

Thanks

>  Makefile.am | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Makefile.am b/Makefile.am
> index 8cdf1db..c12f2f6 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -506,6 +506,7 @@ dist_systemunit_DATA = \
> units/systemd-udevd-control.socket \
> units/systemd-udevd-kernel.socket \
> units/system-update.target \
> +   units/machines.target \
> units/initrd-switch-root.target
>
>  if ENABLE_KDBUS
> @@ -549,7 +550,6 @@ nodist_systemunit_DATA = \
> units/initrd-udevadm-cleanup-db.service \
> units/initrd-switch-root.service \
> units/systemd-nspawn@.service \
> -   units/machines.target \
> units/systemd-update-done.service
>
>  if HAVE_UTMP
> --
> 1.8.3.1
>
> ___
> 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

Re: [systemd-devel] [PATCH] fix zsh completion typo

[Ronny Chevalier]

2015-01-14 15:33 GMT+01:00 Moez Bouhlel :
> json-see => json-sse
>
> ---
>  shell-completion/zsh/_sd_outputmodes | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/shell-completion/zsh/_sd_outputmodes 
> b/shell-completion/zsh/_sd_outputmodes
> index 2ce84a7..3836f79 100644
> --- a/shell-completion/zsh/_sd_outputmodes
> +++ b/shell-completion/zsh/_sd_outputmodes
> @@ -1,5 +1,5 @@
>  #autoload
>
>  local -a _output_opts
> -_output_opts=(short short-iso short-precise short-monotonic verbose export 
> json json-pretty json-see cat)
> +_output_opts=(short short-iso short-precise short-monotonic verbose export 
> json json-pretty json-sse cat)
>  _describe -t output 'output mode' _output_opts || compadd "$@"
> --
> 2.2.2

Hi,

Applied, thanks.

>
> ___
> 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

Re: [systemd-devel] Using `systemctl edit` on "invalid" unit names

[Ronny Chevalier]

2014-12-13 11:33 GMT+01:00 Ivan Shapovalov :
> Hello all,

Hi,

>
> it seems that the newly added `systemctl edit` command requires its arguments
> to be valid unit names.
>
> This causes `edit` operation to fail in apparently valid use-cases like
>
> systemctl edit getty@.service

This is fixed in git now, thanks!

> or
> systemctl edit autovt@tty1.service
>
> In second case, the error message is especially cryptic:
> "autovt@tty1.service ignored: not found".

It worked before and it still works for me.

>
> Actually I understand what it does mean: systemctl asks the manager to show
> unit's FragmentPath -> the manager tries to load the unit -> loading fails 
> with
> "File exists" because getty@tty1.service is already instantiated.

I don't see why it should fail for this reason ?

>
> (BTW, it's a separate question: is this failure valid or is it a bug?)
>

systemctl edit getty@.service, should have worked before so yes this was a bug.

> But well. I guess that `edit` operation should always work with unit files
> directly, just like enable/disable commands do.

systemctl edit try to use the bus if it is possible, because this is
the only way you can know where is the unit file of the unit
foo.service currently running. Is it in /etc/systemd or
/usr/lib/systemd? If we check directly the file system we will assume
this is the first directory with the highest priority, which is wrong
in some cases.

But systemctl edit wanted to work with valid unit names which is wrong
in some cases too, so this is fixed now.

>
> Is this all correct? Can anyone please comment on these two issues?

Thanks for the report!

>
> --
> Ivan Shapovalov / intelfx /
> ___
> 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

Re: [systemd-devel] [PATCH] localed: forward xkbcommon errors

[Ronny Chevalier]

2014-12-02 14:02 GMT+01:00 Jan Synacek :
Hi,

> The errors are prefixed with "libxkbcommon", because they are quite
> confusing. With the prefix, we at least know where they come from.
> ---
>  src/locale/localed.c | 15 +++
>  1 file changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/src/locale/localed.c b/src/locale/localed.c
> index 4e56382..ea54798 100644
> --- a/src/locale/localed.c
> +++ b/src/locale/localed.c
> @@ -1011,10 +1011,16 @@ static int method_set_vc_keyboard(sd_bus *bus, 
> sd_bus_message *m, void *userdata
>
>  #ifdef HAVE_XKBCOMMON
>  static void log_xkb(struct xkb_context *ctx, enum xkb_log_level lvl, const 
> char *format, va_list args) {
> -/* suppress xkb messages for now */
> +_cleanup_free_ char *fmt = NULL;
> +sd_bus_error *e;
> +
> +if (asprintf(&fmt, "libxkbcommon: %s", format) < 0)
> +(void) log_oom();
If you only need to concatenate you can use strjoin and since this is
only valid for this scope, strappenda is even more appropriate here.

> +e = xkb_context_get_user_data(ctx);
> +bus_error_setfv(e, SD_BUS_ERROR_INVALID_ARGS, fmt, args);
>  }
>
> -static int verify_xkb_rmlvo(const char *model, const char *layout, const 
> char *variant, const char *options) {
> +static int verify_xkb_rmlvo(const char *model, const char *layout, const 
> char *variant, const char *options, sd_bus_error *error) {
>  const struct xkb_rule_names rmlvo = {
>  .model  = model,
>  .layout = layout,
> @@ -1033,6 +1039,7 @@ static int verify_xkb_rmlvo(const char *model, const 
> char *layout, const char *v
>  goto exit;
>  }
>
> +xkb_context_set_user_data(ctx, (void *)error);
>  xkb_context_set_log_fn(ctx, log_xkb);
>
>  km = xkb_keymap_new_from_names(ctx, &rmlvo, 
> XKB_KEYMAP_COMPILE_NO_FLAGS);
> @@ -1049,7 +1056,7 @@ exit:
>  return r;
>  }
>  #else
> -static int verify_xkb_rmlvo(const char *model, const char *layout, const 
> char *variant, const char *options) {
> +static int verify_xkb_rmlvo(const char *model, const char *layout, const 
> char *variant, const char *options, sd_bus_error *error) {
>  return 0;
>  }
>  #endif
> @@ -1087,7 +1094,7 @@ static int method_set_x11_keyboard(sd_bus *bus, 
> sd_bus_message *m, void *userdat
>  (options && !string_is_safe(options)))
>  return sd_bus_error_set_errnof(error, -EINVAL, 
> "Received invalid keyboard data");
>
> -r = verify_xkb_rmlvo(model, layout, variant, options);
> +r = verify_xkb_rmlvo(model, layout, variant, options, error);
>  if (r < 0)
>  log_warning("Cannot compile XKB keymap for new x11 
> keyboard layout ('%s' / '%s' / '%s' / '%s'): %s",
>  strempty(model), strempty(layout), 
> strempty(variant), strempty(options), strerror(-r));
> --
> 1.9.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

Re: [systemd-devel] [PATCH] man: fix link to systemd-networkd-wait-online.service in systemd-networkd.service(8)

[Ronny Chevalier]

2014-11-30 15:44 GMT+01:00 Chris Mayo :
>
> ---
>  man/systemd-networkd.service.xml | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/man/systemd-networkd.service.xml 
> b/man/systemd-networkd.service.xml
> index 0570798..7c20fa4 100644
> --- a/man/systemd-networkd.service.xml
> +++ b/man/systemd-networkd.service.xml
> @@ -95,7 +95,7 @@
>  
> systemd.link5,
>  
> systemd.network5,
>  
> systemd.netdev5,
> -
> systemd-network-wait-online.service8
> +
> systemd-networkd-wait-online.service8
>  
>  
Applied.

Thanks.

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

Re: [systemd-devel] [PATCH] man: add a link to systemd-coredump(8) in Description of coredump.conf(5)

[Ronny Chevalier]

2014-11-30 15:45 GMT+01:00 Chris Mayo :
>
> ---
>  man/coredump.conf.xml | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/man/coredump.conf.xml b/man/coredump.conf.xml
> index 37916f0..fe7479f 100644
> --- a/man/coredump.conf.xml
> +++ b/man/coredump.conf.xml
> @@ -59,7 +59,7 @@
>
>  Description
>
> -These files configure the behaviour of 
> systemd-coredump,
> +These files configure the behaviour of 
> systemd-coredump8,
>  a handler for core dumps invoked by the kernel.
>
>
Applied (I added a newline to avoid a long line).

Thanks.

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

Re: [systemd-devel] [PATCH] systemctl: add edit verb

[Ronny Chevalier]

2014-11-28 23:41 GMT+01:00 Zbigniew Jędrzejewski-Szmek :
> On Fri, Nov 28, 2014 at 09:48:55PM +0100, Ronny Chevalier wrote:
>> 2014-10-29 16:22 GMT+01:00 Ronny Chevalier :
>> > It helps editing units by either creating a drop-in file, like
>> > /etc/systemd/system/my.service.d/override.conf, or by copying the
>> > original unit from /usr/lib/systemd/ to /etc/systemd/ if the --full
>> > option is specified.
>> >
>> > It invokes an editor on temporary files related to the unit files and
>> > if the editor exited successfully, then it renames the temporary files
>> > to their original names (e.g. my.service or override.conf) and
>> > daemon-reload is invoked.
>> >
>> > If the temporary file is empty the modification is canceled.
>> >
>> > See https://bugzilla.redhat.com/show_bug.cgi?id=906824
>> > ---
>>
>> Is it ok if I apply this patch, or is there other remarks ? (Minus the
>> recent log_error_errno changes)
> Yes please go ahead! Your patch was one of the big things on my TODO list,
> it's great that you that your account is finally created and you can push
> it yourself.
>
> I replied to the last version of the patch with some comments.

Pushed.
Thanks for the review

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

Re: [systemd-devel] [PATCH] systemctl: add edit verb

[Ronny Chevalier]

2014-10-29 16:22 GMT+01:00 Ronny Chevalier :
> It helps editing units by either creating a drop-in file, like
> /etc/systemd/system/my.service.d/override.conf, or by copying the
> original unit from /usr/lib/systemd/ to /etc/systemd/ if the --full
> option is specified.
>
> It invokes an editor on temporary files related to the unit files and
> if the editor exited successfully, then it renames the temporary files
> to their original names (e.g. my.service or override.conf) and
> daemon-reload is invoked.
>
> If the temporary file is empty the modification is canceled.
>
> See https://bugzilla.redhat.com/show_bug.cgi?id=906824
> ---

Is it ok if I apply this patch, or is there other remarks ? (Minus the
recent log_error_errno changes)

>
> lookup_paths_init does not concatenate root_dir, so I added a path_join with 
> arg_root
>
>  TODO  |   4 +-
>  man/less-variables.xml|   4 +-
>  man/systemctl.xml |  64 +-
>  src/systemctl/systemctl.c | 525 
> +-
>  4 files changed, 587 insertions(+), 10 deletions(-)
>
> diff --git a/TODO b/TODO
> index abe89b7..1cbedd4 100644
> --- a/TODO
> +++ b/TODO
> @@ -84,7 +84,7 @@ Features:
>
>  * systemctl: if it fails, show log output?
>
> -* maybe add "systemctl edit" that copies unit files from 
> /usr/lib/systemd/system to /etc/systemd/system and invokes vim on them
> +* systemctl edit: add commented help text to the end, like git commit
>
>  * maybe introduce AssertXYZ= similar to ConditionXYZ= that causes a unit to 
> fail (instead of skipping it) if some condition is not true...
>
> @@ -776,7 +776,7 @@ External:
>
>  * zsh shell completion:
>-   - should complete options, but currently does not
> -  - systemctl add-wants,add-requires
> +  - systemctl add-wants,add-requires, edit
>
>
>  Regularly:
> diff --git a/man/less-variables.xml b/man/less-variables.xml
> index 09cbd42..0fb4d7f 100644
> --- a/man/less-variables.xml
> +++ b/man/less-variables.xml
> @@ -6,7 +6,7 @@
>  Environment
>
>  
> -
> +
>  $SYSTEMD_PAGER
>
>  Pager to use when
> @@ -17,7 +17,7 @@
>  --no-pager.
>  
>
> -
> +
>  $SYSTEMD_LESS
>
>  Override the default
> diff --git a/man/systemctl.xml b/man/systemctl.xml
> index 7cbaa6c..26f5235 100644
> --- a/man/systemctl.xml
> +++ b/man/systemctl.xml
> @@ -465,7 +465,7 @@ along with systemd; If not, see 
> <http://www.gnu.org/licenses/>.
>
>  
>When used with enable,
> -  disable,
> +  disable, edit,
>(and related commands), make changes only temporarily, so
>that they are lost on the next reboot. This will have the
>effect that changes are not made in subdirectories of
> @@ -1150,6 +1150,43 @@ kobject-uevent 1 systemd-udevd-kernel.socket 
> systemd-udevd.service
>  default.target to the given unit.
>
>  
> +
> +
> +  edit 
> NAME...
> +
> +  
> +Edit a drop-in snippet or a whole replacement file if
> +--full is specified, to extend or override the
> +specified unit.
> +
> +Depending on whether --system (the 
> default),
> +--user, or --global is 
> specified,
> +this create a drop-in file for each units either for the system,
> +for the calling user or for all futures logins of all users. Then
> +the editor (see section "Environment" below) is invoked on 
> temporary
> +files which will be saved as their corresponding files if the 
> editor
> +exited successfully.
> +
> +If --full is specified, this will copy the
> +original units instead of creating drop-in files.
> +
> +If --runtime is specified, the changes 
> will
> +be made temporarily in /run and they will be
> +lost on the next reboot.
> +
> +If the temporary file is empty the modification of the 
> related
> +unit is canceled
> +
> +After the units have been edited, the systemd 
> configuration is
> +reloaded (in a way that is equivalent to 
> daemon-reload),
> +but it does not restart or reload the units.
> +
> +Note that this command cannot be used to remotely edit 
> units
> +and that you c

Re: [systemd-devel] [PATCH 2/2] tty-ask-password-agent: fix CID 996261

[Ronny Chevalier]

2014-11-17 19:36 GMT+01:00 Greg KH :
> On Mon, Nov 17, 2014 at 06:47:33PM +0100, Ronny Chevalier wrote:
>> 2014-11-17 18:31 GMT+01:00 Greg KH :
>> > On Mon, Nov 17, 2014 at 10:44:14PM +0530, Susant Sahani wrote:
>> >> On 11/17/2014 10:39 PM, Greg KH wrote:
>> >> >On Mon, Nov 17, 2014 at 10:36:53PM +0530, Susant Sahani wrote:
>> >> >>On 11/17/2014 10:26 PM, Greg KH wrote:
>> >> >>>On Mon, Nov 17, 2014 at 04:28:58PM +0530, Susant Sahani wrote:
>> >> >>>>---
>> >> >>>>  src/tty-ask-password-agent/tty-ask-password-agent.c | 2 +-
>> >> >>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> >> >>>>
>> >> >>>>diff --git a/src/tty-ask-password-agent/tty-ask-password-agent.c 
>> >> >>>>b/src/tty-ask-password-agent/tty-ask-password-agent.c
>> >> >>>>index e6dc84b..1fc792b 100644
>> >> >>>>--- a/src/tty-ask-password-agent/tty-ask-password-agent.c
>> >> >>>>+++ b/src/tty-ask-password-agent/tty-ask-password-agent.c
>> >> >>>>@@ -376,8 +376,8 @@ static int wall_tty_block(void) {
>> >> >>>>  return -ENOMEM;
>> >> >>>>
>> >> >>>>  mkdir_parents_label(p, 0700);
>> >> >>>>-mkfifo(p, 0600);
>> >> >>>>
>> >> >>>>+(void)mkfifo(p, 0600);
>> >> >>>
>> >> >>>You really aren't "fixing" anything in these patches, just merely
>> >> >>>papering over the Coverity issues.  Which is fine, if you really want 
>> >> >>>to
>> >> >>>do that, but don't think it's anything other than that...
>> >> >>
>> >> >>Yes my intention is to for coverity only Any way next line 'open' 
>> >> >>handling
>> >> >>the error case .
>> >> >
>> >> >I'm sorry, but I don't understand this sentance at all, can you rephrase
>> >> >it?
>> >> >
>> >>
>> >> Sorry let me rephrase it. This patch only for coverity . The next like of
>> >> mkfifo is open .
>> >>
>> >> (void)mkfifo(p, 0600);
>> >> fd = open(p, O_RDONLY|O_CLOEXEC|O_NONBLOCK|O_NOCTTY);
>> >> if (fd < 0)
>> >> return -errno;
>> >>
>> >> and open is handling the failure.
>> >
>> > Then coverity should be fixed, don't paper over stupid bugs in tools for
>> > no reason.
>> I disagree.
>>
>> Coverity can not infer this in any possible way. How can coverity
>> infer that we do not care about the return value of mkfifo ?
>> It really depends of the semantic here.
>
> Coverity is a "semantic checker", why can't it be changed to determine
> if mkfifo() is followed by open() and an error check, that it is safe
> code?  It does this for lots of other common patterns.
For me I see this as a warning, for some cases it is safe and there is
no problem like this one so we can document the code for us and tools
like Coverity, but it can be a mistake and maybe it should have been
checked. So Coverity assumes the worst case by warning us, and I don't
see the problem.

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

Re: [systemd-devel] [PATCH 2/2] tty-ask-password-agent: fix CID 996261

[Ronny Chevalier]

2014-11-17 18:31 GMT+01:00 Greg KH :
> On Mon, Nov 17, 2014 at 10:44:14PM +0530, Susant Sahani wrote:
>> On 11/17/2014 10:39 PM, Greg KH wrote:
>> >On Mon, Nov 17, 2014 at 10:36:53PM +0530, Susant Sahani wrote:
>> >>On 11/17/2014 10:26 PM, Greg KH wrote:
>> >>>On Mon, Nov 17, 2014 at 04:28:58PM +0530, Susant Sahani wrote:
>> ---
>>   src/tty-ask-password-agent/tty-ask-password-agent.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/src/tty-ask-password-agent/tty-ask-password-agent.c 
>> b/src/tty-ask-password-agent/tty-ask-password-agent.c
>> index e6dc84b..1fc792b 100644
>> --- a/src/tty-ask-password-agent/tty-ask-password-agent.c
>> +++ b/src/tty-ask-password-agent/tty-ask-password-agent.c
>> @@ -376,8 +376,8 @@ static int wall_tty_block(void) {
>>   return -ENOMEM;
>> 
>>   mkdir_parents_label(p, 0700);
>> -mkfifo(p, 0600);
>> 
>> +(void)mkfifo(p, 0600);
>> >>>
>> >>>You really aren't "fixing" anything in these patches, just merely
>> >>>papering over the Coverity issues.  Which is fine, if you really want to
>> >>>do that, but don't think it's anything other than that...
>> >>
>> >>Yes my intention is to for coverity only Any way next line 'open' handling
>> >>the error case .
>> >
>> >I'm sorry, but I don't understand this sentance at all, can you rephrase
>> >it?
>> >
>>
>> Sorry let me rephrase it. This patch only for coverity . The next like of
>> mkfifo is open .
>>
>> (void)mkfifo(p, 0600);
>> fd = open(p, O_RDONLY|O_CLOEXEC|O_NONBLOCK|O_NOCTTY);
>> if (fd < 0)
>> return -errno;
>>
>> and open is handling the failure.
>
> Then coverity should be fixed, don't paper over stupid bugs in tools for
> no reason.
I disagree.

Coverity can not infer this in any possible way. How can coverity
infer that we do not care about the return value of mkfifo ?
It really depends of the semantic here. In this case Susant is
documenting the fact that he does not care about the return value of
mkfifo because he thinks that it is already handled by open. In
another program one can just forgot to check the return value of
mkfifo and doing an open after, but maybe in this program checking the
return value of mkfifo is important.

>
> thanks,
>
> greg k-h
> ___
> 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

Re: [systemd-devel] [PATCH] udev: fix TOCTOU when creating a directory

[Ronny Chevalier]

2014-11-16 19:39 GMT+01:00 David Herrmann :
> Hi
>
> On Sun, Nov 16, 2014 at 7:34 PM, David Herrmann  wrote:
>> Hi
>>
>> On Sun, Nov 9, 2014 at 3:42 PM, Ronny Chevalier
>>  wrote:
>>> CID#979416
>>> ---
>>>  src/udev/collect/collect.c | 7 ---
>>>  1 file changed, 4 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/src/udev/collect/collect.c b/src/udev/collect/collect.c
>>> index dc849bd..6cb10fe 100644
>>> --- a/src/udev/collect/collect.c
>>> +++ b/src/udev/collect/collect.c
>>> @@ -86,12 +86,13 @@ static void usage(void)
>>>   */
>>>  static int prepare(char *dir, char *filename)
>>>  {
>>> -struct stat statbuf;
>>>  char buf[512];
>>>  int fd;
>>> +int r;
>>>
>>> -if (stat(dir, &statbuf) < 0)
>>> -mkdir(dir, 0700);
>>> +r = mkdir(dir, 0700);
>>> +if (r < 0 && errno != EEXIST)
>>> +return -errno;
>>>
>>>  snprintf(buf, sizeof(buf), "%s/%s", dir, filename);
>>
>> So the race you describe is if the directory is removed after we
>> stat() it, but before we use it somewhere down in the code. Applying
>> your patch avoids the stat(), but it still fails if the dir is removed
>> after your mkdir(). So this doesn't fix anything, does it?
Right

>>
>> The code is definitely nicer than before, so I guess I'll apply it,
>> anyway. But I don't see how it would fix anything, but silence a
>> coverity warning. Or am I missing something? Feel free to prove me
>> wrong ;)
>
> One more addition: your code avoids an additional syscall, so yeah,
> it's nicer. So I applied it now!
It was the purpose of this patch. The stat() call was useless and we
did not check if the mkdir() succeeded.
But you are right, it does not fix any TOCTOU, I think I named it this
way because of the coverity report.

So you can change the commit message if you want :)

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

Re: [systemd-devel] [PATCH v2] localed: validate set-x11-keymap input

[Ronny Chevalier]

2014-11-14 12:42 GMT+01:00 Jan Synacek :
> Try to validate the input similarly to how setxkbmap does it. Multiple
> layouts and variants can be specified, separated by a comma. Variants
> can also be left out, meaning that the user doesn't want any particular
> variant for the respective layout.
>
> Variants are validated respectively to their layouts. First variant
> validates against first layout, second variant to second layout, etc. If
> there are more entries of either layouts or variants, only their
> respective counterparts are validated, the rest is ignored.
>
> Examples:
> $ set-x11-keymap us,cz  pc105 ,qwerty
> "us" is not validated, because no respective variant was specified. "cz"
> is checked for an existing "qwerty" variant, the check succeeds.
>
> $ set-x11-keymap us pc105 ,qwerty
> "us" is not validated as in the above example. The rest of the variants
> is ignored, because there are no respective layouts.
>
> $ set-x11-keymap us,cz pc105 qwerty
> "us" is validated against the "qwerty" variant, check fails, because
> there is no "qwerty" variant for the "us" layout.
>
> $ set-x11-keymap us,cz pc105 euro,qwerty
> Validation succeeds, there is the "euro" variant for the "us" layout,
> and "qwerty" variant for the "cz" layout.
>
> http://lists.freedesktop.org/archives/systemd-devel/2014-October/024411.html
> ---
> v2:
> - rewrite to use the X11Keymap struct
> - use greedy allocation where it makes sense
> - rename enum keymap_state to KeymapComponent
> - on the server side, propagate error to the client and don't log it
> - on the server side, change SD_BUS_ERROR_FAILED to SD_BUS_ERROR_INVALID_ARGS
>
>  Makefile.am|   2 +
>  src/locale/localectl.c |  85 ++---
>  src/locale/localed.c   |   6 +
>  src/shared/xkb-util.c  | 319 
> +
>  src/shared/xkb-util.h  |  50 
>  5 files changed, 385 insertions(+), 77 deletions(-)
>  create mode 100644 src/shared/xkb-util.c
>  create mode 100644 src/shared/xkb-util.h
>
> diff --git a/Makefile.am b/Makefile.am
> index 701666c..224582c 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -772,6 +772,8 @@ libsystemd_shared_la_SOURCES = \
> src/shared/time-util.h \
> src/shared/locale-util.c \
> src/shared/locale-util.h \
> +   src/shared/xkb-util.c \
> +   src/shared/xkb-util.h \
> src/shared/mempool.c \
> src/shared/mempool.h \
> src/shared/hashmap.c \
> diff --git a/src/locale/localectl.c b/src/locale/localectl.c
> index d4a2d29..08a1011 100644
> --- a/src/locale/localectl.c
> +++ b/src/locale/localectl.c
> @@ -46,6 +46,7 @@
>  #include "virt.h"
>  #include "fileio.h"
>  #include "locale-util.h"
> +#include "xkb-util.h"
>
>  static bool arg_no_pager = false;
>  static bool arg_ask_password = true;
> @@ -388,15 +389,8 @@ static int set_x11_keymap(sd_bus *bus, char **args, 
> unsigned n) {
>
>  static int list_x11_keymaps(sd_bus *bus, char **args, unsigned n) {
>  _cleanup_fclose_ FILE *f = NULL;
> -_cleanup_strv_free_ char **list = NULL;
> -char line[LINE_MAX];
> -enum {
> -NONE,
> -MODELS,
> -LAYOUTS,
> -VARIANTS,
> -OPTIONS
> -} state = NONE, look_for;
> +_cleanup_(xkb_keymap_free_components) X11Keymap keymap = {};
> +enum KeymapComponent look_for;
>  int r;
>
>  if (n > 2) {
> @@ -404,12 +398,6 @@ static int list_x11_keymaps(sd_bus *bus, char **args, 
> unsigned n) {
>  return -EINVAL;
>  }
>
> -f = fopen("/usr/share/X11/xkb/rules/base.lst", "re");
> -if (!f) {
> -log_error("Failed to open keyboard mapping list. %m");
> -return -errno;
> -}
> -
>  if (streq(args[0], "list-x11-keymap-models"))
>  look_for = MODELS;
>  else if (streq(args[0], "list-x11-keymap-layouts"))
> @@ -421,71 +409,14 @@ static int list_x11_keymaps(sd_bus *bus, char **args, 
> unsigned n) {
>  else
>  assert_not_reached("Wrong parameter");
>
> -FOREACH_LINE(line, f, break) {
> -char *l, *w;
> -
> -l = strstrip(line);
> -
> -if (isempty(l))
> -continue;
> -
> -if (l[0] == '!') {
> -if (startswith(l, "! model"))
> -state = MODELS;
> -else if (startswith(l, "! layout"))
> -state = LAYOUTS;
> -else if (startswith(l, "! variant"))
> -state = VARIANTS;
> -else if (startswith(l, "! option"))
> -state = OPTIONS;
> -else
> -state = NONE;
> -
> -continue;
> -}
> -
> -   

Re: [systemd-devel] [PATCH] networkd: Support VXlan parameters

[Ronny Chevalier]

2014-11-14 8:44 GMT+01:00 Susant Sahani :
Hi,

> Add vxlan paramertes to config.
> ---
>  man/systemd.netdev.xml  | 30 +
>  src/network/networkd-netdev-gperf.gperf |  7 ++-
>  src/network/networkd-netdev-vxlan.c | 75 
> +
>  src/network/networkd-netdev-vxlan.h |  8 
>  src/network/networkd.h  | 11 +
>  5 files changed, 130 insertions(+), 1 deletion(-)
>
> diff --git a/man/systemd.netdev.xml b/man/systemd.netdev.xml
> index 275ee52..e25c1c4 100644
> --- a/man/systemd.netdev.xml
> +++ b/man/systemd.netdev.xml
> @@ -272,6 +272,36 @@
>  to discover remote MAC 
> addresses.
>  
>  
> +
> +
> FDBAgeingSec=
> +
> +The lifetime of FDB 
> entries learnt by the kernel in seconds.
> +
> +
> +
> +
> ARPProxy=
> +
> +A boolean. When true, 
> enables ARP proxy.
> +
> +
> +
> +
> L2Miss=
> +
> +A boolean. When true, 
> enables netlink LLADDR miss notifications.
> +
> +
> +
> +
> L3Miss=
> +
> +A boolean. When true, 
> enables netlink IP ADDR miss notifications.
> +
> +
> +
> +
> RouteSC=
> +
> +A boolean. When true 
> route short circuit is turned on.
> +
> +
>  
>  
>  
> diff --git a/src/network/networkd-netdev-gperf.gperf 
> b/src/network/networkd-netdev-gperf.gperf
> index c524ee5..5ee5380 100644
> --- a/src/network/networkd-netdev-gperf.gperf
> +++ b/src/network/networkd-netdev-gperf.gperf
> @@ -37,10 +37,15 @@ Tunnel.DiscoverPathMTU,  config_parse_bool,   
>0,
>  Peer.Name,   config_parse_ifname,0,  
>offsetof(Veth, ifname_peer)
>  Peer.MACAddress, config_parse_hwaddr,0,  
>offsetof(Veth, mac_peer)
>  VXLAN.Id,config_parse_uint64,0,  
>offsetof(VxLan, id)
> -VXLAN.Group, config_parse_tunnel_address,0,  
>offsetof(VxLan, group)
> +VXLAN.Group, config_parse_vxlan_group_address,   0,  
>offsetof(VxLan, group)
>  VXLAN.TOS,   config_parse_unsigned,  0,  
>offsetof(VxLan, tos)
>  VXLAN.TTL,   config_parse_unsigned,  0,  
>offsetof(VxLan, ttl)
>  VXLAN.MacLearning,   config_parse_bool,  0,  
>offsetof(VxLan, learning)
> +VXLAN.ARPProxy,  config_parse_bool,  0,  
>offsetof(VxLan, arp_proxy)
> +VXLAN.L2Miss,config_parse_bool,  0,  
>offsetof(VxLan, l2miss)
> +VXLAN.L3Miss,config_parse_bool,  0,  
>offsetof(VxLan, l3miss)
> +VXLAN.RouteSC,   config_parse_bool,  0,  
>offsetof(VxLan, route_short_circuit)
> +VXLAN.FDBAgeingSec,  config_parse_sec,   0,  
>offsetof(VxLan, fdb_ageing)
>  Tun.OneQueue,config_parse_bool,  0,  
>offsetof(TunTap, one_queue)
>  Tun.MultiQueue,  config_parse_bool,  0,  
>offsetof(TunTap, multi_queue)
>  Tun.PacketInfo,  config_parse_bool,  0,  
>offsetof(TunTap, packet_info)
> diff --git a/src/network/networkd-netdev-vxlan.c 
> b/src/network/networkd-netdev-vxlan.c
> index 326ac54..076e266 100644
> --- a/src/network/networkd-netdev-vxlan.c
> +++ b/src/network/networkd-netdev-vxlan.c
> 

[systemd-devel] [PATCH] shared: explicitly ignore the return value of wait_for_terminate

[Ronny Chevalier]

CID#1237532
CID#1237523
CID#1237522
---
 src/shared/pager.c| 2 +-
 src/shared/spawn-ask-password-agent.c | 2 +-
 src/shared/spawn-polkit-agent.c   | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/shared/pager.c b/src/shared/pager.c
index 5479094..001927c 100644
--- a/src/shared/pager.c
+++ b/src/shared/pager.c
@@ -143,7 +143,7 @@ void pager_close(void) {
 /* Inform pager that we are done */
 fclose(stdout);
 kill(pager_pid, SIGCONT);
-wait_for_terminate(pager_pid, NULL);
+(void) wait_for_terminate(pager_pid, NULL);
 pager_pid = 0;
 }
 
diff --git a/src/shared/spawn-ask-password-agent.c 
b/src/shared/spawn-ask-password-agent.c
index c1a9c58..ff121f8 100644
--- a/src/shared/spawn-ask-password-agent.c
+++ b/src/shared/spawn-ask-password-agent.c
@@ -62,6 +62,6 @@ void ask_password_agent_close(void) {
 /* Inform agent that we are done */
 kill(agent_pid, SIGTERM);
 kill(agent_pid, SIGCONT);
-wait_for_terminate(agent_pid, NULL);
+(void) wait_for_terminate(agent_pid, NULL);
 agent_pid = 0;
 }
diff --git a/src/shared/spawn-polkit-agent.c b/src/shared/spawn-polkit-agent.c
index 29b01db..7a90ef8 100644
--- a/src/shared/spawn-polkit-agent.c
+++ b/src/shared/spawn-polkit-agent.c
@@ -82,7 +82,7 @@ void polkit_agent_close(void) {
 /* Inform agent that we are done */
 kill(agent_pid, SIGTERM);
 kill(agent_pid, SIGCONT);
-wait_for_terminate(agent_pid, NULL);
+(void) wait_for_terminate(agent_pid, NULL);
 agent_pid = 0;
 }
 
-- 
2.1.3

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

[systemd-devel] [PATCH] udev: fix TOCTOU when creating a directory

[Ronny Chevalier]

CID#979416
---
 src/udev/collect/collect.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/udev/collect/collect.c b/src/udev/collect/collect.c
index dc849bd..6cb10fe 100644
--- a/src/udev/collect/collect.c
+++ b/src/udev/collect/collect.c
@@ -86,12 +86,13 @@ static void usage(void)
  */
 static int prepare(char *dir, char *filename)
 {
-struct stat statbuf;
 char buf[512];
 int fd;
+int r;
 
-if (stat(dir, &statbuf) < 0)
-mkdir(dir, 0700);
+r = mkdir(dir, 0700);
+if (r < 0 && errno != EEXIST)
+return -errno;
 
 snprintf(buf, sizeof(buf), "%s/%s", dir, filename);
 
-- 
2.1.3

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

[systemd-devel] [PATCH] bus: fix null pointer dereference

[Ronny Chevalier]

CID#1237620
---
 src/libsystemd/sd-bus/bus-message.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/libsystemd/sd-bus/bus-message.c 
b/src/libsystemd/sd-bus/bus-message.c
index be36d9f..edadacf 100644
--- a/src/libsystemd/sd-bus/bus-message.c
+++ b/src/libsystemd/sd-bus/bus-message.c
@@ -2048,6 +2048,7 @@ static int bus_message_close_variant(sd_bus_message *m, 
struct bus_container *c)
 
 assert(m);
 assert(c);
+assert(c->signature);
 
 if (!BUS_MESSAGE_IS_GVARIANT(m))
 return 0;
@@ -2174,6 +2175,8 @@ _public_ int 
sd_bus_message_close_container(sd_bus_message *m) {
 if (c->enclosing != SD_BUS_TYPE_ARRAY)
 if (c->signature && c->signature[c->index] != 0)
 return -EINVAL;
+if (!c->signature && c->enclosing == SD_BUS_TYPE_VARIANT)
+return -EINVAL;
 
 m->n_containers--;
 
-- 
2.1.3

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

Re: [systemd-devel] Leak mempool/hashmap

[Ronny Chevalier]

2014-11-06 19:59 GMT+01:00 Jan Janssen :
Hi,

>
>
> On 2014-11-06 19:05, Lennart Poettering wrote:
>>
>> On Thu, 06.11.14 18:36, Jan Janssen (medhe...@web.de) wrote:
>>
>>> Hi,
>>>
>>> I just noticed that mempool/hashmap leaks memory. It's as simple as this
>>> to
>>> trigger:
>>>
>>> #include "hashmap.h"
>>> int main(int argc, const char *argv[]) {
>>>  Hashmap *m = hashmap_new(&string_hash_ops);
>>>  hashmap_free(m);
>>> }
>>
>>
>> How did you determine the leak?
>>
>> Note that the hashmap uses an allocation cache. It's not freed on
>> shutdown, but it's not leaked either...
>>
>> Lennart
>>
>
> I've noticed while testing my cryptsetup-generator rewrite with valgrind.
> It's still reachable according to valgrind, but a silent output would be
> nice to have.
If you add -DVALGRIND=1 to the CFLAGS there will be no false positive
leaks reported.

>
> Jan
>
> ___
> 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

Re: [systemd-devel] [PATCH] udev: Fix parsing of udev.event-timeout kernel parameter.

[Ronny Chevalier]

2014-11-05 0:31 GMT+01:00 Richard W.M. Jones :
Hi,

> ---
>  src/udev/udevd.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/udev/udevd.c b/src/udev/udevd.c
> index 2e6c713..206a4d3 100644
> --- a/src/udev/udevd.c
> +++ b/src/udev/udevd.c
> @@ -994,7 +994,7 @@ static void kernel_cmdline_options(struct udev *udev) {
>  if (r < 0)
>  log_warning("Invalid udev.exec-delay 
> ignored: %s", opt + 16);
>  } else if (startswith(opt, "udev.event-timeout=")) {
> -r = safe_atou64(opt + 16, &arg_event_timeout_usec);
> +r = safe_atou64(opt + 19, &arg_event_timeout_usec);
>  if (r < 0) {
>  log_warning("Invalid udev.event-timeout 
> ignored: %s", opt + 16);
You need to fix this one too.

Maybe use opt + strlen("udev.event-timeout=") since it is optimized by gcc ?

>  break;
> --
> 2.1.0
>
> ___
> 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] tests: add tests for strv.c

[Ronny Chevalier]

add tests for:
- strv_find_startswith
- strv_push_prepend
- strv_consume_prepend
---
 src/test/test-strv.c | 39 +++
 1 file changed, 39 insertions(+)

diff --git a/src/test/test-strv.c b/src/test/test-strv.c
index bbfe306..915fa46 100644
--- a/src/test/test-strv.c
+++ b/src/test/test-strv.c
@@ -113,6 +113,22 @@ static void test_strv_find_prefix(void) {
 assert_se(!strv_find_prefix((char **)input_table_multiple, "onee"));
 }
 
+static void test_strv_find_startswith(void) {
+char *r;
+
+r = strv_find_startswith((char **)input_table_multiple, "o");
+assert_se(r && streq(r, "ne"));
+
+r = strv_find_startswith((char **)input_table_multiple, "one");
+assert_se(r && streq(r, ""));
+
+r = strv_find_startswith((char **)input_table_multiple, "");
+assert_se(r && streq(r, "one"));
+
+assert_se(!strv_find_startswith((char **)input_table_multiple, "xxx"));
+assert_se(!strv_find_startswith((char **)input_table_multiple, 
"onee"));
+}
+
 static void test_strv_join(void) {
 _cleanup_free_ char *p = NULL, *q = NULL, *r = NULL, *s = NULL, *t = 
NULL;
 
@@ -416,6 +432,27 @@ static void test_strv_from_stdarg_alloca(void) {
 test_strv_from_stdarg_alloca_one(STRV_MAKE_EMPTY, NULL);
 }
 
+static void test_strv_push_prepend(void) {
+_cleanup_strv_free_ char **a = NULL;
+
+a = strv_new("foo", "bar", "three", NULL);
+
+assert_se(strv_push_prepend(&a, strdup("first")) >= 0);
+assert_se(streq(a[0], "first"));
+assert_se(streq(a[1], "foo"));
+assert_se(streq(a[2], "bar"));
+assert_se(streq(a[3], "three"));
+assert_se(!a[4]);
+
+assert_se(strv_consume_prepend(&a, strdup("first2")) >= 0);
+assert_se(streq(a[0], "first2"));
+assert_se(streq(a[1], "first"));
+assert_se(streq(a[2], "foo"));
+assert_se(streq(a[3], "bar"));
+assert_se(streq(a[4], "three"));
+assert_se(!a[5]);
+}
+
 int main(int argc, char *argv[]) {
 test_specifier_printf();
 test_strv_foreach();
@@ -423,6 +460,7 @@ int main(int argc, char *argv[]) {
 test_strv_foreach_pair();
 test_strv_find();
 test_strv_find_prefix();
+test_strv_find_startswith();
 test_strv_join();
 
 test_strv_quote_unquote(input_table_multiple, "\"one\" \"two\" 
\"three\"");
@@ -462,6 +500,7 @@ int main(int argc, char *argv[]) {
 test_strv_extend();
 test_strv_extendf();
 test_strv_from_stdarg_alloca();
+test_strv_push_prepend();
 
 return 0;
 }
-- 
2.1.3

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

[systemd-devel] [PATCH] tests: add tests for fileio.c

[Ronny Chevalier]

add tests for the following functions:
- write_string_file_no_create
- load_env_file_pairs
---
 src/test/test-fileio.c | 63 ++
 1 file changed, 63 insertions(+)

diff --git a/src/test/test-fileio.c b/src/test/test-fileio.c
index 7e7b4ac..a713abd 100644
--- a/src/test/test-fileio.c
+++ b/src/test/test-fileio.c
@@ -331,6 +331,23 @@ static void test_write_string_file(void) {
 unlink(fn);
 }
 
+static void test_write_string_file_no_create(void) {
+char fn[] = "/tmp/test-write_string_file_no_create-XX";
+_cleanup_close_ int fd;
+char buf[64] = {0};
+
+fd = mkostemp_safe(fn, O_RDWR);
+assert_se(fd >= 0);
+
+
assert_se(write_string_file_no_create("/a/file/which/does/not/exists/i/guess", 
"boohoo") < 0);
+assert_se(write_string_file_no_create(fn, "boohoo") == 0);
+
+assert_se(read(fd, buf, sizeof(buf)));
+assert_se(streq(buf, "boohoo\n"));
+
+unlink(fn);
+}
+
 static void test_sendfile_full(void) {
 char in_fn[] = "/tmp/test-sendfile_full-XX";
 char out_fn[] = "/tmp/test-sendfile_full-XX";
@@ -355,6 +372,50 @@ static void test_sendfile_full(void) {
 unlink(out_fn);
 }
 
+static void test_load_env_file_pairs(void) {
+char fn[] = "/tmp/test-load_env_file_pairs-XX";
+int fd;
+int r;
+_cleanup_fclose_ FILE *f = NULL;
+_cleanup_strv_free_ char **l = NULL;
+char **k, **v;
+
+fd = mkostemp_safe(fn, O_RDWR);
+assert_se(fd >= 0);
+
+r = write_string_file(fn,
+"NAME=\"Arch Linux\"\n"
+"ID=arch\n"
+"PRETTY_NAME=\"Arch Linux\"\n"
+"ANSI_COLOR=\"0;36\"\n"
+"HOME_URL=\"https://www.archlinux.org/\"\n";
+"SUPPORT_URL=\"https://bbs.archlinux.org/\"\n";
+"BUG_REPORT_URL=\"https://bugs.archlinux.org/\"\n";
+);
+assert_se(r == 0);
+
+f = fdopen(fd, "r");
+assert_se(f);
+
+r = load_env_file_pairs(f, fn, NULL, &l);
+assert_se(r >= 0);
+
+assert_se(strv_length(l) == 14);
+STRV_FOREACH_PAIR(k, v, l) {
+assert_se(STR_IN_SET(*k, "NAME", "ID", "PRETTY_NAME", 
"ANSI_COLOR", "HOME_URL", "SUPPORT_URL", "BUG_REPORT_URL"));
+printf("%s=%s\n", *k, *v);
+if (streq(*k, "NAME")) assert_se(streq(*v, "Arch Linux"));
+if (streq(*k, "ID")) assert_se(streq(*v, "arch"));
+if (streq(*k, "PRETTY_NAME")) assert_se(streq(*v, "Arch 
Linux"));
+if (streq(*k, "ANSI_COLOR")) assert_se(streq(*v, "0;36"));
+if (streq(*k, "HOME_URL")) assert_se(streq(*v, 
"https://www.archlinux.org/";));
+if (streq(*k, "SUPPORT_URL")) assert_se(streq(*v, 
"https://bbs.archlinux.org/";));
+if (streq(*k, "BUG_REPORT_URL")) assert_se(streq(*v, 
"https://bugs.archlinux.org/";));
+}
+
+unlink(fn);
+}
+
 int main(int argc, char *argv[]) {
 log_parse_environment();
 log_open();
@@ -366,7 +427,9 @@ int main(int argc, char *argv[]) {
 test_capeff();
 test_write_string_stream();
 test_write_string_file();
+test_write_string_file_no_create();
 test_sendfile_full();
+test_load_env_file_pairs();
 
 return 0;
 }
-- 
2.1.3

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

[systemd-devel] [PATCH] shared: fix typo

[Ronny Chevalier]

---
 src/shared/capability.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/shared/capability.c b/src/shared/capability.c
index d2b9013..0226542 100644
--- a/src/shared/capability.c
+++ b/src/shared/capability.c
@@ -228,7 +228,7 @@ int drop_privileges(uid_t uid, gid_t gid, uint64_t 
keep_capabilities) {
  * which we want to avoid. */
 
 if (setresgid(gid, gid, gid) < 0) {
-log_error("Failed change group ID: %m");
+log_error("Failed to change group ID: %m");
 return -errno;
 }
 
@@ -244,7 +244,7 @@ int drop_privileges(uid_t uid, gid_t gid, uint64_t 
keep_capabilities) {
 
 r = setresuid(uid, uid, uid);
 if (r < 0) {
-log_error("Failed change user ID: %m");
+log_error("Failed to change user ID: %m");
 return -errno;
 }
 
-- 
2.1.3

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

[systemd-devel] [PATCH 2/4] tests: add test-locale-util

[Ronny Chevalier]

---
 .gitignore  |  1 +
 Makefile.am |  9 ++-
 src/test/test-locale-util.c | 59 +
 3 files changed, 68 insertions(+), 1 deletion(-)
 create mode 100644 src/test/test-locale-util.c

diff --git a/.gitignore b/.gitignore
index 14f1691..7d5f04d 100644
--- a/.gitignore
+++ b/.gitignore
@@ -198,6 +198,7 @@
 /test-libudev
 /test-libudev-sym*
 /test-list
+/test-locale-util
 /test-log
 /test-login
 /test-login-shared
diff --git a/Makefile.am b/Makefile.am
index 3b273d4..552f41a 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -1358,7 +1358,8 @@ tests += \
test-ratelimit \
test-condition-util \
test-uid-range \
-   test-bus-policy
+   test-bus-policy \
+   test-locale-util
 
 EXTRA_DIST += \
test/a.service \
@@ -1502,6 +1503,12 @@ test_async_SOURCES = \
 test_async_LDADD = \
libsystemd-shared.la
 
+test_locale_util_SOURCES = \
+   src/test/test-locale-util.c
+
+test_locale_util_LDADD = \
+   libsystemd-shared.la
+
 test_condition_util_SOURCES = \
src/test/test-condition-util.c
 
diff --git a/src/test/test-locale-util.c b/src/test/test-locale-util.c
new file mode 100644
index 000..1398a3a
--- /dev/null
+++ b/src/test/test-locale-util.c
@@ -0,0 +1,59 @@
+/***
+  This file is part of systemd
+
+  Copyright 2014 Ronny Chevalier
+
+  systemd is free software; you can redistribute it and/or modify it
+  under the terms of the GNU Lesser General Public License as published by
+  the Free Software Foundation; either version 2.1 of the License, or
+  (at your option) any later version.
+
+  systemd is distributed in the hope that it will be useful, but
+  WITHOUT ANY WARRANTY; without even the implied warranty of
+  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+  Lesser General Public License for more details.
+
+  You should have received a copy of the GNU Lesser General Public License
+  along with systemd; If not, see <http://www.gnu.org/licenses/>.
+***/
+
+#include 
+
+#include "locale-util.h"
+#include "strv.h"
+#include "macro.h"
+
+static void test_get_locales(void) {
+_cleanup_strv_free_ char **locales = NULL;
+char **p;
+int r;
+
+r = get_locales(&locales);
+assert_se(r >= 0);
+assert_se(locales);
+
+STRV_FOREACH(p, locales) {
+puts(*p);
+assert_se(locale_is_valid(*p));
+}
+}
+
+static void test_locale_is_valid(void) {
+assert_se(locale_is_valid("en_EN.utf8"));
+assert_se(locale_is_valid("fr_FR.utf8"));
+assert_se(locale_is_valid("fr_FR@euro"));
+assert_se(locale_is_valid("fi_FI"));
+assert_se(locale_is_valid("POSIX"));
+assert_se(locale_is_valid("C"));
+
+assert_se(!locale_is_valid(""));
+assert_se(!locale_is_valid("/usr/bin/foo"));
+assert_se(!locale_is_valid("\x01gar\x02 bage\x03"));
+}
+
+int main(int argc, char *argv[]) {
+test_get_locales();
+test_locale_is_valid();
+
+return 0;
+}
-- 
2.1.3

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

[systemd-devel] [PATCH 4/4] tests: add test-copy

[Ronny Chevalier]

---
 .gitignore   |   1 +
 Makefile.am  |   9 +++-
 src/test/test-copy.c | 115 +++
 3 files changed, 124 insertions(+), 1 deletion(-)
 create mode 100644 src/test/test-copy.c

diff --git a/.gitignore b/.gitignore
index 7d5f04d..23522ed 100644
--- a/.gitignore
+++ b/.gitignore
@@ -159,6 +159,7 @@
 /test-compress-benchmark
 /test-condition-util
 /test-conf-files
+/test-copy
 /test-coredump-vacuum
 /test-daemon
 /test-date
diff --git a/Makefile.am b/Makefile.am
index 552f41a..8cb369f 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -1359,7 +1359,8 @@ tests += \
test-condition-util \
test-uid-range \
test-bus-policy \
-   test-locale-util
+   test-locale-util \
+   test-copy
 
 EXTRA_DIST += \
test/a.service \
@@ -1509,6 +1510,12 @@ test_locale_util_SOURCES = \
 test_locale_util_LDADD = \
libsystemd-shared.la
 
+test_copy_SOURCES = \
+   src/test/test-copy.c
+
+test_copy_LDADD = \
+   libsystemd-shared.la
+
 test_condition_util_SOURCES = \
src/test/test-condition-util.c
 
diff --git a/src/test/test-copy.c b/src/test/test-copy.c
new file mode 100644
index 000..6aa86a0
--- /dev/null
+++ b/src/test/test-copy.c
@@ -0,0 +1,115 @@
+/***
+  This file is part of systemd
+
+  Copyright 2014 Ronny Chevalier
+
+  systemd is free software; you can redistribute it and/or modify it
+  under the terms of the GNU Lesser General Public License as published by
+  the Free Software Foundation; either version 2.1 of the License, or
+  (at your option) any later version.
+
+  systemd is distributed in the hope that it will be useful, but
+  WITHOUT ANY WARRANTY; without even the implied warranty of
+  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+  Lesser General Public License for more details.
+
+  You should have received a copy of the GNU Lesser General Public License
+  along with systemd; If not, see <http://www.gnu.org/licenses/>.
+***/
+
+#include 
+
+#include "copy.h"
+#include "path-util.h"
+#include "fileio.h"
+#include "mkdir.h"
+#include "strv.h"
+#include "macro.h"
+#include "util.h"
+
+static void test_copy_file(void) {
+_cleanup_free_ char *buf = NULL;
+char fn[] = "/tmp/test-copy_file.XX";
+char fn_copy[] = "/tmp/test-copy_file.XX";
+size_t sz = 0;
+int fd;
+
+fd = mkostemp_safe(fn, O_RDWR|O_CLOEXEC);
+assert_se(fd >= 0);
+close(fd);
+
+fd = mkostemp_safe(fn_copy, O_RDWR|O_CLOEXEC);
+assert_se(fd >= 0);
+close(fd);
+
+assert_se(write_string_file(fn, "foo bar bar bar foo") == 0);
+
+assert_se(copy_file(fn, fn_copy, 0, 0644) == 0);
+
+assert_se(read_full_file(fn_copy, &buf, &sz) == 0);
+assert_se(streq(buf, "foo bar bar bar foo\n"));
+
+unlink(fn);
+unlink(fn_copy);
+}
+
+static void test_copy_tree(void) {
+char original_dir[] = "/tmp/test-copy_tree/";
+char copy_dir[] = "/tmp/test-copy_tree-copy/";
+char **files = STRV_MAKE("file", "dir1/file", "dir1/dir2/file", 
"dir1/dir2/dir3/dir4/dir5/file");
+char **links = STRV_MAKE("link", "file",
+ "link2", "dir1/file");
+char **p, **link;
+
+rm_rf_dangerous(copy_dir, false, true, false);
+rm_rf_dangerous(original_dir, false, true, false);
+
+STRV_FOREACH(p, files) {
+char *f = strappenda(original_dir, *p);
+
+assert_se(mkdir_parents(f, 0755) >= 0);
+assert_se(write_string_file(f, "file") == 0);
+}
+
+STRV_FOREACH_PAIR(link, p, links) {
+char *f = strappenda(original_dir, *p);
+char *l = strappenda(original_dir, *link);
+
+assert_se(mkdir_parents(l, 0755) >= 0);
+assert_se(symlink(f, l) == 0);
+}
+
+assert_se(copy_tree(original_dir, copy_dir, true) == 0);
+
+STRV_FOREACH(p, files) {
+_cleanup_free_ char *buf = NULL;
+size_t sz = 0;
+char *f = strappenda(copy_dir, *p);
+
+assert_se(access(f, F_OK) == 0);
+assert_se(read_full_file(f, &buf, &sz) == 0);
+assert_se(streq(buf, "file\n"));
+}
+
+STRV_FOREACH_PAIR(link, p, links) {
+_cleanup_free_ char *target = NULL;
+char *f = strappenda(original_dir, *p);
+char *l = strappenda(copy_dir, *link);
+
+assert_se(readlink_and_canonicalize(l, &target) == 0);
+assert_se(path_equal(f, target));
+}
+
+assert_se(copy_tree(original

[systemd-devel] [PATCH 3/4] tests: add missing entry for LocalVariable to test-tables

[Ronny Chevalier]

---
 src/test/test-tables.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/test/test-tables.c b/src/test/test-tables.c
index 907958e..2138442 100644
--- a/src/test/test-tables.c
+++ b/src/test/test-tables.c
@@ -48,6 +48,7 @@
 #include "link-config.h"
 #include "bus-policy.h"
 #include "journald-server.h"
+#include "locale-util.h"
 
 #include "test-tables.h"
 
@@ -116,6 +117,7 @@ int main(int argc, char **argv) {
 test_table(unit_file_state, UNIT_FILE_STATE);
 test_table(unit_load_state, UNIT_LOAD_STATE);
 test_table(unit_type, UNIT_TYPE);
+test_table(locale_variable, VARIABLE_LC);
 
 test_table_sparse(object_compressed, OBJECT_COMPRESSED);
 
-- 
2.1.3

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

[systemd-devel] [PATCH 1/4] shared: add missing includes

[Ronny Chevalier]

---
 src/shared/copy.h| 3 +++
 src/shared/locale-util.h | 4 
 2 files changed, 7 insertions(+)

diff --git a/src/shared/copy.h b/src/shared/copy.h
index 0bf2598..6b93107 100644
--- a/src/shared/copy.h
+++ b/src/shared/copy.h
@@ -21,6 +21,9 @@
   along with systemd; If not, see .
 ***/
 
+#include 
+#include 
+
 int copy_file(const char *from, const char *to, int flags, mode_t mode);
 int copy_tree(const char *from, const char *to, bool merge);
 int copy_bytes(int fdf, int fdt, off_t max_bytes);
diff --git a/src/shared/locale-util.h b/src/shared/locale-util.h
index d7a3e4f..e48aa3d 100644
--- a/src/shared/locale-util.h
+++ b/src/shared/locale-util.h
@@ -21,6 +21,10 @@
   along with systemd; If not, see .
 ***/
 
+#include 
+
+#include "macro.h"
+
 typedef enum LocaleVariable {
 /* We don't list LC_ALL here on purpose. People should be
  * using LANG instead. */
-- 
2.1.3

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

[systemd-devel] [PATCH] systemctl: add edit verb

[Ronny Chevalier]

It helps editing units by either creating a drop-in file, like
/etc/systemd/system/my.service.d/override.conf, or by copying the
original unit from /usr/lib/systemd/ to /etc/systemd/ if the --full
option is specified.

It invokes an editor on temporary files related to the unit files and
if the editor exited successfully, then it renames the temporary files
to their original names (e.g. my.service or override.conf) and
daemon-reload is invoked.

If the temporary file is empty the modification is canceled.

See https://bugzilla.redhat.com/show_bug.cgi?id=906824
---

lookup_paths_init does not concatenate root_dir, so I added a path_join with 
arg_root

 TODO  |   4 +-
 man/less-variables.xml|   4 +-
 man/systemctl.xml |  64 +-
 src/systemctl/systemctl.c | 525 +-
 4 files changed, 587 insertions(+), 10 deletions(-)

diff --git a/TODO b/TODO
index abe89b7..1cbedd4 100644
--- a/TODO
+++ b/TODO
@@ -84,7 +84,7 @@ Features:
 
 * systemctl: if it fails, show log output?
 
-* maybe add "systemctl edit" that copies unit files from 
/usr/lib/systemd/system to /etc/systemd/system and invokes vim on them
+* systemctl edit: add commented help text to the end, like git commit
 
 * maybe introduce AssertXYZ= similar to ConditionXYZ= that causes a unit to 
fail (instead of skipping it) if some condition is not true...
 
@@ -776,7 +776,7 @@ External:
 
 * zsh shell completion:
   -   - should complete options, but currently does not
-  - systemctl add-wants,add-requires
+  - systemctl add-wants,add-requires, edit
 
 
 Regularly:
diff --git a/man/less-variables.xml b/man/less-variables.xml
index 09cbd42..0fb4d7f 100644
--- a/man/less-variables.xml
+++ b/man/less-variables.xml
@@ -6,7 +6,7 @@
 Environment
 
 
-
+
 $SYSTEMD_PAGER
 
 Pager to use when
@@ -17,7 +17,7 @@
 --no-pager.
 
 
-
+
 $SYSTEMD_LESS
 
 Override the default
diff --git a/man/systemctl.xml b/man/systemctl.xml
index 7cbaa6c..26f5235 100644
--- a/man/systemctl.xml
+++ b/man/systemctl.xml
@@ -465,7 +465,7 @@ along with systemd; If not, see 
.
 
 
   When used with enable,
-  disable,
+  disable, edit,
   (and related commands), make changes only temporarily, so
   that they are lost on the next reboot. This will have the
   effect that changes are not made in subdirectories of
@@ -1150,6 +1150,43 @@ kobject-uevent 1 systemd-udevd-kernel.socket 
systemd-udevd.service
 default.target to the given unit.
   
 
+
+
+  edit 
NAME...
+
+  
+Edit a drop-in snippet or a whole replacement file if
+--full is specified, to extend or override the
+specified unit.
+
+Depending on whether --system (the default),
+--user, or --global is specified,
+this create a drop-in file for each units either for the system,
+for the calling user or for all futures logins of all users. Then
+the editor (see section "Environment" below) is invoked on 
temporary
+files which will be saved as their corresponding files if the 
editor
+exited successfully.
+
+If --full is specified, this will copy the
+original units instead of creating drop-in files.
+
+If --runtime is specified, the changes will
+be made temporarily in /run and they will be
+lost on the next reboot.
+
+If the temporary file is empty the modification of the 
related
+unit is canceled
+
+After the units have been edited, the systemd configuration 
is
+reloaded (in a way that is equivalent to 
daemon-reload),
+but it does not restart or reload the units.
+
+Note that this command cannot be used to remotely edit units
+and that you cannot temporarily edit units which are in
+/etc since they take precedence over
+/run.
+  
+
   
 
 
@@ -1608,7 +1645,28 @@ kobject-uevent 1 systemd-udevd-kernel.socket 
systemd-udevd.service
 code otherwise.
   
 
-  
+  
+Environment
+
+
+  
+$SYSTEMD_EDITOR
+
+Editor to use when editing units; overrides
+$EDITOR and $VISUAL. If neither
+$SYSTEMD_EDITOR nor $EDITOR nor
+$VISUAL are present or if it is set to an empty
+string or if their execution failed, systemctl will try to execute well
+known editors in this order:
+
nano1,
+
vim1,
+
vi1.
+
+  
+
+
+
+  
 
   
 See Also
@@ -1621,7 +1679,7 @@ kobject-uevent 1 systemd-udevd-kernel.socket 
systemd

[systemd-devel] [PATCH] remove references of readahead

[Ronny Chevalier]

---
 .gitignore | 1 -
 README | 1 -
 TODO   | 7 ---
 3 files changed, 9 deletions(-)

diff --git a/.gitignore b/.gitignore
index 0b71f09..14f1691 100644
--- a/.gitignore
+++ b/.gitignore
@@ -101,7 +101,6 @@
 /systemd-quotacheck
 /systemd-random-seed
 /systemd-rc-local-generator
-/systemd-readahead
 /systemd-remount-api-vfs
 /systemd-remount-fs
 /systemd-reply-password
diff --git a/README b/README
index 99b66a8..1440367 100644
--- a/README
+++ b/README
@@ -30,7 +30,6 @@ AUTHOR:
 
 LICENSE:
 LGPLv2.1+ for all code
-- except sd-readahead.[ch] which is MIT
 - except src/shared/MurmurHash2.c which is Public Domain
 - except src/shared/siphash24.c which is CC0 Public Domain
 - except src/journal/lookup3.c which is Public Domain
diff --git a/TODO b/TODO
index b07d664..abe89b7 100644
--- a/TODO
+++ b/TODO
@@ -646,13 +646,6 @@ Features:
 
 * and a dbus call to generate target from current state
 
-* readahead:
-  - drop /.readahead on bigger upgrades with yum
-  - move readahead files into /var (look for them with .path units?)
-  - readahead: use BTRFS_IOC_DEFRAG_RANGE instead of BTRFS_IOC_DEFRAG ioctl, 
with START_IO
-  - readahead: when bumping /sys readahead variable save mtime and compare 
later to detect changes
-  - readahead: make use of EXT4_IOC_MOVE_EXT, as used by 
http://e4rat.sourceforge.net/
-
 * GC unreferenced jobs (such as .device jobs)
 
 * write blog stories about:
-- 
2.1.2

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

[systemd-devel] [PATCH] systemctl: add edit verb

[Ronny Chevalier]

It helps editing units by either creating a drop-in file, like
/etc/systemd/system/my.service.d/override.conf, or by copying the
original unit from /usr/lib/systemd/ to /etc/systemd/ if the --full
option is specified.

It invokes an editor on temporary files related to the unit files and
if the editor exited successfully, then it renames the temporary files
to their original names (e.g. my.service or override.conf) and
daemon-reload is invoked.

If the temporary file is empty the modification is canceled.

See https://bugzilla.redhat.com/show_bug.cgi?id=906824
---
changes:
  * --runtime is handled
  * changes are made atomically by creating temporary files
  * man page improved
  * no heap allocation for execlp editor
  * arg_root is handled properly

 TODO  |   4 +-
 man/less-variables.xml|   4 +-
 man/systemctl.xml |  64 +-
 src/systemctl/systemctl.c | 525 +-
 4 files changed, 587 insertions(+), 10 deletions(-)

diff --git a/TODO b/TODO
index abe89b7..1cbedd4 100644
--- a/TODO
+++ b/TODO
@@ -84,7 +84,7 @@ Features:
 
 * systemctl: if it fails, show log output?
 
-* maybe add "systemctl edit" that copies unit files from 
/usr/lib/systemd/system to /etc/systemd/system and invokes vim on them
+* systemctl edit: add commented help text to the end, like git commit
 
 * maybe introduce AssertXYZ= similar to ConditionXYZ= that causes a unit to 
fail (instead of skipping it) if some condition is not true...
 
@@ -776,7 +776,7 @@ External:
 
 * zsh shell completion:
   -   - should complete options, but currently does not
-  - systemctl add-wants,add-requires
+  - systemctl add-wants,add-requires, edit
 
 
 Regularly:
diff --git a/man/less-variables.xml b/man/less-variables.xml
index 09cbd42..0fb4d7f 100644
--- a/man/less-variables.xml
+++ b/man/less-variables.xml
@@ -6,7 +6,7 @@
 Environment
 
 
-
+
 $SYSTEMD_PAGER
 
 Pager to use when
@@ -17,7 +17,7 @@
 --no-pager.
 
 
-
+
 $SYSTEMD_LESS
 
 Override the default
diff --git a/man/systemctl.xml b/man/systemctl.xml
index 7cbaa6c..26f5235 100644
--- a/man/systemctl.xml
+++ b/man/systemctl.xml
@@ -465,7 +465,7 @@ along with systemd; If not, see 
.
 
 
   When used with enable,
-  disable,
+  disable, edit,
   (and related commands), make changes only temporarily, so
   that they are lost on the next reboot. This will have the
   effect that changes are not made in subdirectories of
@@ -1150,6 +1150,43 @@ kobject-uevent 1 systemd-udevd-kernel.socket 
systemd-udevd.service
 default.target to the given unit.
   
 
+
+
+  edit 
NAME...
+
+  
+Edit a drop-in snippet or a whole replacement file if
+--full is specified, to extend or override the
+specified unit.
+
+Depending on whether --system (the default),
+--user, or --global is specified,
+this create a drop-in file for each units either for the system,
+for the calling user or for all futures logins of all users. Then
+the editor (see section "Environment" below) is invoked on 
temporary
+files which will be saved as their corresponding files if the 
editor
+exited successfully.
+
+If --full is specified, this will copy the
+original units instead of creating drop-in files.
+
+If --runtime is specified, the changes will
+be made temporarily in /run and they will be
+lost on the next reboot.
+
+If the temporary file is empty the modification of the 
related
+unit is canceled
+
+After the units have been edited, the systemd configuration 
is
+reloaded (in a way that is equivalent to 
daemon-reload),
+but it does not restart or reload the units.
+
+Note that this command cannot be used to remotely edit units
+and that you cannot temporarily edit units which are in
+/etc since they take precedence over
+/run.
+  
+
   
 
 
@@ -1608,7 +1645,28 @@ kobject-uevent 1 systemd-udevd-kernel.socket 
systemd-udevd.service
 code otherwise.
   
 
-  
+  
+Environment
+
+
+  
+$SYSTEMD_EDITOR
+
+Editor to use when editing units; overrides
+$EDITOR and $VISUAL. If neither
+$SYSTEMD_EDITOR nor $EDITOR nor
+$VISUAL are present or if it is set to an empty
+string or if their execution failed, systemctl will try to execute well
+known editors in this order:
+
nano1,
+
vim1,
+
vi1.
+
+  
+
+
+  

[systemd-devel] [PATCH] NEWS: fix typos

[Ronny Chevalier]

---
 NEWS | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/NEWS b/NEWS
index c5f0db2..f4afcf6 100644
--- a/NEWS
+++ b/NEWS
@@ -70,7 +70,7 @@ CHANGES WITH 217:
 * Udev rules can now remove tags on devices with TAG-="foobar".
 
 * systemd's readahead implementation has been removed. In many
-  circumstatances it didn't give expected benefits even for
+  circumstances it didn't give expected benefits even for
   rotational disk drives and was becoming less relevant in the
   age of SSDs. As none of the developers has been using
   rotating media anymore, and nobody stepped up to actively
@@ -133,7 +133,7 @@ CHANGES WITH 217:
   rootfstype= but allow mounting a specific file system to
   /usr.
 
-* The $NOTIFY_SOCKET is now also passed to control processesof
+* The $NOTIFY_SOCKET is now also passed to control processes of
   services, not only the main process.
 
 * This version reenables support for fsck's -l switch. This
-- 
2.1.2

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

Re: [systemd-devel] [PATCH v3] systemctl: add edit verb

[Ronny Chevalier]

2014-10-22 11:15 GMT+02:00 Lennart Poettering :
> On Wed, 22.10.14 01:48, Zbigniew Jędrzejewski-Szmek (zbys...@in.waw.pl) wrote:
>
>> On Sat, Oct 18, 2014 at 06:30:02PM +0200, Ronny Chevalier wrote:
>> > It helps editing units by either creating a drop-in file, like
>> > /etc/systemd/system/my.service.d/amendments.conf, or by copying the
>> > original unit from /usr/lib/systemd/ to /etc/systemd/ if the --full
>> > option is specified. Then it invokes an editor to the related files
>> > and daemon-reload is invoked when the editor exited successfully.
>>
>> Hm, this sequence doesn't sound right. A temporary file should be
>> created, edited, and then atomically put in place, iff the editor
>> exits successfully.  I think we should follow in the footsteps of git
>> here... and abort if the editor exits with an error.
>
> Hmm, don't smart editors do this anyway when saving a file?
Yes, but what Zbigniew meant, is that I mkdir_p and in the case of
--full I copy the file. So if the editor doesn't exit successfully, we
will not delete the dirs and files.

>
>> I'm not sure abou the name 'amendments.conf'. Wouldn't 'local.conf'
>> be more idiomatic, and also easier to type?
>
> I was thinking about this too, and I wanted to propose "override.conf"
> instead?
>
> The word "amendment" I only know from the US constitution...
Ok, both are fine for me.

>
> 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] systemctl: add edit verb

[Ronny Chevalier]

2014-10-13 13:40 GMT+02:00 David Herrmann :
> Hi
Hi,

>
> On Sat, Oct 11, 2014 at 8:17 PM, Daniel Buch  wrote:
>> Nice, I was in the process of implementing this. Looks good to me. But I
>> think it would be better to use "vi" instead of "vim" if no &editor is set.
>> Vim is not installed on every system as default but vi is most likely.
>
> I'd prefer doing nothing. "vi" has quite different behavior than
> "vim", especially in Ex mode. And for people uncomfortable with 'vi'
> it's even worth. And everyone should have set EDITOR anyway..
Yes, the first version that I made just shown an error saying that you
should set SYSTEMD_EDITOR or EDITOR, but
the RFE suggested using vim, and since no one was against it, I
thought this was common to use vi/vim when EDITOR is not set.

And, even as a vim user, I agree that for many people vi/vim is really
not a good choice.

I will resend the patch with the modification.

Thanks !

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

Re: [systemd-devel] [PATCH] Add timesync-wait tool

[Ronny Chevalier]

2014-10-23 21:24 GMT+02:00 Łukasz Stelmach :
> ---
>  src/timesync/timesync-wait.c | 43 +++
>  1 file changed, 43 insertions(+)
>  create mode 100644 src/timesync/timesync-wait.c
>
> I am afraid TFD_TIMER_CANCEL_ON_SET doesn't help much here. You can
> watch for time changes but it is not the moment adjtimex() starts to
> return TIME_OK and STA_UNSYNC flag unset.
>
> Where would you like this to be patched in?
>
> diff --git a/src/timesync/timesync-wait.c b/src/timesync/timesync-wait.c
> new file mode 100644
> index 000..9648b09
> --- /dev/null
> +++ b/src/timesync/timesync-wait.c
> @@ -0,0 +1,43 @@
> +/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
> +
> +/***
> +  This file is part of systemd.
> +
> +  Copyright 2014 Łukasz Stelmach
> +
> +  systemd is free software; you can redistribute it and/or modify it
> +  under the terms of the GNU Lesser General Public License as published by
> +  the Free Software Foundation; either version 2.1 of the License, or
> +  (at your option) any later version.
> +
> +  systemd is distributed in the hope that it will be useful, but
> +  WITHOUT ANY WARRANTY; without even the implied warranty of
> +  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> +  Lesser General Public License for more details.
> +
> +  You should have received a copy of the GNU Lesser General Public License
> +  along with systemd; If not, see .
> +***/
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +int main(int argc, char *argv[]) {
You should add a minimal parse_argv() like
systemd-networkd-wait-online in order to handle --help and --version,
like all systemd binaries.

> +struct timex tbuf;
> +int r;
> +
> +memset(&tbuf, 0, sizeof(tbuf));
> +r = adjtimex(&tbuf);
> +
> +while (r != TIME_OK) {
> +sleep(1);
> +/* Unfortunately there seem to be no other way than
> +polling to get this information. */
> +memset(&tbuf, 0, sizeof(tbuf));
> +r = adjtimex(&tbuf);
> +}
> +
> +return 0;
> +}
> --
> 2.0.4
You should add the .service and a minimal man-page for this tool.

Also, you should add a little summary in the commit message explaining
why this is needed and/or a link to the discussion:
http://lists.freedesktop.org/archives/systemd-devel/2014-October/024346.html
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH v3] systemctl: add edit verb

[Ronny Chevalier]

2014-10-22 18:30 GMT+02:00 Lennart Poettering :
> On Wed, 22.10.14 18:28, Ronny Chevalier (chevalier.ro...@gmail.com) wrote:
>
>> >> I'm not sure abou the name 'amendments.conf'. Wouldn't 'local.conf'
>> >> be more idiomatic, and also easier to type?
>> >
>> > I was thinking about this too, and I wanted to propose "override.conf"
>> > instead?
>> >
>> > The word "amendment" I only know from the US constitution...
>> Ok, both are fine for me.
>
> I'd prefer "override" still...
Yeah I misspoke, I meant "local" and "override" were fine. I'll set it
to "override.conf" ;)

>
> 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 v3] systemctl: add edit verb

[Ronny Chevalier]

2014-10-22 2:13 GMT+02:00 Ronny Chevalier :
> 2014-10-22 1:48 GMT+02:00 Zbigniew Jędrzejewski-Szmek :
>> On Sat, Oct 18, 2014 at 06:30:02PM +0200, Ronny Chevalier wrote:
>>> It helps editing units by either creating a drop-in file, like
>>> /etc/systemd/system/my.service.d/amendments.conf, or by copying the
>>> original unit from /usr/lib/systemd/ to /etc/systemd/ if the --full
>>> option is specified. Then it invokes an editor to the related files
>>> and daemon-reload is invoked when the editor exited successfully.
>>
>> Hm, this sequence doesn't sound right. A temporary file should be
>> created, edited, and then atomically put in place, iff the editor
>> exits successfully.  I think we should follow in the footsteps of git
>> here... and abort if the editor exits with an error.
> You are right, I will rework it this way.
>
>>
>> I'm not sure abou the name 'amendments.conf'. Wouldn't 'local.conf'
>> be more idiomatic, and also easier to type?
> Ok
>
>>
>>> See https://bugzilla.redhat.com/show_bug.cgi?id=906824
>>> ---
>>>  TODO  |   2 -
>>>  man/journalctl.xml|   6 +-
>>>  man/less-variables.xml|  40 +++--
>>>  man/localectl.xml |   6 +-
>>>  man/loginctl.xml  |   6 +-
>>>  man/machinectl.xml|   6 +-
>>>  man/systemctl.xml |  49 +-
>>>  man/systemd-analyze.xml   |   6 +-
>>>  man/timedatectl.xml   |   6 +-
>>>  src/systemctl/systemctl.c | 394 
>>> +-
>>>  10 files changed, 488 insertions(+), 33 deletions(-)
>> There's no need to mangle all the xml files. It is possible
>> to include specific parts of a file. See how standard-options.xml
>> incorporated whole, and sometimes just specific parts using
>> .
> Ok I will look into this.
>
>>
>>> -
>>> +
>>> +Environment
>>> +
>>> +
>>> +
>>>
>>
>>> +
>>> +  edit 
>>> NAME...
>>> +
>>> +  
>>> +Edit one or more unit files, as specified on the command
>>> +line.
>> This wording is misleading, because the unit file actually will not be 
>> *edited*,
>> but extended in the normal case where --full is not used.
>>
>> I'm missing an explanatory sentence here, something like "An editor will be 
>> launched
>> to edit a drop-in snippet (or a whole replacement file if --full is used), 
>> to extend
>> or override the specified unit." Then the next paragraph about 
>> --system/--user/--global
>> will be more natural.
> You are right, it's better this way.
>
>>
>>> +
>>> +Depending on whether --system (the 
>>> default),
>>> +--user, or --global is 
>>> specified,
>>> +this create a drop-in file for each units either for the 
>>> system,
>>> +for the calling user or for all futures logins of all users. 
>>> Then
>>> +the editor is invoked on them (see section "Environment" 
>>> below).
>>> +
>>> +If --full is specified, this will copy 
>>> the original
>>> +units instead of creating drop-in files.
>>> +
>>> +After the units have been edited, the systemd 
>>> configuration is
>>> +reloaded (in a way that is equivalent to 
>>> daemon-reload),
>>> +but it does not restart or reload the units.
>>> +
>>> +Note that this command cannot be used with 
>>> --runtime or
>>> +to remotely edit units.
>>> +  
>>> +
>>>
>>>  
>>>
>>
>>> +
>>> +  
>>> +$SYSTEMD_EDITOR
>>> +
>>> +Editor to use when editing units; overrides
>>> +$EDITOR and $VISUAL. If 
>>> neither
>>> +$SYSTEMD_EDITOR nor $EDITOR 
>>> nor
>>> +$VISUAL are present or if it is set to an empty
>>> +string or if their execution failed, systemctl will try to execute 
>>> well
>>> +known editors in this order:
>>> +
>>> nano1,
>>> +
>>> vim1,
>>> +
>>> vi1.
&

Re: [systemd-devel] [PATCH v3] systemctl: add edit verb

[Ronny Chevalier]

2014-10-22 1:48 GMT+02:00 Zbigniew Jędrzejewski-Szmek :
> On Sat, Oct 18, 2014 at 06:30:02PM +0200, Ronny Chevalier wrote:
>> It helps editing units by either creating a drop-in file, like
>> /etc/systemd/system/my.service.d/amendments.conf, or by copying the
>> original unit from /usr/lib/systemd/ to /etc/systemd/ if the --full
>> option is specified. Then it invokes an editor to the related files
>> and daemon-reload is invoked when the editor exited successfully.
>
> Hm, this sequence doesn't sound right. A temporary file should be
> created, edited, and then atomically put in place, iff the editor
> exits successfully.  I think we should follow in the footsteps of git
> here... and abort if the editor exits with an error.
You are right, I will rework it this way.

>
> I'm not sure abou the name 'amendments.conf'. Wouldn't 'local.conf'
> be more idiomatic, and also easier to type?
Ok

>
>> See https://bugzilla.redhat.com/show_bug.cgi?id=906824
>> ---
>>  TODO  |   2 -
>>  man/journalctl.xml|   6 +-
>>  man/less-variables.xml|  40 +++--
>>  man/localectl.xml |   6 +-
>>  man/loginctl.xml  |   6 +-
>>  man/machinectl.xml|   6 +-
>>  man/systemctl.xml |  49 +-
>>  man/systemd-analyze.xml   |   6 +-
>>  man/timedatectl.xml   |   6 +-
>>  src/systemctl/systemctl.c | 394 
>> +-
>>  10 files changed, 488 insertions(+), 33 deletions(-)
> There's no need to mangle all the xml files. It is possible
> to include specific parts of a file. See how standard-options.xml
> incorporated whole, and sometimes just specific parts using
> .
Ok I will look into this.

>
>> -
>> +
>> +Environment
>> +
>> +
>> +
>>
>
>> +
>> +  edit 
>> NAME...
>> +
>> +  
>> +Edit one or more unit files, as specified on the command
>> +line.
> This wording is misleading, because the unit file actually will not be 
> *edited*,
> but extended in the normal case where --full is not used.
>
> I'm missing an explanatory sentence here, something like "An editor will be 
> launched
> to edit a drop-in snippet (or a whole replacement file if --full is used), to 
> extend
> or override the specified unit." Then the next paragraph about 
> --system/--user/--global
> will be more natural.
You are right, it's better this way.

>
>> +
>> +Depending on whether --system (the 
>> default),
>> +--user, or --global is 
>> specified,
>> +this create a drop-in file for each units either for the system,
>> +for the calling user or for all futures logins of all users. 
>> Then
>> +the editor is invoked on them (see section "Environment" 
>> below).
>> +
>> +If --full is specified, this will copy 
>> the original
>> +units instead of creating drop-in files.
>> +
>> +After the units have been edited, the systemd 
>> configuration is
>> +reloaded (in a way that is equivalent to 
>> daemon-reload),
>> +but it does not restart or reload the units.
>> +
>> +Note that this command cannot be used with 
>> --runtime or
>> +to remotely edit units.
>> +  
>> +
>>
>>  
>>
>
>> +
>> +  
>> +$SYSTEMD_EDITOR
>> +
>> +Editor to use when editing units; overrides
>> +$EDITOR and $VISUAL. If 
>> neither
>> +$SYSTEMD_EDITOR nor $EDITOR 
>> nor
>> +$VISUAL are present or if it is set to an empty
>> +string or if their execution failed, systemctl will try to execute 
>> well
>> +known editors in this order:
>> +
>> nano1,
>> +
>> vim1,
>> +
>> vi1.
>> +
>> +  
>> +
>> +
>> +  
>>
>>
>>  See Also
>> @@ -1572,7 +1617,7 @@ kobject-uevent 1 systemd-udevd-kernel.socket 
>> systemd-udevd.service
>>
>> systemd.resource-management5,
>>
>> systemd.special7,
>>> project='man-pages'>wall1,
>> -  
>> systemd.preset5
>> +  
>> systemd.preset5,
>>
>> glob7
>>  
>>
&

Re: [systemd-devel] [PATCH v3] systemctl: add edit verb

[Ronny Chevalier]

2014-10-21 0:51 GMT+02:00 Lennart Poettering :
> On Sat, 18.10.14 18:30, Ronny Chevalier (chevalier.ro...@gmail.com) wrote:
>
> Looks pretty good. A few comments.
>
>> +
>> +static int unit_file_copy_if_needed(const char *unit_name, const char 
>> *fragment_path, char **new_path) {
>> +char *tmp_path;
>> +int r;
>> +
>> +assert(fragment_path);
>> +assert(unit_name);
>> +assert(new_path);
>> +
>> +/* If it's a unit for the --user scope there is no need to copy it, 
>> it's already in the right directory.
>> + * Same if this is --system/--global scope and the file is in 
>> {SYSTEM,USER}_CONFIG_UNIT_PATH
>> + */
>> +if (arg_scope == UNIT_FILE_USER
>> +|| startswith(fragment_path, SYSTEM_CONFIG_UNIT_PATH)
>> +|| startswith(fragment_path, USER_CONFIG_UNIT_PATH)) {
>> +*new_path = strdup(fragment_path);
>> +if (!*new_path)
>> +return log_oom();
>> +return 0;
>> +}
>> +
>> +switch (arg_scope) {
>> +case UNIT_FILE_SYSTEM:
>> +tmp_path = strjoin(SYSTEM_CONFIG_UNIT_PATH, "/", 
>> unit_name, NULL);
>> +break;
>> +case UNIT_FILE_GLOBAL:
>> +tmp_path = strjoin(USER_CONFIG_UNIT_PATH, "/", 
>> unit_name, NULL);
>> +break;
>> +default:
>> +assert_not_reached("Invalid scope");
>> +}
>> +if (!tmp_path)
>> +return log_oom();
>> +
>> +if (access(tmp_path, F_OK) == 0) {
>> +char response;
>> +
>> +r = ask_char(&response, "yn", "%s already exists, are you 
>> sure to overwrite it with %s? [(y)es, (n)o] ", tmp_path, fragment_path);
>> +if (r < 0) {
>> +free(tmp_path);
>> +return r;
>> +}
>> +if (response != 'y') {
>> +log_warning("%s ignored", unit_name);
>> +free(tmp_path);
>> +return -1;
>> +}
>> +}
>> +
>> +r = mkdir_parents(tmp_path, 0755);
>> +if (r < 0) {
>> +log_error("Failed to create directories for %s: %s", 
>> tmp_path, strerror(-r));
>> +free(tmp_path);
>> +return r;
>> +}
>> +
>> +r = copy_file(fragment_path, tmp_path, 0, 0644);
>> +if (r < 0) {
>> +log_error("Failed to copy %s to %s: %s", fragment_path, 
>> tmp_path, strerror(-r));
>> +free(tmp_path);
>> +return r;
>> +}
>> +
>> +*new_path = tmp_path;
>> +
>> +return 0;
>> +}
>> +
>> +static int get_editors(char ***editors) {
>> +char **tmp_editors = strv_new("nano", "vim", "vi", NULL);
>
> Please avoid calling functions and declaring variables in one line.
>
> Also, there's an OOM check missing for this line.
>
>> +char *editor;
>> +
>> +/* SYSTEMD_EDITOR takes precedence over EDITOR which takes 
>> precedence over VISUAL
>> + * If neither SYSTEMD_EDITOR nor EDITOR nor VISUAL are present,
>> + * we try to execute well known editors
>> + */
>> +editor = getenv("SYSTEMD_EDITOR");
>> +if (!editor)
>> +editor = getenv("EDITOR");
>> +if (!editor)
>> +editor = getenv("VISUAL");
>> +
>> +if (editor) {
>> +int r;
>> +
>> +editor = strdup(editor);
>> +if (!editor)
>> +return log_oom();
>> +
>> +r = strv_consume_prepend(&tmp_editors, editor);
>> +if (r < 0)
>> +return log_oom();
>> +}
>> +
>> +*editors = tmp_editors;
>> +
>> +return 0;
>> +}
>
> Hmm, I don't like this bit. Instead of figuring out the editor in one
> step, and then executing it, I'd really prefer if we'd try to execute
> 

Re: [systemd-devel] [PATCH v3] systemctl: add edit verb

[Ronny Chevalier]

2014-10-21 11:09 GMT+02:00 Lennart Poettering :
> On Sat, 18.10.14 18:30, Ronny Chevalier (chevalier.ro...@gmail.com) wrote:
>
> Three more suggestions after thinking about this a bit more...
>
>> +static int unit_file_drop_in(const char *unit_name, const char 
>> *config_home, char **new_path) {
>> +char *tmp_path;
>> +int r;
>> +
>> +assert(unit_name);
>> +assert(new_path);
>> +
>> +switch (arg_scope) {
>> +case UNIT_FILE_SYSTEM:
>> +tmp_path = strjoin(SYSTEM_CONFIG_UNIT_PATH, "/", 
>> unit_name, ".d/amendments.conf", NULL);
>> +break;
>> +case UNIT_FILE_GLOBAL:
>> +tmp_path = strjoin(USER_CONFIG_UNIT_PATH, "/", 
>> unit_name, ".d/amendments.conf", NULL);
>> +break;
>> +case UNIT_FILE_USER:
>> +assert(config_home);
>> +tmp_path = strjoin(config_home, "/", unit_name, 
>> ".d/amendments.conf", NULL);
>> +break;
>> +default:
>> +assert_not_reached("Invalid scope");
>> +}
>> +if (!tmp_path)
>> +return log_oom();
>> +
>> +r = mkdir_parents(tmp_path, 0755);
>> +if (r < 0) {
>> +log_error("Failed to create directories for %s: %s", 
>> tmp_path, strerror(-r));
>> +free(tmp_path);
>> +return r;
>> +}
>> +
>> +*new_path = tmp_path;
>> +
>> +return 0;
>
> I think we should really take some inspiration here from how git
> prepares the editor when asking for commit messages: adding a really
> useful commented help text to the end of the new file to edit would be
> really awesome. More specifically, I think we should include an output
> equivalent to "systemctl cat"'s add the end, commented and prefixed
> with some clear marker that we can then use to remove the bits again
> before saving things.
Yeah It seems nice.

Do you prefer that I add this in the correction of this patch, or with
follow-up patches ?

>
>> +if (arg_scope == UNIT_FILE_USER
>> +|| startswith(fragment_path, SYSTEM_CONFIG_UNIT_PATH)
>> +|| startswith(fragment_path, USER_CONFIG_UNIT_PATH)) {
>
> Please use path_startswith() for this.
Oh I didn't know this one, thanks.

>
>> +if (access(tmp_path, F_OK) == 0) {
>> +char response;
>> +
>> +r = ask_char(&response, "yn", "%s already exists, are you 
>> sure to overwrite it with %s? [(y)es, (n)o] ", tmp_path, fragment_path);
>> +if (r < 0) {
>> +free(tmp_path);
>> +return r;
>> +}
>
> Hmm, when precisely is this actually triggered? I mean, if the file
> already exists in /etc we should just edit it there...
There is a specific case when this can happen.

Imagine you have a unit foobar with
FragmentPath=/usr/lib/systemd/system/foobar.service, and you added
manually /etc/systemd/system/foobar.service without doing
daemon-reload. When avoid_bus() == false, we will get the /usr/***
path, so we need to be sure that the user don't want to overwrite the
one in /etc/***.

We can avoid this by doing daemon-reload before editing, but I'm not
sure this is wanted.

>
> Lennart
>
> --
> Lennart Poettering, Red Hat

Thanks for the review!
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH v3] systemctl: add edit verb

[Ronny Chevalier]

2014-10-21 11:01 GMT+02:00 Lennart Poettering :
> On Tue, 21.10.14 03:01, Ronny Chevalier (chevalier.ro...@gmail.com) wrote:
>
>> >> +static int get_editors(char ***editors) {
>> >> +char **tmp_editors = strv_new("nano", "vim", "vi", NULL);
>> >
>> > Please avoid calling functions and declaring variables in one line.
>> >
>> > Also, there's an OOM check missing for this line.
>> >
>> >> +char *editor;
>> >> +
>> >> +/* SYSTEMD_EDITOR takes precedence over EDITOR which takes 
>> >> precedence over VISUAL
>> >> + * If neither SYSTEMD_EDITOR nor EDITOR nor VISUAL are present,
>> >> + * we try to execute well known editors
>> >> + */
>> >> +editor = getenv("SYSTEMD_EDITOR");
>> >> +if (!editor)
>> >> +editor = getenv("EDITOR");
>> >> +if (!editor)
>> >> +editor = getenv("VISUAL");
>> >> +
>> >> +if (editor) {
>> >> +int r;
>> >> +
>> >> +editor = strdup(editor);
>> >> +if (!editor)
>> >> +return log_oom();
>> >> +
>> >> +r = strv_consume_prepend(&tmp_editors, editor);
>> >> +if (r < 0)
>> >> +return log_oom();
>> >> +}
>> >> +
>> >> +*editors = tmp_editors;
>> >> +
>> >> +return 0;
>> >> +}
>> >
>> > Hmm, I don't like this bit. Instead of figuring out the editor in one
>> > step, and then executing it, I'd really prefer if we'd try to execute
>> > the editors one-by-one until we find one that works. When we invoke
>> > the pager we follow a similar logic.
>>
>> Actually that's what I do. I fill a strv of editors and try to execute
>> each one of them, until one works. But we fail if the error is
>> different than "No such file or directory".
>>
>> This function (get_editors) is used in run_editor, where I do a
>> STRV_FOREACH in the child to try to execute every editor, with the one
>> comming from SYSTEMD_EDITOR or EDITOR or VISUAL, first.
>
> Oh, well, indeed. Hmm, so I figure I'd just prefer if we wouldn't have
> to allocate memory for this. or in other words, I'd like to see an alg
> like this:
>
> if (fork() == 0) {
>
> ...
>
> e = getenv("SYSTEMD_EDITOR");
> if (!e)
> e = getenv("EDITOR");
> if (!e)
> e = getenv("VISUAL");
> if (e)
> execlp(e, ...);
>
> execlp("nano", ...);
> execlp("vim", ...);
> execlp("vi", ...);
>
> ...
> }
>
> That way we wouldn't have to allocate an strv, and just open code the
> search logic. This would then mirror nicely how the pager logic works...
I agree but I can't (or I don't know how to do this), because we need
to give the paths of the units to execvp, and since the first argument
must be the name of the editor, I need to add it to the strv. The
original strv is a strv of units path, so I copy it and prepend the
name of the editor.

We can do this for the pager because there is no arguments to give,
here we have a list of paths.

>
>> Or maybe, you meant that if the one coming from SYSTEMD_EDITOR failed,
>> we should try to execute EDITOR, then VISUAL, too? Because I think we
>> should do that, but with the current code we don't.
>
> I think it's OK checking only the first of SYSTEMD_EDITOR, EDITOR,
> VISUAL that exists, and then proceed with nano/vim/vi
Ok

>
>> >> +if (arg_transport != BUS_TRANSPORT_LOCAL) {
>> >> +log_error("Cannot remotely edit units");
>> >> +return -EINVAL;
>> >> +}
>> >> +
>> >> +if (arg_runtime) {
>> >> +log_error("Cannot edit runtime units");
>> >> +return -EINVAL;
>> >> +}
>> >
>> > Hmm, why not support this? And imply --runtime or so?
>>
>> You are right, I forgot to ask a question about this one, sorry.
>>
>> If there is a unit in /etc/systemd/system/ and someone wants to edit
>> this 

Re: [systemd-devel] [PATCH v3] systemctl: add edit verb

[Ronny Chevalier]

2014-10-21 0:51 GMT+02:00 Lennart Poettering :
> On Sat, 18.10.14 18:30, Ronny Chevalier (chevalier.ro...@gmail.com) wrote:
>
> Looks pretty good. A few comments.
>
>> +
>> +static int unit_file_copy_if_needed(const char *unit_name, const char 
>> *fragment_path, char **new_path) {
>> +char *tmp_path;
>> +int r;
>> +
>> +assert(fragment_path);
>> +assert(unit_name);
>> +assert(new_path);
>> +
>> +/* If it's a unit for the --user scope there is no need to copy it, 
>> it's already in the right directory.
>> + * Same if this is --system/--global scope and the file is in 
>> {SYSTEM,USER}_CONFIG_UNIT_PATH
>> + */
>> +if (arg_scope == UNIT_FILE_USER
>> +|| startswith(fragment_path, SYSTEM_CONFIG_UNIT_PATH)
>> +|| startswith(fragment_path, USER_CONFIG_UNIT_PATH)) {
>> +*new_path = strdup(fragment_path);
>> +if (!*new_path)
>> +return log_oom();
>> +return 0;
>> +}
>> +
>> +switch (arg_scope) {
>> +case UNIT_FILE_SYSTEM:
>> +tmp_path = strjoin(SYSTEM_CONFIG_UNIT_PATH, "/", 
>> unit_name, NULL);
>> +break;
>> +case UNIT_FILE_GLOBAL:
>> +tmp_path = strjoin(USER_CONFIG_UNIT_PATH, "/", 
>> unit_name, NULL);
>> +break;
>> +default:
>> +assert_not_reached("Invalid scope");
>> +}
>> +if (!tmp_path)
>> +return log_oom();
>> +
>> +if (access(tmp_path, F_OK) == 0) {
>> +char response;
>> +
>> +r = ask_char(&response, "yn", "%s already exists, are you 
>> sure to overwrite it with %s? [(y)es, (n)o] ", tmp_path, fragment_path);
>> +if (r < 0) {
>> +free(tmp_path);
>> +return r;
>> +}
>> +if (response != 'y') {
>> +log_warning("%s ignored", unit_name);
>> +free(tmp_path);
>> +return -1;
>> +}
>> +}
>> +
>> +r = mkdir_parents(tmp_path, 0755);
>> +if (r < 0) {
>> +log_error("Failed to create directories for %s: %s", 
>> tmp_path, strerror(-r));
>> +free(tmp_path);
>> +return r;
>> +}
>> +
>> +r = copy_file(fragment_path, tmp_path, 0, 0644);
>> +if (r < 0) {
>> +log_error("Failed to copy %s to %s: %s", fragment_path, 
>> tmp_path, strerror(-r));
>> +free(tmp_path);
>> +return r;
>> +}
>> +
>> +*new_path = tmp_path;
>> +
>> +return 0;
>> +}
>> +
>> +static int get_editors(char ***editors) {
>> +char **tmp_editors = strv_new("nano", "vim", "vi", NULL);
>
> Please avoid calling functions and declaring variables in one line.
>
> Also, there's an OOM check missing for this line.
>
>> +char *editor;
>> +
>> +/* SYSTEMD_EDITOR takes precedence over EDITOR which takes 
>> precedence over VISUAL
>> + * If neither SYSTEMD_EDITOR nor EDITOR nor VISUAL are present,
>> + * we try to execute well known editors
>> + */
>> +editor = getenv("SYSTEMD_EDITOR");
>> +if (!editor)
>> +editor = getenv("EDITOR");
>> +if (!editor)
>> +editor = getenv("VISUAL");
>> +
>> +if (editor) {
>> +int r;
>> +
>> +editor = strdup(editor);
>> +if (!editor)
>> +return log_oom();
>> +
>> +r = strv_consume_prepend(&tmp_editors, editor);
>> +if (r < 0)
>> +return log_oom();
>> +}
>> +
>> +*editors = tmp_editors;
>> +
>> +return 0;
>> +}
>
> Hmm, I don't like this bit. Instead of figuring out the editor in one
> step, and then executing it, I'd really prefer if we'd try to execute
>

[systemd-devel] [PATCH v3] systemctl: add edit verb

[Ronny Chevalier]

It helps editing units by either creating a drop-in file, like
/etc/systemd/system/my.service.d/amendments.conf, or by copying the
original unit from /usr/lib/systemd/ to /etc/systemd/ if the --full
option is specified. Then it invokes an editor to the related files
and daemon-reload is invoked when the editor exited successfully.

See https://bugzilla.redhat.com/show_bug.cgi?id=906824
---
 TODO  |   2 -
 man/journalctl.xml|   6 +-
 man/less-variables.xml|  40 +++--
 man/localectl.xml |   6 +-
 man/loginctl.xml  |   6 +-
 man/machinectl.xml|   6 +-
 man/systemctl.xml |  49 +-
 man/systemd-analyze.xml   |   6 +-
 man/timedatectl.xml   |   6 +-
 src/systemctl/systemctl.c | 394 +-
 10 files changed, 488 insertions(+), 33 deletions(-)

diff --git a/TODO b/TODO
index 3206420..cc8d8c4 100644
--- a/TODO
+++ b/TODO
@@ -66,8 +66,6 @@ Features:
 
 * systemctl: if it fails, show log output?
 
-* maybe add "systemctl edit" that copies unit files from 
/usr/lib/systemd/system to /etc/systemd/system and invokes vim on them
-
 * dbus: add new message hdr field for allowing interactive auth, write spec 
for it. update dbus spec to mandate that unknown flags *must* be ignored...
 
 * maybe introduce AssertXYZ= similar to ConditionXYZ= that causes a unit to 
fail (instead of skipping it) if some condition is not true...
diff --git a/man/journalctl.xml b/man/journalctl.xml
index 7fb6afc..d36889f 100644
--- a/man/journalctl.xml
+++ b/man/journalctl.xml
@@ -891,7 +891,11 @@
 failure code is returned.
 
 
-
+
+Environment
+
+
+
 
 
 Examples
diff --git a/man/less-variables.xml b/man/less-variables.xml
index 09cbd42..1b8aae0 100644
--- a/man/less-variables.xml
+++ b/man/less-variables.xml
@@ -2,28 +2,24 @@
 http://www.oasis-open.org/docbook/xml/4.5/docbookx.dtd";>
 
-
-Environment
+
+   
+   $SYSTEMD_PAGER
 
-
-
-$SYSTEMD_PAGER
+   Pager to use when
+   --no-pager is not given;
+   overrides $PAGER.  Setting
+   this to an empty string or the value
+   cat is equivalent to passing
+   --no-pager.
+   
 
-Pager to use when
---no-pager is not given;
-overrides $PAGER.  Setting
-this to an empty string or the value
-cat is equivalent to passing
---no-pager.
-
+   
+   $SYSTEMD_LESS
 
-
-$SYSTEMD_LESS
-
-Override the default
-options passed to
-less
-(FRSXMK).
-
-
-
+   Override the default
+   options passed to
+   less
+   (FRSXMK).
+   
+
diff --git a/man/localectl.xml b/man/localectl.xml
index 38e73c7..7ae6c60 100644
--- a/man/localectl.xml
+++ b/man/localectl.xml
@@ -223,7 +223,11 @@
 code otherwise.
 
 
-
+
+Environment
+
+
+
 
 
 See Also
diff --git a/man/loginctl.xml b/man/loginctl.xml
index 749db92..4754790 100644
--- a/man/loginctl.xml
+++ b/man/loginctl.xml
@@ -438,7 +438,11 @@
 code otherwise.
 
 
-
+
+Environment
+
+
+
 
 
 See Also
diff --git a/man/machinectl.xml b/man/machinectl.xml
index 2f2e257..b95b7fe 100644
--- a/man/machinectl.xml
+++ b/man/machinectl.xml
@@ -281,7 +281,11 @@
 code otherwise.
 
 
-
+
+Environment
+
+
+
 
 
 See Also
diff --git a/man/systemctl.xml b/man/systemctl.xml
index 61a23de..44dc7cb 100644
--- a/man/systemctl.xml
+++ b/man/systemctl.xml
@@ -1150,6 +1150,31 @@ kobject-uevent 1 systemd-udevd-kernel.socket 
systemd-udevd.service
 default.target to the given unit.
   
 
+
+
+  edit 
NAME...
+
+  
+Edit one or more unit files, as specified on the command
+line.
+
+Depending on whether --system (the default),
+--user, or --global is specified,
+this create a drop-in file for each units either for the system,
+for the calling user or for all futures logins of all users. Then
+the editor is invoked on them (see section "Environment" 
below).
+
+If --full is specified, this will copy the 
original
+units instead of creating drop-in files.
+
+After the units have been edited, the sys

Re: [systemd-devel] [PATCH v2] systemctl: add edit verb

[Ronny Chevalier]

Ignore this one, I rebased, then saved a modification in vim, so the
rebase is reverted in the patch.

Sorry for the noise.

2014-10-18 18:12 GMT+02:00 Ronny Chevalier :
> It helps editing units by either creating a drop-in file, like
> /etc/systemd/system/my.service.d/amendments.conf, or by copying the
> original unit from /usr/lib/systemd/ to /etc/systemd/ if the --full
> option is specified. Then it invokes an editor to the related files
> and daemon-reload is invoked when the editor exited successfully.
>
> See https://bugzilla.redhat.com/show_bug.cgi?id=906824
> ---
>  TODO  |   2 -
>  man/journalctl.xml|   6 +-
>  man/less-variables.xml|  40 ++-
>  man/localectl.xml |   6 +-
>  man/loginctl.xml  |   6 +-
>  man/machinectl.xml|   6 +-
>  man/systemctl.xml |  49 +++-
>  man/systemd-analyze.xml   |   6 +-
>  man/timedatectl.xml   |   6 +-
>  src/systemctl/systemctl.c | 650 
> +-
>  10 files changed, 564 insertions(+), 213 deletions(-)
>
> diff --git a/TODO b/TODO
> index 3206420..cc8d8c4 100644
> --- a/TODO
> +++ b/TODO
> @@ -66,8 +66,6 @@ Features:
>
>  * systemctl: if it fails, show log output?
>
> -* maybe add "systemctl edit" that copies unit files from 
> /usr/lib/systemd/system to /etc/systemd/system and invokes vim on them
> -
>  * dbus: add new message hdr field for allowing interactive auth, write spec 
> for it. update dbus spec to mandate that unknown flags *must* be ignored...
>
>  * maybe introduce AssertXYZ= similar to ConditionXYZ= that causes a unit to 
> fail (instead of skipping it) if some condition is not true...
> diff --git a/man/journalctl.xml b/man/journalctl.xml
> index 7fb6afc..d36889f 100644
> --- a/man/journalctl.xml
> +++ b/man/journalctl.xml
> @@ -891,7 +891,11 @@
>  failure code is returned.
>  
>
> -
> +
> +Environment
> +
> +
> +
>
>  
>  Examples
> diff --git a/man/less-variables.xml b/man/less-variables.xml
> index 09cbd42..1b8aae0 100644
> --- a/man/less-variables.xml
> +++ b/man/less-variables.xml
> @@ -2,28 +2,24 @@
> "http://www.oasis-open.org/docbook/xml/4.5/docbookx.dtd";>
>
> -
> -Environment
> +
> +   
> +   $SYSTEMD_PAGER
>
> -
> -
> -$SYSTEMD_PAGER
> +   Pager to use when
> +   --no-pager is not given;
> +   overrides $PAGER.  Setting
> +   this to an empty string or the value
> +   cat is equivalent to passing
> +   --no-pager.
> +   
>
> -Pager to use when
> ---no-pager is not given;
> -overrides $PAGER.  Setting
> -this to an empty string or the value
> -cat is equivalent to passing
> ---no-pager.
> -
> +   
> +   $SYSTEMD_LESS
>
> -
> -$SYSTEMD_LESS
> -
> -Override the default
> -options passed to
> -less
> -(FRSXMK).
> -
> -
> -
> +   Override the default
> +   options passed to
> +   less
> +   (FRSXMK).
> +   
> +
> diff --git a/man/localectl.xml b/man/localectl.xml
> index 38e73c7..7ae6c60 100644
> --- a/man/localectl.xml
> +++ b/man/localectl.xml
> @@ -223,7 +223,11 @@
>  code otherwise.
>  
>
> -
> +
> +Environment
> +
> +
> +
>
>  
>  See Also
> diff --git a/man/loginctl.xml b/man/loginctl.xml
> index 749db92..4754790 100644
> --- a/man/loginctl.xml
> +++ b/man/loginctl.xml
> @@ -438,7 +438,11 @@
>  code otherwise.
>  
>
> -
> +
> +Environment
> +
> +
> +
>
>  
>  See Also
> diff --git a/man/machinectl.xml b/man/machinectl.xml
> index 2f2e257..b95b7fe 100644
> --- a/man/machinectl.xml
> +++ b/man/machinectl.xml
> @@ -281,7 +281,11 @@
>  code otherwise.
>  
>
> -
> +
> +Environment
> +
> +
> +
>
>  
>

[systemd-devel] [PATCH v2] systemctl: add edit verb

[Ronny Chevalier]

It helps editing units by either creating a drop-in file, like
/etc/systemd/system/my.service.d/amendments.conf, or by copying the
original unit from /usr/lib/systemd/ to /etc/systemd/ if the --full
option is specified. Then it invokes an editor to the related files
and daemon-reload is invoked when the editor exited successfully.

See https://bugzilla.redhat.com/show_bug.cgi?id=906824
---
 TODO  |   2 -
 man/journalctl.xml|   6 +-
 man/less-variables.xml|  40 ++-
 man/localectl.xml |   6 +-
 man/loginctl.xml  |   6 +-
 man/machinectl.xml|   6 +-
 man/systemctl.xml |  49 +++-
 man/systemd-analyze.xml   |   6 +-
 man/timedatectl.xml   |   6 +-
 src/systemctl/systemctl.c | 650 +-
 10 files changed, 564 insertions(+), 213 deletions(-)

diff --git a/TODO b/TODO
index 3206420..cc8d8c4 100644
--- a/TODO
+++ b/TODO
@@ -66,8 +66,6 @@ Features:
 
 * systemctl: if it fails, show log output?
 
-* maybe add "systemctl edit" that copies unit files from 
/usr/lib/systemd/system to /etc/systemd/system and invokes vim on them
-
 * dbus: add new message hdr field for allowing interactive auth, write spec 
for it. update dbus spec to mandate that unknown flags *must* be ignored...
 
 * maybe introduce AssertXYZ= similar to ConditionXYZ= that causes a unit to 
fail (instead of skipping it) if some condition is not true...
diff --git a/man/journalctl.xml b/man/journalctl.xml
index 7fb6afc..d36889f 100644
--- a/man/journalctl.xml
+++ b/man/journalctl.xml
@@ -891,7 +891,11 @@
 failure code is returned.
 
 
-
+
+Environment
+
+
+
 
 
 Examples
diff --git a/man/less-variables.xml b/man/less-variables.xml
index 09cbd42..1b8aae0 100644
--- a/man/less-variables.xml
+++ b/man/less-variables.xml
@@ -2,28 +2,24 @@
 http://www.oasis-open.org/docbook/xml/4.5/docbookx.dtd";>
 
-
-Environment
+
+   
+   $SYSTEMD_PAGER
 
-
-
-$SYSTEMD_PAGER
+   Pager to use when
+   --no-pager is not given;
+   overrides $PAGER.  Setting
+   this to an empty string or the value
+   cat is equivalent to passing
+   --no-pager.
+   
 
-Pager to use when
---no-pager is not given;
-overrides $PAGER.  Setting
-this to an empty string or the value
-cat is equivalent to passing
---no-pager.
-
+   
+   $SYSTEMD_LESS
 
-
-$SYSTEMD_LESS
-
-Override the default
-options passed to
-less
-(FRSXMK).
-
-
-
+   Override the default
+   options passed to
+   less
+   (FRSXMK).
+   
+
diff --git a/man/localectl.xml b/man/localectl.xml
index 38e73c7..7ae6c60 100644
--- a/man/localectl.xml
+++ b/man/localectl.xml
@@ -223,7 +223,11 @@
 code otherwise.
 
 
-
+
+Environment
+
+
+
 
 
 See Also
diff --git a/man/loginctl.xml b/man/loginctl.xml
index 749db92..4754790 100644
--- a/man/loginctl.xml
+++ b/man/loginctl.xml
@@ -438,7 +438,11 @@
 code otherwise.
 
 
-
+
+Environment
+
+
+
 
 
 See Also
diff --git a/man/machinectl.xml b/man/machinectl.xml
index 2f2e257..b95b7fe 100644
--- a/man/machinectl.xml
+++ b/man/machinectl.xml
@@ -281,7 +281,11 @@
 code otherwise.
 
 
-
+
+Environment
+
+
+
 
 
 See Also
diff --git a/man/systemctl.xml b/man/systemctl.xml
index 61a23de..44dc7cb 100644
--- a/man/systemctl.xml
+++ b/man/systemctl.xml
@@ -1150,6 +1150,31 @@ kobject-uevent 1 systemd-udevd-kernel.socket 
systemd-udevd.service
 default.target to the given unit.
   
 
+
+
+  edit 
NAME...
+
+  
+Edit one or more unit files, as specified on the command
+line.
+
+Depending on whether --system (the default),
+--user, or --global is specified,
+this create a drop-in file for each units either for the system,
+for the calling user or for all futures logins of all users. Then
+the editor is invoked on them (see section "Environment" 
below).
+
+If --full is specified, this will copy the 
original
+units instead of creating drop-in files.
+
+After the units have been edited, the system

Re: [systemd-devel] [PATCH, REVIEW, v2] find_symlinks: adds a cache of enabled unit symbolic link state

[Ronny Chevalier]

Hi,

2014-10-18 1:34 GMT+02:00 Ken Sedgwick :
> The current find_symlinks_fd code traverses the config directories
> duplicatively. This is a performance problem if 1000s of units are
> being controlled. This patch adds a hashmap cache of symbolic link
> state which is filled in one traversal and then consulted as needed to
> prevent re-traversal.
>
> The enabled_context cache lives in the manager struct.  Initially the
> cache is empty and is filled on first use.  A pointer to the cache is
> passed to all routines which can call find_symlinks_fd.  If a NULL is
> passed to find_symlinks_fd a temporary cache is created and used for
> the single call.
>
> The cache has two levels, the first is keyed by config_path and the
> second by the names and paths of the units.
>
> The cache contains both forward and reverse mappings; from symbolic
> link name to target and from target to symbolic link name.
>
> The cache contains entries for both the basename of the unit and the
> full path of the link name and target.
>
> The test-enabled patch (previously submitted) checks that the results
> of unit_file_get_state and unit_file_get_list do not change as a
> result of this cache. This patch presumes the test-enabled patch has
> already been applied.
>
> The test-manyunits patch (previously submitted) checks that the performance
> of unit_file_get_list is acceptable in the face of thousands of units.
> This patch presumes the test-manyunits patch has already been applied.
If there is dependencies between your patches, you should use
something like "git format-patch -3" to generate 3 patchs from the 3
last commits. It will add [PATCH 1/3] [PATCH 2/3]... on the subjects
of the mails. It helps knowing if a patch needs to be applied before
applying another.

> ---
>  src/core/dbus-manager.c   |   6 +-
>  src/core/manager.c|   6 +
>  src/core/manager.h|   2 +
>  src/core/unit.c   |   2 +-
>  src/shared/install.c  | 273 
> +-
>  src/shared/install.h  |  18 ++-
>  src/systemctl/systemctl.c |   6 +-
>  src/test/test-enabled.c   |  24 ++--
>  src/test/test-install.c   |  87 ---
>  src/test/test-manyunits.c |  11 +-
>  src/test/test-unit-file.c |   2 +-
>  11 files changed, 325 insertions(+), 112 deletions(-)
>
> diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c
> index 57db1c9..0ce41b0 100644
> --- a/src/core/dbus-manager.c
> +++ b/src/core/dbus-manager.c
> @@ -1403,7 +1403,7 @@ static int method_list_unit_files(sd_bus *bus, 
> sd_bus_message *message, void *us
>  if (!h)
>  return -ENOMEM;
>
> -r = unit_file_get_list(m->running_as == SYSTEMD_SYSTEM ? 
> UNIT_FILE_SYSTEM : UNIT_FILE_USER, NULL, h);
> +r = unit_file_get_list(m->running_as == SYSTEMD_SYSTEM ? 
> UNIT_FILE_SYSTEM : UNIT_FILE_USER, NULL, h, m->enabled);
>  if (r < 0)
>  goto fail;
>
> @@ -1454,7 +1454,7 @@ static int method_get_unit_file_state(sd_bus *bus, 
> sd_bus_message *message, void
>
>  scope = m->running_as == SYSTEMD_SYSTEM ? UNIT_FILE_SYSTEM : 
> UNIT_FILE_USER;
>
> -state = unit_file_get_state(scope, NULL, name);
> +state = unit_file_get_state(scope, NULL, name, m->enabled);
>  if (state < 0)
>  return state;
>
> @@ -1843,7 +1843,7 @@ static int method_add_dependency_unit_files(sd_bus 
> *bus, sd_bus_message *message
>
>  scope = m->running_as == SYSTEMD_SYSTEM ? UNIT_FILE_SYSTEM : 
> UNIT_FILE_USER;
>
> -r = unit_file_add_dependency(scope, runtime, NULL, l, target, dep, 
> force, &changes, &n_changes);
> +r = unit_file_add_dependency(scope, runtime, NULL, l, target, dep, 
> force, NULL, &changes, &n_changes);
>  if (r < 0)
>  return r;
>
> diff --git a/src/core/manager.c b/src/core/manager.c
> index 726977f..f8f41fb 100644
> --- a/src/core/manager.c
> +++ b/src/core/manager.c
> @@ -465,6 +465,10 @@ int manager_new(SystemdRunningAs running_as, bool 
> test_run, Manager **_m) {
>  if (r < 0)
>  goto fail;
>
> +m->enabled = enabled_context_new();
> +if (!m->enabled)
> +goto fail;
> +
>  r = set_ensure_allocated(&m->startup_units, NULL);
>  if (r < 0)
>  goto fail;
> @@ -822,6 +826,8 @@ void manager_free(Manager *m) {
>  hashmap_free(m->watch_pids2);
>  hashmap_free(m->watch_bus);
>
> +enabled_context_free(m->enabled);
> +
>  set_free(m->startup_units);
>  set_free(m->failed_units);
>
> diff --git a/src/core/manager.h b/src/core/manager.h
> index 8e3c146..3f54fe0 100644
> --- a/src/core/manager.h
> +++ b/src/core/manager.h
> @@ -72,6 +72,7 @@ typedef enum ManagerExitCode {
>  #include "unit-name.h"
>  #include "exit-status.h"
>  #include "show-status.h"
> +#include "install.h"
>  #include "failure-action.h"
>
>  struct Manager {
> @@ -82,6 +83,7 @@ struct Manager {
>

Re: [systemd-devel] [PATCH] systemctl: add edit verb

[Ronny Chevalier]

2014-10-13 16:13 GMT+02:00 Simon McVittie :
> On 13/10/14 14:38, Dale R. Worley wrote:
>> My general understanding is that the traditional behavior when "you
>> need an editor but the user hasn't specified one" is to use "vi", and
>> so people who don't want "vi" *always* set $VISUAL in their
>> environment.
>
> The Right Thing™ is distro-specific. Debian and its derivatives have
> sensible-editor(1) which is a shell script that uses $VISUAL, $EDITOR,
> nano[1] or vi; I would expect systemd in Debian to use sensible-editor
> as its fallback, either via a configure option or a patch.
>
> In distros without sensible-editor, I'm tempted to say the solution is
> "stop being a distro without sensible-editor". New systemd API? :-)
>
Before I resend the patch, we should agree on what to do here, for me
it makes more sense to raise an error asking to set either EDITOR or
SYSTEMD_EDITOR than running vi or a "sensible-editor" like you said.
Because, someone used to its editor will quit vi or "sensible-editor"
and will try to find out how to set the editor for systemctl (Even if
it is well known for EDITOR or VISUAL, some people don't set these
variables).

If we raise an error, someone not used to these variables but having a
favorite editor will know how to change this. And if this person does
not have a favorite editor and never used one (unlikely but I think
this is the purpose of the sensible-editor ?), we can advice to set
EDITOR to nano or an equivalent.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH 1/4] man: add examples for coredumpctl

[Ronny Chevalier]

Add examples to clarify how to use coredumpctl

See https://bugs.freedesktop.org/show_bug.cgi?id=83437
---
 man/coredumpctl.xml | 28 
 1 file changed, 28 insertions(+)

diff --git a/man/coredumpctl.xml b/man/coredumpctl.xml
index a7b8793..03552b7 100644
--- a/man/coredumpctl.xml
+++ b/man/coredumpctl.xml
@@ -208,6 +208,34 @@
 
 
 
+Examples
+
+
+List all the coredumps of a program named 
foo
+
+# coredumpctl list foo
+
+
+
+Invoke gdb on the last coredump
+
+# coredumpctl gdb
+
+
+
+Show information about a process that dumped 
core
+
+# coredumpctl info 
6654
+
+
+
+Extract the last coredump of /usr/bin/bar to a 
file named bar.coredump
+
+# coredumpctl -o bar.coredump dump 
/usr/bin/bar
+
+
+
+
 See Also
 
 
systemd-coredump8,
-- 
2.1.2

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

[systemd-devel] [PATCH 4/4] man: fix project reference for archlinux

[Ronny Chevalier]

---
 man/systemd-nspawn.xml | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/man/systemd-nspawn.xml b/man/systemd-nspawn.xml
index 22f2df4..c02aa88 100644
--- a/man/systemd-nspawn.xml
+++ b/man/systemd-nspawn.xml
@@ -106,7 +106,7 @@
 yum8,
 debootstrap8,
 or
-pacman8
+pacman8
 to set up an OS directory tree suitable as file system
 hierarchy for systemd-nspawn
 containers.
@@ -774,7 +774,7 @@
 chroot1,
 yum8,
 debootstrap8,
-pacman8,
+pacman8,
 
systemd.slice5,
 
machinectl1
 
-- 
2.1.2

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

[systemd-devel] [PATCH 3/4] man: use instead of multiple for examples

[Ronny Chevalier]

---
 man/systemd-activate.xml | 19 ++--
 man/systemd-nspawn.xml   | 75 
 2 files changed, 47 insertions(+), 47 deletions(-)

diff --git a/man/systemd-activate.xml b/man/systemd-activate.xml
index 717f5c0..cde4263 100644
--- a/man/systemd-activate.xml
+++ b/man/systemd-activate.xml
@@ -144,20 +144,19 @@ along with systemd; If not, see 
.
   
 
   
-Example 1
+Examples
 
-$ /usr/lib/systemd/systemd-activate -l 2000 -a 
cat
+
+  Run an echo server on port 2000
 
-This runs an echo server on port 2000.
-  
-
-  
-Example 2
+  $ /usr/lib/systemd/systemd-activate -l 2000 -a 
cat
+
 
-$ /usr/lib/systemd/systemd-activate -l 19531 
/usr/lib/systemd/systemd-journal-gatewayd
+
+  Run a socket activated instance of 
systemd-journal-gatewayd8
 
-This runs a socket activated instance of
-
systemd-journal-gatewayd8.
+  $ /usr/lib/systemd/systemd-activate -l 19531 
/usr/lib/systemd/systemd-journal-gatewayd
+
   
 
   
diff --git a/man/systemd-nspawn.xml b/man/systemd-nspawn.xml
index 820a79b..22f2df4 100644
--- a/man/systemd-nspawn.xml
+++ b/man/systemd-nspawn.xml
@@ -694,69 +694,70 @@
 
 
 
-Example 1
+Examples
+
+Boot a minimal Fedora distribution in a 
container
 
-# yum -y --releasever=19 --nogpg 
--installroot=/srv/mycontainer --disablerepo='*' --enablerepo=fedora install 
systemd passwd yum fedora-release vim-minimal
+# yum -y --releasever=19 --nogpg 
--installroot=/srv/mycontainer --disablerepo='*' --enablerepo=fedora install 
systemd passwd yum fedora-release vim-minimal
 # systemd-nspawn -bD /srv/mycontainer
 
-This installs a minimal Fedora distribution into
-the directory /srv/mycontainer/ and
-then boots an OS in a namespace container in
-it.
-
+This installs a minimal Fedora distribution into
+the directory /srv/mycontainer/ and
+then boots an OS in a namespace container in
+it.
+
 
-
-Example 2
+
+Spawn a shell in a container of a minimal 
Debian unstable distribution
 
-# debootstrap --arch=amd64 unstable 
~/debian-tree/
+# debootstrap --arch=amd64 unstable 
~/debian-tree/
 # systemd-nspawn -D ~/debian-tree/
 
-This installs a minimal Debian unstable
-distribution into the directory
-~/debian-tree/ and then spawns a
-shell in a namespace container in it.
-
+This installs a minimal Debian unstable
+distribution into the directory
+~/debian-tree/ and then spawns a
+shell in a namespace container in it.
+
 
-
-Example 3
+
+Boot a minimal Arch Linux distribution in a 
container
 
-# pacstrap -c -d ~/arch-tree/ base
+# pacstrap -c -d ~/arch-tree/ base
 # systemd-nspawn -bD ~/arch-tree/
 
-This installs a mimimal Arch Linux distribution into
-the directory ~/arch-tree/ and then
-boots an OS in a namespace container in it.
-
+This installs a mimimal Arch Linux distribution 
into
+the directory ~/arch-tree/ and 
then
+boots an OS in a namespace container in it.
+
 
-
-Example 4
+
+Enable Arch Linux container on boot
 
-# mv ~/arch-tree /var/lib/container/arch
+# mv ~/arch-tree 
/var/lib/container/arch
 # systemctl enable systemd-nspawn@arch.service
 # systemctl start systemd-nspawn@arch.service
 
-This makes the Arch Linux container part of the
-multi-user.target on the host.
-
-
+This makes the Arch Linux container part of the
+multi-user.target on the host.
+
+
 
-
-Example 5
+
+Boot into a btrfs snapshot of the host 
system
 
-# btrfs subvolume snapshot / /.tmp
+# btrfs subvolume snapshot / /.tmp
 # systemd-nspawn --private-network -D /.tmp -b
 
-This runs a copy of the host system in a
-btrfs snapshot.
+This runs a copy of the host system in a
+btrfs snapshot.
+
 
 
 
-

[systemd-devel] [PATCH 2/4] man: add missing commas

[Ronny Chevalier]

---
 man/sd_journal_get_catalog.xml  | 2 +-
 man/sysctl.d.xml| 2 +-
 man/systemd-bus-pro...@.service.xml | 6 +++---
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/man/sd_journal_get_catalog.xml b/man/sd_journal_get_catalog.xml
index 485bbba..c38a645 100644
--- a/man/sd_journal_get_catalog.xml
+++ b/man/sd_journal_get_catalog.xml
@@ -133,7 +133,7 @@
 
sd-journal3,
 
sd_journal_open3,
 
sd_journal_next3,
-
sd_journal_get_data3
+
sd_journal_get_data3,
 
malloc3
 
 
diff --git a/man/sysctl.d.xml b/man/sysctl.d.xml
index 3002ebd..7b51b68 100644
--- a/man/sysctl.d.xml
+++ b/man/sysctl.d.xml
@@ -194,7 +194,7 @@ net.bridge.bridge-nf-call-arptables = 0
 
systemd-sysctl.service8,
 
systemd-delta1,
 
sysctl8,
-
sysctl.conf5
+
sysctl.conf5,
 
modprobe8
 
 
diff --git a/man/systemd-bus-pro...@.service.xml 
b/man/systemd-bus-pro...@.service.xml
index 3a5930d..f8e4576 100644
--- a/man/systemd-bus-pro...@.service.xml
+++ b/man/systemd-bus-pro...@.service.xml
@@ -72,9 +72,9 @@ along with systemd; If not, see 
.
 See Also
 
 
-  
systemd-bus-proxyd8
-  
dbus-daemon1
-  http://freedesktop.org/wiki/Software/dbus";>D-Bus
+  
systemd-bus-proxyd8,
+  
dbus-daemon1,
+  http://freedesktop.org/wiki/Software/dbus";>D-Bus,
   https://code.google.com/p/d-bus/";>kdbus
 
   
-- 
2.1.2

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

[systemd-devel] [PATCH] avoid duplication of TIME_T_MAX

[Ronny Chevalier]

---
 src/core/manager.c   | 3 +--
 src/shared/time-util.h   | 2 ++
 src/timesync/timesyncd-manager.c | 3 +--
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/core/manager.c b/src/core/manager.c
index e0c1cd1..dfd8cda 100644
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -75,6 +75,7 @@
 #include "dbus-job.h"
 #include "dbus-manager.h"
 #include "bus-kernel.h"
+#include "time-util.h"
 
 /* As soon as 5s passed since a unit was added to our GC queue, make sure to 
run a gc sweep */
 #define GC_QUEUE_USEC_MAX (10*USEC_PER_SEC)
@@ -84,8 +85,6 @@
 #define JOBS_IN_PROGRESS_PERIOD_USEC (USEC_PER_SEC / 3)
 #define JOBS_IN_PROGRESS_PERIOD_DIVISOR 3
 
-#define TIME_T_MAX (time_t)((1UL << ((sizeof(time_t) << 3) - 1)) - 1)
-
 static int manager_dispatch_notify_fd(sd_event_source *source, int fd, 
uint32_t revents, void *userdata);
 static int manager_dispatch_signal_fd(sd_event_source *source, int fd, 
uint32_t revents, void *userdata);
 static int manager_dispatch_time_change_fd(sd_event_source *source, int fd, 
uint32_t revents, void *userdata);
diff --git a/src/shared/time-util.h b/src/shared/time-util.h
index 05369d2..b55a660 100644
--- a/src/shared/time-util.h
+++ b/src/shared/time-util.h
@@ -65,6 +65,8 @@ typedef struct dual_timestamp {
 #define FORMAT_TIMESTAMP_RELATIVE_MAX 256
 #define FORMAT_TIMESPAN_MAX 64
 
+#define TIME_T_MAX (time_t)((1UL << ((sizeof(time_t) << 3) - 1)) - 1)
+
 #define DUAL_TIMESTAMP_NULL ((struct dual_timestamp) { 0, 0 })
 
 usec_t now(clockid_t clock);
diff --git a/src/timesync/timesyncd-manager.c b/src/timesync/timesyncd-manager.c
index 46f8bd6..e5b156b 100644
--- a/src/timesync/timesyncd-manager.c
+++ b/src/timesync/timesyncd-manager.c
@@ -54,8 +54,7 @@
 #include "mkdir.h"
 #include "timesyncd-conf.h"
 #include "timesyncd-manager.h"
-
-#define TIME_T_MAX (time_t)((1UL << ((sizeof(time_t) << 3) - 1)) - 1)
+#include "time-util.h"
 
 #ifndef ADJ_SETOFFSET
 #define ADJ_SETOFFSET   0x0100  /* add 'time' to current time 
*/
-- 
2.1.2

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

[systemd-devel] [PATCH] shared: remove unused functions

[Ronny Chevalier]

- mkdir_p_prefix: It has never been used
- mkdir_parents_prefix_label: Unused since 
1434ae6fd49f8377b0ddbd4c675736e0d3226ea6
---
 src/shared/mkdir-label.c | 4 
 src/shared/mkdir.c   | 4 
 src/shared/mkdir.h   | 2 --
 3 files changed, 10 deletions(-)

diff --git a/src/shared/mkdir-label.c b/src/shared/mkdir-label.c
index 4ee6251..43ea2c2 100644
--- a/src/shared/mkdir-label.c
+++ b/src/shared/mkdir-label.c
@@ -44,10 +44,6 @@ int mkdir_parents_label(const char *path, mode_t mode) {
 return mkdir_parents_internal(NULL, path, mode, label_mkdir);
 }
 
-int mkdir_parents_prefix_label(const char *prefix, const char *path, mode_t 
mode) {
-return mkdir_parents_internal(prefix, path, mode, label_mkdir);
-}
-
 int mkdir_p_label(const char *path, mode_t mode) {
 return mkdir_p_internal(NULL, path, mode, label_mkdir);
 }
diff --git a/src/shared/mkdir.c b/src/shared/mkdir.c
index f941efb..fabd9e2 100644
--- a/src/shared/mkdir.c
+++ b/src/shared/mkdir.c
@@ -144,7 +144,3 @@ int mkdir_p_internal(const char *prefix, const char *path, 
mode_t mode, mkdir_fu
 int mkdir_p(const char *path, mode_t mode) {
 return mkdir_p_internal(NULL, path, mode, mkdir);
 }
-
-int mkdir_p_prefix(const char *prefix, const char *path, mode_t mode) {
-return mkdir_p_internal(prefix, path, mode, mkdir);
-}
diff --git a/src/shared/mkdir.h b/src/shared/mkdir.h
index dd5b41e..ca38d21 100644
--- a/src/shared/mkdir.h
+++ b/src/shared/mkdir.h
@@ -28,14 +28,12 @@
 int mkdir_safe(const char *path, mode_t mode, uid_t uid, gid_t gid);
 int mkdir_parents(const char *path, mode_t mode);
 int mkdir_p(const char *path, mode_t mode);
-int mkdir_p_prefix(const char *prefix, const char *path, mode_t mode);
 
 /* selinux versions */
 int mkdir_label(const char *path, mode_t mode);
 int mkdir_safe_label(const char *path, mode_t mode, uid_t uid, gid_t gid);
 int mkdir_parents_label(const char *path, mode_t mode);
 int mkdir_p_label(const char *path, mode_t mode);
-int mkdir_parents_prefix_label(const char *prefix, const char *path, mode_t 
mode);
 
 /* internally used */
 typedef int (*mkdir_func_t)(const char *pathname, mode_t mode);
-- 
2.1.2

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

[systemd-devel] [PATCH] systemctl: add edit verb

[Ronny Chevalier]

It helps editing units by either creating a drop-in file, like
/etc/systemd/system/my.service.d/amendments.conf, or by copying the
original unit from /usr/lib/systemd/ to /etc/systemd/ if the --full
option is specified. Then it invokes the $SYSTEMD_EDITOR or $EDITOR or
vim to the related files and daemon-reload is invoked when the editor exited
successfully.

See https://bugzilla.redhat.com/show_bug.cgi?id=906824
---
 TODO  |   2 -
 man/journalctl.xml|   6 +-
 man/less-variables.xml|  40 +++---
 man/localectl.xml |   6 +-
 man/loginctl.xml  |   6 +-
 man/machinectl.xml|   6 +-
 man/systemctl.xml |  44 +-
 man/systemd-analyze.xml   |   6 +-
 man/timedatectl.xml   |   6 +-
 src/systemctl/systemctl.c | 346 +-
 10 files changed, 435 insertions(+), 33 deletions(-)

diff --git a/TODO b/TODO
index 0aaa9f2..857bdd0 100644
--- a/TODO
+++ b/TODO
@@ -65,8 +65,6 @@ Features:
 
 * systemctl: if it fails, show log output?
 
-* maybe add "systemctl edit" that copies unit files from 
/usr/lib/systemd/system to /etc/systemd/system and invokes vim on them
-
 * dbus: add new message hdr field for allowing interactive auth, write spec 
for it. update dbus spec to mandate that unknown flags *must* be ignored...
 
 * maybe introduce AssertXYZ= similar to ConditionXYZ= that causes a unit to 
fail (instead of skipping it) if some condition is not true...
diff --git a/man/journalctl.xml b/man/journalctl.xml
index 7fb6afc..d36889f 100644
--- a/man/journalctl.xml
+++ b/man/journalctl.xml
@@ -891,7 +891,11 @@
 failure code is returned.
 
 
-
+
+Environment
+
+
+
 
 
 Examples
diff --git a/man/less-variables.xml b/man/less-variables.xml
index 09cbd42..1b8aae0 100644
--- a/man/less-variables.xml
+++ b/man/less-variables.xml
@@ -2,28 +2,24 @@
 http://www.oasis-open.org/docbook/xml/4.5/docbookx.dtd";>
 
-
-Environment
+
+   
+   $SYSTEMD_PAGER
 
-
-
-$SYSTEMD_PAGER
+   Pager to use when
+   --no-pager is not given;
+   overrides $PAGER.  Setting
+   this to an empty string or the value
+   cat is equivalent to passing
+   --no-pager.
+   
 
-Pager to use when
---no-pager is not given;
-overrides $PAGER.  Setting
-this to an empty string or the value
-cat is equivalent to passing
---no-pager.
-
+   
+   $SYSTEMD_LESS
 
-
-$SYSTEMD_LESS
-
-Override the default
-options passed to
-less
-(FRSXMK).
-
-
-
+   Override the default
+   options passed to
+   less
+   (FRSXMK).
+   
+
diff --git a/man/localectl.xml b/man/localectl.xml
index 38e73c7..7ae6c60 100644
--- a/man/localectl.xml
+++ b/man/localectl.xml
@@ -223,7 +223,11 @@
 code otherwise.
 
 
-
+
+Environment
+
+
+
 
 
 See Also
diff --git a/man/loginctl.xml b/man/loginctl.xml
index 749db92..4754790 100644
--- a/man/loginctl.xml
+++ b/man/loginctl.xml
@@ -438,7 +438,11 @@
 code otherwise.
 
 
-
+
+Environment
+
+
+
 
 
 See Also
diff --git a/man/machinectl.xml b/man/machinectl.xml
index 2f2e257..b95b7fe 100644
--- a/man/machinectl.xml
+++ b/man/machinectl.xml
@@ -281,7 +281,11 @@
 code otherwise.
 
 
-
+
+Environment
+
+
+
 
 
 See Also
diff --git a/man/systemctl.xml b/man/systemctl.xml
index 61a23de..34f689b 100644
--- a/man/systemctl.xml
+++ b/man/systemctl.xml
@@ -1150,6 +1150,31 @@ kobject-uevent 1 systemd-udevd-kernel.socket 
systemd-udevd.service
 default.target to the given unit.
   
 
+
+
+  edit 
NAME...
+
+  
+Edit one or more unit files, as specified on the command
+line.
+
+Depending on whether --system (the default),
+--user, or --global is specified,
+this create a drop-in file for each units either for the system,
+for the calling user or for all futures logins of all users. Then
+the editor is invoked on them (see section "Environment" 
below).
+
+If --full is specified, this will copy the 
original
+units instead of creating drop-in files.
+
+After the un

Re: [systemd-devel] [PATCH] fstab-generator: Honor usr=, usrfstype= and usrflags=

[Ronny Chevalier]

Hi,

2014-09-30 21:18 GMT+02:00 Tobias Hunger :
> Any feedback at all? Please?
>
> Am I doing something wrong in posting my patch here?
No you just need to wait that someone review your patch. It rarely
takes more than 2 weeks.

>
> Best Regards,
> Tobias
>
> On Sat, Sep 27, 2014 at 5:59 PM, Tobias Hunger  
> wrote:
>> Ping?
>>
>> This is really useful to test out the changes proposed in
>> http://0pointer.net/blog/revisiting-how-we-put-together-linux-systems.html
>>
>> Yes, Lennart seems to want to move to something more strict that can
>> also work with the uefi secure boot, but this helps me get a test
>> system of the ground where the / und /usr mounts are defined in the
>> bootloader entries. Even works with encrypted partitions here;-)
>>
>> Best Regards,
>> Tobias
> ___
> 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] tests: fix resource & mem leaks

[Ronny Chevalier]

---
 src/test/test-condition-util.c | 2 +-
 src/test/test-fileio.c | 8 
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/test/test-condition-util.c b/src/test/test-condition-util.c
index 4ee5600..35ee916 100644
--- a/src/test/test-condition-util.c
+++ b/src/test/test-condition-util.c
@@ -45,7 +45,7 @@ static void test_condition_test_host(void) {
 sd_id128_t id;
 int r;
 char sid[SD_ID128_STRING_MAX];
-char *hostname;
+_cleanup_free_ char *hostname = NULL;
 
 r = sd_id128_get_machine(&id);
 assert_se(r >= 0);
diff --git a/src/test/test-fileio.c b/src/test/test-fileio.c
index 344cc1f..078e802 100644
--- a/src/test/test-fileio.c
+++ b/src/test/test-fileio.c
@@ -303,7 +303,7 @@ static void test_write_string_stream(void) {
 assert_se(f);
 assert_se(write_string_stream(f, "boohoo") < 0);
 
-f = fdopen(fd, "r+");
+f = freopen(fn, "r+", f);
 assert_se(f);
 
 assert_se(write_string_stream(f, "boohoo") == 0);
@@ -317,7 +317,7 @@ static void test_write_string_stream(void) {
 
 static void test_write_string_file(void) {
 char fn[] = "/tmp/test-write_string_file-XX";
-int fd;
+_cleanup_close_ int fd;
 char buf[64] = {0};
 
 fd = mkostemp_safe(fn, O_RDWR);
@@ -333,7 +333,7 @@ static void test_write_string_file(void) {
 
 static void test_write_string_file_no_create(void) {
 char fn[] = "/tmp/test-write_string_file_no_create-XX";
-int fd;
+_cleanup_close_ int fd;
 char buf[64] = {0};
 
 fd = mkostemp_safe(fn, O_RDWR);
@@ -352,7 +352,7 @@ static void test_sendfile_full(void) {
 char in_fn[] = "/tmp/test-sendfile_full-XX";
 char out_fn[] = "/tmp/test-sendfile_full-XX";
 _cleanup_close_ int in_fd = -1;
-int out_fd;
+_cleanup_close_ int out_fd = -1;
 char text[] = "boohoo\nfoo\n\tbar\n";
 char buf[64] = {0};
 
-- 
2.1.0

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

[systemd-devel] [PATCH] logind: fix typo

[Ronny Chevalier]

---
 src/login/logind-session-dbus.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/login/logind-session-dbus.c b/src/login/logind-session-dbus.c
index 7d81500..58836fc 100644
--- a/src/login/logind-session-dbus.c
+++ b/src/login/logind-session-dbus.c
@@ -249,7 +249,7 @@ static int method_set_idle_hint(sd_bus *bus, sd_bus_message 
*message, void *user
 return r;
 
 if (uid != 0 && uid != s->user->uid)
-return sd_bus_error_setf(error, SD_BUS_ERROR_ACCESS_DENIED, 
"Only owner of session my set idle hint");
+return sd_bus_error_setf(error, SD_BUS_ERROR_ACCESS_DENIED, 
"Only owner of session may set idle hint");
 
 session_set_idle_hint(s, b);
 
-- 
2.1.0

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

[systemd-devel] [PATCH] man: fix typos

[Ronny Chevalier]

---
 man/journalctl.xml| 6 +++---
 man/systemctl.xml | 2 +-
 man/systemd-firstboot.xml | 4 ++--
 man/systemd.network.xml   | 2 +-
 4 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/man/journalctl.xml b/man/journalctl.xml
index bf18756..e10918a 100644
--- a/man/journalctl.xml
+++ b/man/journalctl.xml
@@ -99,9 +99,9 @@
 _KERNEL_DEVICE= match for the
 device.
 
-Additional contraints may be added using options
+Additional constraints may be added using options
 --boot, --unit=,
-etc, to futher limit what entries will be shown
+etc, to further limit what entries will be shown
 (logical AND).
 
 Output is interleaved from all accessible
@@ -128,7 +128,7 @@
 --no-pager option and the "Environment"
 section below.
 
-When outputing to a tty, lines are colored
+When outputting to a tty, lines are colored
 according to priority: lines of level ERROR and higher
 are colored red; lines of level NOTICE and higher are
 highlighted; other lines are displayed normally.
diff --git a/man/systemctl.xml b/man/systemctl.xml
index db7798d..2818bcb 100644
--- a/man/systemctl.xml
+++ b/man/systemctl.xml
@@ -1305,7 +1305,7 @@ kobject-uevent 1 systemd-udevd-kernel.socket 
systemd-udevd.service
   
 Checks whether the system is running. This returns
 success when the system is fully up and running, meaning
-not in startup, shutdown or maintainance mode. Failure is
+not in startup, shutdown or maintenance mode. Failure is
 returned otherwise. In addition, the current state is
 printed in a short string to standard output. Use
 --quiet to suppress output of this state
diff --git a/man/systemd-firstboot.xml b/man/systemd-firstboot.xml
index c3fe0ed..5da0a75 100644
--- a/man/systemd-firstboot.xml
+++ b/man/systemd-firstboot.xml
@@ -213,7 +213,7 @@
 Query the user for locale,
 timezone, hostname and root
 password. This is equivalent to
-specifiying
+specifying
 --prompt-locale,
 --prompt-timezone,
 --prompt-hostname,
@@ -238,7 +238,7 @@
 
 Copy locale, time zone and root
 password from the host. This is
-equivalent to specifiying
+equivalent to specifying
 --copy-locale,
 --copy-timezone,
 --copy-root-password
diff --git a/man/systemd.network.xml b/man/systemd.network.xml
index 0245099..641e02a 100644
--- a/man/systemd.network.xml
+++ b/man/systemd.network.xml
@@ -412,7 +412,7 @@
 
Destination=
 
 The destination prefix 
of the route. Possibly followed by a slash and the
-prefixlength. If ommitted, a 
full-length host route is assumed.
+prefixlength. If omitted, a 
full-length host route is assumed.
 
 
 
-- 
2.0.4

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

Re: [systemd-devel] [PATCH] journalctl: add "-t --identifier=STRING" option

[Ronny Chevalier]

2014-08-19 12:12 GMT+02:00  :
> From: Harald Hoyer 
>
> This turns journalctl to the counterpart of systemd-cat.
> Messages sent with
>
> systemd-cat --identifier foo --prioritiy debug
>
> can now be shown with
>
> journalctl --identifier foo --prioritiy debug
>
> "--identifier" is not merged with "--unit" to make a clear
> distinction between syslog and systemd units.
> syslog identifiers can be chosen freely by anyone.
> ---
>  man/journalctl.xml   | 14 ++
>  src/journal/journalctl.c | 43 ++-
>  2 files changed, 56 insertions(+), 1 deletion(-)
>
> diff --git a/man/journalctl.xml b/man/journalctl.xml
> index bf18756..906ffd0 100644
> --- a/man/journalctl.xml
> +++ b/man/journalctl.xml
> @@ -498,6 +498,20 @@
>  
>
>  
> +-t
> +
> --identifier=SYSLOG_IDENTIFIER|PATTERN
> +
> +Show messages for the
> +specified syslog identifier
> +
> SYSLOG_IDENTIFIER, or
> +for any of the messages with a 
> SYSLOG_IDENTIFIER
> +matched by 
> PATTERN.
> +
> +This parameter can be specified
> +multiple times.
> +
> +
> +
>  -u
>  
> --unit=UNIT|PATTERN
>
> diff --git a/src/journal/journalctl.c b/src/journal/journalctl.c
> index 5c4a71d..3037fb8 100644
> --- a/src/journal/journalctl.c
> +++ b/src/journal/journalctl.c
> @@ -89,6 +89,7 @@ static bool arg_force = false;
>  #endif
>  static usec_t arg_since, arg_until;
>  static bool arg_since_set = false, arg_until_set = false;
> +static char **arg_syslog_identifier = NULL;
>  static char **arg_system_units = NULL;
>  static char **arg_user_units = NULL;
>  static const char *arg_field = NULL;
> @@ -180,6 +181,7 @@ static void help(void) {
> "  -k --dmesg   Show kernel message log from the 
> current boot\n"
> "  -u --unit=UNIT   Show data only from the specified 
> unit\n"
> " --user-unit=UNIT  Show data only from the specified 
> user session unit\n"
> +   "  -t --identifier=STRING   Show only messages wit the 
> specified syslog identifier\n"
with*

> "  -p --priority=RANGE  Show only messages within the 
> specified priority range\n"
> "  -e --pager-end   Immediately jump to end of the 
> journal in the pager\n"
> "  -f --follow  Follow the journal\n"
> @@ -276,6 +278,7 @@ static int parse_argv(int argc, char *argv[]) {
>  { "file",   required_argument, NULL, ARG_FILE
>},
>  { "root",   required_argument, NULL, ARG_ROOT
>},
>  { "header", no_argument,   NULL, ARG_HEADER  
>},
> +{ "identifier", required_argument, NULL, 't' 
>},
>  { "priority",   required_argument, NULL, 'p' 
>},
>  { "setup-keys", no_argument,   NULL, ARG_SETUP_KEYS  
>},
>  { "interval",   required_argument, NULL, ARG_INTERVAL
>},
> @@ -304,7 +307,7 @@ static int parse_argv(int argc, char *argv[]) {
>  assert(argc >= 0);
>  assert(argv);
>
> -while ((c = getopt_long(argc, argv, 
> "hefo:aln::qmb::kD:p:c:u:F:xrM:", options, NULL)) >= 0)
> +while ((c = getopt_long(argc, argv, 
> "hefo:aln::qmb::kD:p:c:t:u:F:xrM:", options, NULL)) >= 0)
>
>  switch (c) {
>
> @@ -590,6 +593,12 @@ static int parse_argv(int argc, char *argv[]) {
>  arg_until_set = true;
>  break;
>
> +case 't':
> +r = strv_extend(&arg_syslog_identifier, optarg);
> +if (r < 0)
> +return log_oom();
> +break;
> +
>  case 'u':
>  r = strv_extend(&arg_system_units, optarg);
>  if (r < 0)
> @@ -1212,6 +1221,32 @@ static int add_priorities(sd_journal *j) {
>  return 0;
>  }
>
> +
> +static int add_syslog_identifier(sd_journal *j) {
> +int r;
> +char **i;
> +
> +assert(j);
> +
> +STRV_FOREACH(i, arg_syslog_identifier) {
> +char *u;
> +
> +u = strappenda("SYSLOG_IDENTIFIER=", *i);
> +r = sd_journal_add_match(j, u, 0);
> +if (r < 0)
> +return r;
> +r = sd_journal_add_disjunction(j);
> +if (r < 0)
> +return r;
> + 

Re: [systemd-devel] [PATCH] util: do not execute files without exec permission

[Ronny Chevalier]

2014-08-18 21:10 GMT+02:00 Lennart Poettering :
> On Mon, 18.08.14 20:47, Ronny Chevalier (chevalier.ro...@gmail.com) wrote:
>
>>
>> 2014-08-18 15:51 GMT+02:00 Lennart Poettering :
>> > On Sat, 16.08.14 14:24, Ronny Chevalier (chevalier.ro...@gmail.com) wrote:
>> >
>> > What's the rationale here? I think it makes a lot of sense to output an
>> > error if people drop non-executable files in such a directory...
>> >
>> Yeah it makes sense. But it is useless to fork & exec() when we know
>> it will fail so maybe leave the check and add a warning ?
>
> Dunno. Is this a real problem? I mean, failing after the fork()
> shouldn't be much of a real-life problem, since it realistically never
> really happens.
>
> In general I always try to be careful with these cases that might be
> vulnerable TOCTTOU issues. Not that this was really an issue in this
> case, but I'd prefer if the kernel's exec() syscall would figure out
> that something isn't executable, rather than us, since we cannot do it
> atomically, and somebody could toggle the x bit of a file right after we
> ran access() on it, but before the exec()... Hence, I'd prefer to avoid
> any explicit access() check, unless we really know that this is a common
> issue.
>
> Hope that makes sense,
Yeah it totally makes sense.

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

[systemd-devel] [PATCH] man: fix typo

[Ronny Chevalier]

---
 man/systemd.exec.xml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/man/systemd.exec.xml b/man/systemd.exec.xml
index cfcf996..af103ff 100644
--- a/man/systemd.exec.xml
+++ b/man/systemd.exec.xml
@@ -1013,7 +1013,7 @@
 made inaccessible and empty for
 processes invoked by this unit. If set
 to read-only, the
-two directores are made read-only
+two directories are made read-only
 instead. It is recommended to enable
 this setting for all long-running
 services (in particular network-facing
-- 
2.0.4

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

[systemd-devel] [PATCH v2] bootchart: use NSEC_PER_SEC

[Ronny Chevalier]

---
 src/bootchart/store.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/bootchart/store.c b/src/bootchart/store.c
index cedcba8..2d2ea42 100644
--- a/src/bootchart/store.c
+++ b/src/bootchart/store.c
@@ -34,6 +34,7 @@
 #include 
 
 #include "util.h"
+#include "time-util.h"
 #include "strxcpyx.h"
 #include "store.h"
 #include "bootchart.h"
@@ -54,14 +55,14 @@ double gettime_ns(void) {
 
 clock_gettime(CLOCK_MONOTONIC, &n);
 
-return (n.tv_sec + (n.tv_nsec / 10.0));
+return (n.tv_sec + (n.tv_nsec / (double) NSEC_PER_SEC));
 }
 
 static double gettime_up(void) {
 struct timespec n;
 
 clock_gettime(CLOCK_BOOTTIME, &n);
-return (n.tv_sec + (n.tv_nsec / 10.0));
+return (n.tv_sec + (n.tv_nsec / (double) NSEC_PER_SEC));
 }
 
 void log_uptime(void) {
-- 
2.0.4

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

Re: [systemd-devel] [PATCH] util: do not execute files without exec permission

[Ronny Chevalier]

2014-08-18 15:51 GMT+02:00 Lennart Poettering :
> On Sat, 16.08.14 14:24, Ronny Chevalier (chevalier.ro...@gmail.com) wrote:
>
> What's the rationale here? I think it makes a lot of sense to output an
> error if people drop non-executable files in such a directory...
>
Yeah it makes sense. But it is useless to fork & exec() when we know
it will fail so maybe leave the check and add a warning ?

>> ---
>>  src/shared/util.c | 4 
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/src/shared/util.c b/src/shared/util.c
>> index 18d40f3..3a03470 100644
>> --- a/src/shared/util.c
>> +++ b/src/shared/util.c
>> @@ -3921,6 +3921,10 @@ void execute_directory(const char *directory, DIR *d, 
>> usec_t timeout, char *argv
>>  _exit(EXIT_FAILURE);
>>  }
>>
>> +if (access(path, X_OK) < 0) {
>> +continue;
>> +}
>> +
>>  pid = fork();
>>  if (pid < 0) {
>>  log_error("Failed to fork: %m");
>
>
> 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] condition-util: do not validate condition if paramater is garbage

[Ronny Chevalier]

2014-08-18 18:34 GMT+02:00 Lennart Poettering :
> On Sat, 16.08.14 14:24, Ronny Chevalier (chevalier.ro...@gmail.com) wrote:
>
>> To follow the same behavior that src/core/condition.c do
>> ---
>>  src/shared/condition-util.c| 2 +-
>>  src/test/test-condition-util.c | 4 
>>  2 files changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/shared/condition-util.c b/src/shared/condition-util.c
>> index ff4a8ec..f21786f 100644
>> --- a/src/shared/condition-util.c
>> +++ b/src/shared/condition-util.c
>> @@ -213,7 +213,7 @@ bool condition_test_ac_power(Condition *c) {
>>
>>  r = parse_boolean(c->parameter);
>>  if (r < 0)
>> -return !c->negate;
>> +return c->negate;
>
> Idon't agree that this would be a good idea. I am pretty sure that if we
> cannot make sense of the ac power condition we should return positive by
> default, not negative.
>
> That's a bit different form the other conditions, but this is simply
> because if don't know let's say a virtualization it's probably a good
> idea to assume that we are not running on it. However, if we can't make
> sense of an AC state, we should probably assume that we have AC power
> and hence say "true"...
>
> Does that make sense?
Both make sense for me, there is no good thing to assume for this, but
I just thought that the same behaviour should be followed and document
about it maybe.

By the way, there is a couple of places, like this one, where we
should add a warning when we could not parse the boolean, I think.

>
> 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] Make journalctl start at the end of the journal by default

[Ronny Chevalier]

2014-08-18 13:46 GMT+02:00 Philippe De Swert :
> Hi,
Hi,

>
> Having to often use journalctl it has slowly driven me insane with the
> default options not matching common use cases.
>
> Attached is already a patch to start the journal at the end. Usually people
> check the logs when something went wrong, and don't care about what happened
> three weeks ago at the beginning of the log. Yes you can press the "end" key
> to skip to the end but in some cases that freezes up the console for over a
> minute.
There is the --reverse or -r option to show the newest entries first,
is this what you are looking for ?

>
> Other gripes are --no-pager... way too long to type on a virtual keyboard
> when you are trying to use the logs old style with common unix
> utilities. Maybe not having it by default, or introducing a shorter command
> switch should not be hard to add.
>
> And also I would like to see the full logs always by default. Usually after
> lots of searching you find the offending log entry for the error, only to
> find out you forgot to pass the right command line options to journalctl and
> the important bit is cut off.
There is the --all or -a option for this.

In the end, you just have to use journalctl -ar

>
> If you guys are willing to also consider those other two gripes I will glady
> submit patches for those also.
>
> Regards,
>
> Philippe
>
> ___
> 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] man: fix typo

[Ronny Chevalier]

---
 man/systemd-firstboot.xml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/man/systemd-firstboot.xml b/man/systemd-firstboot.xml
index 42fd753..c3fe0ed 100644
--- a/man/systemd-firstboot.xml
+++ b/man/systemd-firstboot.xml
@@ -249,7 +249,7 @@
 
--setup-machine-id
 
 Initialize the system's machine
-ID to a random ID. This only works
+ID to a random ID. This only works in
 combination with
 --root=.
 
-- 
2.0.4

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

[systemd-devel] [PATCH] condition-util: do not validate condition if paramater is garbage

[Ronny Chevalier]

To follow the same behavior that src/core/condition.c do
---
 src/shared/condition-util.c| 2 +-
 src/test/test-condition-util.c | 4 
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/shared/condition-util.c b/src/shared/condition-util.c
index ff4a8ec..f21786f 100644
--- a/src/shared/condition-util.c
+++ b/src/shared/condition-util.c
@@ -213,7 +213,7 @@ bool condition_test_ac_power(Condition *c) {
 
 r = parse_boolean(c->parameter);
 if (r < 0)
-return !c->negate;
+return c->negate;
 
 return ((on_ac_power() != 0) == !!r) == !c->negate;
 }
diff --git a/src/test/test-condition-util.c b/src/test/test-condition-util.c
index 4ee5600..3de0b67 100644
--- a/src/test/test-condition-util.c
+++ b/src/test/test-condition-util.c
@@ -38,6 +38,10 @@ static void test_condition_test_ac_power(void) {
 condition = condition_new(CONDITION_AC_POWER, "false", false, true);
 assert_se(condition_test_ac_power(condition) == on_ac_power());
 condition_free(condition);
+
+condition = condition_new(CONDITION_AC_POWER, "garbage value", false, 
false);
+assert_se(!condition_test_ac_power(condition));
+condition_free(condition);
 }
 
 static void test_condition_test_host(void) {
-- 
2.0.4

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