[systemd-devel] Bugfix release(s)

2019-01-14 Thread Jan Synacek
Hi,

since v240 didn't go too well, I would like to suggest that the next one
(preferably two) release(s) are bugfix only. Please, consider it.

Thank you,
-- 
Jan Synacek
Software Engineer, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] Strange problem: Failed to get properties: Failed to activate service

2018-10-22 Thread Jan Synacek
On Thu, Oct 18, 2018 at 3:29 PM Cecil Westerhof  wrote:
> I found out that in reality no reboot had taken place. For one reason or 
> another that did not work. Doing:
> systemctl reboot
> systemctl poweroff
> init 0
> halt
>
> All did not work. Those gave messages like:
> Transport endpoint is not connected

This is very familiar to me. Try this:

1) Boot the computer.
2) Run "systemctl daemon-reexec" as root.
3) Run "dmesg" as root.
4) Look for a pid 1 segfault or a message that says "Freezing execution".

If you see a SIGSEGV, let me know. Note that this problem is *not*
present in the upstream code base.

Cheers,
-- 
Jan Synacek
Software Engineer, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel


[systemd-devel] Use of SystemKeepFree=

2018-10-11 Thread Jan Synacek
Hello all,

looking at the current code, SystemKeepFree= is not accounted for when
doing vacuuming, only SystemMaxUse= is used. There was an ancient
RHEL-7 bug for systemd-219 with the exact same problem. Now I'm not
sure if that's actually a problem or not, but the documentation
suggests that SystemKeepFree= should be honored.

Is it a bug?

When is SystemKeepFree= actually used?

Why have SystemKeepFree= at all if it's the "other way around" of
SystemMaxUse= ?

Cheers,
-- 
Jan Synacek
Software Engineer, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] Github systemd issue 6237

2017-07-11 Thread Jan Synacek
On Mon, Jul 10, 2017 at 4:41 PM, Lennart Poettering
<lenn...@poettering.net> wrote:
> On Mon, 10.07.17 15:58, Lennart Poettering (lenn...@poettering.net) wrote:
>
>> On Mon, 10.07.17 15:16, Jan Synacek (jsyna...@redhat.com) wrote:
>>
>> > On Mon, Jul 10, 2017 at 12:42 PM, Lennart Poettering
>> > <lenn...@poettering.net> wrote:
>> > > Now, because this is so weakly defined, we hence do not follow POSIX
>> > > rules, but filter out more that might be dangerous. Specifically:
>> > >
>> > > 1. We do not permit empty usernames
>> > > 2. We don't permit the first character to be numeric
>> > >(This also filters out fully numeric user names)
>> > > 3. We do not permit dots in usernames, neither at the beginning nor in
>> > >the middle.
>> > > 4. We do not permit "-" at the beginning of usernames (something which
>> > >POSIX explicitly suggests, btw)
>> > > 5. We require that the user name fits in the utmp user name field, so
>> > >that we can always log properly about it.
>> >
>> > Is this documented somewhere? If not, it would be great to have it
>> > documented. I'm pretty sure that this exact paragraph would be ok.
>>
>> There's a longer (and not entirely complete) comment about this in the
>> sources, but other than that it's not explicitly documented.
>>
>> If you prep a patch that adds this to the User=/Group= man page, this
>> would certainly be welcome. However, it should be reworded, as we
>> shouldn't say "We" there, and probably drop explicit references to
>> POSIX and utmp there, and instead just dryly state the accepted
>> character set + minimum and maximum string lengths.
>
> I have posted a PR documenting this just now:
>
> https://github.com/systemd/systemd/pull/6321

Thanks for the fast response!

-- 
Jan Synacek
Software Engineer, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] Github systemd issue 6237

2017-07-10 Thread Jan Synacek
On Mon, Jul 10, 2017 at 12:42 PM, Lennart Poettering
<lenn...@poettering.net> wrote:
> Now, because this is so weakly defined, we hence do not follow POSIX
> rules, but filter out more that might be dangerous. Specifically:
>
> 1. We do not permit empty usernames
> 2. We don't permit the first character to be numeric
>(This also filters out fully numeric user names)
> 3. We do not permit dots in usernames, neither at the beginning nor in
>the middle.
> 4. We do not permit "-" at the beginning of usernames (something which
>POSIX explicitly suggests, btw)
> 5. We require that the user name fits in the utmp user name field, so
>that we can always log properly about it.

Is this documented somewhere? If not, it would be great to have it
documented. I'm pretty sure that this exact paragraph would be ok.

Cheers,
-- 
Jan Synacek
Software Engineer, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] Disabling 'Predictable Network Interface Names'

2017-03-07 Thread Jan Synacek
On Tue, Mar 7, 2017 at 9:25 AM, Lucas Ventura Carro
<useyour.m...@gmail.com> wrote:
> Hello,
>
> I've recently installed a CentOS 7 minimal ISO[1], where I found the new
> Predictable Network Interface Naming strategy enabled.
>
> But in my current environment this naming strategy is not viable, and I'd
> like to get back to old _unpredictable_ strategy for all the interfaces. So
> according to documentation on how to disable[2] there are 2 options:
> (1) Creating a symlink
> (2) Changing a kernel boot parameter
>
> But, using option (1) doesn't work, and I'm still having predictable names.
> It is a CentOS 7 issue? Because '/etc/systemd/network' folder did not
> existed in this clean install, I had to create myself.
> Inspecting the '/lib/udev/rules.d/80-net-name-slot.rules' will disable
> predictable names only if kernel boot param is present.

Hi,

check out the official Red Hat documentation [1] on how to disable the
naming scheme. If what's described there doesn't work, please, file a
bug report.

[1] 
https://access.redhat.com/documentation/en-US/Red_Hat_Enterprise_Linux/7/html/Networking_Guide/sec-Disabling_Consistent_Network_Device_Naming.html

Cheers,
-- 
Jan Synacek
Software Engineer, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] Scheme bindings

2015-11-20 Thread Jan Synacek
Zbigniew Jędrzejewski-Szmek <zbys...@in.waw.pl> writes:

> On Fri, Nov 13, 2015 at 09:27:17AM +0100, Jan Synáček wrote:
>> Hello,
>> 
>> if anybody lurking here and hacking on systemd also likes scheme, I
>> created bindings for GNU Guile [1]. The API is far from covered, but
>> journal API and sd_listen* stuff is usable. You can now write socket
>> activated services in scheme!
>> 
>> [1] https://github.com/jsynacek/guile-systemd
>
> Hi,
>
> when you construct a list like this, do you have normal or reverse order:
> for (i = 0; i < r; i++)
>s_fds = scm_cons(scm_from_int(SD_LISTEN_FDS_START+i), s_fds);
> ?

Good catch, it's reversed. But, does it really matter in practice?

> return sd_booted() ? SCM_BOOL_T : SCM_BOOL_F;
> → sd_booted can return negative for error.

_public_ int sd_booted(void) {
return laccess("/run/systemd/system/", F_OK) >= 0;
}

This returns a "boolean" value. I'm not really sure why it would return
anything else. But, the documentation indeed says that it can return
negative values when it fails.

Cheers,
-- 
Jan Synacek
Software Engineer, Red Hat


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


Re: [systemd-devel] Scheme bindings

2015-11-20 Thread Jan Synacek
David Timothy Strauss <da...@davidstrauss.net> writes:

> Would you be interested in moving this work to the systemd umbrella project
> on GitHub? You would still manage the team, but it may get more visibility.

Sure, why not.

> On Fri, Nov 13, 2015, 19:27 Jan Synáček <jsyna...@redhat.com> wrote:
>
>> Hello,
>>
>> if anybody lurking here and hacking on systemd also likes scheme, I
>> created bindings for GNU Guile [1]. The API is far from covered, but
>> journal API and sd_listen* stuff is usable. You can now write socket
>> activated services in scheme!
>>
>> [1] https://github.com/jsynacek/guile-systemd
>>
>> Have fun,
>> --
>> Jan Synacek
>> Software Engineer, Red Hat
>> ___
>> systemd-devel mailing list
>> systemd-devel@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/systemd-devel
>>

-- 
Jan Synacek
Software Engineer, Red Hat


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


Re: [systemd-devel] systemd-211 patch for FailureAction

2015-11-18 Thread Jan Synacek
Jay Burger <jay.bur...@us.fujitsu.com> writes:

> Hi,
>
> I am a complete newbie to this list and I am trying to find
> out if I can patch my systemd-211 to include the FailureAction
> feature. If so where can I obtain the patche(s)?
>
> We are using the yocto daisy distribution which has systemd-211
> and  due to silly process restrictions getting the dizzy version is not
> going to happen any time soon.
>
> Any help is greatly appreciated.
>
> -Jay

Hi,

upstream development is done on Github [1]. That's also where you find
all the patches. Be prepared to hunt for them, though, as systemd-211 is
pretty old.

[1] https://github.com/systemd/systemd/

Cheers,
-- 
Jan Synacek
Software Engineer, Red Hat


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


Re: [systemd-devel] systemd and intltool

2015-10-09 Thread Jan Synacek
Jan Synacek <jsyna...@redhat.com> writes:

> Lennart Poettering <lenn...@poettering.net> writes:
>
>> On Thu, 10.09.15 19:10, Michael Biebl (mbi...@gmail.com) wrote:
>>
>>> Hi,
>>> 
>>> reading https://wiki.gnome.org/Projects/GnomeCommon/Migration, it says
>>> that intltool is practically dead and one should use gettext directly.
>>> 
>>> Do we still need intltool in systemd? Does gettext have support for
>>> translating PolicyKit policy files?
>>
>> Happy to take a patch that removes the intltool hookup if it replaces
>> it with the right gettext hookup instead.
>
> I have investigated this a bit... AFAIK, gettext cannot be directly used
> to parse and merge translations into XML files. However, a simple python
> script instead of intltools should be enough for systemd's needs. I'll
> investigate further and possibly submit a pull request.

Submitted as https://github.com/systemd/systemd/pull/1513.

-- 
Jan Synacek
Software Engineer, Red Hat


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


Re: [systemd-devel] systemd and intltool

2015-10-08 Thread Jan Synacek
Michael Biebl <mbi...@gmail.com> writes:

> 2015-10-07 14:43 GMT+02:00 Jan Synacek <jsyna...@redhat.com>:
>> Lennart Poettering <lenn...@poettering.net> writes:
>
>>> Happy to take a patch that removes the intltool hookup if it replaces
>>> it with the right gettext hookup instead.
>>
>> I have investigated this a bit... AFAIK, gettext cannot be directly used
>> to parse and merge translations into XML files. However, a simple python
>> script instead of intltools should be enough for systemd's needs. I'll
>> investigate further and possibly submit a pull request.
>
> Afaiu, the plan is not to teach gettext about all different XML
> formats, but use itstool for that.

I didn't suggest that.

> https://mail.gnome.org/archives/desktop-devel-list/2015-October/msg0.html
>
> That's still work in progress, so maybe we just have to wait a little
> until this is sorted out.

I have tried itstool and works just fine.

@Lennart: Happy to take a patch to replace intltool with itstool?

-- 
Jan Synacek
Software Engineer, Red Hat


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


Re: [systemd-devel] systemd and intltool

2015-10-07 Thread Jan Synacek
Lennart Poettering <lenn...@poettering.net> writes:

> On Thu, 10.09.15 19:10, Michael Biebl (mbi...@gmail.com) wrote:
>
>> Hi,
>> 
>> reading https://wiki.gnome.org/Projects/GnomeCommon/Migration, it says
>> that intltool is practically dead and one should use gettext directly.
>> 
>> Do we still need intltool in systemd? Does gettext have support for
>> translating PolicyKit policy files?
>
> Happy to take a patch that removes the intltool hookup if it replaces
> it with the right gettext hookup instead.

I have investigated this a bit... AFAIK, gettext cannot be directly used
to parse and merge translations into XML files. However, a simple python
script instead of intltools should be enough for systemd's needs. I'll
investigate further and possibly submit a pull request.

-- 
Jan Synacek
Software Engineer, Red Hat


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


Re: [systemd-devel] remote-fs dependency/ordering on network

2015-06-23 Thread Jan Synacek
Lennart Poettering lenn...@poettering.net writes:

 On Mon, 22.06.15 14:49, Jan Synacek (jsyna...@redhat.com) wrote:

 Lukáš Nykrýn lnyk...@redhat.com writes:
 
  Jan Synáček píše v Čt 18. 06. 2015 v 15:41 +0200:
  Is remote-fs.target somehow dependent/ordered on network.target or
  network-online.target? I can't find anything that would suggest it
  actually is.
  
  Cheers,
 
  If I am not mistaken remote-fs.target should be after all netdev mounts
  and netdev mounts should be after network-online.target.
 
 I'm sure it should, but I don't see any evidence that it really is. My
 mnt-nfs.mount that was generated by the fstab generator is ordered
 before remote-fs.target, which is correct. However, I can't find any
 dependency between remote-fs.target, and network*. I'm quite puzzled how
 NFS mounts mounted on boot can actually work correctly right now.

 There's also remote-fs-pre.target. That's ordered before all NFS
 mounts, and that's what the online stuff should be ordered before.

All seems to be in order when the system is booting up. However, during
shutdown, the order in which network* and remote* are taken down seems
to be incorrect. If you could take a look at [1], that would help a bit,
since I'm really clueless right now.

[1] https://bugzilla.redhat.com/show_bug.cgi?id=1214466

Cheers,
-- 
Jan Synacek
Software Engineer, Red Hat


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


Re: [systemd-devel] remote-fs dependency/ordering on network

2015-06-22 Thread Jan Synacek
Lukáš Nykrýn lnyk...@redhat.com writes:

 Jan Synáček píše v Čt 18. 06. 2015 v 15:41 +0200:
 Is remote-fs.target somehow dependent/ordered on network.target or
 network-online.target? I can't find anything that would suggest it
 actually is.
 
 Cheers,

 If I am not mistaken remote-fs.target should be after all netdev mounts
 and netdev mounts should be after network-online.target.

I'm sure it should, but I don't see any evidence that it really is. My
mnt-nfs.mount that was generated by the fstab generator is ordered
before remote-fs.target, which is correct. However, I can't find any
dependency between remote-fs.target, and network*. I'm quite puzzled how
NFS mounts mounted on boot can actually work correctly right now.

-- 
Jan Synacek
Software Engineer, Red Hat


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


Re: [systemd-devel] [PATCH] journal, coredump: allow relative values in some configuration options

2015-05-28 Thread Jan Synacek
Lennart Poettering lenn...@poettering.net writes:

 Hmm, this doesn't look right. here we choose the hash table sizes to
 use for a file, and I doubt we should base this on the currently
 available disk space, since sizing the hashtable will have an effect
 on the entire lifetime of the file, during which the available disk
 space might change wildly.

 I think it would be best not to do relative sizes for the journal file
 max size at all, and continue to only support and absolute value for
 that. 

 +
 +uint64_t size_parameter_evaluate(const SizeParameter *sp, uint64_t 
 available) {
 +if (sp-value == (uint64_t) -1)
 +return (uint64_t) -1;
 +
 +if (sp-relative)
 +return sp-value * 0.01 * available;

 Hmm, so this implements this as percentage after all. as mentioned in
 my earlier mail, I think this should be normalized to 2^32 instead, so
 that 2^32 maps to 100%...

I realized that I got the patch wrong. What I really wanted was to take
percentage values of *disk size*, not available space. Using disk size
would make it constant. Having said that, is it ok to change even the
options that you said were the bad idea? Also, does it really make sense
to implement the relative values as a mapping as you have suggested? To
me it really doesn't, since you can't take more than 100% of disk space
is not possible (I don't really count thin LVs), and mapping to a
huge interval is just not as readable as using percentage. What is the
advantage of the mapping again? Sorry if I'm being thick.

Cheers,
-- 
Jan Synacek
Software Engineer, Red Hat


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


Re: [systemd-devel] [PATCH] WIP: conf-parser: allow config_parse_iec_off to parse percentages

2015-05-22 Thread Jan Synacek
Lennart Poettering lenn...@poettering.net writes:

 On Wed, 20.05.15 10:37, jsyna...@redhat.com (jsyna...@redhat.com) wrote:

 From: Jan Synacek jsyna...@redhat.com
 
 Allow certain configuration options to be specified as percentages. For
 example, in journald.conf, SystemMaxUse= can now also be specified as 33%.
 
 There is a slight problem with the patch. It parses option names to 
 determine
 what filesystem it should use to get the available space from. This approach
 is probably not the rigth thing to do, but I couldn't think of a better one.
 Another approach that came to my mind was to use the highest bit of the off_t
 value returned by the parser to indicate if the value was a percentage, or
 a normal value. This would require to rewrite a significant amount of code
 which now counts on the values being definite (not percentages), and would,
 IMHO, be hardly any improvement at all.
 
 What do you think? Is there a better way to implement this functionality? Is 
 it
 a real problem to parse the option lvalues like that?

 Yes, it's ugly! Also, it's problematic since disk sizes and space
 change dynamically, hence evaluating this only when parsing is not
 enough.

 What about this: introduce a new type:

 typedef struct SizeParameter {
 uint64_t value;
 bool relative;
 } SizeParameter;

 When .relative is false, then .value is an absolute value, otherwise
 it's a relative value normalized to let's say 0x1 (so that
 this value equals 100%, and values below it  100% and above it 
 100%).

Would you mind explaining this a bit more? I'm not sure if I understand
this correctly, especially the  100% and  %100 parts. It doesn't
seem to be needed at all. You always need the info from statvfs anyway,
if the value is a percentage. If not, the value can be used as-is.

 Then add new helper calls:

  int size_parameter_from_string(const char *s, SizeParameter *ret);
  uint64 size_parameter_evaluate(SizeParameter *p, uint64_t base);


 The latter should return .value as-is if p-relative is false, and
 (base * .value)  32 otherwise.

Why is base needed here?

 THen, change the appropriate places to use SizeParameter instead of
 simple uint64_t where necessary, and use size_parameter_evaluate()
 with the data from statvfs when the actual value is required.

Maybe I totally misunderstood your suggestion:) However, I tried to
implement something similar and it turned out to be non-trivial, since
there are quite a few places that need to be rewritten and tested, plus
there is some duplicate code in coredump.c that needs the testing as
well, so it may take a while.

 Lennart

Cheers,
-- 
Jan Synacek
Software Engineer, Red Hat


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


Re: [systemd-devel] [PATCH] systemctl: introduce -e and -d for start and stop

2015-05-14 Thread Jan Synacek
Lennart Poettering lenn...@poettering.net writes:

 On Wed, 13.05.15 15:21, jsyna...@redhat.com (jsyna...@redhat.com) wrote:

 From: Jan Synacek jsyna...@redhat.com

 Hmm, do we really need two options for this? I mean, since -e would
 only ever be combined with start, and -d only with stop it could as
 well be a single option that works for both?

Makes sense. I actually wanted to implement the one argument version
first, but then decided to make it more explicit. Will fix.

 Also, I am tempted to say that this should be reversed: instead of
 making this options that alter start/stop behaviour, it should be
 options that alter enable/disable behaviour, and actually take into
 account the precise units that were enabled, including everything
 referenced in the [Install] section of the unit files.

 hence, I think I would prefer something like this instead:

systemctl enable --now foobar.service
systemctl disable --now foobar.service

 Where --now simply means that the service shall be started after
 enabling, and stopped after disabling. 

 Does this make sense?

Yes, it does. I'll rewrite the patch.

Cheers,
-- 
Jan Synacek
Software Engineer, Red Hat


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


Re: [systemd-devel] [PATCH] cryptsetup-generator: support rd.luks.key=keyfile:keyfile_device

2015-04-24 Thread Jan Synacek
Lennart Poettering lenn...@poettering.net writes:

 On Fri, 20.02.15 10:56, Jan Synacek (jsyna...@redhat.com) wrote:

 Sorry for the late review.

 What's the precise background of this? Can you elaborate? Is there
 some feature request for this?

Hi,

I can see that Andrei already answered most of your questions.

Some time after writing this patch, I realized that I should just fix
dracut, so I did [1], and I forgot to leave a mention in this thread.
I'm not sure what happened to the dracut patch since then, though.

[1] http://thread.gmane.org/gmane.linux.kernel.initramfs/4072

Cheers,
-- 
Jan Synacek
Software Engineer, Red Hat


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


Re: [systemd-devel] [PATCH] tmpfiles: don't create subvolumes in chroot

2015-04-02 Thread Jan Synacek
Lennart Poettering lenn...@poettering.net writes:

 On Wed, 01.04.15 15:45, Jan Synacek (jsyna...@redhat.com) wrote:

  I am also against this since chrooting is an implementation detail of
  mock, nothing more, and the fact that mock's recursive deletion logic
  cannot handle removal of subvolumes is not directly connected to the
  fact that mock uses chroot.
 
  Sorry, but we need to find a different solution for this.
 
  Maybe mock should use seccomp to make the subvolume creation ioctls
  unavailable, or it should be updated to deal with subvolumes properly.
 
 I agree that mock should be enhanced to cope with subvolumes, but I also
 think that systemd shouldn't create them where it doesn't make
 sense. I don't think that that's achievable with the current logic. Am I
 missing something?

 But why do you say when it doesn't make sense? Why do you think this
 doesn't make sense...

I think that in mock root it doesn't. But that's a special case.

-- 
Jan Synacek
Software Engineer, Red Hat


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


Re: [systemd-devel] [PATCH] tmpfiles: don't create subvolumes in chroot

2015-04-02 Thread Jan Synacek
Lennart Poettering lenn...@poettering.net writes:

 On Thu, 02.04.15 08:59, Jan Synacek (jsyna...@redhat.com) wrote:

  think that systemd shouldn't create them where it doesn't make
  sense. I don't think that that's achievable with the current logic. Am I
  missing something?
 
  But why do you say when it doesn't make sense? Why do you think this
  doesn't make sense...
 
 I think that in mock root it doesn't. But that's a special case.

 Why not? Subvolumes are fully recursive, hence it doesn't really
 matter whether they are created on the host or in a chroot environment
 or a container or whatever else.

Ok, I discussed this with the maintainer of mock and wrote a patch for
it to be able to deal with subvolumes.

Thanks for the pointers!
-- 
Jan Synacek
Software Engineer, Red Hat


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


[systemd-devel] [PATCH] tmpfiles: don't create subvolumes in chroot

2015-04-01 Thread Jan Synacek
Creating subvolumes in chrooted environments makes them
undeletable and breaks mock.

https://bugzilla.redhat.com/show_bug.cgi?id=1205564

Jan Synacek (1):
  tmpfiles: don't create subvolumes in chroot

 src/tmpfiles/tmpfiles.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

-- 
2.1.0

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


[systemd-devel] [PATCH] tmpfiles: don't create subvolumes in chroot

2015-04-01 Thread Jan Synacek
---
 src/tmpfiles/tmpfiles.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c
index 494fd1a..9280fd7 100644
--- a/src/tmpfiles/tmpfiles.c
+++ b/src/tmpfiles/tmpfiles.c
@@ -1099,9 +1099,15 @@ static int create_item(Item *i) {
 
 break;
 
+case CREATE_SUBVOLUME:
+
+/* Don't create subvolumes in chrooted environments. */
+if (running_in_chroot())
+break;
+/* FALLTHROUGH */
+
 case CREATE_DIRECTORY:
 case TRUNCATE_DIRECTORY:
-case CREATE_SUBVOLUME:
 
 RUN_WITH_UMASK()
 mkdir_parents_label(i-path, 0755);
-- 
2.1.0

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


Re: [systemd-devel] [PATCH] tmpfiles: don't create subvolumes in chroot

2015-04-01 Thread Jan Synacek
Lennart Poettering lenn...@poettering.net writes:

 On Wed, 01.04.15 14:33, Jan Synacek (jsyna...@redhat.com) wrote:

 Creating subvolumes in chrooted environments makes them
 undeletable and breaks mock.

 Humm, I am not convinced that this is a good idea.

 The chroot environments are hardly undeletable, they just require
 you to delete them explicitly. There's work going on to tech
 btrfs-progs recursive deleting of subvolumes. I am pretty sure that's
 the right fix and mock should really be updated to deal with that...

undeletable was a bad wording from my side, sorry for that. What I
really meant is that mock simply couldn't deal with it... 

 I am also against this since chrooting is an implementation detail of
 mock, nothing more, and the fact that mock's recursive deletion logic
 cannot handle removal of subvolumes is not directly connected to the
 fact that mock uses chroot.

 Sorry, but we need to find a different solution for this.

 Maybe mock should use seccomp to make the subvolume creation ioctls
 unavailable, or it should be updated to deal with subvolumes properly.

I agree that mock should be enhanced to cope with subvolumes, but I also
think that systemd shouldn't create them where it doesn't make
sense. I don't think that that's achievable with the current logic. Am I
missing something?

Cheers,
-- 
Jan Synacek
Software Engineer, Red Hat


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


Re: [systemd-devel] SELinux labels on unix sockets

2015-03-10 Thread Jan Synacek
Lennart Poettering lenn...@poettering.net writes:

 On Fri, 06.03.15 13:04, Jan Synáček (jsyna...@redhat.com) wrote:

 Hello,
 
 when systemd creates a socket file, it explicitly calls a selinux
 procedure to label it. I don't think that is needed, as the kernel does
 the right thing when the socket is created. Am I missing something? Why
 is the explicit labeling in place?

 Well, it's complicated.

 If we use socket activation we label a socket taking into account the
 label of the binary that is eventually started for it.

 And then, for file system sockets the kernel could traditionally only
 derive the label to use from the directory the socket was created in,
 which makes it difficult to have multiple sockets in the same
 directory with different labels, which is pretty frequently done
 though. That said, I think this limitation was lifted a while back in
 the kernel, and the policy can now also take the socket file name into
 consideration and derive different labels automatically.

 Ultimately, I only superficially understand the selinux code. We rely
 on patches from Dan  co to keep it up-to-date. Better keep him in the
 loop.

If there is a way to specify the automatic labeling of the socket files
according to their names, and not the directory that they reside in, in
the policy, then the code that does the explicit labeling isn't
necessary. If not, the code would label the sockets incorrectly, which
is what actually happens now. Plus the fact that systemd doesn't
correctly re-require the libselinux handle (meaning that policy
updates/reloads are not recognized) on policy changes makes the logic
not work.

I've tried to write a small piece of code that would execute whenever a
policy is modified, but failed to do so. Calling
selinux_set_callback(SELINUX_CB_POLICYLOAD, cb) doesn't do anything.

So, I think that the code that explictly labels the socket files should
be removed.

It would be nice to hear from Dan, though.

Cheers,
-- 
Jan Synacek
Software Engineer, Red Hat


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


Re: [systemd-devel] [PATCH] cryptsetup-generator: support rd.luks.key=keyfile:keyfile_device

2015-02-24 Thread Jan Synacek
Andrei Borzenkov arvidj...@gmail.com writes:

 В Fri, 20 Feb 2015 10:56:41 +0100
 Jan Synacek jsyna...@redhat.com пишет:

 First version of the patch that allows rd.luks.key to be specified almost 
 the same way as dracut can
 read it.
 

 This sounds like working around dracut bug. Dracut already has code to
 deal with it, it updates /etc/crypttab and reload systemd to run
 generators but it completely ignores keyfile parameter in non-systemd
 branch.

 The code in dracut for systemd-enabled case does

 echo $luks $dev - timeout=0,$allowdiscards  /etc/crypttab
 systemctl daemon-reload
 systemctl start cryptsetup.target

 which means it won't even use keyfile anyway.

 Why do not you simply mount device here? Dracut already has code to
 temporary mount keyfile device in non-systemd-enabled case, you can
 simply reuse it.

I've done some digging around and...

I don't get it. It makes sense to put the functionality to dracut, so
why is this implemented *differently* in both dracut and systemd? Why is
there code to make this systemd-independent task in a systemd-enabled
and non-systemd-enabled case? It's quite confusing.

 The solution creates a temporary mount unit mnt.mount that the generated 
 cryptsetup service wants.
 The partition where the keyfile is then mounted to /mnt and the absolute 
 path to the keyfile is
 changed so it points to this temporary mount instead.
 
 I'm not sure if temporarily mounting something to /mnt in initrd is safe. If 
 not, what would be a
 preffered way to do this?
 

 Dracut creates unique name for it already.

Ok, I found the code. Thanks for the pointers!

 Also, there's a problem that I'm not sure how to solve. If the 
 keyfile_device is somehow
 misspecified (for example, the uuid simply isn't valid), the boot stalls. I 
 believe that this was
 one of the problems in https://bugzilla.redhat.com/show_bug.cgi?id=905683. I 
 was thinking about
 using udev to verify the uuid and its device, but I'm not sure if this makes 
 sense to do in
 initrd. Any pointers would be appreciated.
 
 Once the above problems are sorted out, an extension of this patch (perhaps 
 another patch?) would be
 to also support the third argument that rd.luks.key can take in dracut.
 
 Jan Synacek (1):
   cryptsetup-generator: support rd.luks.key=keyfile:keyfile_device
 
  src/cryptsetup/cryptsetup-generator.c | 163 
 +++---
  1 file changed, 150 insertions(+), 13 deletions(-)
 


-- 
Jan Synacek
Software Engineer, Red Hat


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


[systemd-devel] [PATCH] nspawn: fix whitespace in partition table blurb

2015-02-23 Thread Jan Synacek
---
 src/nspawn/nspawn.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c
index 0d8d199..9c5b8cd 100644
--- a/src/nspawn/nspawn.c
+++ b/src/nspawn/nspawn.c
@@ -2754,7 +2754,7 @@ static int setup_image(char **device_path, int *loop_nr) {
 
 #define PARTITION_TABLE_BLURB \
 Note that the disk image needs to either contain only a single MBR 
partition of\n \
-type 0x83 that is marked bootable, or a sinlge GPT partition of type 
\
+type 0x83 that is marked bootable, or a sinlge GPT partition of type 
 \
 0FC63DAF-8483-4772-8E79-3D69D8477DE4 or follow\n \
 
http://www.freedesktop.org/wiki/Specifications/DiscoverablePartitionsSpec/\n; \
 to be bootable with systemd-nspawn.
-- 
2.1.0

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


[systemd-devel] [PATCH] nspawn: fix whitespace and typo in partition table blurb

2015-02-23 Thread Jan Synacek
---
Changes in v2:
- fix additional typo on the same line, doh

 src/nspawn/nspawn.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c
index 0d8d199..a9b9a3e 100644
--- a/src/nspawn/nspawn.c
+++ b/src/nspawn/nspawn.c
@@ -2754,7 +2754,7 @@ static int setup_image(char **device_path, int *loop_nr) {
 
 #define PARTITION_TABLE_BLURB \
 Note that the disk image needs to either contain only a single MBR 
partition of\n \
-type 0x83 that is marked bootable, or a sinlge GPT partition of type 
\
+type 0x83 that is marked bootable, or a single GPT partition of type 
 \
 0FC63DAF-8483-4772-8E79-3D69D8477DE4 or follow\n \
 
http://www.freedesktop.org/wiki/Specifications/DiscoverablePartitionsSpec/\n; \
 to be bootable with systemd-nspawn.
-- 
2.1.0

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


Re: [systemd-devel] [PATCH] nspawn: fix whitespace in partition table blurb

2015-02-23 Thread Jan Synacek
Mantas Mikulėnas graw...@gmail.com writes:

 While you're at it,

 -sinlge
 +single

I have no idea how this one escaped me... Will fix, thanks!

-- 
Jan Synacek
Software Engineer, Red Hat


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


Re: [systemd-devel] [PATCH] cryptsetup-generator: support rd.luks.key=keyfile:keyfile_device

2015-02-22 Thread Jan Synacek
Andrei Borzenkov arvidj...@gmail.com writes:

 В Fri, 20 Feb 2015 10:56:42 +0100
 Jan Synacek jsyna...@redhat.com пишет:

 To be more consistent with how dracut parses rd.luks.key, it is now
 allowed to specified it in the format keyfile[:keyfile_device].
 
 Should keyfile_device be provided, it needs to be in UUID=uuid-here
 format. Also, keyfile path is then treated relatively to the root of the
 keyfile device.
 

 What makes UUID special? Why it cannot be anything mount understands
 (like LABEL=..., /dev/disk/by-uuid/...). systemd already has code to
 resolve it.

UUID uniquely identifies the device and is always there. AFAIK that is
not true for LABEL. Supporting LABEL would complicate things and since I
wasn't even sure if my solution was the way to go, I didn't bother with
it.

-- 
Jan Synacek
Software Engineer, Red Hat


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


Re: [systemd-devel] [PATCH] cryptsetup-generator: support rd.luks.key=keyfile:keyfile_device

2015-02-20 Thread Jan Synacek
Przemyslaw Rudy pru...@o2.pl writes:

 Could you use the rd.luks.key.tout= instead of hardcoded
 JobTimeoutSec=30, as the dracut does?

I didn't know about such parameter. In fact, I don't see it anywhere in
dracut.cmdline(5). If it really exists and just isn't documented, then
yes, it would probably make sense.

-- 
Jan Synacek
Software Engineer, Red Hat


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


[systemd-devel] [PATCH] cryptsetup-generator: support rd.luks.key=keyfile:keyfile_device

2015-02-20 Thread Jan Synacek
First version of the patch that allows rd.luks.key to be specified almost the 
same way as dracut can
read it.

The solution creates a temporary mount unit mnt.mount that the generated 
cryptsetup service wants.
The partition where the keyfile is then mounted to /mnt and the absolute path 
to the keyfile is
changed so it points to this temporary mount instead.

I'm not sure if temporarily mounting something to /mnt in initrd is safe. If 
not, what would be a
preffered way to do this?

Also, there's a problem that I'm not sure how to solve. If the keyfile_device 
is somehow
misspecified (for example, the uuid simply isn't valid), the boot stalls. I 
believe that this was
one of the problems in https://bugzilla.redhat.com/show_bug.cgi?id=905683. I 
was thinking about
using udev to verify the uuid and its device, but I'm not sure if this makes 
sense to do in
initrd. Any pointers would be appreciated.

Once the above problems are sorted out, an extension of this patch (perhaps 
another patch?) would be
to also support the third argument that rd.luks.key can take in dracut.

Jan Synacek (1):
  cryptsetup-generator: support rd.luks.key=keyfile:keyfile_device

 src/cryptsetup/cryptsetup-generator.c | 163 +++---
 1 file changed, 150 insertions(+), 13 deletions(-)

-- 
2.1.0

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


[systemd-devel] [PATCH] cryptsetup-generator: support rd.luks.key=keyfile:keyfile_device

2015-02-20 Thread Jan Synacek
To be more consistent with how dracut parses rd.luks.key, it is now
allowed to specified it in the format keyfile[:keyfile_device].

Should keyfile_device be provided, it needs to be in UUID=uuid-here
format. Also, keyfile path is then treated relatively to the root of the
keyfile device.

If no keyfile_device appears on the command line, keyfile is then
treated as an absolute path.

Examples:

rd.luks.key=/etc/key/secret-partition.key

The keyfile is treated as an absolute path.

rd.luks.key=/root.key:UUID=dead-beef

First, the device UUID=dead-beef is temporarily mounted in /mnt and the
absolute path to the keyfile is constructed as /mnt/root.key.
---
 src/cryptsetup/cryptsetup-generator.c | 163 +++---
 1 file changed, 150 insertions(+), 13 deletions(-)

diff --git a/src/cryptsetup/cryptsetup-generator.c 
b/src/cryptsetup/cryptsetup-generator.c
index 05061c0..3590787 100644
--- a/src/cryptsetup/cryptsetup-generator.c
+++ b/src/cryptsetup/cryptsetup-generator.c
@@ -43,6 +43,12 @@ typedef struct crypto_device {
 bool create;
 } crypto_device;
 
+typedef struct key_device {
+const crypto_device *device;
+char *keyfile;
+char *name;
+} key_device;
+
 static const char *arg_dest = /tmp;
 static bool arg_enabled = true;
 static bool arg_read_crypttab = true;
@@ -50,6 +56,39 @@ static bool arg_whitelist = false;
 static Hashmap *arg_disks = NULL;
 static char *arg_default_options = NULL;
 static char *arg_default_keyfile = NULL;
+static key_device *arg_key_device = NULL;
+
+static char *crypt_service_name_build(const char *name)
+{
+_cleanup_free_ char *e = NULL;
+
+e = unit_name_escape(name);
+if (!e)
+return e;
+
+return unit_name_build(systemd-cryptsetup, e, .service);
+}
+
+static key_device *get_key_device(void)
+{
+key_device *d;
+
+if (arg_key_device)
+return arg_key_device;
+
+d = new0(struct key_device, 1);
+if (!d)
+return NULL;
+
+arg_key_device = d;
+return arg_key_device;
+}
+
+static void free_key_device(key_device *kd)
+{
+free(kd-keyfile);
+free(kd-name);
+}
 
 static int create_disk(
 const char *name,
@@ -77,11 +116,7 @@ static int create_disk(
 return -EINVAL;
 }
 
-e = unit_name_escape(name);
-if (!e)
-return log_oom();
-
-n = unit_name_build(systemd-cryptsetup, e, .service);
+n = crypt_service_name_build(name);
 if (!n)
 return log_oom();
 
@@ -233,6 +268,76 @@ static int create_disk(
 return 0;
 }
 
+static int create_temporary_mount(void)
+{
+_cleanup_fclose_ FILE *f = NULL;
+_cleanup_free_ char *p = NULL, *n = NULL, *c = NULL, *wants_dir = 
NULL, *to = NULL, *u = NULL;
+const char *m = mnt.mount;
+key_device *kd;
+
+kd = get_key_device();
+if (!kd)
+return log_oom();
+
+/* no uuid where we should search for the key was specified */
+if (!kd-name)
+return 0;
+
+
+if (!kd-device) {
+log_warning(No rd.luks.uuid specified. Can't generate a 
temporary mount unit);
+return 0;
+}
+
+p = strjoin(arg_dest, /, m, NULL);
+if (!p)
+return log_oom();
+
+
+f = fopen(p, wxe);
+if (!f)
+return log_error_errno(errno, Failed to open %s: %m, p);
+
+fprintf(f, # Automatically generated by 
systemd-cryptsetup-generator\n\n
+[Unit]\n
+Description=Temporary keyfile mount point.\n
+JobTimeoutSec=30\n);
+
+n = strjoin(luks-, kd-device-uuid, NULL);
+if (!n)
+return log_oom();
+
+c = crypt_service_name_build(n);
+if (!c)
+return log_oom();
+
+u = fstab_node_to_udev_node(kd-name);
+if (!u)
+return log_oom();
+
+fprintf(f, Before=%s\n\n
+[Mount]\n
+What=%s\n
+Where=/mnt\n,
+c, u);
+
+wants_dir = strjoin(arg_dest, /, c, .wants, NULL);
+if (!wants_dir)
+return log_oom();
+
+to = strjoin(wants_dir, /, m, NULL);
+if (!to)
+return log_oom();
+
+if (mkdir_safe(wants_dir, 0700, 0, 0)  0)
+log_error(Failed to create %s: %m, wants_dir);
+
+if (symlink(../mnt.mount, to)  0)
+return log_error_errno(errno, Failed to create symlink %s: 
%m, to);
+
+return 0;
+}
+
 static void free_arg_disks(void) {
 crypto_device *d;
 
@@ -282,6 +387,7 @@ static crypto_device *get_crypto_device(const char *uuid) {
 static int parse_proc_cmdline_item(const char *key, const char *value) {
 int r;
 crypto_device *d;
+key_device *kd;
 _cleanup_free_ 

Re: [systemd-devel] test-dhcp-client failing in mock builds

2015-02-01 Thread Jan Synacek
Zbigniew Jędrzejewski-Szmek zbys...@in.waw.pl writes:

 Hi,

 I was trying to enable tests in the %check part of systemd rpm.
 Something strange happens which causes test-dhcp-client when building
 in mock:

 FAIL: test-dhcp-client
 ==

http://lists.freedesktop.org/archives/systemd-devel/2014-December/026190.html

I haven't got time to properly analyze the problem since then...

-- 
Jan Synacek
Software Engineer, Red Hat


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


[systemd-devel] [PATCH] systemctl: fix argument handling when invoked as shutdown

2014-12-15 Thread Jan Synacek
---
 src/systemctl/systemctl.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c
index 8400bc8..91e8f7c 100644
--- a/src/systemctl/systemctl.c
+++ b/src/systemctl/systemctl.c
@@ -6942,7 +6942,7 @@ static int shutdown_parse_argv(int argc, char *argv[]) {
 assert(argc = 0);
 assert(argv);
 
-while ((c = getopt_long(argc, argv, HPrhkt:afFc, options, NULL)) = 
0)
+while ((c = getopt_long(argc, argv, HPrhkKt:afFc, options, NULL)) = 
0)
 switch (c) {
 
 case ARG_HELP:
@@ -6983,6 +6983,8 @@ static int shutdown_parse_argv(int argc, char *argv[]) {
 
 case 't':
 case 'a':
+case 'f':
+case 'F':
 /* Compatibility nops */
 break;
 
-- 
2.2.0

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


[systemd-devel] [PATCH] virt: fix container detection when we're not PID 1

2014-12-10 Thread Jan Synacek
systemd-detect-virt would print none when using nspawn to run a shell
inside a container and then running systemd-detect-virt in it, because
the shell would be PID 1, not the actuall systemd-detect-virt process.
---
 src/shared/virt.c | 19 ++-
 1 file changed, 6 insertions(+), 13 deletions(-)

diff --git a/src/shared/virt.c b/src/shared/virt.c
index f9c4e67..298e005 100644
--- a/src/shared/virt.c
+++ b/src/shared/virt.c
@@ -275,18 +275,10 @@ int detect_container(const char **id) {
 goto finish;
 }
 
-if (getpid() == 1) {
-/* If we are PID 1 we can just check our own
- * environment variable */
-
-e = getenv(container);
-if (isempty(e)) {
-r = 0;
-goto finish;
-}
-} else {
-
-/* Otherwise, PID 1 dropped this information into a
+/* Check our own environment variable */
+e = getenv(container);
+if (isempty(e)) {
+/* PID 1 dropped this information into a
  * file in /run. This is better than accessing
  * /proc/1/environ, since we don't need CAP_SYS_PTRACE
  * for that. */
@@ -300,7 +292,8 @@ int detect_container(const char **id) {
 return r;
 
 e = m;
-}
+} else
+r = 0;
 
 /* We only recognize a selected few here, since we want to
  * enforce a redacted namespace */
-- 
1.9.3

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


Re: [systemd-devel] [PATCH] test: fix some tests when running inside a container

2014-12-10 Thread Jan Synacek
Jan Synacek jsyna...@redhat.com writes:
 ---
  src/test/test-execute.c | 2 +-
  src/test/test-util.c| 3 ---
  test/udev-test.pl   | 8 
  3 files changed, 9 insertions(+), 4 deletions(-)

 diff --git a/src/test/test-util.c b/src/test/test-util.c
 index 20e711d..c055955 100644
 --- a/src/test/test-util.c
 +++ b/src/test/test-util.c
 @@ -495,7 +495,6 @@ static void test_get_process_comm(void) {
  pid_t e;
  uid_t u;
  gid_t g;
 -dev_t h;
  int r;
  pid_t me;
  
 @@ -544,8 +543,6 @@ static void test_get_process_comm(void) {
  assert_se(r = 0 || r == -EACCES);
  log_info(self strlen(environ): '%zd', strlen(env));
  
 -assert_se(get_ctty_devnr(1, h) == -ENOENT);
 -
  getenv_for_pid(1, PATH, i);
  log_info(pid1 $PATH: '%s', strna(i));

This part is wrong, sorry about that, I'll send a new patch.

-- 
Jan Synacek
Software Engineer, Red Hat


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


[systemd-devel] [PATCH v2] test: fix some tests when running inside a container

2014-12-10 Thread Jan Synacek
---
v2:
* don't remove the assert, run in if not in a container

 src/test/test-execute.c | 2 +-
 src/test/test-util.c| 4 +++-
 test/udev-test.pl   | 8 
 3 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/src/test/test-execute.c b/src/test/test-execute.c
index 85deb27..60466f0 100644
--- a/src/test/test-execute.c
+++ b/src/test/test-execute.c
@@ -164,7 +164,7 @@ int main(int argc, char *argv[]) {
 r = manager_new(SYSTEMD_USER, true, m);
 if (IN_SET(r, -EPERM, -EACCES, -EADDRINUSE, -EHOSTDOWN, -ENOENT)) {
 printf(Skipping test: manager_new: %s, strerror(-r));
-return -EXIT_TEST_SKIP;
+return EXIT_TEST_SKIP;
 }
 assert_se(r = 0);
 assert_se(manager_startup(m, NULL, NULL) = 0);
diff --git a/src/test/test-util.c b/src/test/test-util.c
index 20e711d..fe54586 100644
--- a/src/test/test-util.c
+++ b/src/test/test-util.c
@@ -35,6 +35,7 @@
 #include def.h
 #include fileio.h
 #include conf-parser.h
+#include virt.h
 
 static void test_streq_ptr(void) {
 assert_se(streq_ptr(NULL, NULL));
@@ -544,7 +545,8 @@ static void test_get_process_comm(void) {
 assert_se(r = 0 || r == -EACCES);
 log_info(self strlen(environ): '%zd', strlen(env));
 
-assert_se(get_ctty_devnr(1, h) == -ENOENT);
+if (!detect_container(NULL))
+assert_se(get_ctty_devnr(1, h) == -ENOENT);
 
 getenv_for_pid(1, PATH, i);
 log_info(pid1 $PATH: '%s', strna(i));
diff --git a/test/udev-test.pl b/test/udev-test.pl
index 14f11df..3e05b61 100755
--- a/test/udev-test.pl
+++ b/test/udev-test.pl
@@ -27,6 +27,7 @@ my $udev_dev= test/dev;
 my $udev_run= test/run;
 my $udev_rules_dir  = $udev_run/udev/rules.d;
 my $udev_rules  = $udev_rules_dir/udev-test.rules;
+my $EXIT_TEST_SKIP  = 77;
 
 my @tests = (
 {
@@ -1485,6 +1486,13 @@ if (!($==0)) {
 exit;
 }
 
+# skip the test when running in a container
+system(systemd-detect-virt, -c, -q);
+if ($?  8 == 0) {
+print Running in a container, skipping the test.\n;
+exit($EXIT_TEST_SKIP);
+}
+
 udev_setup();
 
 my $test_num = 1;
-- 
1.9.3

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


[systemd-devel] [PATCH] test: fix some tests when running inside a container

2014-12-10 Thread Jan Synacek
---
 src/test/test-execute.c | 2 +-
 src/test/test-util.c| 3 ---
 test/udev-test.pl   | 8 
 3 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/src/test/test-execute.c b/src/test/test-execute.c
index 85deb27..60466f0 100644
--- a/src/test/test-execute.c
+++ b/src/test/test-execute.c
@@ -164,7 +164,7 @@ int main(int argc, char *argv[]) {
 r = manager_new(SYSTEMD_USER, true, m);
 if (IN_SET(r, -EPERM, -EACCES, -EADDRINUSE, -EHOSTDOWN, -ENOENT)) {
 printf(Skipping test: manager_new: %s, strerror(-r));
-return -EXIT_TEST_SKIP;
+return EXIT_TEST_SKIP;
 }
 assert_se(r = 0);
 assert_se(manager_startup(m, NULL, NULL) = 0);
diff --git a/src/test/test-util.c b/src/test/test-util.c
index 20e711d..c055955 100644
--- a/src/test/test-util.c
+++ b/src/test/test-util.c
@@ -495,7 +495,6 @@ static void test_get_process_comm(void) {
 pid_t e;
 uid_t u;
 gid_t g;
-dev_t h;
 int r;
 pid_t me;
 
@@ -544,8 +543,6 @@ static void test_get_process_comm(void) {
 assert_se(r = 0 || r == -EACCES);
 log_info(self strlen(environ): '%zd', strlen(env));
 
-assert_se(get_ctty_devnr(1, h) == -ENOENT);
-
 getenv_for_pid(1, PATH, i);
 log_info(pid1 $PATH: '%s', strna(i));
 }
diff --git a/test/udev-test.pl b/test/udev-test.pl
index 14f11df..3e05b61 100755
--- a/test/udev-test.pl
+++ b/test/udev-test.pl
@@ -27,6 +27,7 @@ my $udev_dev= test/dev;
 my $udev_run= test/run;
 my $udev_rules_dir  = $udev_run/udev/rules.d;
 my $udev_rules  = $udev_rules_dir/udev-test.rules;
+my $EXIT_TEST_SKIP  = 77;
 
 my @tests = (
 {
@@ -1485,6 +1486,13 @@ if (!($==0)) {
 exit;
 }
 
+# skip the test when running in a container
+system(systemd-detect-virt, -c, -q);
+if ($?  8 == 0) {
+print Running in a container, skipping the test.\n;
+exit($EXIT_TEST_SKIP);
+}
+
 udev_setup();
 
 my $test_num = 1;
-- 
1.9.3

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


Re: [systemd-devel] [PATCH] virt: fix container detection when we're not PID 1

2014-12-10 Thread Jan Synacek
Lennart Poettering lenn...@poettering.net writes:
 On Wed, 10.12.14 09:21, Jan Synacek (jsyna...@redhat.com) wrote:

 systemd-detect-virt would print none when using nspawn to run a shell
 inside a container and then running systemd-detect-virt in it, because
 the shell would be PID 1, not the actuall systemd-detect-virt
 process.

 So, previously the code read the env var directly from
 /proc/1/environ, but that file is only readable with privs, hence I
 added code to PID 1 to write the value of that env var to
 /run/systemd/container which is readable without privs. Now, if you
 run a shell as PID 1 that will obviously not happen and the detection
 won't work after all. 

 Simply relying that $container is inherited from PID 1 down is
 something I'd really like to avoid, though.

Could you please explain why? I don't see why that might be a problem.

 I have now made a change to the code that falls back to
 getenv_for_pid() if /rub/systemd/container does not exist. THis will
 only be ffective with perms however. The new code hence still isn't
 perfect: if you boot up with only a shell as PID 1 and drop privileges
 the code will still not be able to detect the container manager. Not
 sure what other option we have, though.

Thanks!

-- 
Jan Synacek
Software Engineer, Red Hat


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


Re: [systemd-devel] emergency, rescue and single-user

2014-12-09 Thread Jan Synacek
Lennart Poettering lenn...@poettering.net writes:
 On Tue, 09.12.14 13:43, Jan Synáček (jsyna...@redhat.com) wrote:

 Hello,
 
 what is the difference between emergency, rescue and single-user?
 On F21, systemd-216-12.fc21.x86_64, they all boot into something that
 presents itself as Welcome to emergency mode! and they all require a
 root password. In case of booting into emergency.target, I can see
 Starting Emergency Shell in the console output. In single-user and
 rescue.target, I can see Starting Rescue Shell, but they all look the
 same. systemd.special(7) doesn't help much.

 rescue is simply how we call the old sysv single user mode. This
 means all early-boot services are started, but no later boot
 service. File systems are hence checked, udev is started, and so
 on. You get your shell right after sysinit.target but before
 basic.target basically.

 emergency maps to the emergency mode that sysvinit already knew:
 it just starts a shell, and does nothing else. No early-boot services
 are run. No udev, no file system checks, no nothing.

 Lennart

Thanks for the explanation!

-- 
Jan Synacek
Software Engineer, Red Hat


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


Re: [systemd-devel] emergency, rescue and single-user

2014-12-09 Thread Jan Synacek
Mantas Mikulėnas graw...@gmail.com writes:
 On Tue, Dec 9, 2014 at 2:43 PM, Jan Synáček jsyna...@redhat.com wrote:

 Hello,

 what is the difference between emergency, rescue and single-user?
 On F21, systemd-216-12.fc21.x86_64, they all boot into something that
 presents itself as Welcome to emergency mode! and they all require a
 root password. In case of booting into emergency.target, I can see
 Starting Emergency Shell in the console output. In single-user and
 rescue.target, I can see Starting Rescue Shell, but they all look the
 same. systemd.special(7) doesn't help much.


 rescue.target pulls in sysinit.target (mounts, swaps, udev, sysctl...),
 while emergency.target starts a sulogin shell and nothing more. See the
 graph in bootup(7).

Ok, good to know about bootup(7), thanks!

-- 
Jan Synacek
Software Engineer, Red Hat


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


[systemd-devel] [PATCH v2] localed: log xkbcommon errors

2014-12-03 Thread Jan Synacek
The errors are prefixed with libxkbcommon to provide some context,
because they are quite confusing without it. With the prefix, we at
least know where they come from.
---
Changes in v2:
- don't log a null message if vasprintf() fails

 src/locale/localed.c | 19 +++
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/src/locale/localed.c b/src/locale/localed.c
index 654b291..c4e4829 100644
--- a/src/locale/localed.c
+++ b/src/locale/localed.c
@@ -1009,7 +1009,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 *msg = NULL;
+const char *fmt;
+char prefix[] = libxkbcommon: ;
+
+fmt = strappenda(prefix, format);
+if (vasprintf(msg, fmt, args)  0) {
+(void) log_oom();
+return;
+}
+log_debug(%s, msg);
 }
 
 static int verify_xkb_rmlvo(const char *model, const char *layout, const char 
*variant, const char *options) {
@@ -1092,9 +1101,11 @@ static int method_set_x11_keyboard(sd_bus *bus, 
sd_bus_message *m, void *userdat
 return 1; /* No authorization for now, but the async 
polkit stuff will call us again when it has it */
 
 r = verify_xkb_rmlvo(model, layout, variant, options);
-if (r  0)
-log_warning_errno(r, Cannot compile XKB keymap for 
new x11 keyboard layout ('%s' / '%s' / '%s' / '%s'): %m,
-  strempty(model), strempty(layout), 
strempty(variant), strempty(options));
+if (r  0) {
+log_error_errno(r, Cannot compile XKB keymap for new 
x11 keyboard layout ('%s' / '%s' / '%s' / '%s'): %m,
+strempty(model), strempty(layout), 
strempty(variant), strempty(options));
+return sd_bus_error_set(error, 
SD_BUS_ERROR_INVALID_ARGS, Cannot compile XKB keymap, refusing);
+}
 
 if (free_and_strdup(c-x11_layout, layout)  0 ||
 free_and_strdup(c-x11_model, model)  0 ||
-- 
1.9.3

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


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

2014-12-02 Thread Jan Synacek
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();
+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


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

2014-12-02 Thread Jan Synacek
Lennart Poettering lenn...@poettering.net writes:
 On Tue, 02.12.14 14:02, Jan Synacek (jsyna...@redhat.com) wrote:

 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();
 +e = xkb_context_get_user_data(ctx);
 +bus_error_setfv(e, SD_BUS_ERROR_INVALID_ARGS, fmt, args);

 I thought the plan now was to log them at debug level but not return
 them to the client?

 Also, strappenda()!

 Lennart

Please, disregard this patch submission. For some reason, I sent a
wrong version.

Sorry,
-- 
Jan Synacek
Software Engineer, Red Hat


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


[systemd-devel] [PATCH] localed: log xkbcommon errors

2014-12-02 Thread Jan Synacek
The errors are prefixed with libxkbcommon to provide some context,
because they are quite confusing without it. With the prefix, we at
least know where they come from.
---
 src/locale/localed.c | 17 +
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/src/locale/localed.c b/src/locale/localed.c
index 654b291..1d2673a 100644
--- a/src/locale/localed.c
+++ b/src/locale/localed.c
@@ -1009,7 +1009,14 @@ 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 *msg = NULL;
+const char *fmt;
+char prefix[] = libxkbcommon: ;
+
+fmt = strappenda(prefix, format);
+if (vasprintf(msg, fmt, args)  0)
+(void) log_oom();
+log_debug(%s, msg);
 }
 
 static int verify_xkb_rmlvo(const char *model, const char *layout, const char 
*variant, const char *options) {
@@ -1092,9 +1099,11 @@ static int method_set_x11_keyboard(sd_bus *bus, 
sd_bus_message *m, void *userdat
 return 1; /* No authorization for now, but the async 
polkit stuff will call us again when it has it */
 
 r = verify_xkb_rmlvo(model, layout, variant, options);
-if (r  0)
-log_warning_errno(r, Cannot compile XKB keymap for 
new x11 keyboard layout ('%s' / '%s' / '%s' / '%s'): %m,
-  strempty(model), strempty(layout), 
strempty(variant), strempty(options));
+if (r  0) {
+log_error_errno(r, Cannot compile XKB keymap for new 
x11 keyboard layout ('%s' / '%s' / '%s' / '%s'): %m,
+strempty(model), strempty(layout), 
strempty(variant), strempty(options));
+return sd_bus_error_set(error, 
SD_BUS_ERROR_INVALID_ARGS, Cannot compile XKB keymap, refusing);
+}
 
 if (free_and_strdup(c-x11_layout, layout)  0 ||
 free_and_strdup(c-x11_model, model)  0 ||
-- 
1.9.3

___
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

2014-12-01 Thread Jan Synacek
Lennart Poettering lenn...@poettering.net writes:
 On Tue, 25.11.14 10:01, David Herrmann (dh.herrm...@gmail.com) wrote:

 I explicitly ignore errors from verify_xkb_rmlvo() and proceed.
 libxkbcommon is still not 100% compatible to libxkb (and doesn't
 intend to be that, I guess). As we write X11 configs here, I just
 continue with a warning.
 
 If you call bus_error_setfv(), then sd-bus will return a method-error
 to the caller. However, you also send a method-return in
 method_set_x11_keyboard(). You thus end up with 2 calls.
 
 I'm not sure how to solve this. Furthermore, libxkbcommon can print a
 lot of informational warnings, and we shouldn't use just the last one.
 One idea would be to copy the same logic into localectl. You could
 also specify XKB_LOG_LEVEL and XKB_LOG_VERBOSITY as environment to get
 more information there.
 Yes, this would mean compiling the keymap twice, but it's for the sake
 of debugging output so I think it's fine.

 What precisely are the error messages libxkbcommon prints? Are they
 really material to pass to the client? I mean, are they really about
 some invalid data the client passed or just about something broken in
 the way the system is set up? If it's the latter than I figure we
 should return just a simple error to the client, and pass the full
 logs to the logging stream...

 Lennart

Thinking about it again, I don't think that it's a good idea to pass
those errors to the client...

A few examples:

# localectl set-x11-keymap QQ

Dec 01 12:02:06 fedora-rawhide-systemd-virt systemd-localed[877]: libxkbcommon: 
Couldn't find file symbols/QQ in include paths
Dec 01 12:02:06 fedora-rawhide-systemd-virt systemd-localed[877]: libxkbcommon: 
1 include paths searched:
Dec 01 12:02:06 fedora-rawhide-systemd-virt systemd-localed[877]: libxkbcommon: 
/usr/share/X11/xkb
Dec 01 12:02:06 fedora-rawhide-systemd-virt systemd-localed[877]: libxkbcommon: 
Abandoning symbols file (unnamed)
Dec 01 12:02:06 fedora-rawhide-systemd-virt systemd-localed[877]: libxkbcommon: 
Failed to compile xkb_symbols
Dec 01 12:02:06 fedora-rawhide-systemd-virt systemd-localed[877]: libxkbcommon: 
Failed to compile keymap
Dec 01 12:02:06 fedora-rawhide-systemd-virt systemd-localed[877]: Cannot 
compile XKB keymap for new x11 keyboard layout ('' / 'QQ' / '' / ''): Invalid 
argument

# localectl set-x11-keymap us pc105 QQ

Dec 01 12:03:03 fedora-rawhide-systemd-virt systemd-localed[896]: libxkbcommon: 
Couldn't process include statement for 'us(QQ)'
Dec 01 12:03:03 fedora-rawhide-systemd-virt systemd-localed[896]: libxkbcommon: 
Abandoning symbols file (unnamed)
Dec 01 12:03:03 fedora-rawhide-systemd-virt systemd-localed[896]: libxkbcommon: 
Failed to compile xkb_symbols
Dec 01 12:03:03 fedora-rawhide-systemd-virt systemd-localed[896]: libxkbcommon: 
Failed to compile keymap
Dec 01 12:03:03 fedora-rawhide-systemd-virt systemd-localed[896]: Cannot 
compile XKB keymap for new x11 keyboard layout ('pc105' / 'us' / 'QQ' / ''): 
Invalid argument

Those errors are just stupid. Maybe they would make sense if there was
also libxkbcommon code in front of me... But from the user's
perspective, they say nothing useful. I added the prefix so the errors
at least have some context.

My original patch would say something like No such layout 'QQ' in the
first case and No variant 'QQ' for layout 'us' in the second.

So, is it worth logging this information? Any other ideas?

Cheers,
-- 
Jan Synacek
Software Engineer, Red Hat


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


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

2014-11-25 Thread Jan Synacek
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();
+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


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

2014-11-25 Thread Jan Synacek
David Herrmann dh.herrm...@gmail.com writes:
 Hi Jan!

Hello!

 On Tue, Nov 25, 2014 at 9:23 AM, Jan Synacek jsyna...@redhat.com wrote:
 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();

 I think you can safely use:

 char fmt[LINE_MAX];

 snprintf(fmt, sizeof(fmt), libxkbcommon: %s, format);
 e = xkb_context_get_user_data(ctx);
 bus_error_setfv(e, SD_BUS_ERROR_INVALID_ARGS, fmt, args);

 We use LINE_MAX as explicit limit on a lot of these calls (and I think
 it makes sense to prevent unbound allocations).

I don't understand this. What do you mean by unbound allocation? That
libxkbcommon could, in theory, return a huge format that would exhaust
all memory?

 +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));

 I explicitly ignore errors from verify_xkb_rmlvo() and proceed.
 libxkbcommon is still not 100% compatible to libxkb (and doesn't
 intend to be that, I guess). As we write X11 configs here, I just
 continue with a warning.

 If you call bus_error_setfv(), then sd-bus will return a method-error
 to the caller. However, you also send a method-return in
 method_set_x11_keyboard(). You thus end up with 2 calls.

Hmm, I thought that bus_error_setfv() only fills the error struct. Is
there any documentation that desribes how the internal bus_* stuff
works?

 I'm not sure how to solve this. Furthermore, libxkbcommon can print a
 lot of informational warnings, and we shouldn't use just the last one.
 One idea would be to copy the same logic into localectl. You could
 also specify XKB_LOG_LEVEL and XKB_LOG_VERBOSITY as environment to get
 more information there.
 Yes, this would mean compiling the keymap twice, but it's for the sake
 of debugging output so I think it's fine.

I don't think that would work. You would have the same code on the
client and on the server and that might not be the same xkb
configuration.

 Thanks
 David

-- 
Jan Synacek
Software Engineer, Red Hat


signature.asc
Description: PGP signature
___
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

2014-11-24 Thread Jan Synacek
Lennart Poettering lenn...@poettering.net writes:
 On Fri, 14.11.14 12:42, Jan Synacek (jsyna...@redhat.com) wrote:
 +if (look_for == LAYOUTS) {
 +Set *s;
 +char *k;
 +Iterator i;
 +/* XXX: Is there a better way to sort Hashmap keys? */
 +_cleanup_strv_free_ char **tmp = NULL;
 +
 +HASHMAP_FOREACH_KEY(s, k, keymap-x11_layouts, i)
 +if (strv_extend(tmp, k)  0)
 +(void) log_oom();

 There's hashmap_get_strv() for cases like this.

I need keys, not values. I didn't find any hashmap_get_strv() equivalent
for keys.


 Also, please neer invoke strv_extend() when appending to unbounded
 arrays. It's slow! 

What is a preffered way to extend a strv? In this case, I know the final
size, so it the array could manually be copied. I don't think that
that's very nice, though.

 +void xkb_keymap_free_components(X11Keymap *keymap) {
 +if (keymap-x11_layouts) {
 +Set *s;
 +char *k;
 +Iterator i;
 +
 +HASHMAP_FOREACH_KEY(s, k, keymap-x11_layouts, i) {
 +free(k);
 +set_free_free(s);
 +}

 Humm, when clearing up hashmaps please just write a loop that steals
 the first entry, free it and then repeat. Iterating through a hashmap
 while deleting its entries is unnecessary...

I need to free its entries *and* keys. I didn't find any function that I
could use for that, apart from hashmap_free_free(), which crashes in my case.

 Lennart

 -- 
 Lennart Poettering, Red Hat

Cheers,
-- 
Jan Synacek
Software Engineer, Red Hat


signature.asc
Description: PGP signature
___
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

2014-11-19 Thread Jan Synacek
David Herrmann dh.herrm...@gmail.com writes:
 Hi

 On Fri, Nov 14, 2014 at 12:42 PM, Jan Synacek jsyna...@redhat.com wrote:
 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

 I didn't follow the discussion on v1, but why don't we use
 libxkbcommon to compile the keymap? If it doesn't compile, print an
 error (or warning and maybe optionally still proceed?).

 Sure, this would add a dependency to libxkbcommon for localed, but we
 could make it optional. And libxkbcommon has no dependencies by
 itself. Furthermore, set-x11-keymap is pretty useless without
 libxkbcommon installed, anyway.

 It'd be a ~10 line patch to use
 xkb_context_new()+xkb_keymap_from_rmlvo(). And it would be guaranteed
 to have the same semantics as the real keymap compilation.

 Thanks
 David

For some reason, I was under the impression that depending on
libxkbcommon would mean depending on plenty of X libraries... Using the
library and making the dependency on it optional sounds like the best
solution to me.

Waiting for more opinions.

Cheers,
-- 
Jan Synacek
Software Engineer, Red Hat


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


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

2014-11-14 Thread 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;
-}
-
-if (state != look_for)
-continue;
-
-w = l + strcspn(l, WHITESPACE);
-
-if (n  1) {
-char *e;
-
-if (*w == 0)
-continue;
-
-*w = 0;
-w++;
-   

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

2014-11-11 Thread Jan Synacek
Lennart Poettering lenn...@poettering.net writes:
 On Tue, 04.11.14 12:05, Jan Synacek (jsyna...@redhat.com) wrote:

 diff --git a/src/locale/localectl.c b/src/locale/localectl.c
 index d4a2d29..8f9e4da 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;
 @@ -389,14 +390,7 @@ 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;
 +enum keymap_state look_for;

 While we don#t follow this rule too strictly we usually define typdefs
 for enums and name them in CamelCase. Hence, please rename this type
 KeymapState instead of enum keymap_state.

 That said, is state really the right name here? Shouldn't it be
 field or component or item or so?

  !streq_ptr(variant, c-x11_variant) ||
  !streq_ptr(options, c-x11_options)) {
 +_cleanup_free_ char *msg = NULL;
  
  if ((layout  !string_is_safe(layout)) ||
  (model  !string_is_safe(model)) ||
 @@ -1050,6 +1052,12 @@ static int method_set_x11_keyboard(sd_bus *bus, 
 sd_bus_message *m, void *userdat
  free_and_strdup(c-x11_options, options)  0)
  return -ENOMEM;
  
 +r = xkb_validate_keymaps(model, layout, variant, options, 
 msg);
 +if (r  0) {
 +log_error(Failed to validate X11 keyboard layout: 
 %s, strerror(-r));
 +return sd_bus_error_set(error, SD_BUS_ERROR_FAILED, 
 msg);
 +}
 +

 Please return the error back to the client instead of logging it
 away. Use sd_bus_error_setf() for that.

The errno part of the error is logged and the digestible by the user
part is returned in the error message. I did this in exactly the same
way as it was already there a few lines below (x11_write_data()
call). Are you sure I should change it? If yes, both log_error()s should
probably be changed.

 Use SD_BUS_ERROR_INVALID_ARGS as error id.

 +
 +static char **xkb_parse_argument(const char *arg)
 +{
 +_cleanup_free_ char *input;
 +char *token;
 +char **result = NULL;
 +int r;
 +
 +assert(arg);
 +
 +input = strdup(arg);
 +if (!input)
 +return NULL;
 +
 +token = strsep(input, ,);
 +while(token) {
 +r = strv_extend(result, token);
 +if (r  0)
 +return NULL;
 +token = strsep(input, ,);
 +}
 +
 +return result;
 +}

 Please place the opening { of a function body on the same line as the
 function declaration.

Oops, old habit, sorry, will fix.

 I figure strv_split() does exactly the same thing, please use that.

That's what I had originally used. The problem is that it throws away
the empty parts, which is what I don't want. I need it to return the
empty strings after splitting as well. That's why I wrote my own that
uses strsep(). Since I figured it's probably not needed anywhere else, I
wrote it locally instead of introducing a new shared strv_something
function.

 +int xkb_get_keymaps(char ***list, enum keymap_state look_for, const char 
 *layout_prefix)
 +{
 +_cleanup_fclose_ FILE *f;
 +char line[LINE_MAX];
 +enum keymap_state state = NONE;
 +int r;
 +
 +f = fopen(/usr/share/X11/xkb/rules/base.lst, re);
 +if (!f) {
 +log_error(Failed to open keyboard mapping list. %m);
 +return -errno;
 +}

 This should probably become a function call that returns errors but
 doesn't log about them, leaving that to the caller.

Again, are you sure? The logged error is very local to what the code is
trying to do.

 +int xkb_validate_keymaps(const char *model,
 + const char *layouts_arg,
 + const char *variants_arg,
 + const char *options_arg,
 + char **error)
 +{
 +int r;
 +
 +if (layouts_arg) {
 +_cleanup_strv_free_ char **layouts_list = NULL;
 +char **it, **layouts;
 +int i = 0;
 +
 +r = xkb_get_keymaps(layouts_list, LAYOUTS, NULL);
 +if (r  0)
 +return r;
 +
 +layouts = strv_split(layouts_arg, ,);
 +if (!layouts)
 +return log_oom();
 +
 +STRV_FOREACH

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

2014-11-11 Thread Jan Synacek
Jan Synacek jsyna...@redhat.com writes:
 Lennart Poettering lenn...@poettering.net writes:
 On Tue, 04.11.14 12:05, Jan Synacek (jsyna...@redhat.com) wrote:
 +int xkb_validate_keymaps(const char *model,
 + const char *layouts_arg,
 + const char *variants_arg,
 + const char *options_arg,
 + char **error)
 +{
 +int r;
 +
 +if (layouts_arg) {
 +_cleanup_strv_free_ char **layouts_list = NULL;
 +char **it, **layouts;
 +int i = 0;
 +
 +r = xkb_get_keymaps(layouts_list, LAYOUTS, NULL);
 +if (r  0)
 +return r;
 +
 +layouts = strv_split(layouts_arg, ,);
 +if (!layouts)
 +return log_oom();
 +
 +STRV_FOREACH(it, layouts) {
 +_cleanup_strv_free_ char **variants_list = NULL;
 +bool variants_left = true;
 +int n;
 +
 +if (!strv_find(layouts_list, *it)) {
 +r = asprintf(error, Requested layout '%s' 
 not available.\n, *it);
 +if (r  0)
 +return log_oom();
 +return -EINVAL;
 +}
 +
 +if (variants_left  variants_arg) {
 +_cleanup_strv_free_ char **variants;
 +
 +r = xkb_get_keymaps(variants_list, 
 VARIANTS, *it);
 +if (r  0)
 +return r;

 Hmm, reading the file over and over and over again sounds less than
 ideal. Maybe we should intrdouce a struct here and then make
 xkb_get_keymaps() return an array of structs really?

 That sounds ok, I'll see what I can do. I wanted to preserve as much of
 the original code as I could, but maybe it wasn't the right decision.

After thinking about it, I can put the layout and its variants into a
structure, but... When parsing /usr/share/X11/xkb/rules/base.lst, is
it safe to depend on the ordering of all the components in the file? I
mean, is it safe to expect layouts being order before variants in that
file? Or can it be vice versa? The code would have to be more general if
one cannot depend on that order, and I think it would be quite ugly. The
strv implementation doesn't have that problem.

-- 
Jan Synacek
Software Engineer, Red Hat


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


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

2014-11-11 Thread Jan Synacek
Lennart Poettering lenn...@poettering.net writes:
 On Tue, 04.11.14 12:05, Jan Synacek (jsyna...@redhat.com) wrote:

 One more addition:

 +}
 +
 +int xkb_get_keymaps(char ***list, enum keymap_state look_for, const char 
 *layout_prefix)
 +{
 +_cleanup_fclose_ FILE *f;
 +char line[LINE_MAX];
 +enum keymap_state state = NONE;
 +int r;
 +
 +f = fopen(/usr/share/X11/xkb/rules/base.lst, re);
 +if (!f) {
 +log_error(Failed to open keyboard mapping list. %m);
 +return -errno;
 +}
 +
 +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;
 +}
 +
 +if (state != look_for)
 +continue;
 +
 +w = l + strcspn(l, WHITESPACE);
 +
 +if (layout_prefix) {
 +char *e;
 +
 +if (*w == 0)
 +continue;
 +
 +*w = 0;
 +w++;
 +w += strspn(w, WHITESPACE);
 +
 +e = strchr(w, ':');
 +if (!e)
 +continue;
 +
 +*e = 0;
 +
 +if (!streq(w, layout_prefix))
 +continue;
 +} else
 +*w = 0;
 +
 +r = strv_extend(list, l);
 +if (r  0)
 +return log_oom();


 I think, while we are at it, this should really be reworked to use
 GREEDY_REALLOC. See strv_split_quoted() for an example.

Could you please explain why? str_extend() uses realloc_multiply()
inside, which, to me, seems to be ok for this case.

 +}
 +
 +if (strv_isempty(*list)) {
 +log_error(Couldn't find any entries.); /* TODO: improve 
 error message */
 +return -ENOENT;
 +}
 +
 +return 0;


 Lennart

 -- 
 Lennart Poettering, Red Hat

-- 
Jan Synacek
Software Engineer, Red Hat


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


Re: [systemd-devel] [PATCH] shared/install: don't report 'static' when unit contains only Also=

2014-11-07 Thread Jan Synacek
Lennart Poettering lenn...@poettering.net writes:
 On Thu, 06.11.14 10:49, Jan Synacek (jsyna...@redhat.com) wrote:

 I think that this patch might be a bit ineffective, as it calls
 unit_file_load() again just to get an InstallContext. I wasn't sure
 how to get Also= targets in any other way.
 
 If such change makes sense, this patch should probably be considered a
 preview rather than something to be committed right away.

 Hmm, wouldn't it be nicer to introduce a new UnitFileState enum value
 for this?

 Maybe UNIT_FILE_ALSO or so? 

 I am not sure I like the idea of implicitly following the Also= setting here, 
 due
 to the awkwarndess if multiple units are listed and how to map exotic
 states of that other unit back to ours...

 Would that make sense?

 Lennart

Yes, that makes sense. What should a string representation of
UNIT_FILE_ALSO be? I don't think that reporting 'also' would feel right.

Also, is there a better way to find out if unit has any Also= targets
than how I did it?

Cheers,
-- 
Jan Synacek
Software Engineer, Red Hat


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


[systemd-devel] [PATCH] shared/install: when unit contains only Also=, report 'indirect'

2014-11-07 Thread Jan Synacek
Continuation of 
http://lists.freedesktop.org/archives/systemd-devel/2014-November/025041.html.

Jan Synacek (1):
  shared/install: when unit contains only Also=, report 'indirect'

 man/systemctl.xml |  5 +
 src/shared/install.c  | 45 +++--
 src/shared/install.h  |  2 ++
 src/systemctl/systemctl.c |  7 +++
 4 files changed, 41 insertions(+), 18 deletions(-)

-- 
1.9.3

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


[systemd-devel] [PATCH] shared/install: when unit contains only Also=, report 'indirect'

2014-11-07 Thread Jan Synacek
If a unit contains only Also=, with no Alias= or WantedBy=, it shouldn't
be reported as static. New 'indirect' status shall be introduced.

https://bugzilla.redhat.com/show_bug.cgi?id=864298
---
 man/systemctl.xml |  5 +
 src/shared/install.c  | 45 +++--
 src/shared/install.h  |  2 ++
 src/systemctl/systemctl.c |  7 +++
 4 files changed, 41 insertions(+), 18 deletions(-)

diff --git a/man/systemctl.xml b/man/systemctl.xml
index 7cbaa6c..fa85d0b 100644
--- a/man/systemctl.xml
+++ b/man/systemctl.xml
@@ -1013,6 +1013,11 @@ kobject-uevent 1 systemd-udevd-kernel.socket 
systemd-udevd.service
 entry0/entry
   /row
   row
+entryliteralindirect/literal/entry
+entryUnit's status is determined indirectly by another 
unit(s) specified in literalAlso=/literal in [Install] section/entry
+entry0/entry
+  /row
+  row
 entryliteraldisabled/literal/entry
 entryUnit is not enabled/entry
 entry1/entry
diff --git a/src/shared/install.c b/src/shared/install.c
index cab93e8..83d1093 100644
--- a/src/shared/install.c
+++ b/src/shared/install.c
@@ -840,6 +840,7 @@ static void install_info_free(InstallInfo *i) {
 strv_free(i-aliases);
 strv_free(i-wanted_by);
 strv_free(i-required_by);
+strv_free(i-also);
 free(i-default_instance);
 free(i);
 }
@@ -948,6 +949,7 @@ static int config_parse_also(
 size_t l;
 const char *word, *state;
 InstallContext *c = data;
+InstallInfo *i = userdata;
 
 assert(filename);
 assert(lvalue);
@@ -964,6 +966,10 @@ static int config_parse_also(
 r = install_info_add(c, n, NULL);
 if (r  0)
 return r;
+
+r = strv_extend(i-also, n);
+if (r  0)
+return r;
 }
 if (!isempty(state))
 log_syntax(unit, LOG_ERR, filename, line, EINVAL,
@@ -1043,7 +1049,8 @@ static int unit_file_load(
 const char *path,
 const char *root_dir,
 bool allow_symlink,
-bool load) {
+bool load,
+char ***also) {
 
 const ConfigTableItem items[] = {
 { Install, Alias,   config_parse_strv, 
0, info-aliases   },
@@ -1087,6 +1094,9 @@ static int unit_file_load(
 if (r  0)
 return r;
 
+if (also  !strv_isempty(info-also))
+*also = strv_copy(info-also);
+
 return
 (int) strv_length(info-aliases) +
 (int) strv_length(info-wanted_by) +
@@ -1099,7 +1109,8 @@ static int unit_file_search(
 LookupPaths *paths,
 const char *root_dir,
 bool allow_symlink,
-bool load) {
+bool load,
+char ***also) {
 
 char **p;
 int r;
@@ -1109,7 +1120,7 @@ static int unit_file_search(
 assert(paths);
 
 if (info-path)
-return unit_file_load(c, info, info-path, root_dir, 
allow_symlink, load);
+return unit_file_load(c, info, info-path, root_dir, 
allow_symlink, load, also);
 
 assert(info-name);
 
@@ -1120,7 +1131,7 @@ static int unit_file_search(
 if (!path)
 return -ENOMEM;
 
-r = unit_file_load(c, info, path, root_dir, allow_symlink, 
load);
+r = unit_file_load(c, info, path, root_dir, allow_symlink, 
load, also);
 if (r = 0) {
 info-path = path;
 path = NULL;
@@ -1149,7 +1160,7 @@ static int unit_file_search(
 if (!path)
 return -ENOMEM;
 
-r = unit_file_load(c, info, path, root_dir, 
allow_symlink, load);
+r = unit_file_load(c, info, path, root_dir, 
allow_symlink, load, also);
 if (r = 0) {
 info-path = path;
 path = NULL;
@@ -1167,7 +1178,8 @@ static int unit_file_can_install(
 LookupPaths *paths,
 const char *root_dir,
 const char *name,
-bool allow_symlink) {
+bool allow_symlink,
+char ***also) {
 
 _cleanup_(install_context_done) InstallContext c = {};
 InstallInfo *i;
@@ -1182,7 +1194,7 @@ static int unit_file_can_install(
 
 assert_se(i = ordered_hashmap_first(c.will_install));
 
-r = unit_file_search(c, i, paths, root_dir, allow_symlink, true);
+r = unit_file_search(c, i, paths, root_dir, allow_symlink, 

Re: [systemd-devel] [PATCH] shared/install: don't report 'static' when unit contains only Also=

2014-11-07 Thread Jan Synacek
Zbigniew Jędrzejewski-Szmek zbys...@in.waw.pl writes:
 On Fri, Nov 07, 2014 at 01:06:41PM +0100, Lennart Poettering wrote:
 On Fri, 07.11.14 09:49, Jan Synacek (jsyna...@redhat.com) wrote:
 
  Lennart Poettering lenn...@poettering.net writes:
   On Thu, 06.11.14 10:49, Jan Synacek (jsyna...@redhat.com) wrote:
  
   I think that this patch might be a bit ineffective, as it calls
   unit_file_load() again just to get an InstallContext. I wasn't sure
   how to get Also= targets in any other way.
   
   If such change makes sense, this patch should probably be considered a
   preview rather than something to be committed right away.
  
   Hmm, wouldn't it be nicer to introduce a new UnitFileState enum value
   for this?
  
   Maybe UNIT_FILE_ALSO or so? 
  
   I am not sure I like the idea of implicitly following the Also= setting 
   here, due
   to the awkwarndess if multiple units are listed and how to map exotic
   states of that other unit back to ours...
  
   Would that make sense?
  
   Lennart
  
  Yes, that makes sense. What should a string representation of
  UNIT_FILE_ALSO be? I don't think that reporting 'also' would feel
  right.
 Maybe I'm missing something, but wouldn't be enough to report is as
 'enabled'?

AFAIK, it can also be disabled... Take systemd-journal-gatewayd.service
as an example. It doesn't have anything but
Also=systemd-journal-gatewayd.socket in the Install section. If you
disable the socket, you would then return enabled, which would be
wrong.

Howerever, I'm not sure about more complicated setups.

 For some units adding another name from Also= really enables the unit,
 and for other units the name from Also= is mostly cosmetic. What I'm
 trying to say is that having or not the Also= name enabled is only approximate
 information and does not always tell us if the unit will be started.

 I'd prefer to redefine enabled/disabled/static as this unit has at
 least on of the declared links in the filesystem/the unit does not
 have any defined links in the filesystem/this unit does not declare
 any links in the filesystem, which is something that we can actually
 check.

 Zbyszek

Cheers,
-- 
Jan Synacek
Software Engineer, Red Hat


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


Re: [systemd-devel] [PATCH] shared/install: don't report 'static' when unit contains only Also=

2014-11-07 Thread Jan Synacek
Lennart Poettering lenn...@poettering.net writes:
 On Fri, 07.11.14 09:49, Jan Synacek (jsyna...@redhat.com) wrote:

 Lennart Poettering lenn...@poettering.net writes:
  On Thu, 06.11.14 10:49, Jan Synacek (jsyna...@redhat.com) wrote:
 
  I think that this patch might be a bit ineffective, as it calls
  unit_file_load() again just to get an InstallContext. I wasn't sure
  how to get Also= targets in any other way.
  
  If such change makes sense, this patch should probably be considered a
  preview rather than something to be committed right away.
 
  Hmm, wouldn't it be nicer to introduce a new UnitFileState enum value
  for this?
 
  Maybe UNIT_FILE_ALSO or so? 
 
  I am not sure I like the idea of implicitly following the Also= setting 
  here, due
  to the awkwarndess if multiple units are listed and how to map exotic
  states of that other unit back to ours...
 
  Would that make sense?
 
  Lennart
 
 Yes, that makes sense. What should a string representation of
 UNIT_FILE_ALSO be? I don't think that reporting 'also' would feel
 right.

 We should always keep the enum name and the string it translates to in
 sync. I can see that reporting also might be confusing, note sure
 what we could name it better though. But if we use a different string
 we should also rename its enum really.

 Maybe indirect? other? Hmm, see-also could work? With the
 counterpart UNIT_FILE_SEE_ALSO?

 Also, is there a better way to find out if unit has any Also= targets
 than how I did it?

 I think the best way would be to extend InstallInfo to get a new also
 strv field, that is upated by config_parse_also(). Then
 unit_file_can_install() can find it and return the fact that the list
 is not empty in an extra parameter.

Thanks for the pointers! I've resubmitted the patch and went with the
indirect version, as it felt the best to me.

-- 
Jan Synacek
Software Engineer, Red Hat


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


Re: [systemd-devel] [PATCH] shared/install: when unit contains only Also=, report 'indirect'

2014-11-07 Thread Jan Synacek
Lennart Poettering lenn...@poettering.net writes:
 On Fri, 07.11.14 15:18, Jan Synacek (jsyna...@redhat.com) wrote:

  }
  if (!isempty(state))
  log_syntax(unit, LOG_ERR, filename, line, EINVAL,
 @@ -1043,7 +1049,8 @@ static int unit_file_load(
  const char *path,
  const char *root_dir,
  bool allow_symlink,
 -bool load) {
 +bool load,
 +char ***also) {

 Hmm, do we really want to return the full list here? I don't think any
 caller really is interested in that, or am I wrong? Wouldn't a bool*
 suffice here that tells us if also is empty, that we fill in? 

No, it's not necessary. I'm not sure why I wrote it that way. Let's
blame it on friday...

 I mean, I think the inner calls should parse the whole strv, i see no
 problem with that, I just don't think we really need to pass the whole
 thing all the way up...

  const ConfigTableItem items[] = {
  { Install, Alias,   config_parse_strv,  
0, info-aliases   },
 @@ -1087,6 +1094,9 @@ static int unit_file_load(
  if (r  0)
  return r;
  
 +if (also  !strv_isempty(info-also))
 +*also = strv_copy(info-also);
 +

 If the argument would just be a bool* we wouldn't have to do the
 expensive strv_copy() here... (which is missing an OOM check btw...)

 Otherwise looks great!

 Lennart

Next version incoming!

Cheers,
-- 
Jan Synacek
Software Engineer, Red Hat


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


[systemd-devel] [PATCH v2] shared/install: when unit contains only Also=, report 'indirect'

2014-11-07 Thread Jan Synacek
If a unit contains only Also=, with no Alias= or WantedBy=, it shouldn't
be reported as static. New 'indirect' status shall be introduced.

https://bugzilla.redhat.com/show_bug.cgi?id=864298
---
Changes in version 2
 - don't pass the whole strv to the higher level calls, use bool instead

 man/systemctl.xml |  5 +
 src/shared/install.c  | 45 +++--
 src/shared/install.h  |  2 ++
 src/systemctl/systemctl.c |  7 +++
 4 files changed, 41 insertions(+), 18 deletions(-)

diff --git a/man/systemctl.xml b/man/systemctl.xml
index 7cbaa6c..fa85d0b 100644
--- a/man/systemctl.xml
+++ b/man/systemctl.xml
@@ -1013,6 +1013,11 @@ kobject-uevent 1 systemd-udevd-kernel.socket 
systemd-udevd.service
 entry0/entry
   /row
   row
+entryliteralindirect/literal/entry
+entryUnit's status is determined indirectly by another 
unit(s) specified in literalAlso=/literal in [Install] section/entry
+entry0/entry
+  /row
+  row
 entryliteraldisabled/literal/entry
 entryUnit is not enabled/entry
 entry1/entry
diff --git a/src/shared/install.c b/src/shared/install.c
index cab93e8..8a7f7e2 100644
--- a/src/shared/install.c
+++ b/src/shared/install.c
@@ -840,6 +840,7 @@ static void install_info_free(InstallInfo *i) {
 strv_free(i-aliases);
 strv_free(i-wanted_by);
 strv_free(i-required_by);
+strv_free(i-also);
 free(i-default_instance);
 free(i);
 }
@@ -948,6 +949,7 @@ static int config_parse_also(
 size_t l;
 const char *word, *state;
 InstallContext *c = data;
+InstallInfo *i = userdata;
 
 assert(filename);
 assert(lvalue);
@@ -964,6 +966,10 @@ static int config_parse_also(
 r = install_info_add(c, n, NULL);
 if (r  0)
 return r;
+
+r = strv_extend(i-also, n);
+if (r  0)
+return r;
 }
 if (!isempty(state))
 log_syntax(unit, LOG_ERR, filename, line, EINVAL,
@@ -1043,7 +1049,8 @@ static int unit_file_load(
 const char *path,
 const char *root_dir,
 bool allow_symlink,
-bool load) {
+bool load,
+bool *also) {
 
 const ConfigTableItem items[] = {
 { Install, Alias,   config_parse_strv, 
0, info-aliases   },
@@ -1087,6 +1094,9 @@ static int unit_file_load(
 if (r  0)
 return r;
 
+if (also)
+*also = !strv_isempty(info-also);
+
 return
 (int) strv_length(info-aliases) +
 (int) strv_length(info-wanted_by) +
@@ -1099,7 +1109,8 @@ static int unit_file_search(
 LookupPaths *paths,
 const char *root_dir,
 bool allow_symlink,
-bool load) {
+bool load,
+bool *also) {
 
 char **p;
 int r;
@@ -1109,7 +1120,7 @@ static int unit_file_search(
 assert(paths);
 
 if (info-path)
-return unit_file_load(c, info, info-path, root_dir, 
allow_symlink, load);
+return unit_file_load(c, info, info-path, root_dir, 
allow_symlink, load, also);
 
 assert(info-name);
 
@@ -1120,7 +1131,7 @@ static int unit_file_search(
 if (!path)
 return -ENOMEM;
 
-r = unit_file_load(c, info, path, root_dir, allow_symlink, 
load);
+r = unit_file_load(c, info, path, root_dir, allow_symlink, 
load, also);
 if (r = 0) {
 info-path = path;
 path = NULL;
@@ -1149,7 +1160,7 @@ static int unit_file_search(
 if (!path)
 return -ENOMEM;
 
-r = unit_file_load(c, info, path, root_dir, 
allow_symlink, load);
+r = unit_file_load(c, info, path, root_dir, 
allow_symlink, load, also);
 if (r = 0) {
 info-path = path;
 path = NULL;
@@ -1167,7 +1178,8 @@ static int unit_file_can_install(
 LookupPaths *paths,
 const char *root_dir,
 const char *name,
-bool allow_symlink) {
+bool allow_symlink,
+bool *also) {
 
 _cleanup_(install_context_done) InstallContext c = {};
 InstallInfo *i;
@@ -1182,7 +1194,7 @@ static int unit_file_can_install(
 
 assert_se(i = ordered_hashmap_first(c.will_install));
 
-r = unit_file_search(c, i, paths, root_dir, allow_symlink, true);
+ 

[systemd-devel] [PATCH] shared/install: don't report 'static' when unit contains only Also=

2014-11-06 Thread Jan Synacek
I think that this patch might be a bit ineffective, as it calls
unit_file_load() again just to get an InstallContext. I wasn't sure
how to get Also= targets in any other way.

If such change makes sense, this patch should probably be considered a
preview rather than something to be committed right away.

Jan Synacek (1):
  shared/install: don't report 'static' when unit contains only Also=

 src/shared/install.c | 23 ++-
 1 file changed, 22 insertions(+), 1 deletion(-)

-- 
1.9.3

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


[systemd-devel] [PATCH] shared/install: don't report 'static' when unit contains only Also=

2014-11-06 Thread Jan Synacek
If a unit contains only Also=, with no Alias= or WantedBy=, it shouldn't
be reported as static. If any target unit specified in Also= is enabled
or disabled, report this unit as enabled or disabled as well.

https://bugzilla.redhat.com/show_bug.cgi?id=864298
---
 src/shared/install.c | 23 ++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/src/shared/install.c b/src/shared/install.c
index cab93e8..781832f 100644
--- a/src/shared/install.c
+++ b/src/shared/install.c
@@ -1874,8 +1874,29 @@ UnitFileState unit_file_get_state(
 return r;
 else if (r  0)
 return UNIT_FILE_DISABLED;
-else if (r == 0)
+else if (r == 0) {
+_cleanup_(install_context_done) InstallContext c = {};
+InstallInfo info = {}, *tmp;
+Iterator it;
+const char *key;
+
+(void) unit_file_load(c, info, path, root_dir, true, 
true);
+
+/* At this point, the unit contains only Also=, no 
Alias= or WantedBy= are specified.
+   It can be enabled/disabled through any of the Also= 
targets, we should check
+   if they're enabled/disabled. */
+if (c.will_install) {
+ORDERED_HASHMAP_FOREACH_KEY(tmp, key, 
c.will_install, it) {
+r = find_symlinks_in_scope(scope, 
root_dir, key, state);
+if (r  0)
+return r;
+else if (r  0)
+return state;
+}
+return UNIT_FILE_DISABLED;
+}
 return UNIT_FILE_STATIC;
+}
 }
 
 return r  0 ? r : state;
-- 
1.9.3

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


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

2014-11-04 Thread Jan Synacek
As mentioned in [1], it would probably be better if the validation
errors were just warnings, but I'm not sure if that can be achieved
over dbus.

[1] http://lists.freedesktop.org/archives/systemd-devel/2014-October/024129.html

Jan Synacek (1):
  localed: validate set-x11-keymap input

 Makefile.am|   2 +
 src/locale/localectl.c |  77 ++--
 src/locale/localed.c   |   8 ++
 src/shared/xkb-util.c  | 194 +
 src/shared/xkb-util.h  |  39 ++
 5 files changed, 248 insertions(+), 72 deletions(-)
 create mode 100644 src/shared/xkb-util.c
 create mode 100644 src/shared/xkb-util.h

-- 
1.9.3

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


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

2014-11-04 Thread 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
---
 Makefile.am|   2 +
 src/locale/localectl.c |  77 ++--
 src/locale/localed.c   |   8 ++
 src/shared/xkb-util.c  | 194 +
 src/shared/xkb-util.h  |  39 ++
 5 files changed, 248 insertions(+), 72 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 ff5f61b..f17bec6 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -770,6 +770,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..8f9e4da 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;
@@ -389,14 +390,7 @@ 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;
+enum keymap_state 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,64 +409,9 @@ 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;
-}
-
-if (state != look_for)
-continue;
-
-w = l + strcspn(l, WHITESPACE);
-
-if (n  1) {
-char *e;
-
-if (*w == 0)
-continue;
-
-*w = 0;
-w++;
-w += strspn(w, WHITESPACE);
-
-e = strchr(w, ':');
-if (!e)
-continue;
-
-*e = 0;
-
-if (!streq(w, args[1]))
-continue;
-} else
-*w = 0;

[systemd-devel] [PATCH] man/tmpfiles.d: fix typo

2014-11-04 Thread Jan Synacek
---
 man/tmpfiles.d.xml | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/man/tmpfiles.d.xml b/man/tmpfiles.d.xml
index f2360ba..1b14d69 100644
--- a/man/tmpfiles.d.xml
+++ b/man/tmpfiles.d.xml
@@ -108,8 +108,8 @@
 filename in lexicographic order, regardless of which
 of the directories they reside in. If multiple files
 specify the same path, the entry in the file with the
-lexicographically earliest name will be applied, all
-all other conflicting entries will be logged as
+lexicographically earliest name will be applied.
+All other conflicting entries will be logged as
 errors. When two lines are prefix and suffix of each
 other, then the prefix is always processed first, the
 suffix later. Otherwise, the files/directories are
-- 
1.9.3

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


[systemd-devel] [PATCH] localectl: fix localectl set-x11-keymap syntax description

2014-11-03 Thread Jan Synacek
This complements the fix in commit cd4c6fb12598435fe24431f1dd616f9582f0e3b.
---
 src/locale/localectl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/locale/localectl.c b/src/locale/localectl.c
index 3690f9f..d4a2d29 100644
--- a/src/locale/localectl.c
+++ b/src/locale/localectl.c
@@ -505,7 +505,7 @@ static void help(void) {
  list-locales Show known locales\n
  set-keymap MAP [MAP] Set virtual console keyboard 
mapping\n
  list-keymaps Show known virtual console keyboard 
mappings\n
- set-x11-keymap LAYOUT [MODEL] [VARIANT] [OPTIONS]\n
+ set-x11-keymap LAYOUT [MODEL [VARIANT [OPTIONS]]]\n
   Set X11 keyboard mapping\n
  list-x11-keymap-models   Show known X11 keyboard mapping 
models\n
  list-x11-keymap-layouts  Show known X11 keyboard mapping 
layouts\n
-- 
1.9.3

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


Re: [systemd-devel] [PATCH] localectl: fix localectl set-x11-keymap syntax description

2014-11-03 Thread Jan Synacek
David Herrmann dh.herrm...@gmail.com writes:
 Hi

 On Mon, Nov 3, 2014 at 2:01 PM, Jan Synacek jsyna...@redhat.com wrote:
 This complements the fix in commit cd4c6fb12598435fe24431f1dd616f9582f0e3b.

 Applied!

I don't think it was, I can't see the patch anywhere in the logs.

 Thanks
 David

 ---
  src/locale/localectl.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/src/locale/localectl.c b/src/locale/localectl.c
 index 3690f9f..d4a2d29 100644
 --- a/src/locale/localectl.c
 +++ b/src/locale/localectl.c
 @@ -505,7 +505,7 @@ static void help(void) {
   list-locales Show known locales\n
   set-keymap MAP [MAP] Set virtual console keyboard 
 mapping\n
   list-keymaps Show known virtual console 
 keyboard mappings\n
 - set-x11-keymap LAYOUT [MODEL] [VARIANT] [OPTIONS]\n
 + set-x11-keymap LAYOUT [MODEL [VARIANT [OPTIONS]]]\n
Set X11 keyboard mapping\n
   list-x11-keymap-models   Show known X11 keyboard mapping 
 models\n
   list-x11-keymap-layouts  Show known X11 keyboard mapping 
 layouts\n
 --
 1.9.3

-- 
Jan Synacek
Software Engineer, Red Hat


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


Re: [systemd-devel] Switch root slowness

2014-10-31 Thread Jan Synacek
Lennart Poettering lenn...@poettering.net writes:
 On Thu, 30.10.14 14:35, Lennart Poettering (lenn...@poettering.net) wrote:
 I wish there was a way how we could use getrandom() in a way like
 /dev/urandom, where we can pull out the non-initialized data
 anyway. In absence of that we can just fallback to /dev/urandom on
 EAGAIN I guess, and always pass GRND_NONBLOCK.

 I have now implemented that. Please test!

 Lennart

It works now, thanks!

-- 
Jan Synacek
Software Engineer, Red Hat


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


[systemd-devel] [PATCH v3] core: improve error message when machine id is missing

2014-10-31 Thread Jan Synacek
---
Changes in v3:
 - check correct errno
Changes in v2:
 - show long explanation only when errno == EROFS

 src/core/machine-id-setup.c | 12 +++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/src/core/machine-id-setup.c b/src/core/machine-id-setup.c
index efb074f..e54b879 100644
--- a/src/core/machine-id-setup.c
+++ b/src/core/machine-id-setup.c
@@ -184,14 +184,24 @@ int machine_id_setup(const char *root) {
  * will be owned by root it doesn't matter much, but maybe
  * people look. */
 
+int old_errno;
+
 mkdir_parents(etc_machine_id, 0755);
 fd = open(etc_machine_id, O_RDWR|O_CREAT|O_CLOEXEC|O_NOCTTY, 
0444);
+old_errno = errno;
 if (fd = 0)
 writable = true;
 else {
 fd = open(etc_machine_id, O_RDONLY|O_CLOEXEC|O_NOCTTY);
 if (fd  0) {
-log_error(Cannot open %s: %m, 
etc_machine_id);
+if (old_errno == EROFS)
+log_error(System cannot boot: Missing 
/etc/machine-id and /etc is mounted read-only.\n
+  Booting up is supported 
only when:\n
+  1) /etc/machine-id exists 
and is populated.\n
+  2) /etc/machine-id exists 
and is empty.\n
+  3) /etc/machine-id is 
missing and /etc is writable.\n);
+else
+log_error(Cannot open %s: %m, 
etc_machine_id);
 return -errno;
 }
 
-- 
1.9.3

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


Re: [systemd-devel] [PATCH] bash-completion: fix systemctl isolate

2014-10-30 Thread Jan Synacek
Jan Synacek jsyna...@redhat.com writes:
 Jan Synacek (1):
   bash-completion: fix systemctl isolate

I've just tested the latest systemd (commit 81333ec) and after the
bash-completion patches, this patch seems obsolete and not needed
anymore.

-- 
Jan Synacek
Software Engineer, Red Hat


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


[systemd-devel] [PATCH v2] core: improve error message when machine id is missing

2014-10-30 Thread Jan Synacek
---
Changes in v2:
 - show long explanation only when errno == EROFS

 src/core/machine-id-setup.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/src/core/machine-id-setup.c b/src/core/machine-id-setup.c
index efb074f..2360904 100644
--- a/src/core/machine-id-setup.c
+++ b/src/core/machine-id-setup.c
@@ -191,7 +191,14 @@ int machine_id_setup(const char *root) {
 else {
 fd = open(etc_machine_id, O_RDONLY|O_CLOEXEC|O_NOCTTY);
 if (fd  0) {
-log_error(Cannot open %s: %m, 
etc_machine_id);
+if (errno == EROFS)
+log_error(System cannot boot: Missing 
/etc/machine-id and /etc is mounted read-only.\n
+  Booting up is supported 
only when:\n
+  1) /etc/machine-id exists 
and is populated.\n
+  2) /etc/machine-id exists 
and is empty.\n
+  3) /etc/machine-id is 
missing and /etc is writable.\n);
+else
+log_error(Cannot open %s: %m, 
etc_machine_id);
 return -errno;
 }
 
-- 
1.9.3

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


Re: [systemd-devel] Switch root slowness

2014-10-30 Thread Jan Synacek
Dave Reisner d...@falconindy.com writes:
 On Thu, Oct 30, 2014 at 01:18:24PM +0100, Jan Synáček wrote:
 Hello,
 
 commit 539618a0ddc2dc7f0fbe28de2ae0e07b34c81e60
 Author: Lennart Poettering lenn...@poettering.net
 Date:   Wed Oct 29 17:06:32 2014 +0100
 
 util: make use of the new getrandom() syscall if it is available when 
 needing entropy
 
 Doesn't require an fd, and could be a bit faster, so let's make use of
 it, if it is available.
 
 Beginning from this commit, switch root takes about a minute on my machine.

 ...

 Probably fixed by:

 http://cgit.freedesktop.org/systemd/systemd/commit/?id=74a550c5d8228

Nope, still slow.

-- 
Jan Synacek
Software Engineer, Red Hat


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


Re: [systemd-devel] [PATCH] tmpfiles: only change device permissions if mknod succeeded

2014-10-29 Thread Jan Synacek
Tom Gundersen t...@jklm.no writes:

 On Mon, Oct 27, 2014 at 4:53 PM, Tom Gundersen t...@jklm.no wrote:
 On Mon, Oct 27, 2014 at 4:48 PM, Lennart Poettering
 mzerq...@0pointer.de wrote:
 On Sat, 25.10.14 01:36, Tom Gundersen (t...@jklm.no) wrote:

 On Mon, Oct 20, 2014 at 9:32 PM, Lennart Poettering
 lenn...@poettering.net wrote:
  On Tue, 14.10.14 16:19, Jan Synacek (jsyna...@redhat.com) wrote:
 
  https://bugzilla.redhat.com/show_bug.cgi?id=1147248
 
  Hmm, so far tmpfiles always adjust access modes, for all types of
  lines, if that's possible. I think this makes sense. The bug
  referenced above seems to suggest though that the access mode of the
  /dev/fuse file node is specified differently in two places
  though. This sounds like something to fix first?

 Well, the /run/tmpfiles.d/kmod.conf one is what the kernel exposes,
 and then the udev rules overrides this. We could surely fix this case,
 but in general I think we should expect that these may differ.

 To me it seems that we should not create devices nodes at all, except
 in systemd-tmpfiles-setup-dev.service, the reason being that udev
 rules are only applied to static nodes at udev startup, so any device
 nodes created (or changed) after that may end up with the wrong
 permissions (as seen here).

 Hmm, so does this mean that the kmod tmpfiles converter really should
 suffixits lines with the exclamation mark? That way, only invocation
 of tmpfiles with --boot would honour those files, which are the ones
 we start at boot.

 Does that make sense?


 Yes, indeed, this is precisely what we want. I had missed that
 feature. I'll do a patch.


 And done: http://permalink.gmane.org/gmane.linux.kernel.modules/1402.

 Jan, does this look like it solves the original problem?

 Cheers,

 Tom

On my current rawhide (updated today, systemd-216-11.fc22.x86_64), with
kmod patched using the patch you've provided, /dev/fuse is not created,
not even on boot. However, invoking systemd-tmpfiles.d --create --boot
correctly creates the node.

# cat /run/tmpfiles.d/kmod.conf 
c! /dev/fuse 0600 - - - 10:229
c! /dev/btrfs-control 0600 - - - 10:234
c! /dev/loop-control 0600 - - - 10:237
d /dev/net 0755 - - -
c! /dev/net/tun 0600 - - - 10:200
c! /dev/ppp 0600 - - - 108:0
c! /dev/uinput 0600 - - - 10:223
c! /dev/uhid 0600 - - - 10:239
d /dev/vfio 0755 - - -
c! /dev/vfio/vfio 0600 - - - 10:196
c! /dev/vhci 0600 - - - 10:137
c! /dev/vhost-net 0600 - - - 10:238
d /dev/snd 0755 - - -
c! /dev/snd/timer 0600 - - - 116:33
d /dev/snd 0755 - - -
c! /dev/snd/seq 0600 - - - 116:1

Is that how it should work?

Cheers,
-- 
Jan Synacek
Software Engineer, Red Hat


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


Re: [systemd-devel] [PATCH] swap: rework discard

2014-10-29 Thread Jan Synacek
Lennart Poettering lenn...@poettering.net writes:

 On Tue, 28.10.14 13:14, Lennart Poettering (lenn...@poettering.net) wrote:

 On Thu, 23.10.14 16:39, Lennart Poettering (lenn...@poettering.net) wrote:
 
 Heya,
 
  Hmm, I think the generator should already treat the option fields the
  same way as I want it to work in the long run, i.e. just read it from
  fstab and write it 1:1 into the unit's Options= string.
 
 I am hacking up a patch for this now, since I really want to get the
 new release out of the door soon.

 OK, landed that patch now. Didn't test it much though. Please test!

 Lennart

Works well on my system.

Thanks again!

-- 
Jan Synacek
Software Engineer, Red Hat


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


Re: [systemd-devel] Possible documentation problems

2014-10-29 Thread Jan Synacek
Lennart Poettering lenn...@poettering.net writes:
 On Wed, 15.10.14 11:07, Jan Synacek (jsyna...@redhat.com) wrote:

 Hello,
 
 in the documentation for systemd.service, under Type= option, it reads:
 
   Behavior of oneshot is similar to simple; however, it is expected that the
   process has to exit before systemd starts follow-up unit RemainAfterExit=
   is particularly useful for this type of service. This is the implied 
 default
   if neither Type= or ExecStart= are specified.
 
 I don't think that the part about not specifying ExecStart is correct. If
 there is no ExecStart in the service file, I get an error.

 As pointed out by Mantas this limitation has been removed a while back.

 
 Also, under Sockets= option:
 
   ...
   Also note that a different service may be activated on incoming
   traffic than that which inherits the sockets.
   ...
 
 I had to reread that sentence about 10 times to actually get it. I'd say
 that rewording it would be benefitial.

 I tried to reword it a bit now in git. Not sure it's a ton more
 understandable though...

 Lennart

It's a bit better, at least for me, thank you.

-- 
Jan Synacek
Software Engineer, Red Hat


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


[systemd-devel] [PATCH] core: improve error message when machine id is missing

2014-10-24 Thread Jan Synacek
---
 src/core/machine-id-setup.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/core/machine-id-setup.c b/src/core/machine-id-setup.c
index efb074f..eba35be 100644
--- a/src/core/machine-id-setup.c
+++ b/src/core/machine-id-setup.c
@@ -191,7 +191,11 @@ int machine_id_setup(const char *root) {
 else {
 fd = open(etc_machine_id, O_RDONLY|O_CLOEXEC|O_NOCTTY);
 if (fd  0) {
-log_error(Cannot open %s: %m, 
etc_machine_id);
+log_error(System cannot boot: Missing 
/etc/machine-id and /etc is mounted read-only.\n
+  Booting up is supported only 
when:\n
+  1) /etc/machine-id exists and is 
populated.\n
+  2) /etc/machine-id exists and is 
empty.\n
+  3) /etc/machine-id is missing and 
/etc is writable.\n);
 return -errno;
 }
 
-- 
1.9.3

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


Re: [systemd-devel] Unicode support in console after boot

2014-10-24 Thread Jan Synacek
Michal Sekletar msekl...@redhat.com writes:
 On Mon, Oct 13, 2014 at 09:36:16AM +0200, Jan Synacek wrote:
 Hello,
 
 currently, unicode characters are not correctly displayed in the
 console. After login, when I run /usr/bin/unicode_start, unicode works
 fine. I tried to create a service file that runs this script, linking
 tty to stdout and stderr, but that didn't work. Is there a way how to
 turn on unicode support in console after boot using a service file? Or
 any other type of unit? Or is this something that has to be patched in
 the source (logind perhaps?)?

 Please try editing /usr/lib/systemd/system/systemd-vconsole-setup.service and
 remove RemainAfterExit=yes, then regenerate your initramfs image by running
 dracut command. Add back RemainAfterExit=yes to service file. Reboot.

Yep, this helped. Could you please explain why? Also, I believe this
should be fixed in all Fedora versions.

 Michal

 
 Cheers,
 -- 
 Jan Synacek
 Software Engineer, Red Hat
 ___
 systemd-devel mailing list
 systemd-devel@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/systemd-devel

-- 
Jan Synacek
Software Engineer, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] Should user mode linux register with machined?

2014-10-24 Thread Jan Synacek
Lennart Poettering lenn...@poettering.net writes:
 On Fri, 10.10.14 18:48, Richard Weinberger (rich...@nod.at) wrote:

 Lennart,
 
 Am 10.10.2014 um 18:44 schrieb Lennart Poettering:
  It's a bit more complex. While UML, qemu, kvm, currently don't, LXC,
  systemd-nspawn and libvirt-lxc all do talk directly to machined. (Note
  that LXC and libvirt-lxc are separate codebases, the latter is *not* a
  wrapper around the former).
  
  So, dunno, it really is up to how you intend UML to be used. If UML
  shall be nice and useful without libvirt, then it's worth doing the
  registration natively, but it's also OK to just leave this to libvirt,
  if that's your primary envisioned usecase...
 
 What is the benefit of this registration?
 I boot all day long UML and qemu-kvm VMs without registering them to systemd,
 so I don't really know what I'm missing. :-)
 But if there is a nice use case I'll happily add the registration to UML.

 Hmm, I figure this mail didn't make it through to you?

 http://lists.freedesktop.org/archives/systemd-devel/2014-October/023875.html

I don't see that mail in my mailbox either and I know that you noticed
some mail not arriving before. It seems to cause quite a lot of
confusion in discussion and patch submission. I'm not sure who to report
this to.

-- 
Jan Synacek
Software Engineer, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


[systemd-devel] [PATCH] swap: rework discard

2014-10-23 Thread Jan Synacek
Instead of a dedicated Discard option, use more general Options. When
the swapon command learns -o, it will be possible to pass the value of
Options as is. The code now assumes that the only possible value to
Options is related to discard.

http://lists.freedesktop.org/archives/systemd-devel/2014-October/024132.html
---
 src/core/dbus-swap.c  |  8 +++
 src/core/load-fragment-gperf.gperf.m4 |  2 +-
 src/core/swap.c   | 28 +++---
 src/core/swap.h   |  3 ++-
 src/fstab-generator/fstab-generator.c | 44 +++
 5 files changed, 25 insertions(+), 60 deletions(-)

diff --git a/src/core/dbus-swap.c b/src/core/dbus-swap.c
index c854716..0792cb4 100644
--- a/src/core/dbus-swap.c
+++ b/src/core/dbus-swap.c
@@ -55,7 +55,7 @@ static int property_get_priority(
 return sd_bus_message_append(reply, i, p);
 }
 
-static int property_get_discard(
+static int property_get_options(
 sd_bus *bus,
 const char *path,
 const char *interface,
@@ -72,9 +72,9 @@ static int property_get_discard(
 assert(s);
 
 if (s-from_fragment)
-p = s-parameters_fragment.discard ?: none;
+p = s-parameters_fragment.options ?: ;
 else
-p = none;
+p = ;
 return sd_bus_message_append(reply, s, p);
 }
 
@@ -84,7 +84,7 @@ const sd_bus_vtable bus_swap_vtable[] = {
 SD_BUS_VTABLE_START(0),
 SD_BUS_PROPERTY(What, s, NULL, offsetof(Swap, what), 
SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE),
 SD_BUS_PROPERTY(Priority, i, property_get_priority, 0, 
SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE),
-SD_BUS_PROPERTY(Discard, s, property_get_discard, 0, 
SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE),
+SD_BUS_PROPERTY(Options, s, property_get_options, 0, 
SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE),
 SD_BUS_PROPERTY(TimeoutUSec, t, bus_property_get_usec, 
offsetof(Swap, timeout_usec), SD_BUS_VTABLE_PROPERTY_CONST),
 SD_BUS_PROPERTY(ControlPID, u, bus_property_get_pid, 
offsetof(Swap, control_pid), SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE),
 SD_BUS_PROPERTY(Result, s, property_get_result, offsetof(Swap, 
result), SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE),
diff --git a/src/core/load-fragment-gperf.gperf.m4 
b/src/core/load-fragment-gperf.gperf.m4
index 8805411..19a79d5 100644
--- a/src/core/load-fragment-gperf.gperf.m4
+++ b/src/core/load-fragment-gperf.gperf.m4
@@ -297,7 +297,7 @@ Automount.DirectoryMode, config_parse_mode, 
 0,
 m4_dnl
 Swap.What,   config_parse_path,  0,
 offsetof(Swap, parameters_fragment.what)
 Swap.Priority,   config_parse_int,   0,
 offsetof(Swap, parameters_fragment.priority)
-Swap.Discard,config_parse_string,0,
 offsetof(Swap, parameters_fragment.discard)
+Swap.Options,config_parse_string,0,
 offsetof(Swap, parameters_fragment.options)
 Swap.TimeoutSec, config_parse_sec,   0,
 offsetof(Swap, timeout_usec)
 EXEC_CONTEXT_CONFIG_ITEMS(Swap)m4_dnl
 CGROUP_CONTEXT_CONFIG_ITEMS(Swap)m4_dnl
diff --git a/src/core/swap.c b/src/core/swap.c
index b2ca048..c183c32 100644
--- a/src/core/swap.c
+++ b/src/core/swap.c
@@ -152,8 +152,8 @@ static void swap_done(Unit *u) {
 free(s-parameters_fragment.what);
 s-parameters_fragment.what = NULL;
 
-free(s-parameters_fragment.discard);
-s-parameters_fragment.discard = NULL;
+free(s-parameters_fragment.options);
+s-parameters_fragment.options = NULL;
 
 s-exec_runtime = exec_runtime_unref(s-exec_runtime);
 exec_command_done_array(s-exec_command, _SWAP_EXEC_COMMAND_MAX);
@@ -607,12 +607,15 @@ static void swap_dump(Unit *u, FILE *f, const char 
*prefix) {
 fprintf(f,
 %sPriority: %i\n
 %sNoAuto: %s\n
-%sNoFail: %s\n
-%sDiscard: %s\n,
+%sNoFail: %s\n,
 prefix, p-priority,
 prefix, yes_no(p-noauto),
-prefix, yes_no(p-nofail),
-prefix, p-discard ?: none);
+prefix, yes_no(p-nofail));
+
+if (p-options)
+fprintf(f,
+%sOptions: %s\n,
+prefix, p-options);
 
 if (s-control_pid  0)
 fprintf(f,
@@ -741,7 +744,7 @@ fail:
 
 static void swap_enter_activating(Swap *s) {
 int r, priority;
-char *discard;
+char *discard = NULL;
 
 assert(s);
 
@@ -750,7 +753,8 @@ static void 

Re: [systemd-devel] [PATCH 2/2] localectl: verify layout, model, variant and options

2014-10-23 Thread Jan Synacek
Lennart Poettering lenn...@poettering.net writes:

 On Mon, 20.10.14 12:43, Jan Synacek (jsyna...@redhat.com) wrote:

 When setting any of those using set-x11-keymap, check that their values
 are available on the system.
 ---
  src/locale/localectl.c | 208 
 +
  1 file changed, 139 insertions(+), 69 deletions(-)
 
 diff --git a/src/locale/localectl.c b/src/locale/localectl.c
 index 3690f9f..0caf467 100644
 --- a/src/locale/localectl.c
 +++ b/src/locale/localectl.c
 @@ -53,6 +53,14 @@ static BusTransport arg_transport = BUS_TRANSPORT_LOCAL;
  static char *arg_host = NULL;
  static bool arg_convert = true;
  
 +enum keymap_state {
 +NONE,
 +MODELS   = 1  0,
 +LAYOUTS  = 1  1,
 +VARIANTS = 1  2,
 +OPTIONS  = 1  3
 +};

 Hmm, why is this a bitfield? Do I get this patch right and you are
 trying to match the passed arguments to all
 models/layouts/variants/options all the time? This means if a layout
 happens to have the same name as a model then things will be
 ambiguous? I really don't like this I must say.

Basic idea was that get_x11_keymaps_internal() gets a flag that says
what kind of information should be parsed and returned from
/usr/share/X11/xkb/rules/base.lst. If only a layout needs to be
specified, then fetch just layouts. If a layout, model and options all
get specified, fetch all of them in one go. If I left the function as it
was, it would require multiple passes to validate layouts, models and
options together.

And yes, there's ambiguity in that solution. I didn't come up with
anything better, I'm not even sure if this has a proper solution without
using X libraries, which is IMO unthinkable in this case. If you have a
better idea, I'll be happy to rework this solution with something
better.

 Or mabye I am not following what you are doing here? Can you elaborate?

 Also, this validating really be done on the server side, not
 on the client side, to take full effect. 

 Doing the listing/completion thing on the client-side appears OK since
 it's really more a help to the user, nothing that validates stuff. But
 if we want to validate input here, we should really do it on the
 server-side, in localed, especially as localed and localectl might run
 on different systems and not share the same map list.

Doing this on the server side makes sense. It didn't occur to me
before. I saw code that already parsed the local xkb rules, so I just
improved on that.

Also, in case of a failed validation, it's probably better to just warn
the user and go on, as mentioned earlier in this thread.

So, if would it make sense to implement the server-side validation
functionality? Would the DBus API need to be changed? Any other ideas?

 Lennart

Cheers,
-- 
Jan Synacek
Software Engineer, Red Hat


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


Re: [systemd-devel] [PATCH] core: improve error message when machine id is missing

2014-10-23 Thread Jan Synacek
Jan Synacek jsyna...@redhat.com writes:

 ---
  src/core/machine-id-setup.c | 6 +-
  1 file changed, 5 insertions(+), 1 deletion(-)

 diff --git a/src/core/machine-id-setup.c b/src/core/machine-id-setup.c
 index efb074f..eba35be 100644
 --- a/src/core/machine-id-setup.c
 +++ b/src/core/machine-id-setup.c
 @@ -191,7 +191,11 @@ int machine_id_setup(const char *root) {
  else {
  fd = open(etc_machine_id, 
 O_RDONLY|O_CLOEXEC|O_NOCTTY);
  if (fd  0) {
 -log_error(Cannot open %s: %m, 
 etc_machine_id);
 +log_error(System cannot boot: Missing 
 /etc/machine-id and /etc is mounted read-only.\n
 +  Booting up is supported only 
 when:\n
 +  1) /etc/machine-id exists and is 
 populated.\n
 +  2) /etc/machine-id exists and is 
 empty.\n
 +  3) /etc/machine-id is missing and 
 /etc is writable.\n);
  return -errno;
  }
  
 -- 
 1.9.3


Not sure if this message ever got through, so resending.

-- 
Jan Synacek
Software Engineer, Red Hat


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


Re: [systemd-devel] [PATCH] swap: introduce Discard property

2014-10-21 Thread Jan Synacek
Lennart Poettering lenn...@poettering.net writes:

 On Mon, 20.10.14 11:08, Karel Zak (k...@redhat.com) wrote:

 On Fri, Oct 03, 2014 at 07:16:55AM +0200, Jan Synacek wrote:
  Karel Zak k...@redhat.com writes:
   Karel, any chance you can add a -o option to swapon?
  
No problem, added to TODO. I'll implement it next week.
 
 Implemented, it's in util-linux git tree, will be in v2.26.
 
  Would you please let me know when that patch makes it to the package in
  rawhide? I would then fix the code I wrote to support it. Thank you.
 
 Probably in December.

 Jan, any chance you could rework the current systemd code to simply
 hackishly accept Options=discard instead of Discard=yes for the
 discard code? Then, later on, when swapon learns -o we can pass the
 string 1:1 over. For now we'd just check if Options= is set to the
 precise string discard, and generate an error for anything else. That
 way we could make the discard functionality available now, but keep
 compatibility with the future mkswap?

Yeah, I'll look into it.

-- 
Jan Synacek
Software Engineer, Red Hat


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


Re: [systemd-devel] [PATCH 2/2] localectl: verify layout, model, variant and options

2014-10-20 Thread Jan Synacek
Ran Benita ran...@gmail.com writes:
 On Fri, Oct 17, 2014 at 02:02:13PM +0200, Jan Synacek wrote:
 When setting any of those using set-x11-keymap, check that their values
 are available on the system.

 I have only skimmed this patch, but generally:

 1. There can only be one model.
 2. There can be multiple layouts and variants, comma separated. E.g.,
layout=us,il variant=,lyx. The amount of layouts and variants should
match (or the variants can be empty), but most stuff you throw at it
will be accepted though.
 3. There can be multiple comma-separated options.

 Do you handle input like layout=us,il variant=,lyx correctly?

 Ran

Nope, I didn't realize that. I'll send a better version of the patch.

The parsing won't be perfect, though. With setxkbmap, I can do the
following without any error:

# setxkbmap us,cz lyx ctrl:nocaps -model idontexist
# echo $?
0

Note that there is no lyx variant for either us or cz. Also, the number
of layouts and variants *doesn't* match and the model doesn't exist as
well. (That's probably what you meant by the last sentence in 2., I'm
just restating it to be clear).

Cheers,
-- 
Jan Synacek
Software Engineer, Red Hat


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


[systemd-devel] [PATCH 1/2] man: fix localectl set-x11-keymap syntax description

2014-10-20 Thread Jan Synacek
---
 man/localectl.xml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/man/localectl.xml b/man/localectl.xml
index 38e73c7..c332027 100644
--- a/man/localectl.xml
+++ b/man/localectl.xml
@@ -178,7 +178,7 @@
 /varlistentry
 
 varlistentry
-termcommandset-x11-keymap LAYOUT [MODEL] 
[VARIANT] [OPTIONS]/command/term
+termcommandset-x11-keymap LAYOUT [MODEL 
[VARIANT [OPTIONS]]]/command/term
 
 listitemparaSet the system default
 keyboard mapping for X11. This takes a
-- 
1.9.3

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


[systemd-devel] [PATCHv2 0/2] localectl: verify layout, model, variant and options; fix man page

2014-10-20 Thread Jan Synacek
Changes in v2:
 - correctly verify multiple layouts, variants and options when specified in a 
comma separated list

Jan Synacek (2):
  man: fix localectl set-x11-keymap syntax description
  localectl: verify layout, model, variant and options

 man/localectl.xml  |   2 +-
 src/locale/localectl.c | 208 +
 2 files changed, 140 insertions(+), 70 deletions(-)

-- 
1.9.3

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


[systemd-devel] [PATCH 2/2] localectl: verify layout, model, variant and options

2014-10-20 Thread Jan Synacek
When setting any of those using set-x11-keymap, check that their values
are available on the system.
---
 src/locale/localectl.c | 208 +
 1 file changed, 139 insertions(+), 69 deletions(-)

diff --git a/src/locale/localectl.c b/src/locale/localectl.c
index 3690f9f..0caf467 100644
--- a/src/locale/localectl.c
+++ b/src/locale/localectl.c
@@ -53,6 +53,14 @@ static BusTransport arg_transport = BUS_TRANSPORT_LOCAL;
 static char *arg_host = NULL;
 static bool arg_convert = true;
 
+enum keymap_state {
+NONE,
+MODELS   = 1  0,
+LAYOUTS  = 1  1,
+VARIANTS = 1  2,
+OPTIONS  = 1  3
+};
+
 static void pager_open_if_enabled(void) {
 
 if (arg_no_pager)
@@ -350,59 +358,12 @@ static int list_vconsole_keymaps(sd_bus *bus, char 
**args, unsigned n) {
 return 0;
 }
 
-static int set_x11_keymap(sd_bus *bus, char **args, unsigned n) {
-_cleanup_bus_error_free_ sd_bus_error error = SD_BUS_ERROR_NULL;
-const char *layout, *model, *variant, *options;
-int r;
-
-assert(bus);
-assert(args);
-
-if (n  5) {
-log_error(Too many arguments.);
-return -EINVAL;
-}
-
-polkit_agent_open_if_enabled();
-
-layout = args[1];
-model = n  2 ? args[2] : ;
-variant = n  3 ? args[3] : ;
-options = n  4 ? args[4] : ;
-
-r = sd_bus_call_method(
-bus,
-org.freedesktop.locale1,
-/org/freedesktop/locale1,
-org.freedesktop.locale1,
-SetX11Keyboard,
-error,
-NULL,
-bb, layout, model, variant, options,
-  arg_convert, arg_ask_password);
-if (r  0)
-log_error(Failed to set keymap: %s, 
bus_error_message(error, -r));
-
-return r;
-}
-
-static int list_x11_keymaps(sd_bus *bus, char **args, unsigned n) {
+static int get_x11_keymaps_internal(char ***list, enum keymap_state look_for, 
const char *layout)
+{
 _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;
-int r;
-
-if (n  2) {
-log_error(Too many arguments.);
-return -EINVAL;
-}
+enum keymap_state state = NONE;
+int r = 0;
 
 f = fopen(/usr/share/X11/xkb/rules/base.lst, re);
 if (!f) {
@@ -410,17 +371,6 @@ static int list_x11_keymaps(sd_bus *bus, char **args, 
unsigned n) {
 return -errno;
 }
 
-if (streq(args[0], list-x11-keymap-models))
-look_for = MODELS;
-else if (streq(args[0], list-x11-keymap-layouts))
-look_for = LAYOUTS;
-else if (streq(args[0], list-x11-keymap-variants))
-look_for = VARIANTS;
-else if (streq(args[0], list-x11-keymap-options))
-look_for = OPTIONS;
-else
-assert_not_reached(Wrong parameter);
-
 FOREACH_LINE(line, f, break) {
 char *l, *w;
 
@@ -444,12 +394,12 @@ static int list_x11_keymaps(sd_bus *bus, char **args, 
unsigned n) {
 continue;
 }
 
-if (state != look_for)
+if (!(state  look_for))
 continue;
 
 w = l + strcspn(l, WHITESPACE);
 
-if (n  1) {
+if (layout) {
 char *e;
 
 if (*w == 0)
@@ -465,23 +415,143 @@ static int list_x11_keymaps(sd_bus *bus, char **args, 
unsigned n) {
 
 *e = 0;
 
-if (!streq(w, args[1]))
+if (!streq(w, layout))
 continue;
 } else
 *w = 0;
 
- r = strv_extend(list, l);
+ r = strv_extend(list, l);
  if (r  0)
  return log_oom();
 }
 
-if (strv_isempty(list)) {
+if (strv_isempty(*list)) {
 log_error(Couldn't find any entries.);
 return -ENOENT;
 }
 
-strv_sort(list);
-strv_uniq(list);
+strv_sort(*list);
+strv_uniq(*list);
+
+return r;
+}
+
+static int set_x11_keymap(sd_bus *bus, char **args, unsigned n) {
+_cleanup_bus_error_free_ sd_bus_error error = SD_BUS_ERROR_NULL;
+_cleanup_strv_free_ char **list = NULL, **layouts = NULL, **variants = 
NULL, **options = NULL;
+char **it;
+const char *layouts_arg, *variants_arg, *options_arg;
+

[systemd-devel] [PATCH 0/2] localectl: verify layout, model, varian and options; fix manpage

2014-10-17 Thread Jan Synacek
Small localectl fixes.

Both patches fix https://bugzilla.redhat.com/show_bug.cgi?id=1049306.

Jan Synacek (2):
  man: fix localectl set-x11-keymap syntax description
  localectl: verify layout, model, variant and options

 man/localectl.xml  |   2 +-
 src/locale/localectl.c | 179 ++---
 2 files changed, 111 insertions(+), 70 deletions(-)

-- 
1.9.3

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


[systemd-devel] [PATCH 1/2] man: fix localectl set-x11-keymap syntax description

2014-10-17 Thread Jan Synacek
---
 man/localectl.xml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/man/localectl.xml b/man/localectl.xml
index 38e73c7..c332027 100644
--- a/man/localectl.xml
+++ b/man/localectl.xml
@@ -178,7 +178,7 @@
 /varlistentry
 
 varlistentry
-termcommandset-x11-keymap LAYOUT [MODEL] 
[VARIANT] [OPTIONS]/command/term
+termcommandset-x11-keymap LAYOUT [MODEL 
[VARIANT [OPTIONS]]]/command/term
 
 listitemparaSet the system default
 keyboard mapping for X11. This takes a
-- 
1.9.3

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


[systemd-devel] [PATCH 2/2] localectl: verify layout, model, variant and options

2014-10-17 Thread Jan Synacek
When setting any of those using set-x11-keymap, check that their values
are available on the system.
---
 src/locale/localectl.c | 179 ++---
 1 file changed, 110 insertions(+), 69 deletions(-)

diff --git a/src/locale/localectl.c b/src/locale/localectl.c
index 3690f9f..79bc2d0 100644
--- a/src/locale/localectl.c
+++ b/src/locale/localectl.c
@@ -53,6 +53,14 @@ static BusTransport arg_transport = BUS_TRANSPORT_LOCAL;
 static char *arg_host = NULL;
 static bool arg_convert = true;
 
+enum keymap_state {
+NONE,
+MODELS   = 1  0,
+LAYOUTS  = 1  1,
+VARIANTS = 1  2,
+OPTIONS  = 1  3
+};
+
 static void pager_open_if_enabled(void) {
 
 if (arg_no_pager)
@@ -350,59 +358,12 @@ static int list_vconsole_keymaps(sd_bus *bus, char 
**args, unsigned n) {
 return 0;
 }
 
-static int set_x11_keymap(sd_bus *bus, char **args, unsigned n) {
-_cleanup_bus_error_free_ sd_bus_error error = SD_BUS_ERROR_NULL;
-const char *layout, *model, *variant, *options;
-int r;
-
-assert(bus);
-assert(args);
-
-if (n  5) {
-log_error(Too many arguments.);
-return -EINVAL;
-}
-
-polkit_agent_open_if_enabled();
-
-layout = args[1];
-model = n  2 ? args[2] : ;
-variant = n  3 ? args[3] : ;
-options = n  4 ? args[4] : ;
-
-r = sd_bus_call_method(
-bus,
-org.freedesktop.locale1,
-/org/freedesktop/locale1,
-org.freedesktop.locale1,
-SetX11Keyboard,
-error,
-NULL,
-bb, layout, model, variant, options,
-  arg_convert, arg_ask_password);
-if (r  0)
-log_error(Failed to set keymap: %s, 
bus_error_message(error, -r));
-
-return r;
-}
-
-static int list_x11_keymaps(sd_bus *bus, char **args, unsigned n) {
+static int get_x11_keymaps_internal(char ***list, enum keymap_state look_for, 
const char *layout)
+{
 _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;
-int r;
-
-if (n  2) {
-log_error(Too many arguments.);
-return -EINVAL;
-}
+enum keymap_state state = NONE;
+int r = 0;
 
 f = fopen(/usr/share/X11/xkb/rules/base.lst, re);
 if (!f) {
@@ -410,17 +371,6 @@ static int list_x11_keymaps(sd_bus *bus, char **args, 
unsigned n) {
 return -errno;
 }
 
-if (streq(args[0], list-x11-keymap-models))
-look_for = MODELS;
-else if (streq(args[0], list-x11-keymap-layouts))
-look_for = LAYOUTS;
-else if (streq(args[0], list-x11-keymap-variants))
-look_for = VARIANTS;
-else if (streq(args[0], list-x11-keymap-options))
-look_for = OPTIONS;
-else
-assert_not_reached(Wrong parameter);
-
 FOREACH_LINE(line, f, break) {
 char *l, *w;
 
@@ -444,12 +394,12 @@ static int list_x11_keymaps(sd_bus *bus, char **args, 
unsigned n) {
 continue;
 }
 
-if (state != look_for)
+if (!(state  look_for))
 continue;
 
 w = l + strcspn(l, WHITESPACE);
 
-if (n  1) {
+if (layout) {
 char *e;
 
 if (*w == 0)
@@ -465,23 +415,114 @@ static int list_x11_keymaps(sd_bus *bus, char **args, 
unsigned n) {
 
 *e = 0;
 
-if (!streq(w, args[1]))
+if (!streq(w, layout))
 continue;
 } else
 *w = 0;
 
- r = strv_extend(list, l);
+ r = strv_extend(list, l);
  if (r  0)
  return log_oom();
 }
 
-if (strv_isempty(list)) {
+if (strv_isempty(*list)) {
 log_error(Couldn't find any entries.);
 return -ENOENT;
 }
 
-strv_sort(list);
-strv_uniq(list);
+strv_sort(*list);
+strv_uniq(*list);
+
+return r;
+}
+
+static int set_x11_keymap(sd_bus *bus, char **args, unsigned n) {
+_cleanup_bus_error_free_ sd_bus_error error = SD_BUS_ERROR_NULL;
+_cleanup_strv_free_ char **list = NULL;
+const char *layout, *model, *variant, *options;
+enum keymap_state look_for;
+int r;
+
+assert(bus);
+

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

2014-10-15 Thread Jan Synacek
---
 man/systemd.service.xml | 6 +++---
 man/systemd.socket.xml  | 2 +-
 man/systemd.swap.xml| 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/man/systemd.service.xml b/man/systemd.service.xml
index 50ff2f5..b9604b8 100644
--- a/man/systemd.service.xml
+++ b/man/systemd.service.xml
@@ -365,7 +365,7 @@
 used, zero or more commands may be
 specified. This can be specified by
 providing multiple command lines in
-the same directive , or alternatively,
+the same directive, or alternatively,
 this directive may be specified more
 than once with the same effect. If the
 empty string is assigned to this
@@ -548,7 +548,7 @@
 when varnameType=oneshot/varname is
 used, in which case the timeout
 is disabled by default
-(see 
citerefentryrefentrytitlesystemd-systemd.conf/refentrytitlemanvolnum5/manvolnum/citerefentry).
+(see 
citerefentryrefentrytitlesystemd-system.conf/refentrytitlemanvolnum5/manvolnum/citerefentry).
 /para/listitem
 /varlistentry
 
@@ -569,7 +569,7 @@
 the timeout logic. Defaults to
 varnameDefaultTimeoutStopSec=/varname from 
the
 manager configuration file
-(see 
citerefentryrefentrytitlesystemd-systemd.conf/refentrytitlemanvolnum5/manvolnum/citerefentry).
+(see 
citerefentryrefentrytitlesystemd-system.conf/refentrytitlemanvolnum5/manvolnum/citerefentry).
 /para/listitem
 /varlistentry
 
diff --git a/man/systemd.socket.xml b/man/systemd.socket.xml
index dad0267..ce04b0b 100644
--- a/man/systemd.socket.xml
+++ b/man/systemd.socket.xml
@@ -846,7 +846,7 @@
 20s. Pass literal0/literal to disable the 
timeout
 logic. Defaults to 
varnameDefaultTimeoutStartSec=/varname from the
 manager configuration file
-(see 
citerefentryrefentrytitlesystemd-systemd.conf/refentrytitlemanvolnum5/manvolnum/citerefentry).
+(see 
citerefentryrefentrytitlesystemd-system.conf/refentrytitlemanvolnum5/manvolnum/citerefentry).
 /para/listitem
 /varlistentry
 
diff --git a/man/systemd.swap.xml b/man/systemd.swap.xml
index 481dc52..00fafdc 100644
--- a/man/systemd.swap.xml
+++ b/man/systemd.swap.xml
@@ -202,7 +202,7 @@
 20s. Pass literal0/literal to disable the 
timeout
 logic. Defaults to 
varnameDefaultTimeoutStartSec=/varname from the
 manager configuration file
-(see 
citerefentryrefentrytitlesystemd-systemd.conf/refentrytitlemanvolnum5/manvolnum/citerefentry).
+(see 
citerefentryrefentrytitlesystemd-system.conf/refentrytitlemanvolnum5/manvolnum/citerefentry).
 /para/listitem
 /varlistentry
 /variablelist
-- 
1.9.3

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


[systemd-devel] Possible documentation problems

2014-10-15 Thread Jan Synacek
Hello,

in the documentation for systemd.service, under Type= option, it reads:

  Behavior of oneshot is similar to simple; however, it is expected that the
  process has to exit before systemd starts follow-up unit RemainAfterExit=
  is particularly useful for this type of service. This is the implied default
  if neither Type= or ExecStart= are specified.

I don't think that the part about not specifying ExecStart is correct. If
there is no ExecStart in the service file, I get an error.

Also, under Sockets= option:

  ...
  Also note that a different service may be activated on incoming
  traffic than that which inherits the sockets.
  ...

I had to reread that sentence about 10 times to actually get it. I'd say
that rewording it would be benefitial.

Since I'm not sure how to fix any of these, I'm sending this email
instead of actually sending patches. Comments welcome.

Cheers,
-- 
Jan Synacek
Software Engineer, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] Possible documentation problems

2014-10-15 Thread Jan Synacek
Mantas Mikulėnas graw...@gmail.com writes:
 On Oct 15, 2014 12:07 PM, Jan Synacek jsyna...@redhat.com wrote:

 Hello,

 in the documentation for systemd.service, under Type= option, it reads:

   Behavior of oneshot is similar to simple; however, it is expected that
 the
   process has to exit before systemd starts follow-up unit
 RemainAfterExit=
   is particularly useful for this type of service. This is the implied
 default
   if neither Type= or ExecStart= are specified.

 I don't think that the part about not specifying ExecStart is correct. If
 there is no ExecStart in the service file, I get an error.

 Recent systemd versions allow this if an ExecStop= is specified instead.

 -- 
 Mantas Mikulėnas

I didn't know that, thanks!

-- 
Jan Synacek
Software Engineer, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


[systemd-devel] [PATCH] tmpfiles: only change device permissions if mknod succeeded

2014-10-14 Thread Jan Synacek
https://bugzilla.redhat.com/show_bug.cgi?id=1147248
---
 src/tmpfiles/tmpfiles.c | 11 +++
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c
index 8108b43..ae0289d 100644
--- a/src/tmpfiles/tmpfiles.c
+++ b/src/tmpfiles/tmpfiles.c
@@ -824,6 +824,7 @@ static int create_item(Item *i) {
 case CREATE_BLOCK_DEVICE:
 case CREATE_CHAR_DEVICE: {
 mode_t file_type;
+bool mknod_succeeded;
 
 if (have_effective_cap(CAP_MKNOD) == 0) {
 /* In a container we lack CAP_MKNOD. We
@@ -842,6 +843,7 @@ static int create_item(Item *i) {
 r = mknod(i-path, i-mode | file_type, 
i-major_minor);
 label_context_clear();
 }
+mknod_succeeded = (r == 0);
 
 if (r  0) {
 if (errno == EPERM) {
@@ -881,10 +883,11 @@ static int create_item(Item *i) {
 }
 }
 
-r = item_set_perms(i, i-path);
-if (r  0)
-return r;
-
+if (mknod_succeeded) {
+r = item_set_perms(i, i-path);
+if (r  0)
+return r;
+}
 break;
 }
 
-- 
1.9.3

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


Re: [systemd-devel] Should user mode linux register with machined?

2014-10-13 Thread Jan Synacek
Jan Synacek jsyna...@redhat.com writes:
 Lennart Poettering lenn...@poettering.net writes:
 On Fri, 10.10.14 18:48, Richard Weinberger (rich...@nod.at) wrote:

 Lennart,
 
 Am 10.10.2014 um 18:44 schrieb Lennart Poettering:
  It's a bit more complex. While UML, qemu, kvm, currently don't, LXC,
  systemd-nspawn and libvirt-lxc all do talk directly to machined. (Note
  that LXC and libvirt-lxc are separate codebases, the latter is *not* a
  wrapper around the former).
  
  So, dunno, it really is up to how you intend UML to be used. If UML
  shall be nice and useful without libvirt, then it's worth doing the
  registration natively, but it's also OK to just leave this to libvirt,
  if that's your primary envisioned usecase...
 
 What is the benefit of this registration?
 I boot all day long UML and qemu-kvm VMs without registering them to 
 systemd,
 so I don't really know what I'm missing. :-)
 But if there is a nice use case I'll happily add the registration to UML.

 Hmm, I figure this mail didn't make it through to you?

 http://lists.freedesktop.org/archives/systemd-devel/2014-October/023875.html

 I don't see that mail in my mailbox either and I know that you noticed
 some mail not arriving before. It seems to cause quite a lot of
 confusion in discussion and patch submission. I'm not sure who to report
 this to.

Bah, never mind, I can see the mail now...

-- 
Jan Synacek
Software Engineer, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


[systemd-devel] Unicode support in console after boot

2014-10-13 Thread Jan Synacek
Hello,

currently, unicode characters are not correctly displayed in the
console. After login, when I run /usr/bin/unicode_start, unicode works
fine. I tried to create a service file that runs this script, linking
tty to stdout and stderr, but that didn't work. Is there a way how to
turn on unicode support in console after boot using a service file? Or
any other type of unit? Or is this something that has to be patched in
the source (logind perhaps?)?

Cheers,
-- 
Jan Synacek
Software Engineer, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] Unicode support in console after boot

2014-10-13 Thread Jan Synacek
Mantas Mikulėnas graw...@gmail.com writes:
 On Mon, Oct 13, 2014 at 10:36 AM, Jan Synacek jsyna...@redhat.com wrote:
 Hello,

 currently, unicode characters are not correctly displayed in the
 console. After login, when I run /usr/bin/unicode_start, unicode works
 fine. I tried to create a service file that runs this script, linking
 tty to stdout and stderr, but that didn't work. Is there a way how to
 turn on unicode support in console after boot using a service file? Or
 any other type of unit? Or is this something that has to be patched in
 the source (logind perhaps?)?

 This is already done by systemd-vconsole-setup [1], but only if the
 system locale is a UTF-8 one [2].

 [1]: 
 http://cgit.freedesktop.org/systemd/systemd/tree/src/vconsole/vconsole-setup.c?h=a158dbf156ac#n70
 [2]: 
 http://cgit.freedesktop.org/systemd/systemd/tree/src/shared/util.c?h=a158dbf156ac#n5547

Thank you. There seems to be something wrong with the
systemd-vconsole-setup.service, it doesn't seem to be run correctly at
boot. If restarted, I get the unicode support.

-- 
Jan Synacek
Software Engineer, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] Unicode support in console after boot

2014-10-13 Thread Jan Synacek
Andrei Borzenkov arvidj...@gmail.com writes:
 On Mon, Oct 13, 2014 at 12:48 PM, Jan Synacek jsyna...@redhat.com wrote:
 Mantas Mikulėnas graw...@gmail.com writes:
 On Mon, Oct 13, 2014 at 10:36 AM, Jan Synacek jsyna...@redhat.com wrote:
 Hello,

 currently, unicode characters are not correctly displayed in the
 console. After login, when I run /usr/bin/unicode_start, unicode works
 fine. I tried to create a service file that runs this script, linking
 tty to stdout and stderr, but that didn't work. Is there a way how to
 turn on unicode support in console after boot using a service file? Or
 any other type of unit? Or is this something that has to be patched in
 the source (logind perhaps?)?

 This is already done by systemd-vconsole-setup [1], but only if the
 system locale is a UTF-8 one [2].

 [1]: 
 http://cgit.freedesktop.org/systemd/systemd/tree/src/vconsole/vconsole-setup.c?h=a158dbf156ac#n70
 [2]: 
 http://cgit.freedesktop.org/systemd/systemd/tree/src/shared/util.c?h=a158dbf156ac#n5547

 Thank you. There seems to be something wrong with the
 systemd-vconsole-setup.service, it doesn't seem to be run correctly at
 boot. If restarted, I get the unicode support.


 Do you use graphical splash screen (plymouth) by any chance?

Yes, I'm trying this in somewhat newish rawhide, I believe plymouth is
on by default. I tried removing rhgb from the kernel command line, but
that didn't change anything.

-- 
Jan Synacek
Software Engineer, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] Unicode support in console after boot

2014-10-13 Thread Jan Synacek
Andrei Borzenkov arvidj...@gmail.com writes:
 On Mon, Oct 13, 2014 at 4:33 PM, Jan Synacek jsyna...@redhat.com wrote:
 Andrei Borzenkov arvidj...@gmail.com writes:
 On Mon, Oct 13, 2014 at 12:48 PM, Jan Synacek jsyna...@redhat.com wrote:
 Mantas Mikulėnas graw...@gmail.com writes:
 On Mon, Oct 13, 2014 at 10:36 AM, Jan Synacek jsyna...@redhat.com wrote:
 Hello,

 currently, unicode characters are not correctly displayed in the
 console. After login, when I run /usr/bin/unicode_start, unicode works
 fine. I tried to create a service file that runs this script, linking
 tty to stdout and stderr, but that didn't work. Is there a way how to
 turn on unicode support in console after boot using a service file? Or
 any other type of unit? Or is this something that has to be patched in
 the source (logind perhaps?)?

 This is already done by systemd-vconsole-setup [1], but only if the
 system locale is a UTF-8 one [2].

 [1]: 
 http://cgit.freedesktop.org/systemd/systemd/tree/src/vconsole/vconsole-setup.c?h=a158dbf156ac#n70
 [2]: 
 http://cgit.freedesktop.org/systemd/systemd/tree/src/shared/util.c?h=a158dbf156ac#n5547

 Thank you. There seems to be something wrong with the
 systemd-vconsole-setup.service, it doesn't seem to be run correctly at
 boot. If restarted, I get the unicode support.


 Do you use graphical splash screen (plymouth) by any chance?

 Yes, I'm trying this in somewhat newish rawhide, I believe plymouth is
 on by default. I tried removing rhgb from the kernel command line, but
 that didn't change anything.


 Does booting with plymouth.enable=0 change anything?

Nope, that doesn't help. After loadkeys cz, I still see white
rectangles instead of proper characters.

-- 
Jan Synacek
Software Engineer, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


[systemd-devel] [PATCH] bash-completion: fix systemctl isolate

2014-10-09 Thread Jan Synacek
I didn't dare fix this in zsh completion (not sure if it's required, though, so
I didn't even remove the TODO entry that mentions this fix is required).

Jan Synacek (1):
  bash-completion: fix systemctl isolate

 shell-completion/bash/systemctl.in | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

-- 
1.9.3

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


  1   2   >