On Wed, Aug 20, 2014 at 09:58:53AM +0200, Michal Privoznik wrote:
On 20.08.2014 05:49, Martin Kletzander wrote:On Tue, Aug 19, 2014 at 03:18:02PM +0200, Michal Privoznik wrote:https://bugzilla.redhat.com/show_bug.cgi?id=1095636 When starting up the domain the domain's NICs are allocated. As of 1f24f682 (v1.0.6) we are able to use multiqueue feature on virtio NICs. It breaks network processing into multiple queues which can be processed in parallel by different host CPUs. The queues are, however, created by opening /dev/net/tun several times. Unfortunately, only the first FD in the row is labelled so when turning the multiqueue feature on in the guest, qemu will get AVC denial. Make sure we label all the FDs needed. Moreover, the default label of /dev/net/tun doesn't allow attaching a queue: type=AVC msg=audit(1399622478.790:893): avc: denied { attach_queue } for pid=7585 comm="qemu-kvm" scontext=system_u:system_r:svirt_t:s0:c638,c877 tcontext=system_u:system_r:virtd_t:s0-s0:c0.c1023Wow, I didn't even know you could have range in the "level" field (I think it doesn't make sense, but I don't understand why it is allowed by SELinux).tclass=tun_socket And as suggested by SELinux maintainers, the tun FD should be labeled
Oh, it's probably too late for s/labeled/labelled/, right? :-/ [...]
+ for (i = 0; i < tapfdSize; i++) { + if (virSecurityManagerSetTapFDLabel(driver->securityManager, + vm->def, tapfd[i]) < 0) + goto cleanup; + } +Shouldn't there be the same loop for vhostfd[i]? Although it won't probably be > 1. Just ckecking.No, vhost FDs are not connected queue to. Frankly, I don't fully understand all the details, but hey - it works :-)
I didn't mean that WRT the multiqueue problem, I just wondered if vhostfd (if passed to qemu as well) cannot have similar problem (only vhostFDs[0] labelled) in the future (which we could fix before it appears). [...]
This looks much better now. Although it looks way too similar to virSecuritySELinuxSetImageFDLabel() :) I think it might be worth keeping just one of these two functions, let's say ...SetFDLabel() in order not to complicate things, and assign it to both needed fields in the domain structure.Yes and no. While SetTapFDLabel needs to set domain process label (seclabel->label) SetImageFDLabel needs to use image label (seclabel->imagelabel). And I don't think it's worth wrap these two
Yep, you're right.
functions into one wrapper function (I doubt it'll be readable more too).
We could argue about the readability, but it doesn't make sense now when they are different.
ACK with that changed.So I'm taking this as ACK without any change required and pushing. Thanks!
If you can fix the commit message, please do, otherwise the ACK stands as guessed. Martin
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list