Re: [libvirt] Resubmission #2: [PATCH 0/3] sVirt AppArmor security driver
On Fri, Sep 25, 2009 at 05:44:35PM -0500, Jamie Strandboge wrote:
This simplifies adding new files. Part of this process involved adding
support for kernel, initrd, loader, serial, and console. I have commented
out code for hostdev (and pci would be easy enough), but it requires a
patch to to move the _usbDevice struct fully into hostusb.h. I figure
once the AppArmor driver is in, I can submit a patch for that.
No, the usbDevice struct is intended to be private because we want the
ability to easily change it without impacting callers. The idea is that
there is not neccessarily just 1 single path associated with a USB
device and thus 'path' should not be exposed to callers. Instead if you
need process the 1-or-more paths associated with a USB device you should
invoke the usbDeviceFileIterate() method and the callback you supply will
be executing once for each path. The same pattern is there for PCI devices
which have between 1 and 4 paths that need dealing with typically.
This iteration also properly works with attach/detach. While I use
virDomainDefFormat() primarily, this does not work with attach because
the definition is not updated with the information yet (neither def nor
newDef), so I pass the disk file on the command line. For now this is ok
since SetSecurityImageLabel currently only deals with a single file, but
this will need to be adjusted depending on how the storage backend patch
works out. I thought about creating a new def and inserting the disk into
it and sending it off to virt-aa-helper, but I couldn't find an easy
way to get an accurate virCapsPtr in security_apparmor.c. I had a
similar problem with virt-aa-helper, but there I just created a fake
virCapsPtr (which defaults to 'hvm', which was a compromise I was
willing to make at this time since sVirt only supports qemu right now).
The issues with virCapsPtr aside, the code is overall cleaner and much
more easily extendable, so thanks for the excellent feedback. :)
For virCapsPtr issues we should probably evaluate whether we can make
the capabilities object passed to virDomainDefParse() optionally
NULLable. The capabilities object is used to fill in missing pieces
of metadata when getting an XML blob in from the client app. By the
time we get into your virt-aa-helper, all this metadata ought to be
present & correct, so in theory there should be no need for a virCapsPtr
object.
Does the virt-aa-helper need the full domain XML when attaching or
detaching a disk ? If not, then you could try an approach where
you pass the full domain XML when initially starting the guest,
and then only pass the domain UUID + the device XML when attaching
or detaching a particular device on the fly.
Regards,
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|