5:09 p.m.
Let's hope it's an improvement :-). See individual patches for details.
Jim Fehlig (3):
libxl: Use the name 'Xen' in driver tables
libxl: Clarify that 'xenlight' should only be used internally
docs: Xen improvements
docs/aclpolkit.html.in | 4 ++--
docs/formatdomain.html.in | 6 +++---
src/libxl/libxl_conf.h | 8 +++++++-
src/libxl/libxl_domain.c | 4 ++--
src/libxl/libxl_driver.c | 20 ++++++++++----------
5 files changed, 24 insertions(+), 18 deletions(-)
--
2.26.0
5:09 p.m.
New subject: [PATCH 1/3] libxl: Use the name 'Xen' in driver tables
The libxl driver declares its name as 'Xen' through the public
virConnectGetType() API. In the virHypervisorDriver table the name is
set to 'xenlight'. To add more confusion, the name is set to 'LIBXL'
in the virStateDriver. For consistency, use the same name in the driver
tables as reported in the public virConnectGetType() API.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/libxl/libxl_conf.h | 1 +
src/libxl/libxl_driver.c | 6 +++---
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h
index 07b3373170..11a922b10a 100644
--- a/src/libxl/libxl_conf.h
+++ b/src/libxl/libxl_conf.h
@@ -38,6 +38,7 @@
#include "libxl_capabilities.h"
#include "libxl_logger.h"
+#define LIBXL_DRIVER_EXTERNAL_NAME "Xen"
#define LIBXL_DRIVER_NAME "xenlight"
#define LIBXL_VNC_PORT_MIN 5900
#define LIBXL_VNC_PORT_MAX 65535
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 980984b199..ebeb91af3c 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -911,7 +911,7 @@ libxlConnectGetType(virConnectPtr conn)
if (virConnectGetTypeEnsureACL(conn) < 0)
return NULL;
- return "Xen";
+ return LIBXL_DRIVER_EXTERNAL_NAME;
}
static int
@@ -6608,7 +6608,7 @@ libxlDomainGetMetadata(virDomainPtr dom,
}
static virHypervisorDriver libxlHypervisorDriver = {
- .name = LIBXL_DRIVER_NAME,
+ .name = LIBXL_DRIVER_EXTERNAL_NAME,
.connectURIProbe = libxlConnectURIProbe,
.connectOpen = libxlConnectOpen, /* 0.9.0 */
.connectClose = libxlConnectClose, /* 0.9.0 */
@@ -6732,7 +6732,7 @@ static virConnectDriver libxlConnectDriver = {
};
static virStateDriver libxlStateDriver = {
- .name = "LIBXL",
+ .name = LIBXL_DRIVER_EXTERNAL_NAME,
.stateInitialize = libxlStateInitialize,
.stateCleanup = libxlStateCleanup,
.stateReload = libxlStateReload,
--
2.26.0
5:09 p.m.
New subject: [PATCH 2/3] libxl: Clarify that 'xenlight' should only be used internally
The libxl driver has suffered an identity crisis since its introduction.
It took on the name 'libxl' since at the time libvirt already contained
a 'xen' driver for the old Xen toolstack implementation. 'libxl' is short
for libxenlight, which is often called xenlight. Unfortunately all forms
of the name are used in the libxl driver.
The only remaining use of the 'xenlight' form is when interacting with
the host device manager, which is difficult to change since it would
cause problems when upgrading the driver.
Rename the #define to make it clear the 'xenlight' form is internal and
add a comment describing why the name exists and that its use should be
discouraged.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/libxl/libxl_conf.h | 7 ++++++-
src/libxl/libxl_domain.c | 4 ++--
src/libxl/libxl_driver.c | 14 +++++++-------
3 files changed, 15 insertions(+), 10 deletions(-)
diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h
index 11a922b10a..b057a9e4ba 100644
--- a/src/libxl/libxl_conf.h
+++ b/src/libxl/libxl_conf.h
@@ -39,7 +39,12 @@
#include "libxl_logger.h"
#define LIBXL_DRIVER_EXTERNAL_NAME "Xen"
-#define LIBXL_DRIVER_NAME "xenlight"
+/*
+ * We are stuck with the 'xenlight' name since it is used by the hostdev
+ * manager. Changing it would break management of any host devices previously
+ * managed under the name 'xenlight'.
+ */
+#define LIBXL_DRIVER_INTERNAL_NAME "xenlight"
#define LIBXL_VNC_PORT_MIN 5900
#define LIBXL_VNC_PORT_MAX 65535
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index cc53a765e1..d9fcde4364 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -873,7 +873,7 @@ libxlDomainCleanup(libxlDriverPrivatePtr driver,
VIR_FREE(xml);
}
- virHostdevReAttachDomainDevices(hostdev_mgr, LIBXL_DRIVER_NAME,
+ virHostdevReAttachDomainDevices(hostdev_mgr, LIBXL_DRIVER_INTERNAL_NAME,
vm->def, hostdev_flags, NULL);
VIR_FREE(priv->lockState);
@@ -1370,7 +1370,7 @@ libxlDomainStart(libxlDriverPrivatePtr driver,
if (cfg->autoballoon && libxlDomainFreeMem(cfg->ctx, &d_config)
< 0)
goto cleanup_dom;
- if (virHostdevPrepareDomainDevices(hostdev_mgr, LIBXL_DRIVER_NAME,
+ if (virHostdevPrepareDomainDevices(hostdev_mgr, LIBXL_DRIVER_INTERNAL_NAME,
vm->def, hostdev_flags) < 0)
goto cleanup_dom;
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index ebeb91af3c..63ec0a2188 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -431,7 +431,7 @@ libxlReconnectDomain(virDomainObjPtr vm,
libxlLoggerOpenFile(cfg->logger, vm->def->id, vm->def->name, NULL);
/* Update hostdev state */
- if (virHostdevUpdateActiveDomainDevices(hostdev_mgr, LIBXL_DRIVER_NAME,
+ if (virHostdevUpdateActiveDomainDevices(hostdev_mgr, LIBXL_DRIVER_INTERNAL_NAME,
vm->def, hostdev_flags) < 0)
goto error;
@@ -3127,7 +3127,7 @@ libxlDomainAttachHostPCIDevice(libxlDriverPrivatePtr driver,
if (VIR_REALLOC_N(vm->def->hostdevs, vm->def->nhostdevs + 1) < 0)
goto cleanup;
- if (virHostdevPreparePCIDevices(hostdev_mgr, LIBXL_DRIVER_NAME,
+ if (virHostdevPreparePCIDevices(hostdev_mgr, LIBXL_DRIVER_INTERNAL_NAME,
vm->def->name, vm->def->uuid,
&hostdev, 1, 0) < 0)
goto cleanup;
@@ -3149,7 +3149,7 @@ libxlDomainAttachHostPCIDevice(libxlDriverPrivatePtr driver,
goto cleanup;
error:
- virHostdevReAttachPCIDevices(hostdev_mgr, LIBXL_DRIVER_NAME,
+ virHostdevReAttachPCIDevices(hostdev_mgr, LIBXL_DRIVER_INTERNAL_NAME,
vm->def->name, &hostdev, 1, NULL);
cleanup:
@@ -3264,7 +3264,7 @@ libxlDomainAttachHostUSBDevice(libxlDriverPrivatePtr driver,
if (VIR_REALLOC_N(vm->def->hostdevs, vm->def->nhostdevs + 1) < 0)
goto cleanup;
- if (virHostdevPrepareUSBDevices(hostdev_mgr, LIBXL_DRIVER_NAME,
+ if (virHostdevPrepareUSBDevices(hostdev_mgr, LIBXL_DRIVER_INTERNAL_NAME,
vm->def->name, &hostdev, 1, 0) < 0)
goto cleanup;
@@ -3284,7 +3284,7 @@ libxlDomainAttachHostUSBDevice(libxlDriverPrivatePtr driver,
goto cleanup;
reattach:
- virHostdevReAttachUSBDevices(hostdev_mgr, LIBXL_DRIVER_NAME,
+ virHostdevReAttachUSBDevices(hostdev_mgr, LIBXL_DRIVER_INTERNAL_NAME,
vm->def->name, &hostdev, 1);
cleanup:
@@ -3689,7 +3689,7 @@ libxlDomainDetachHostPCIDevice(libxlDriverPrivatePtr driver,
virDomainHostdevRemove(vm->def, idx);
- virHostdevReAttachPCIDevices(hostdev_mgr, LIBXL_DRIVER_NAME,
+ virHostdevReAttachPCIDevices(hostdev_mgr, LIBXL_DRIVER_INTERNAL_NAME,
vm->def->name, &hostdev, 1, NULL);
ret = 0;
@@ -3811,7 +3811,7 @@ libxlDomainDetachHostUSBDevice(libxlDriverPrivatePtr driver,
virDomainHostdevRemove(vm->def, idx);
- virHostdevReAttachUSBDevices(hostdev_mgr, LIBXL_DRIVER_NAME,
+ virHostdevReAttachUSBDevices(hostdev_mgr, LIBXL_DRIVER_INTERNAL_NAME,
vm->def->name, &hostdev, 1);
ret = 0;
--
2.26.0
5:09 p.m.
New subject: [PATCH 3/3] docs: Xen improvements
In formatdomain, using 'libxl' and 'xen' is redundant since they now
both refer to the same driver. 'xen' predates 'libxl' and unambiguously
identifies the Xen hypervisor, so drop the use of 'libxl'.
In aclpolkit, the connection URI was erroneously identified as 'libxl'
and the name 'xenlight'. Change the URI to 'xen' and driver name to
'Xen'.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
docs/aclpolkit.html.in | 4 ++--
docs/formatdomain.html.in | 6 +++---
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/docs/aclpolkit.html.in b/docs/aclpolkit.html.in
index 04cb39006a..3e5d30a5dd 100644
--- a/docs/aclpolkit.html.in
+++ b/docs/aclpolkit.html.in
@@ -342,8 +342,8 @@
<td>interface</td>
</tr>
<tr>
- <td>libxl</td>
- <td>xenlight</td>
+ <td>xen</td>
+ <td>Xen</td>
</tr>
<tr>
<td>lxc</td>
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 91d6f6c0d3..23eb029234 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -2533,8 +2533,8 @@
The <code>name</code> attribute selects which timer is
being modified, and can be one of
"platform" (currently unsupported),
- "hpet" (libxl, xen, qemu, lxc), "kvmclock" (qemu),
- "pit" (qemu), "rtc" (qemu, lxc), "tsc" (libxl,
qemu -
+ "hpet" (xen, qemu, lxc), "kvmclock" (qemu),
+ "pit" (qemu), "rtc" (qemu, lxc), "tsc" (xen,
qemu -
<span class="since">since 3.2.0</span>),
"hypervclock"
(qemu - <span class="since">since 1.2.2</span>) or
"armvtimer" (qemu - <span class="since">since
6.1.0</span>).
@@ -7518,7 +7518,7 @@ qemu-kvm -net nic,model=? /dev/null
<p>
You can provide the amount of video memory in kibibytes (blocks of
1024 bytes) using <code>vram</code>. This is supported only for
guest
- type of "libxl", "vz", "qemu", "vbox",
"vmx" and "xen". If no
+ type of "vz", "qemu", "vbox", "vmx" and
"xen". If no
value is provided the default is used. If the size is not a power of
two it will be rounded to closest one.
</p>
--
2.26.0
3:36 a.m.
On Mon, May 04, 2020 at 04:09:16PM -0600, Jim Fehlig wrote:
Let's hope it's an improvement :-). See individual patches
for details.
Jim Fehlig (3):
libxl: Use the name 'Xen' in driver tables
libxl: Clarify that 'xenlight' should only be used internally
docs: Xen improvements
docs/aclpolkit.html.in | 4 ++--
docs/formatdomain.html.in | 6 +++---
src/libxl/libxl_conf.h | 8 +++++++-
src/libxl/libxl_domain.c | 4 ++--
src/libxl/libxl_driver.c | 20 ++++++++++----------
5 files changed, 24 insertions(+), 18 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange(a)redhat.com>
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
1692
days inactive
1693
days old
4 comments
2 participants
participants (2)
-
Daniel P. Berrangé -
Jim Fehlig