Re: [libvirt] [PATCH v4 0/3] Add support for Veritas HyperScale (VxHS) block device protocol
Hi,
Thanks for the review!
On Fri, Jun 30, 2017 at 12:41 AM, Peter Krempa <pkrempa(a)redhat.com> wrote:
On Thu, Jun 29, 2017 at 19:02:38 -0700, Ashish Mittal wrote:
> From: Ashish Mittal <ashish.mittal(a)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