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

Ashish Mittal posted 3 patches 116 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/1498788161-46841-1-git-send-email-Ashish.Mittal@veritas.com
docs/formatdomain.html.in                          |  31 ++++-
docs/schemas/domaincommon.rng                      |  18 +++
src/conf/domain_conf.c                             |  19 +++
src/libxl/libxl_conf.c                             |   1 +
src/qemu/libvirtd_qemu.aug                         |   4 +
src/qemu/qemu.conf                                 |  23 +++
src/qemu/qemu_command.c                            | 155 +++++++++++++++++++++
src/qemu/qemu_conf.c                               |   7 +
src/qemu/qemu_conf.h                               |   3 +
src/qemu/qemu_driver.c                             |   3 +
src/qemu/qemu_parse_command.c                      |  25 ++++
src/qemu/test_libvirtd_qemu.aug.in                 |   2 +
src/util/virstoragefile.c                          |  77 +++++++++-
src/util/virstoragefile.h                          |  10 ++
src/xenconfig/xen_xl.c                             |   1 +
.../qemuargv2xml-disk-drive-network-vxhs-fail.args |  24 ++++
tests/qemuargv2xmltest.c                           |  17 ++-
...ml2argv-disk-drive-network-tlsx509-err-vxhs.xml |  34 +++++
...-disk-drive-network-tlsx509-multidisk-vxhs.args |  41 ++++++
...k-drive-network-tlsx509-multidisk-vxhs.args.new |  41 ++++++
...v-disk-drive-network-tlsx509-multidisk-vxhs.xml |  56 ++++++++
...muxml2argv-disk-drive-network-tlsx509-vxhs.args |  28 ++++
...emuxml2argv-disk-drive-network-tlsx509-vxhs.xml |  34 +++++
.../qemuxml2argv-disk-drive-network-vxhs.args      |  25 ++++
.../qemuxml2argv-disk-drive-network-vxhs.xml       |  34 +++++
tests/qemuxml2argvtest.c                           |  10 ++
tests/virstoragetest.c                             |  30 ++++
27 files changed, 748 insertions(+), 5 deletions(-)
create mode 100644 tests/qemuargv2xmldata/qemuargv2xml-disk-drive-network-vxhs-fail.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-err-vxhs.xml
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-multidisk-vxhs.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-multidisk-vxhs.args.new
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-multidisk-vxhs.xml
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-vxhs.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-vxhs.xml
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-vxhs.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-vxhs.xml

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

Posted by Ashish Mittal 116 weeks ago
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.

Ashish Mittal (3):
  Add support for Veritas HyperScale (VxHS) block device protocol
  conf: Introduce TLS options for VxHS block device clients
  Add TLS support for Veritas HyperScale (VxHS) block device protocol

 docs/formatdomain.html.in                          |  31 ++++-
 docs/schemas/domaincommon.rng                      |  18 +++
 src/conf/domain_conf.c                             |  19 +++
 src/libxl/libxl_conf.c                             |   1 +
 src/qemu/libvirtd_qemu.aug                         |   4 +
 src/qemu/qemu.conf                                 |  23 +++
 src/qemu/qemu_command.c                            | 155 +++++++++++++++++++++
 src/qemu/qemu_conf.c                               |   7 +
 src/qemu/qemu_conf.h                               |   3 +
 src/qemu/qemu_driver.c                             |   3 +
 src/qemu/qemu_parse_command.c                      |  25 ++++
 src/qemu/test_libvirtd_qemu.aug.in                 |   2 +
 src/util/virstoragefile.c                          |  77 +++++++++-
 src/util/virstoragefile.h                          |  10 ++
 src/xenconfig/xen_xl.c                             |   1 +
 .../qemuargv2xml-disk-drive-network-vxhs-fail.args |  24 ++++
 tests/qemuargv2xmltest.c                           |  17 ++-
 ...ml2argv-disk-drive-network-tlsx509-err-vxhs.xml |  34 +++++
 ...-disk-drive-network-tlsx509-multidisk-vxhs.args |  41 ++++++
 ...k-drive-network-tlsx509-multidisk-vxhs.args.new |  41 ++++++
 ...v-disk-drive-network-tlsx509-multidisk-vxhs.xml |  56 ++++++++
 ...muxml2argv-disk-drive-network-tlsx509-vxhs.args |  28 ++++
 ...emuxml2argv-disk-drive-network-tlsx509-vxhs.xml |  34 +++++
 .../qemuxml2argv-disk-drive-network-vxhs.args      |  25 ++++
 .../qemuxml2argv-disk-drive-network-vxhs.xml       |  34 +++++
 tests/qemuxml2argvtest.c                           |  10 ++
 tests/virstoragetest.c                             |  30 ++++
 27 files changed, 748 insertions(+), 5 deletions(-)
 create mode 100644 tests/qemuargv2xmldata/qemuargv2xml-disk-drive-network-vxhs-fail.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-err-vxhs.xml
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-multidisk-vxhs.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-multidisk-vxhs.args.new
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-multidisk-vxhs.xml
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-vxhs.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-vxhs.xml
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-vxhs.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-vxhs.xml

-- 
2.5.5

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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

Posted by Peter Krempa 116 weeks ago
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'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.

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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

Posted by ashish mittal 116 weeks ago
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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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

Posted by John Ferlan 116 weeks ago

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

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.

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
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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

Posted by ashish mittal 116 weeks ago
On Fri, Jun 30, 2017 at 1:07 PM, John Ferlan <jferlan@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@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.
>>>
>
> 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
>>

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list