7:30 a.m.
[...]
>>>
>>> Given that adding TLS support for VxHS (and disk devices in general)
>>> will not be trivial, I want to check if this can be taken up at a
>>> later time?
>>
>> No, it'd be preferable to have a complete solution. That solution
>> should be a separately compilable/checkable multi-patch series where
>> each patch makes forward progress towards your goal.
>>
>> Consider first getting the non-TLS environment working. You have some of
>> it working with the existing patches - it's updating the generation of
>> the command line that would seem to need the adjustment. I think I've
>> already pointed out a series or two that should help you in that endeavor.
>>
>
> I think this part is taken care of. Please see my earlier emails and
> do let me know if something is missing on the base functionality.
>
Well considering the "original" [PATCH v3] was posted 1/19/2017:
https://www.redhat.com/archives/libvir-list/2017-January/msg00892.html
it's been long since paged out of short term memory by today! I also
know that there's been a lot of adjustments along the way with the QEMU
patches and I haven't followed each of those that closely.
I hope when I see a new series I can start with a "fresh perspective".
>> Then for TLS, again it's a multi-step process
>>
>> 1. Modify the qemu_conf.c and qemu.conf files to search for new
>> disk_vxhs_tls_x509* definitions since I assume that either you'll use
>> the existing qemu definitions or you (more likely) want to have some
>> directory that would have a VxHS specific environment. I suggest a
>> "disk_vxhs_" - just to make it more specific. If some day RBD, iSCSI,
>> etc. support TLS, then they'd conceivably have their own "disk_rbd"
or
>> "disk_icsci" options since each would have their own server to
>> authenticate using TLS creds.
>>
>
> Thanks for laying out the steps so nicely. I plan to refer to chardev
> TLS commits to keep my checkins to manageable chunks. For example, the
> first TLS related commit on top of the base VxHS patch, which
> corresponds to chardev commit ID 3f60a9c3, would look like:
>
https://github.com/MittalAshish/libvirt/compare/basechanges_withoutTLS......
>
> If this looks fine, I will send out the next patch series as the base
> VxHS patch + TLS part 1 patch.
>
> Two questions please -
> (1) Can the base VxHS patch be merged to master before the TLS is
> fully implemented?
> (2) Should the TLS support be added as multiple patches, or one patch
> with multiple incremental commits?
Every commit becomes a patch, so I guess the question is incorrect.
What I'm trying to understand is - do the patches get merged as they
get reviewed/approved, or do they get merged after the last bit is
complete? If it's the latter, then I guess it should be OK to make
changes to an older patch in the series in a newer version?
Start here:
http://libvirt.org/docs.html
See contributor guidelines:
http://libvirt.org/hacking.html
Look at the "history" of patches a bit and you'll get a feel for what
the general flow should be. Dumping everything into one patch is a no
go, creating a 100 patch series won't get your patches looked at
quickly. Find a happy medium.
Since having "basic" support and all the TLS bells and whistles cannot
be done in one commit (far too big), use the opportunity to logically
build up support. Patches should logically add the capabilities you'll
need, but must also pass the "make check syntax-check" for each patch.
In the long run what is "fully functionally complete" is a decision left
for the contributor. In this case, you're trying to get to having TLS in
order to have enough functionality in libvirt to match what's been added
to QEMU. If it's possible in QEMU to start up and use without TLS, then
you can use that inflection point in libvirt to get something pushed;
however, since a fully functional and complete solution includes TLS,
that capability needs to be added in via libvirt as well.
The question you have to ask yourself though is - if the partial
solution makes it in 3.4 and the TLS cannot make it until 3.5, would any
extra changes would be required to handle version differences? Migration
makes this tricky. That's (more or less) what happened with chardev.
Some TLS bits made it in one release but full support crossed releases
and things got complex.
I really think though that if you target being "fully functionally
complete", then that will be the best solution. Upstream libvirt
releases are on a month by month cadence, so don't expect to post
something in the last days prior to a release and have it reviewed and
merged unless it perhaps a cleanup of something that was already mostly
agreed upon earlier in the month. Still it cannot be pushed during
freeze which is typically the last 4-7 days before release.
John
BTW: Not sure how this series of responses has broken from the original
patch, but it's becoming "too deep"... I think there's 10 responses
deep
in a few places.
[...]