FS#56354 - [qemu] drop udev rule for KERNEL=="kvm"

Attached to Project: Arch Linux
Opened by Tom Yan (tom.ty89) - Thursday, 16 November 2017, 19:28 GMT
Last edited by Eli Schwartz (eschwartz) - Monday, 18 December 2017, 17:31 GMT
Task Type Bug Report
Category Packages: Extra
Status Closed
Assigned To Tobias Powalowski (tpowa)
Anatol Pomozov (anatolik)
Architecture All
Severity Low
Priority Normal
Reported Version
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 0
Private No

Details

Description:

systemd already ships the same rule:

[tom@archlinux ~]$ grep 'KERNEL=="kvm"' /usr/lib/udev/rules.d/*
/usr/lib/udev/rules.d/50-udev-default.rules:KERNEL=="kvm", GROUP="kvm", MODE="0660"
/usr/lib/udev/rules.d/65-kvm.rules:KERNEL=="kvm", GROUP="kvm", MODE="0660"
/usr/lib/udev/rules.d/70-uaccess.rules:SUBSYSTEM=="misc", KERNEL=="kvm", TAG+="uaccess"

More importantly, there is a coming change in systemd that changes the default mode from 0660 to 0666 and drops the uaccess rule:

https://github.com/systemd/systemd/commit/b8fd3d82205f632ce001fade74fed287e1564a1a

The rule shipped downstream will not only be redundunt but confusing and likely problematic.

Attached are the updated version of relevant files.



Additional info:
* package version(s) 2.10.1-1
   65-kvm.rules (0.1 KiB)
   qemu.install (0.2 KiB)
This task depends upon

Closed by  Eli Schwartz (eschwartz)
Monday, 18 December 2017, 17:31 GMT
Reason for closing:  Fixed
Additional comments about closing:  qemu 2.11.0-1
Comment by loqs (loqs) - Thursday, 16 November 2017, 21:57 GMT
Duplicate  FS#54943 
Comment by Tom Yan (tom.ty89) - Thursday, 16 November 2017, 22:24 GMT
@loqs No, it is not. This is report is about a duplicate (for now) udev rule which will cause unexpected permission (not ownership) problem because of an upcoming change (from 0660+uaccess to 0666). Please read more carefully. The one you referred is due to Arch's obsession on GID and practically affects libvirt only, while this one will affect users who run qemu directly.

P.S. Though among the tons of revisions of patches (no idea why it was made so complicated), it seems to be addressed.
Comment by loqs (loqs) - Thursday, 16 November 2017, 22:52 GMT
@tom.ty89 yes the scope of  FS#54943  expanded to cover the duplicate udev rule as well as the duplicate sysuser rule as they were both caused by systemd 234 and the PKGBUILDs would need adjustment.
If this issue was limited to https://github.com/systemd/systemd/commit/b8fd3d82205f632ce001fade74fed287e1564a1a then it seems premature as that commit will be for systemd 236 which is not in arch yet.
Edit:
I made so many different patches to provide the maintainer with alternative options which resulted in six initial versions
the later updated patches addresses shortcomings in the initial version and flysray does not allow the removal or updating attachments.
Comment by Tom Yan (tom.ty89) - Thursday, 16 November 2017, 23:02 GMT
It's the same thing, it's a dup, so it should be get rid of anyway, and as long as the dup is gone, we will not get into trouble when the change arrives. If you are sure the dev will apply your patch(es), I don't mind if this is closed. Just make sure both the .rules and the .install are updated (not that the latter matters but well).
Comment by loqs (loqs) - Thursday, 16 November 2017, 23:13 GMT
I do not know how  FS#54943  will be resolved https://bugs.archlinux.org/task/54943#comment160609 is the last maintainer comment since then both qemu and libvirt have been updated multiple times with no relevant patches.
systemd could add the option -Ddev-kvm-mode=0660 now which will have no effect as that is the current default and would prevent the change from the later commit.
Edit:
0660 instead of 600
Comment by Tom Yan (tom.ty89) - Thursday, 16 November 2017, 23:44 GMT
That's why I think this should be recorded/handled separately. The duplicate udev rule doesn't really have anything to do the GID problem anyway.

No we don't need any option set downstream, and we do not want 600 anyway. It's a matter of granting rw to others once in for all or doing it with the uaccess ACL approach. (I mean, it's a matter of upstream is gonna change from the latter to the former and this now not-a-problem dup will become a problem) Seems like you still aren't sure what this is about?
Comment by loqs (loqs) - Friday, 17 November 2017, 00:03 GMT
Yes I missed user access drop I was just addressing keeping /usr/lib/udev/rules.d/50-udev-default.rules consistant.
So that comes back to my point about it being premature as this addresses an issue the arch packages are not affected by but its not my decision anyway.
Edit:
Fixed sorry 0660 instead of 600
Comment by Eli Schwartz (eschwartz) - Friday, 17 November 2017, 03:03 GMT
The Arch packages are certainly affected by the low-importance fact that we are shipping duplicate udev rules. It may become a bigger issue once the duplicate rules start having additional effects on top of being duplicative... but that doesn't mean it is not, in fact, duplicative *right now*.
Comment by loqs (loqs) - Friday, 17 November 2017, 10:23 GMT
@eschwartz should I prepare new proposed patches for  FS#54943  which do not fix the duplicate udev rule as well?
Comment by Anatol Pomozov (anatolik) - Friday, 17 November 2017, 18:33 GMT
Thanks for the patch. It got merged to repository as revision 310218 and will be available with the next qemu package update.
Comment by loqs (loqs) - Friday, 17 November 2017, 18:40 GMT
@anatolik is there any reason why you took the patch for this so readily but not the patches for  FS#54943  is there any point in submitting more patches for  FS#54943 ?
Comment by Anatol Pomozov (anatolik) - Friday, 17 November 2017, 18:43 GMT
Giancarlo Razzolini is working on  FS#54943 . Please check with him what is the patches submission status.
Comment by Tom Yan (tom.ty89) - Friday, 17 November 2017, 19:10 GMT
@loq  FS#54943  is a way more nasty issue. It is not just a qemu/libvirt issue tbh, but to some extent a general policy conflict. AFAIR Arch decided sometime ago that it should not only used fixed ID/name mapping for the downstream system groups (while systemd does enumerated mapping AFAIK), but also try to reference to them with ID and avoid names everywhere. (Personally I found that silly and ugly ever since it was introduced) It would have been trivial if there's no such policy, but now the devs may need to think it through because replacing every 78 to kvm in the libvirt package means that the policy has to be violated.
Comment by loqs (loqs) - Friday, 17 November 2017, 22:55 GMT
@tom.ty89 there is precedent for dropping already assigned uid/gid systemd-journal-gateway, systemd-timesync, systemd-network, systemd-bus-proxy, systemd-resolve had uids/gid assigned when provided by filesystem but they were dropped when they became provided by systemd, in testing filesystem also removes assigned uid/gid from nobody and assigned gid from users.
It was also agreed in principal that was the intended fix for  FS#54943  https://bugs.archlinux.org/task/54943#comment160609 I assume anatolik also agreed that was the right approach and no other package apart from libvirt
relies on kvm having the gid 78 fortunately there were not many packages depending on qemu to check.

Loading...