3:21 p.m.
This small series fixes some capatiblity issues in the libxl driver wrt
the legacy xen driver.
Jim Fehlig (2):
libxl: Report connect type as Xen
libxl: set bootloader for PV domains if not specified
src/libxl/libxl_conf.c | 13 +++++++++++--
src/libxl/libxl_conf.h | 1 +
src/libxl/libxl_driver.c | 2 +-
3 files changed, 13 insertions(+), 3 deletions(-)
--
1.7.7
3:21 p.m.
New subject: [libvirt] [PATCH 1/2] libxl: Report connect type as Xen
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
--
1.7.7
4:43 a.m.
New subject: [libvirt] [PATCH 1/2] libxl: Report connect type as Xen
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
a application (like Openstack) doesn't want, nothing is easier than:
type = virConnectGetType(conn);
if (!strcmp(type, "Xen") || !strcmp(type, "xenlight")) {
DoTheMagic();
} else {
ReportError("XEN not supported);
}
IOW I think this is an OpenStack bug, not a libvirt one.
Michal
9:12 a.m.
New subject: [libvirt] [PATCH 1/2] libxl: Report connect type as Xen
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
If
a application (like Openstack) doesn't want, nothing is easier than:
type = virConnectGetType(conn);
if (!strcmp(type, "Xen") || !strcmp(type, "xenlight")) {
DoTheMagic();
} else {
ReportError("XEN not supported);
}
Yes, but every app would have to do that, all because some lower layer
tool was reworked.
Regards,
Jim
11:19 a.m.
New subject: [libvirt] [PATCH 1/2] libxl: Report connect type as Xen
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.
> If
> a application (like Openstack) doesn't want, nothing is easier than:
>
> type = virConnectGetType(conn);
> if (!strcmp(type, "Xen") || !strcmp(type, "xenlight")) {
> DoTheMagic();
> } else {
> ReportError("XEN not supported);
> }
>
Yes, but every app would have to do that, all because some lower layer
tool was reworked.
It wasn't reworked. It's here from initial introduction of libxl :)
Regards,
Jim
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
11:27 a.m.
New subject: [libvirt] [PATCH 1/2] libxl: Report connect type as Xen
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.
>> If
>> a application (like Openstack) doesn't want, nothing is easier than:
>>
>> type = virConnectGetType(conn);
>> if (!strcmp(type, "Xen") || !strcmp(type, "xenlight")) {
>> DoTheMagic();
>> } else {
>> ReportError("XEN not supported);
>> }
>>
>>
> Yes, but every app would have to do that, all because some lower layer
> tool was reworked.
>
It wasn't reworked. It's here from initial introduction of libxl :)
I meant the reworking of xen tools, e.g. the replacement of xm/xend with
xl/libxl. libvirt apps should be insulated from these types of
infrastructure changes at the lower levels. I'd expect my libvirt apps
to continue working after a Xen upgrade, similar to how they continue to
work after installing a new qemu.
Regards,
Jim
3 a.m.
New subject: [libvirt] [PATCH 1/2] libxl: Report connect type as Xen
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. Jim if you have the evironment to test
both it might be interesting to compare the 2 outputs returned from
virConnectGetSysinfo and see if infrastructure code can still make a
difference from both environment with it.
Daniel
--
Daniel Veillard | Open Source and Standards, Red Hat
veillard(a)redhat.com | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | virtualization library http://libvirt.org/
9:26 a.m.
New subject: [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. Jim if you have the evironment to test
both it might be interesting to compare the 2 outputs returned from
virConnectGetSysinfo and see if infrastructure code can still make a
difference from both environment with it.
Unfortunately, connectGetSysinfo is not implemented in the legacy xen or
libxl drivers :(. I created a trivial patch for the libxl driver based
on the qemu impl, but as expected only get the smbios info provided by
virSysinfoRead(), which doesn't include anything to differentiate
between libxl vs old xen toolstack.
Daniel, I too would like a way for users to know if their libvirt is
talking to libxl or the old toolstack, and would like to explore your
virConnectGetSysinfo() suggestion. Currently, virConnectGetSysinfo()
returns a bunch of smbios info within <sysinfo type='smbios'> ...
</sysinfo>. Adding some info about which hypervisor toolstack is in use
under the smbios sysinfo doesn't seem right, since it really isn't
smbios info. Were you thinking of augmenting the current info with
something like
<sysinfo type='hypervisor'>
<version>x.y.z</version>
<api>libxl 4.2.x</api>
...
</sysinfo>
Regards,
Jim
10:08 a.m.
New subject: [libvirt] [PATCH 1/2] libxl: Report connect type as Xen
On Wed, Jun 12, 2013 at 08:26:48AM -0600, Jim Fehlig wrote:
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. Jim if you have the evironment to test
> both it might be interesting to compare the 2 outputs returned from
> virConnectGetSysinfo and see if infrastructure code can still make a
> difference from both environment with it.
>
Unfortunately, connectGetSysinfo is not implemented in the legacy xen or
libxl drivers :(. I created a trivial patch for the libxl driver based
on the qemu impl, but as expected only get the smbios info provided by
virSysinfoRead(), which doesn't include anything to differentiate
between libxl vs old xen toolstack.
Daniel, I too would like a way for users to know if their libvirt is
talking to libxl or the old toolstack, and would like to explore your
virConnectGetSysinfo() suggestion. Currently, virConnectGetSysinfo()
returns a bunch of smbios info within <sysinfo type='smbios'> ...
</sysinfo>. Adding some info about which hypervisor toolstack is in use
under the smbios sysinfo doesn't seem right, since it really isn't
smbios info. Were you thinking of augmenting the current info with
something like
<sysinfo type='hypervisor'>
<version>x.y.z</version>
<api>libxl 4.2.x</api>
...
</sysinfo>
My bad, indeed virConnectGetSysinfo is not the right call to carry
this. On the other hand virConnectGetCapabilities()
"Provides capabilities of the hypervisor / driver"
is the right API for this, since it returns XML we can extend the amount
of details without risk of incompatibilities. But that would also
requires some tweaking as it is all based on virCapabilitiesFormatXML()
which currently just dumps virCaps into a full capabilities block.
This could be extended by passing arguments about the driver type and
driver versions which the various drivers could pass (or ignore):
<capabilities driver="libxl" version="4.10">
<host>
....
</host>
<guest>
...
</guest>
</capabilities>
and have drivers where the informations are not available drop the
attributes.
That sounds one way to expose the informations without leading to the
problem exposed by OpenStack ...
Daniel
--
Daniel Veillard | Open Source and Standards, Red Hat
veillard(a)redhat.com | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | virtualization library http://libvirt.org/
9:44 a.m.
New subject: [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
3:21 p.m.
New subject: [libvirt] [PATCH 2/2] libxl: set bootloader for PV domains if not specified
The legacy xen toolstack will set pygrub as the bootloader if not
specified. For compatibility, do the same in the libxl driver
iff not using direct kernel boot.
---
src/libxl/libxl_conf.c | 13 +++++++++++--
src/libxl/libxl_conf.h | 1 +
2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 90d563b..8b76536 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -414,8 +414,17 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, libxl_domain_config
*d_config)
b_info->shadow_memkb = 4 * (256 *
libxl_bitmap_count_set(&b_info->avail_vcpus) +
2 * (b_info->max_memkb / 1024));
} else {
- if (VIR_STRDUP(b_info->u.pv.bootloader, def->os.bootloader) < 0)
- goto error;
+ /*
+ * For compatibility with the legacy xen toolstack, default to pygrub
+ * if bootloader is not specified AND direct kernel boot is not specified.
+ */
+ if (def->os.bootloader) {
+ if (VIR_STRDUP(b_info->u.pv.bootloader, def->os.bootloader) < 0)
+ goto error;
+ } else if (def->os.kernel == NULL) {
+ if (VIR_STRDUP(b_info->u.pv.bootloader, LIBXL_BOOTLOADER_PATH) < 0)
+ goto error;
+ }
if (def->os.bootloaderArgs) {
if (!(b_info->u.pv.bootloader_args =
virStringSplit(def->os.bootloaderArgs, " \t\n", 0)))
diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h
index 44ecd41..754fc40 100644
--- a/src/libxl/libxl_conf.h
+++ b/src/libxl/libxl_conf.h
@@ -45,6 +45,7 @@
# define LIBXL_LOG_DIR LOCALSTATEDIR "/log/libvirt/libxl"
# define LIBXL_LIB_DIR LOCALSTATEDIR "/lib/libvirt/libxl"
# define LIBXL_SAVE_DIR LIBXL_LIB_DIR "/save"
+# define LIBXL_BOOTLOADER_PATH BINDIR "/pygrub"
typedef struct _libxlDriverPrivate libxlDriverPrivate;
--
1.7.7
4:37 a.m.
New subject: [libvirt] [PATCH 2/2] libxl: set bootloader for PV domains if not specified
On 10.06.2013 22:21, Jim Fehlig wrote:
The legacy xen toolstack will set pygrub as the bootloader if not
specified. For compatibility, do the same in the libxl driver
iff not using direct kernel boot.
---
src/libxl/libxl_conf.c | 13 +++++++++++--
src/libxl/libxl_conf.h | 1 +
2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 90d563b..8b76536 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -414,8 +414,17 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, libxl_domain_config
*d_config)
b_info->shadow_memkb = 4 * (256 *
libxl_bitmap_count_set(&b_info->avail_vcpus) +
2 * (b_info->max_memkb / 1024));
} else {
- if (VIR_STRDUP(b_info->u.pv.bootloader, def->os.bootloader) < 0)
- goto error;
+ /*
+ * For compatibility with the legacy xen toolstack, default to pygrub
+ * if bootloader is not specified AND direct kernel boot is not specified.
+ */
+ if (def->os.bootloader) {
+ if (VIR_STRDUP(b_info->u.pv.bootloader, def->os.bootloader) < 0)
+ goto error;
+ } else if (def->os.kernel == NULL) {
+ if (VIR_STRDUP(b_info->u.pv.bootloader, LIBXL_BOOTLOADER_PATH) < 0)
+ goto error;
+ }
if (def->os.bootloaderArgs) {
if (!(b_info->u.pv.bootloader_args =
virStringSplit(def->os.bootloaderArgs, " \t\n", 0)))
diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h
index 44ecd41..754fc40 100644
--- a/src/libxl/libxl_conf.h
+++ b/src/libxl/libxl_conf.h
@@ -45,6 +45,7 @@
# define LIBXL_LOG_DIR LOCALSTATEDIR "/log/libvirt/libxl"
# define LIBXL_LIB_DIR LOCALSTATEDIR "/lib/libvirt/libxl"
# define LIBXL_SAVE_DIR LIBXL_LIB_DIR "/save"
+# define LIBXL_BOOTLOADER_PATH BINDIR "/pygrub"
typedef struct _libxlDriverPrivate libxlDriverPrivate;
ACK
Michal
4181
days inactive
4183
days old
11 comments
3 participants
participants (3)
-
Daniel Veillard -
Jim Fehlig -
Michal Privoznik