[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 1/2] libxl: Report connect type as Xen



Daniel Veillard wrote:
> On Tue, Jun 11, 2013 at 10:27:37AM -0600, Jim Fehlig wrote:
>   
>> Michal Privoznik wrote:
>>     
>>> On 11.06.2013 16:12, Jim Fehlig wrote:
>>>   
>>>       
>>>> Michal Privoznik wrote:
>>>>     
>>>>         
>>>>> On 10.06.2013 22:21, Jim Fehlig wrote:
>>>>>   
>>>>>       
>>>>>           
>>>>>> Currently, the libxl driver reports a connection type of "xenlight".
>>>>>> To be compatible with the legacy Xen driver, it should return "Xen".
>>>>>>
>>>>>> Note: I noticed this while testing the libxl driver on OpenStack.
>>>>>> After switching my Xen compute nodes to use the libxl stack, I
>>>>>> could no longer launch instances on those nodes since
>>>>>> hypervisor_type was reported as "xenlight" instead of "xen".
>>>>>> ---
>>>>>>  src/libxl/libxl_driver.c |    2 +-
>>>>>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>>>>>
>>>>>> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
>>>>>> index 3990354..935919b 100644
>>>>>> --- a/src/libxl/libxl_driver.c
>>>>>> +++ b/src/libxl/libxl_driver.c
>>>>>> @@ -1405,7 +1405,7 @@ libxlConnectClose(virConnectPtr conn ATTRIBUTE_UNUSED)
>>>>>>  static const char *
>>>>>>  libxlConnectGetType(virConnectPtr conn ATTRIBUTE_UNUSED)
>>>>>>  {
>>>>>> -    return "xenlight";
>>>>>> +    return "Xen";
>>>>>>  }
>>>>>>  
>>>>>>  static int
>>>>>>
>>>>>>     
>>>>>>         
>>>>>>             
>>>>> I am not so convinced about this one. I think there exist some
>>>>> applications which want to distinguish between "Xen" and "xenlight".
>>>>>       
>>>>>           
>>>> If that was the case, we would have went with a libxl:// URI.  In fact,
>>>> the original libxl driver patch introduced a libxl:// URI, but Daniel V.
>>>> pointed out that approach conflicted with libvirt's goal of minimizing
>>>> the impact on application stacks as the lower layers churn
>>>>
>>>> https://www.redhat.com/archives/libvir-list/2011-March/msg00449.html
>>>>     
>>>>         
>>> Right, you certainly got a point there. But I'd like to see DV's input
>>> in here then.
>>>       
>
>   Yes, the main service that libvirt provide for a given hypervisor
> stack is hide the inevitable changes that the evolution of the
> underlying technology implies. When the libxl driver was introduced
> it was then normal to try to minimize the impact on existing Xen users,
> and I still think that's the right approach, especially if there have
> been report of breakages in the field.
>   If we really want to expose a way to differenciate a xen from libxl
> (or future) then virConnectGetSysinfo which is a structured kind of
> reply is a better way to carry this. virConnectGetType is better left
> undifferencied in those cases.
>
>   So to me ACK to the patch.

BTW, I pushed these two patches.  Thanks for the reviews Daniel and Michal!

Regards,
Jim


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]