On Fri, Jun 30, 2017 at 1:07 PM, John Ferlan <jferlan@xxxxxxxxxx> wrote: > > > On 06/30/2017 11:30 AM, ashish mittal wrote: >> Hi, >> >> Thanks for the review! >> >> On Fri, Jun 30, 2017 at 12:41 AM, Peter Krempa <pkrempa@xxxxxxxxxx> wrote: >>> On Thu, Jun 29, 2017 at 19:02:38 -0700, Ashish Mittal wrote: >>>> From: Ashish Mittal <ashish.mittal@xxxxxxxxxxx> >>>> >>>> QEMU changes for VxHS (including TLS support) are already upstream. >>>> This series of patches adds support for VxHS block devices in libvirt. >>>> >>>> Patch 1 adds the base functionality for supporting VxHS protocol. >>>> >>>> Patch 2 adds two new configuration options in qemu.conf to enable TLS >>>> for VxHS devices. >>>> >>>> Patch 3 implements the main TLS functionality. >>> >>> This ordering is wrong and also your patches have a lot of stuff going >>> on at once. >>> > > Recall what I pointed out last week when you were @ Westford. Try to > find a "happy medium" between throwing everything into a few patches and > over creating patches for the sake of having really small compileable > and testable patches. It is a delicate balance... > >>> I suggest you add the TLS support (Which should be designed to be >>> generic with other protocols which may start using TLS in mind) first >>> and then start adding the rest of the stuff. >>> >> >> I'm not sure I understand this point. libvirt currently does not have >> support of VxHS devices. So I cannot add TLS support for VxHS before >> base VxHS functionality. If you mean that I should add generic TLS >> handling functionality for block device protocols, then it would >> probably just be some new variables in structures and bare-bone >> functions (1 or 2) that don't do much. None of the block devices at >> present use TLS, so even if I add generic TLS code, how would I test >> it in the same patch unit? >> > > I'm not so sure we could have generic block TLS env. IIUC, the > _tls_x509_cert_dir (or "s:dir" to the tls-creds-x509 object) is the > location for the server certificate that QEMU would present to say VxHS > server. Whereas, NBD which could use the migrate TLS environment (or > it's own I suppose, but we use it for migration). If Gluster, iSCSI, > RBD, etc. allowed the ability to use TLS instead of <auth ...> secrets, > then they too could conceivably have their own environment. > > This isn't a QEMU to QEMU communication, right? It's QEMU to some server > where the storage is located from whence you'll get your storage. > That is correct! In this case qemu is the client and the actual VxHS storage server is remote. The use of TLS/SSL here is to secure this communication. > John > > I'm sure if I have this wrong someone will correct me... > >>> I'll reply to some parts of the patches separately, but in this state >>> it's kind of a mess to go through, so please re-send the patch split >>> into reasonable chunks. >>> >>> Note that each patch needs to make sense and can be compiled and tested >>> individually. >>> >> >> Regards, >> Ashish >> -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
