Hi, Thanks for the review! On Fri, Jun 30, 2017 at 12:41 AM, Peter Krempa <pkrempa@redhat.com> wrote:
On Thu, Jun 29, 2017 at 19:02:38 -0700, Ashish Mittal wrote:
From: Ashish Mittal <ashish.mittal@veritas.com>
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