[libvirt] [PATCH 1/4] esx: parse new URI param 'allow_unsafe'

Martin Kletzander mkletzan at redhat.com
Sun May 11 10:08:58 UTC 2014


On Sat, May 10, 2014 at 05:06:27PM +0200, Matthias Bolte wrote:
>2014-05-06 10:10 GMT+02:00 Martin Kletzander <mkletzan at redhat.com>:
>> On Mon, May 05, 2014 at 01:48:03PM -0600, Eric Blake wrote:
>>>
>>> On 05/05/2014 03:47 AM, Martin Kletzander wrote:
>>>>
>>>> Signed-off-by: Martin Kletzander <mkletzan at redhat.com>
>>>> ---
>>>>  docs/drvesx.html.in | 14 ++++++++++++++
>>>>  src/esx/esx_util.c  | 13 ++++++++++++-
>>>>  src/esx/esx_util.h  |  2 ++
>>>>  3 files changed, 28 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/docs/drvesx.html.in b/docs/drvesx.html.in
>>>> index 0816baf..d941763 100644
>>>> --- a/docs/drvesx.html.in
>>>> +++ b/docs/drvesx.html.in
>>>> @@ -185,6 +185,20 @@
>>>> vpx://example-vcenter.com/folder1/dc1/folder2/example-esx.com
>>>>                  override the default port 1080.
>>>>              </td>
>>>>          </tr>
>>>> +        <tr>
>>>> +            <td>
>>>> +                <code>allow_unsafe</code>
>>>> +            </td>
>>>> +            <td>
>>>> +                <code>0</code> or <code>1</code>
>>>> +            </td>
>>>> +            <td>
>>>> +                If set to 1, this disables check for supported
>>>> +                virtualHW.version, so connections to newer versions
>>>> +                than supported is possible.  The default value is 0.
>>>
>>>
>>> "connections" is plural, so s/is possible/are possible/
>>>
>>> On the surface this (and the rest of the series) makes sense - it allows
>>> older libvirt to be used with a newer ESX version which may or may not
>>> work, until such time as newer libvirt has been tested to explicitly
>>> work with that ESX.  But I'd like an opinion from someone that actually
>>> uses esx:// URIs before checking this in.
>>>
>>
>> Since the vmx parsing was moved to vmx/vmx.c (so basically since it
>> was introduced), we were blindly (or at least in some cases) adding
>> "support" for newer virtualHW_versions (version 8 in commit 5759a5cc,
>> 9 in e0eb672e and finally 10 now in 5cc3cce5).  I tried getting
>> someone to test the version I added, but since there's so few of us
>> working with ESX (I had to ask Matthias if he can have a look at the
>> code), we can only assume that it will work.  I even remember someone
>> filing a bug that the connection works and everything, but there's a
>> warning.  My thinking was that we have two options here:
>>
>> 1) continue messing the code with blindly adding versions of
>>    virtualHW or
>>
>> 2) conditionally allow anything the user wants, with the condition
>>    showing that we don't "support" it.
>>
>> My reasoning behind choosing version 2 was that the development around
>> VMX-related backends is not that active as with other ones.  So if
>> anything doesn't work because of HW version it is somehow visible that
>> the user is not using "supported" versions of vmx descriptors (or how
>> to call the .vmx file).  I designed the patch so there is no
>> functional nor API change with current usage, it merely allows to
>> check whether our code works with newer versions without touching the
>> code (currently, when someone wants to try that, he needs to add the
>> version to supported ones, obviously).  And it still leaves the list
>> of supported versions in place and whoever wants some version to be
>> supported can add it to the list.
>>
>> I know it is impossible to force people fix everything for versions
>> they added, but it could be at least a bit visible that we currently
>> don't have the capacity for that.
>>
>> Martin
>
>Let's take a step back and look at the complete picture.
>
>Initially it seemed to me that the virtualHW version would have a
>larger effect on the content and format of the VMX file than to
>actually turned out to have. That's why I added this strict check to
>the parser in the first place. But after two major vSphere version (4
>and 5) and multiple virtualHW version came along since the initial
>implementation of the VMX parser I'm now pretty confident that the
>actual virtualHW version is not that important to the parser as I
>initially thought it would be.
>
>I see no gain in adding any extra logic here to allow "unsafe"
>versions. Actually, I dislike this word in this context, it gives the
>user a wrong impression about the situation. There is nothing unsafe
>and nothing to test for compatibility here, it's only me misjudging
>the importance and meaning of this config entry.
>
>I suggest a different approach to deal with future virtualHW versions:
>basically ignore the value, as it's not that important. It should have
>been that way from the beginning. Here's a patch for that:
>
>https://www.redhat.com/archives/libvir-list/2014-May/msg00313.html
>

That patch has way better approach, so Self-NACK to this series.

Martin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140511/056fb5af/attachment-0001.sig>


More information about the libvir-list mailing list