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

[ashish mittal]

On Fri, Jun 30, 2017 at 1:07 PM, John Ferlan <jferlan at redhat.com> 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 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.
>>>
>
> 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
>>