[libvirt] [PATCH v4 0/3] Add support for Veritas HyperScale (VxHS) block device protocol

[ashish mittal]

Hi,

Thanks for the review!

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