On Tue, Jun 07, 2016 at 02:34:39PM -0400, John Ferlan wrote: > > > On 06/07/2016 12:01 PM, Daniel P. Berrange wrote: > > On Tue, Jun 07, 2016 at 10:45:29AM -0400, John Ferlan wrote: > >> Patches 1-3 were posted separately: > >> > >> http://www.redhat.com/archives/libvir-list/2016-June/msg00256.html > >> > >> But perhaps seeing the final direction will make things more clear as > >> to why a "real" flag system wasn't used and keeping the current paradigm > >> of constant value returns still works just fine. > >> > >> Patches 4-5 were posted separately: > >> > >> http://www.redhat.com/archives/libvir-list/2016-June/msg00091.html (4) > >> http://www.redhat.com/archives/libvir-list/2016-June/msg00094.html (5) > >> > >> Although at one point patch 4 had an ACK: > >> > >> http://www.redhat.com/archives/libvir-list/2016-May/msg02115.html > >> > >> It wasn't clear if the more recent review rescinded that, so it still > >> remains "in the list". I understand the concern about adding secret to > >> cfg.mk checking, but without a better idea of how to handle - I left > >> things as they were. > >> > >> Patches 6-16 are all new. Some parts are separable, but rather than continue > >> piecemeal I just figured going with an RFC will at least > >> > >> Patch 6 is only there to "prove" that using the current encryption paradigm > >> XML still works, although if I've read the tea leaves correctly, the qemu > >> support isn't working as desired/expected. > >> > >> Patch 7 adds "usage" as an XML attribute for encryption and the associated > >> tests with that. I've chosen to "reuse" the <encryption> XML element rather > >> than inventing something new. I'm not opposed to something new, but let's > >> decide up a name quickly... > >> > >> Patch 8-9 adds the ability for the storage backend to create/recognize a > >> luks volume > >> > >> Patches 10-13 adds support for luks encryption in the storage backend. > >> The new "<secret>" format uses "luks" as the usage type and "<key>" as > >> the 'name'. If those names cause angst, I'm fine with changing, but just > >> give a better suggestion! Adding <cipher> and <ivgen> were a result of > >> using qemu constructs from qemu commit id '3e308f20'. Since we are parsing > >> something new, I figure failing in the domain parse code for this new type > >> was acceptible as opposed to some post processing check. > >> > >> Patches 14-16 adds support for luks encryption to the domain using > >> <encryption type='luks'... <secret format='key' usage/uuid='xxx'>> > >> > >> I've tested using a "good" and "bad" password and got the expected results > >> for starting a domain. I did not add 'virsh vol-create-as' support just > >> yet. I figured that would be less to go back and redo if the names of > >> elements changes. I've also run the changes through Coverity with no > >> new issues detected. > >> > >> The whole series is a result of the following bz: > >> > >> https://bugzilla.redhat.com/show_bug.cgi?id=1301021 > > > > BTW, one of the core reasons for the LUKS support in QEMU is to facilitate > > use of LUKS with non-local-file based disks. eg, LUKS over RBD, or LUKS > > over NBD or LUKS over iSCSI, etc. > > > > The diffstat for the files shows you've wired up luks-over-plain-file > > ok, but I'm wondering if the code posted here is also dealing with the > > broader support, or if that's something to be addressed later. > > > > No big deal either way, particularly since qemu-img can't actually > > create LUKS volumes on anything other than plain files/block devs > > yet. > > > > Hmm, right, sigh, but I had to start somewhere. So rather than be > either/or in qemuDomainSecretDiskPrepare and qemuBuildDriveStr, there > would need to be a way to have multiple 'secinfo' structs per disk or > maybe one for <auth> and one for <encrypt>. Can we predict the future > and go with a list or just two secinfo's... Any preference? I'd suggest just two secinfos. Then again if we have a chain of backing files that are encrpted or need authentication - of course I don't think our XML lets us deal with that yet anyway. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list