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 :| -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
