[systemd-devel] Bugfix release(s)
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
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=
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
On Mon, Jul 10, 2017 at 4:41 PM, Lennart Poettering 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 >> > 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
On Mon, Jul 10, 2017 at 12:42 PM, Lennart Poettering 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'
On Tue, Mar 7, 2017 at 9:25 AM, Lucas Ventura Carro 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
Zbigniew Jędrzejewski-Szmek 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
David Timothy Strauss 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 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
Jay Burger 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
Jan Synacek writes: > Lennart Poettering 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
Michael Biebl writes: > 2015-10-07 14:43 GMT+02:00 Jan Synacek : >> Lennart Poettering 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
Lennart Poettering 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
Lennart Poettering writes: > On Mon, 22.06.15 14:49, Jan Synacek (jsyna...@redhat.com) wrote: > >> Lukáš Nykrýn 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
Lukáš Nykrýn 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
Lennart Poettering 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
Lennart Poettering writes:
> On Wed, 20.05.15 10:37, jsyna...@redhat.com (jsyna...@redhat.com) wrote:
>
>> From: Jan Synacek
>>
>> 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
Lennart Poettering writes: > On Wed, 13.05.15 15:21, jsyna...@redhat.com (jsyna...@redhat.com) wrote: > >> From: Jan Synacek > > 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
Lennart Poettering 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
Lennart Poettering 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
Re: [systemd-devel] [PATCH] tmpfiles: don't create subvolumes in chroot
Lennart Poettering 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
Lennart Poettering 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
[systemd-devel] [PATCH] tmpfiles: don't create subvolumes in chroot
---
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
[systemd-devel] [PATCH] tmpfiles: don't create subvolumes in chroot
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
Re: [systemd-devel] SELinux labels on unix sockets
Lennart Poettering 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
Andrei Borzenkov writes: > В Fri, 20 Feb 2015 10:56:41 +0100 > Jan Synacek пишет: > >> 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 and typo in partition table blurb
---
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
Mantas Mikulėnas 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
[systemd-devel] [PATCH] nspawn: fix whitespace in partition table blurb
---
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
Re: [systemd-devel] [PATCH] cryptsetup-generator: support rd.luks.key=keyfile:keyfile_device
Andrei Borzenkov writes: > В Fri, 20 Feb 2015 10:56:42 +0100 > 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. >> > > 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
Przemyslaw Rudy 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
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;
crypt
[systemd-devel] [PATCH] cryptsetup-generator: support rd.luks.key=keyfile:keyfile_device
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
Re: [systemd-devel] test-dhcp-client failing in mock builds
Zbigniew Jędrzejewski-Szmek 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"
---
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
Re: [systemd-devel] libsystemd-network tests failing in mock
Tom Gundersen writes:
> On Thu, Dec 11, 2014 at 9:14 AM, Jan Synáček wrote:
>> test-dhcp-{client,server} are failing in mock:
>>
>> FAIL: test-dhcp-client
>> ==
>> Assertion 'client' failed at ../src/libsystemd-network/sd-dhcp-client.c:138,
>> function sd_dhcp_client_set_request_option(). Ignoring.
>> Assertion 'client' failed at ../src/libsystemd-network/sd-dhcp-client.c:169,
>> function sd_dhcp_client_set_request_address(). Ignoring.
>> Assertion 'client' failed at ../src/libsystemd-network/sd-dhcp-client.c:182,
>> function sd_dhcp_client_set_index(). Ignoring.
>> Assertion 'interface_index > 0' failed at
>> ../src/libsystemd-network/sd-dhcp-client.c:185, function
>> sd_dhcp_client_set_index(). Ignoring.
>> Assertion 'interface_index > 0' failed at
>> ../src/libsystemd-network/sd-dhcp-client.c:185, function
>> sd_dhcp_client_set_index(). Ignoring.
>> Assertion 'interface_index > 0' failed at
>> ../src/libsystemd-network/sd-dhcp-client.c:185, function
>> sd_dhcp_client_set_index(). Ignoring.
>> DHCP CLIENT (0x0): FREE
>> DHCP CLIENT (0xbe31128b): STARTED on ifindex 42
>> DHCP CLIENT (0xbe31128b): DISCOVER
>> DHCP CLIENT (0xbe31128b): STOPPED
>> DHCP CLIENT (0x0): FREE
>> DHCP CLIENT (0x45d3fd67): STARTED on ifindex 42
>> DHCP CLIENT (0x45d3fd67): DISCOVER
>> DHCP CLIENT (0x45d3fd67): OFFER
>> DHCP CLIENT (0x45d3fd67): REQUEST (requesting)
>> DHCP CLIENT (0x45d3fd67): ACK
>> DHCP CLIENT (0x45d3fd67): lease expires in 9min 57.698157s
>> DHCP CLIENT (0x45d3fd67): T2 expires in 8min 42.895614s
>> DHCP CLIENT (0x45d3fd67): T1 expires in 5min 101.292ms
>> DHCP CLIENT (0x45d3fd67): STOPPED: Operation not permitted
>
>
> This is really odd. I cannot reproduce, and looking at the code I
> cannot figure out how this could ever happen. I pushed a bit of extra
> logging just now, but even so I really don't see it.
>
> Any chance you can reproduce this locally and either let me know how,
> or go step through it with gdb and see where the EPERM originates?
Assuming that you run Fedora, you can patch the current specfile that is
in rawhide with the patch in [1]. Then, just execute "fedpkg
mockbuild". I tried to manually make the test fail directly inside the
mock buildroot, but couldn't make it work. After patching the spec, you
can create an srpm with "fedpkg srpm" and then use mock direcly like so:
$ mock -r fedora-rawhide-x86_64 --no-cleanup --no-cleanup-after
You'll then have the root and the results in
/var/lib/mock/fedora-rawhide-x86_64/result.
[1] https://bugzilla.redhat.com/show_bug.cgi?id=1076119
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] virt: fix container detection when we're not PID 1
Lennart Poettering 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
[systemd-devel] [PATCH] test: fix some tests when running inside a container
---
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
[systemd-devel] [PATCH v2] test: fix some tests when running inside a container
---
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
Re: [systemd-devel] [PATCH] test: fix some tests when running inside a container
Jan Synacek 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] virt: fix container detection when we're not PID 1
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] emergency, rescue and single-user
Mantas Mikulėnas writes: > On Tue, Dec 9, 2014 at 2:43 PM, Jan Synáček 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
Re: [systemd-devel] emergency, rescue and single-user
Lennart Poettering 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
[systemd-devel] [PATCH v2] localed: log xkbcommon errors
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: log xkbcommon errors
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
Lennart Poettering 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: forward xkbcommon errors
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
Lennart Poettering 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
Re: [systemd-devel] [PATCH] localed: forward xkbcommon errors
David Herrmann writes:
> Hi Jan!
Hello!
> On Tue, Nov 25, 2014 at 9:23 AM, Jan Synacek 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
[systemd-devel] [PATCH] localed: forward xkbcommon errors
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 v2] localed: validate set-x11-keymap input
Lennart Poettering 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
David Herrmann writes: > Hi > > On Fri, Nov 14, 2014 at 12:42 PM, Jan Synacek 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
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;
-
-
Re: [systemd-devel] [PATCH] localed: validate set-x11-keymap input
Lennart Poettering 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] localed: validate set-x11-keymap input
Jan Synacek writes:
> Lennart Poettering 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
Lennart Poettering 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_
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");
&g
[systemd-devel] [PATCH v2] shared/install: when unit contains only Also=, report 'indirect'
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
0
+indirect
+Unit's status is determined indirectly by another
unit(s) specified in Also= in [Install] section
+0
+
+
disabled
Unit is not enabled
1
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);
+r = unit_file_search(&c, i, paths, root_dir, allow_symlink, true,
also);
if (r >= 0)
Re: [systemd-devel] [PATCH] shared/install: when unit contains only Also=, report 'indirect'
Lennart Poettering 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
Re: [systemd-devel] [PATCH] shared/install: don't report 'static' when unit contains only Also=
Lennart Poettering writes: > On Fri, 07.11.14 09:49, Jan Synacek (jsyna...@redhat.com) wrote: > >> Lennart Poettering 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: don't report 'static' when unit contains only Also=
Zbigniew Jędrzejewski-Szmek 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 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
[systemd-devel] [PATCH] shared/install: when unit contains only Also=, report 'indirect'
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
0
+indirect
+Unit's status is determined indirectly by another
unit(s) specified in Also= in [Install] section
+0
+
+
disabled
Unit is not enabled
1
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, true,
also);
if (r >= 0)
r =
@@ -1415,7 +1427,7 @@ static int install_contex
[systemd-devel] [PATCH] shared/install: when unit contains only Also=, report 'indirect'
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
Re: [systemd-devel] [PATCH] shared/install: don't report 'static' when unit contains only Also=
Lennart Poettering 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: don't report 'static' when unit contains only Also=
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] shared/install: don't report 'static' when unit contains only Also=
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] man/tmpfiles.d: fix typo
--- 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] localed: validate set-x11-keymap input
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;
-
[systemd-devel] [PATCH] localed: validate set-x11-keymap input
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
Re: [systemd-devel] [PATCH] localectl: fix localectl set-x11-keymap syntax description
David Herrmann writes:
> Hi
>
> On Mon, Nov 3, 2014 at 2:01 PM, Jan Synacek 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
[systemd-devel] [PATCH] localectl: fix localectl set-x11-keymap syntax description
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
[systemd-devel] [PATCH v3] core: improve error message when machine id is missing
---
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] Switch root slowness
Lennart Poettering 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
Re: [systemd-devel] Switch root slowness
Dave Reisner writes: > On Thu, Oct 30, 2014 at 01:18:24PM +0100, Jan Synáček wrote: >> Hello, >> >> commit 539618a0ddc2dc7f0fbe28de2ae0e07b34c81e60 >> Author: Lennart Poettering >> 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
[systemd-devel] [PATCH v2] core: improve error message when machine id is missing
---
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] [PATCH] bash-completion: fix systemctl isolate
Jan Synacek 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
Re: [systemd-devel] Possible documentation problems
Lennart Poettering 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
Re: [systemd-devel] [PATCH] swap: rework discard
Lennart Poettering 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] [PATCH] tmpfiles: only change device permissions if mknod succeeded
Tom Gundersen writes: > On Mon, Oct 27, 2014 at 4:53 PM, Tom Gundersen wrote: >> On Mon, Oct 27, 2014 at 4:48 PM, Lennart Poettering >> 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 >>>> 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 1/2] man: fix localectl set-x11-keymap syntax description
Ran Benita writes: > On Fri, Oct 17, 2014 at 02:02:12PM +0200, Jan Synacek wrote: >> --- >> 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 @@ >> >> >> >> -set-x11-keymap LAYOUT >> [MODEL] [VARIANT] [OPTIONS] >> +set-x11-keymap LAYOUT [MODEL >> [VARIANT [OPTIONS]]] > > Would have been nice if this used the same order as setxkbmap (which is > more sensible), but I suppose this cannot be changed? I'm not sure. The man page of setxkbmap doesn't specify model in the optional argumets at all, you can change it with the -model option. Otherwise, the order of arguments is the same. If you want to put the model argument somewhere, I think that the order that localectl uses now makes sense. That's just my opinion, though. -- 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] Should user mode linux register with machined?
Lennart Poettering 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
Re: [systemd-devel] Unicode support in console after boot
Michal Sekletar 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
[systemd-devel] [PATCH] core: improve error message when machine id is missing
---
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
[systemd-devel] [PATCH] bash-completion: fix systemctl isolate
---
shell-completion/bash/systemctl.in | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/shell-completion/bash/systemctl.in
b/shell-completion/bash/systemctl.in
index 0150018..e47c028 100644
--- a/shell-completion/bash/systemctl.in
+++ b/shell-completion/bash/systemctl.in
@@ -67,6 +67,8 @@ __get_disabled_units () { __systemctl $1 list-unit-files \
| { while read -r a b c ; do [[ $b == "disabled" ]] && echo " $a";
done; }; }
__get_masked_units () { __systemctl $1 list-unit-files \
| { while read -r a b c ; do [[ $b == "masked" ]] && echo " $a";
done; }; }
+__get_all_targets() { { __systemctl $1 list-units -t target --all; } \
+| { while read -r a b; do echo " $a"; done; }; }
_systemctl () {
local cur=${COMP_WORDS[COMP_CWORD]} prev=${COMP_WORDS[COMP_CWORD-1]}
@@ -198,7 +200,7 @@ _systemctl () {
elif __contains_word "$verb" ${VERBS[ISOLATABLE_UNITS]}; then
comps=$( __filter_units_by_property $mode AllowIsolate yes \
- $( __get_all_units $mode ) )
+ $( __get_all_targets $mode ) )
compopt -o filenames
elif __contains_word "$verb" ${VERBS[FAILED_UNITS]}; then
--
1.9.3
___
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
Jan Synacek 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 2/2] localectl: verify layout, model, variant and options
Lennart Poettering 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
[systemd-devel] [PATCH] swap: rework discard
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 = NUL
Re: [systemd-devel] [PATCH] swap: introduce Discard property
Lennart Poettering 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 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
[systemd-devel] [PATCH 2/2] localectl: verify layout, model, variant and options
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;
+c
[systemd-devel] [PATCH 1/2] man: fix localectl set-x11-keymap syntax description
--- 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 @@ -set-x11-keymap LAYOUT [MODEL] [VARIANT] [OPTIONS] +set-x11-keymap LAYOUT [MODEL [VARIANT [OPTIONS]]] Set 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
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
Re: [systemd-devel] [PATCH 2/2] localectl: verify layout, model, variant and options
Ran Benita 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 2/2] localectl: verify layout, model, variant and options
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 lo
[systemd-devel] [PATCH 1/2] man: fix localectl set-x11-keymap syntax description
--- 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 @@ -set-x11-keymap LAYOUT [MODEL] [VARIANT] [OPTIONS] +set-x11-keymap LAYOUT [MODEL [VARIANT [OPTIONS]]] Set 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 0/2] localectl: verify layout, model, varian and options; fix manpage
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
Re: [systemd-devel] Possible documentation problems
Mantas Mikulėnas writes: > On Oct 15, 2014 12:07 PM, "Jan Synacek" 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] Possible documentation problems
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
[systemd-devel] [PATCH] man: fix typos
--- 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 Type=oneshot is used, in which case the timeout is disabled by default -(see systemd-systemd.conf5). +(see systemd-system.conf5). @@ -569,7 +569,7 @@ the timeout logic. Defaults to DefaultTimeoutStopSec= from the manager configuration file -(see systemd-systemd.conf5). +(see systemd-system.conf5). 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 0 to disable the timeout logic. Defaults to DefaultTimeoutStartSec= from the manager configuration file -(see systemd-systemd.conf5). +(see systemd-system.conf5). 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 0 to disable the timeout logic. Defaults to DefaultTimeoutStartSec= from the manager configuration file -(see systemd-systemd.conf5). +(see systemd-system.conf5). -- 1.9.3 ___ 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
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] Unicode support in console after boot
Andrei Borzenkov writes: > On Mon, Oct 13, 2014 at 4:33 PM, Jan Synacek wrote: >> Andrei Borzenkov writes: >>> On Mon, Oct 13, 2014 at 12:48 PM, Jan Synacek wrote: >>>> Mantas Mikulėnas writes: >>>>> On Mon, Oct 13, 2014 at 10:36 AM, 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?)? >>>>> >>>>> 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
Re: [systemd-devel] Unicode support in console after boot
Andrei Borzenkov writes: > On Mon, Oct 13, 2014 at 12:48 PM, Jan Synacek wrote: >> Mantas Mikulėnas writes: >>> On Mon, Oct 13, 2014 at 10:36 AM, 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?)? >>> >>> 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
Mantas Mikulėnas writes: > On Mon, Oct 13, 2014 at 10:36 AM, 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?)? > > 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
