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. > > 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'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
