8:54 p.m.
This are some additional features to libxl driver. Some of them require change
in domain config structures/syntax. Details described with each patch.
There are two bugfix patches for deadlock during daemon startup.
Changes since v1:
- dropped 'script' disk parameter patches
- updated 'script' interface parameter handling
- dropped PCI passthrough patches - duplicates Chunyan's work
- dropped RFC: libxl: special 'stubdom-dm' emulator to use qemu in stub domain
- rebased on 1.0.6+
- VIR_STRDUP
- other changes described in individual patches
- new patches:
libxl: initialize device structures
libxl: support paused domain restore in virDomainRestoreFlags
libxl: support domain config modification in virDomainRestoreFlags
libxl: support network device attach/detach
libxl: pass ipaddr to libxl toolstack
libxl: implement lifecycle actions from domain
libxl: add tablet/mouse input device support
8:54 p.m.
New subject: [libvirt] [PATCH v2 01/14] libxl: allow only 'ethernet' and 'bridge' interfaces, allow script there
Actually only those interface types are handled correctly so reject
others instead of ignoring settings (i.e. treating as bridge/ethernet
anyway).
Also allow <script/> in 'ethernet' (which should be the only
script-allowing type). Keep <script/> allowed in bridge to be compatible
with legacy 'xen' driver.
Changes in v2:
- reject interfaces other than 'ethernet' or 'bridge'
- change title to better match patch content (was "libxl: allow script
for any network interface, not only bridge")
- update description
Signed-off-by: Marek Marczykowski-Górecki <marmarek(a)invisiblethingslab.com>
---
src/libxl/libxl_conf.c | 24 +++++++++++++-----------
1 file changed, 13 insertions(+), 11 deletions(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 90d563b..b9cb61e 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -563,18 +563,20 @@ libxlMakeNic(virDomainNetDefPtr l_nic, libxl_device_nic *x_nic)
if (VIR_STRDUP(x_nic->ifname, l_nic->ifname) < 0)
return -1;
- if (l_nic->type == VIR_DOMAIN_NET_TYPE_BRIDGE) {
- if (VIR_STRDUP(x_nic->bridge, l_nic->data.bridge.brname) < 0)
- return -1;
- if (VIR_STRDUP(x_nic->script, l_nic->script) < 0)
- return -1;
- } else {
- if (l_nic->script) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("scripts are not supported on interfaces of type
%s"),
- virDomainNetTypeToString(l_nic->type));
+ switch (l_nic->type) {
+ case VIR_DOMAIN_NET_TYPE_BRIDGE:
+ if (VIR_STRDUP(x_nic->bridge, l_nic->data.bridge.brname) < 0)
+ return -1;
+ /* fallthrough */
+ case VIR_DOMAIN_NET_TYPE_ETHERNET:
+ if (VIR_STRDUP(x_nic->script, l_nic->script) < 0)
+ return -1;
+ break;
+ default:
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("libxenlight does not support network device type %s"),
+ virDomainNetTypeToString(l_nic->type));
return -1;
- }
}
return 0;
--
1.8.1.4
4:28 p.m.
New subject: [libvirt] [PATCH v2 01/14] libxl: allow only 'ethernet' and 'bridge' interfaces, allow script there
Marek Marczykowski-Górecki wrote:
Actually only those interface types are handled correctly so reject
others instead of ignoring settings (i.e. treating as bridge/ethernet
anyway).
Also allow <script/> in 'ethernet' (which should be the only
script-allowing type). Keep <script/> allowed in bridge to be compatible
with legacy 'xen' driver.
Changes in v2:
- reject interfaces other than 'ethernet' or 'bridge'
- change title to better match patch content (was "libxl: allow script
for any network interface, not only bridge")
- update description
No need to add notes about the evolution of a patch in the commit
message. Standard practice here is to add them after the '---' using
the '--annotate' option to git send-email.
ACK to the patch though :). I removed the notes about changes in v2 and
pushed.
Thanks!
Jim
Signed-off-by: Marek Marczykowski-Górecki
<marmarek(a)invisiblethingslab.com>
---
src/libxl/libxl_conf.c | 24 +++++++++++++-----------
1 file changed, 13 insertions(+), 11 deletions(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 90d563b..b9cb61e 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -563,18 +563,20 @@ libxlMakeNic(virDomainNetDefPtr l_nic, libxl_device_nic *x_nic)
if (VIR_STRDUP(x_nic->ifname, l_nic->ifname) < 0)
return -1;
- if (l_nic->type == VIR_DOMAIN_NET_TYPE_BRIDGE) {
- if (VIR_STRDUP(x_nic->bridge, l_nic->data.bridge.brname) < 0)
- return -1;
- if (VIR_STRDUP(x_nic->script, l_nic->script) < 0)
- return -1;
- } else {
- if (l_nic->script) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("scripts are not supported on interfaces of type
%s"),
- virDomainNetTypeToString(l_nic->type));
+ switch (l_nic->type) {
+ case VIR_DOMAIN_NET_TYPE_BRIDGE:
+ if (VIR_STRDUP(x_nic->bridge, l_nic->data.bridge.brname) < 0)
+ return -1;
+ /* fallthrough */
+ case VIR_DOMAIN_NET_TYPE_ETHERNET:
+ if (VIR_STRDUP(x_nic->script, l_nic->script) < 0)
+ return -1;
+ break;
+ default:
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("libxenlight does not support network device type %s"),
+ virDomainNetTypeToString(l_nic->type));
return -1;
- }
}
return 0;
8:54 p.m.
New subject: [libvirt] [PATCH v2 02/14] libxl: populate xenstore memory entries at startup, handle dom0_mem
libxl uses some xenstore entries for hints in memory management
(especially when starting new domain). This includes dom0 memory limit
and Xen free memory margin, based on current system state. Entries are
created at first function usage, so force such call at daemon startup,
which most likely will be before any domain startup.
Also prevent automatic memory management if dom0_mem= option passed to
xen hypervisor - it is known to be incompatible with autoballoon.
Changes in v2:
- disable autoballoon when dom0_mem option detected
- rebase on 1.0.6+
Signed-off-by: Marek Marczykowski-Górecki <marmarek(a)invisiblethingslab.com>
---
src/libxl/libxl_conf.h | 4 ++++
src/libxl/libxl_driver.c | 36 +++++++++++++++++++++++++++++++++++-
2 files changed, 39 insertions(+), 1 deletion(-)
diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h
index 44ecd41..e8fb9c4 100644
--- a/src/libxl/libxl_conf.h
+++ b/src/libxl/libxl_conf.h
@@ -62,6 +62,10 @@ struct _libxlDriverPrivate {
virPortAllocatorPtr reservedVNCPorts;
+ /* should libxl/libvirt take care of getting memory for new domain from
+ * dom0? */
+ bool autoballoon;
+
size_t nactive;
virStateInhibitCallback inhibitCallback;
void *inhibitOpaque;
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 3990354..0a0690d 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -29,6 +29,7 @@
#include <math.h>
#include <libxl.h>
#include <fcntl.h>
+#include <regex.h>
#include "internal.h"
#include "virlog.h"
@@ -963,7 +964,7 @@ libxlVmStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm,
if (libxlBuildDomainConfig(driver, vm->def, &d_config) < 0)
goto error;
- if (libxlFreeMem(priv, &d_config) < 0) {
+ if (driver->autoballoon && libxlFreeMem(priv, &d_config) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("libxenlight failed to get free memory for domain
'%s'"),
d_config.c_info.name);
@@ -1145,6 +1146,28 @@ libxlStateCleanup(void)
return 0;
}
+static int auto_autoballoon(libxlDriverPrivatePtr driver)
+{
+ const libxl_version_info *info;
+ regex_t regex;
+ int ret;
+
+ info = libxl_get_version_info(driver->ctx);
+ if (!info)
+ return 1; /* default to on */
+
+ ret = regcomp(®ex,
+ "(^| )dom0_mem=((|min:|max:)[0-9]+[bBkKmMgG]?,?)+($| )",
+ REG_NOSUB | REG_EXTENDED);
+ if (ret)
+ return 1;
+
+ ret = regexec(®ex, info->commandline, 0, NULL, 0);
+ regfree(®ex);
+ return ret == REG_NOMATCH;
+}
+
+
static int
libxlStateInitialize(bool privileged,
virStateInhibitCallback callback ATTRIBUTE_UNUSED,
@@ -1154,6 +1177,7 @@ libxlStateInitialize(bool privileged,
char *log_file = NULL;
virCommandPtr cmd;
int status, ret = 0;
+ unsigned int free_mem;
char ebuf[1024];
/* Disable libxl driver if non-root */
@@ -1278,6 +1302,16 @@ libxlStateInitialize(bool privileged,
NULL)))
goto error;
+ /* This will fill xenstore info about free and dom0 memory if missing,
+ * should be called before starting first domain */
+ if (libxl_get_free_memory(libxl_driver->ctx, &free_mem)) {
+ VIR_ERROR(_("cannot get free memory info"));
+ goto error;
+ }
+
+ /* setup autoballoon */
+ libxl_driver->autoballoon = auto_autoballoon(libxl_driver);
+
/* Load running domains first. */
if (virDomainObjListLoadAllConfigs(libxl_driver->domains,
libxl_driver->stateDir,
--
1.8.1.4
12:13 p.m.
New subject: [libvirt] [PATCH v2 02/14] libxl: populate xenstore memory entries at startup, handle dom0_mem
Marek Marczykowski-Górecki wrote:
libxl uses some xenstore entries for hints in memory management
(especially when starting new domain). This includes dom0 memory limit
and Xen free memory margin, based on current system state. Entries are
created at first function usage, so force such call at daemon startup,
which most likely will be before any domain startup.
Also prevent automatic memory management if dom0_mem= option passed to
xen hypervisor - it is known to be incompatible with autoballoon.
Changes in v2:
- disable autoballoon when dom0_mem option detected
- rebase on 1.0.6+
As mentioned previously, patch history should not be in the commit message.
Signed-off-by: Marek Marczykowski-Górecki
<marmarek(a)invisiblethingslab.com>
---
src/libxl/libxl_conf.h | 4 ++++
src/libxl/libxl_driver.c | 36 +++++++++++++++++++++++++++++++++++-
2 files changed, 39 insertions(+), 1 deletion(-)
diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h
index 44ecd41..e8fb9c4 100644
--- a/src/libxl/libxl_conf.h
+++ b/src/libxl/libxl_conf.h
@@ -62,6 +62,10 @@ struct _libxlDriverPrivate {
virPortAllocatorPtr reservedVNCPorts;
+ /* should libxl/libvirt take care of getting memory for new domain from
+ * dom0? */
This comment sounds as though we are not quite sure about autoballoon,
like we need more info to handle autoballoon properly. Better to just
state its purpose IMO, e.g.
Controls automatic ballooning of domain0. If true, get memory for new
domains from domain0.
+ bool autoballoon;
+
size_t nactive;
virStateInhibitCallback inhibitCallback;
void *inhibitOpaque;
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 3990354..0a0690d 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -29,6 +29,7 @@
#include <math.h>
#include <libxl.h>
#include <fcntl.h>
+#include <regex.h>
#include "internal.h"
#include "virlog.h"
@@ -963,7 +964,7 @@ libxlVmStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm,
if (libxlBuildDomainConfig(driver, vm->def, &d_config) < 0)
goto error;
- if (libxlFreeMem(priv, &d_config) < 0) {
+ if (driver->autoballoon && libxlFreeMem(priv, &d_config) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("libxenlight failed to get free memory for domain
'%s'"),
d_config.c_info.name);
@@ -1145,6 +1146,28 @@ libxlStateCleanup(void)
return 0;
}
+static int auto_autoballoon(libxlDriverPrivatePtr driver)
Please use the pattern I mentioned in patch 1 when declaring/defining
functions. Also, this function should return a bool, and use a more
libvirt libxl-ish name, e.g. libxlGetAutoballoon :).
+{
+ const libxl_version_info *info;
+ regex_t regex;
+ int ret;
+
+ info = libxl_get_version_info(driver->ctx);
+ if (!info)
+ return 1; /* default to on */
+
+ ret = regcomp(®ex,
+ "(^| )dom0_mem=((|min:|max:)[0-9]+[bBkKmMgG]?,?)+($| )",
+ REG_NOSUB | REG_EXTENDED);
+ if (ret)
+ return 1;
+
+ ret = regexec(®ex, info->commandline, 0, NULL, 0);
+ regfree(®ex);
+ return ret == REG_NOMATCH;
+}
+
+
static int
libxlStateInitialize(bool privileged,
virStateInhibitCallback callback ATTRIBUTE_UNUSED,
@@ -1154,6 +1177,7 @@ libxlStateInitialize(bool privileged,
char *log_file = NULL;
virCommandPtr cmd;
int status, ret = 0;
+ unsigned int free_mem;
char ebuf[1024];
/* Disable libxl driver if non-root */
@@ -1278,6 +1302,16 @@ libxlStateInitialize(bool privileged,
NULL)))
goto error;
+ /* This will fill xenstore info about free and dom0 memory if missing,
+ * should be called before starting first domain */
+ if (libxl_get_free_memory(libxl_driver->ctx, &free_mem)) {
+ VIR_ERROR(_("cannot get free memory info"));
I think the error message could be improved since the driver won't load
if libxl_get_free_memory fails. Something like "Unable to configure
libxl's memory management parameters"?
Regards,
Jim
+ goto error;
+ }
+
+ /* setup autoballoon */
+ libxl_driver->autoballoon = auto_autoballoon(libxl_driver);
+
/* Load running domains first. */
if (virDomainObjListLoadAllConfigs(libxl_driver->domains,
libxl_driver->stateDir,
9:46 p.m.
New subject: [libvirt] [PATCH v3] libxl: populate xenstore memory entries at startup, handle dom0_mem
libxl uses some xenstore entries for hints in memory management
(especially when starting new domain). This includes dom0 memory limit
and Xen free memory margin, based on current system state. Entries are
created at first function usage, so force such call at daemon startup,
which most likely will be before any domain startup.
Also prevent automatic memory management if dom0_mem= option passed to
xen hypervisor - it is known to be incompatible with autoballoon.
Signed-off-by: Marek Marczykowski-Górecki <marmarek(a)invisiblethingslab.com>
---
Changes in v2:
- disable autoballoon when dom0_mem option detected
- rebase on 1.0.6+
Changes in v3:
- improve comments and error messages
- rename auto_autoballoon
src/libxl/libxl_conf.h | 4 ++++
src/libxl/libxl_driver.c | 37 ++++++++++++++++++++++++++++++++++++-
2 files changed, 40 insertions(+), 1 deletion(-)
diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h
index 44ecd41..42bcdbc 100644
--- a/src/libxl/libxl_conf.h
+++ b/src/libxl/libxl_conf.h
@@ -62,6 +62,10 @@ struct _libxlDriverPrivate {
virPortAllocatorPtr reservedVNCPorts;
+ /* Controls automatic ballooning of domain0. If true, get memory for new
+ * domains from domain0. */
+ bool autoballoon;
+
size_t nactive;
virStateInhibitCallback inhibitCallback;
void *inhibitOpaque;
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 3990354..89f4f12 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -29,6 +29,7 @@
#include <math.h>
#include <libxl.h>
#include <fcntl.h>
+#include <regex.h>
#include "internal.h"
#include "virlog.h"
@@ -963,7 +964,7 @@ libxlVmStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm,
if (libxlBuildDomainConfig(driver, vm->def, &d_config) < 0)
goto error;
- if (libxlFreeMem(priv, &d_config) < 0) {
+ if (driver->autoballoon && libxlFreeMem(priv, &d_config) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("libxenlight failed to get free memory for domain
'%s'"),
d_config.c_info.name);
@@ -1146,6 +1147,29 @@ libxlStateCleanup(void)
}
static int
+libxlGetAutoballoon(libxlDriverPrivatePtr driver)
+{
+ const libxl_version_info *info;
+ regex_t regex;
+ int ret;
+
+ info = libxl_get_version_info(driver->ctx);
+ if (!info)
+ return 1; /* default to on */
+
+ ret = regcomp(®ex,
+ "(^| )dom0_mem=((|min:|max:)[0-9]+[bBkKmMgG]?,?)+($| )",
+ REG_NOSUB | REG_EXTENDED);
+ if (ret)
+ return 1;
+
+ ret = regexec(®ex, info->commandline, 0, NULL, 0);
+ regfree(®ex);
+ return ret == REG_NOMATCH;
+}
+
+
+static int
libxlStateInitialize(bool privileged,
virStateInhibitCallback callback ATTRIBUTE_UNUSED,
void *opaque ATTRIBUTE_UNUSED)
@@ -1154,6 +1178,7 @@ libxlStateInitialize(bool privileged,
char *log_file = NULL;
virCommandPtr cmd;
int status, ret = 0;
+ unsigned int free_mem;
char ebuf[1024];
/* Disable libxl driver if non-root */
@@ -1278,6 +1303,16 @@ libxlStateInitialize(bool privileged,
NULL)))
goto error;
+ /* This will fill xenstore info about free and dom0 memory if missing,
+ * should be called before starting first domain */
+ if (libxl_get_free_memory(libxl_driver->ctx, &free_mem)) {
+ VIR_ERROR(_("Unable to configure libxl's memory management
parameters"));
+ goto error;
+ }
+
+ /* setup autoballoon */
+ libxl_driver->autoballoon = libxlGetAutoballoon(libxl_driver);
+
/* Load running domains first. */
if (virDomainObjListLoadAllConfigs(libxl_driver->domains,
libxl_driver->stateDir,
--
1.8.1.4
5:26 p.m.
New subject: [libvirt] [PATCH v3] libxl: populate xenstore memory entries at startup, handle dom0_mem
Marek Marczykowski-Górecki wrote:
libxl uses some xenstore entries for hints in memory management
(especially when starting new domain). This includes dom0 memory limit
and Xen free memory margin, based on current system state. Entries are
created at first function usage, so force such call at daemon startup,
which most likely will be before any domain startup.
Also prevent automatic memory management if dom0_mem= option passed to
xen hypervisor - it is known to be incompatible with autoballoon.
Signed-off-by: Marek Marczykowski-Górecki <marmarek(a)invisiblethingslab.com>
---
Changes in v2:
- disable autoballoon when dom0_mem option detected
- rebase on 1.0.6+
Changes in v3:
- improve comments and error messages
- rename auto_autoballoon
src/libxl/libxl_conf.h | 4 ++++
src/libxl/libxl_driver.c | 37 ++++++++++++++++++++++++++++++++++++-
2 files changed, 40 insertions(+), 1 deletion(-)
diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h
index 44ecd41..42bcdbc 100644
--- a/src/libxl/libxl_conf.h
+++ b/src/libxl/libxl_conf.h
@@ -62,6 +62,10 @@ struct _libxlDriverPrivate {
virPortAllocatorPtr reservedVNCPorts;
+ /* Controls automatic ballooning of domain0. If true, get memory for new
+ * domains from domain0. */
+ bool autoballoon;
+
size_t nactive;
virStateInhibitCallback inhibitCallback;
void *inhibitOpaque;
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 3990354..89f4f12 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -29,6 +29,7 @@
#include <math.h>
#include <libxl.h>
#include <fcntl.h>
+#include <regex.h>
#include "internal.h"
#include "virlog.h"
@@ -963,7 +964,7 @@ libxlVmStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm,
if (libxlBuildDomainConfig(driver, vm->def, &d_config) < 0)
goto error;
- if (libxlFreeMem(priv, &d_config) < 0) {
+ if (driver->autoballoon && libxlFreeMem(priv, &d_config) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("libxenlight failed to get free memory for domain
'%s'"),
d_config.c_info.name);
@@ -1146,6 +1147,29 @@ libxlStateCleanup(void)
}
static int
+libxlGetAutoballoon(libxlDriverPrivatePtr driver)
I changed this function to return a bool as suggested in v2 and pushed.
Regards,
Jim
+{
+ const libxl_version_info *info;
+ regex_t regex;
+ int ret;
+
+ info = libxl_get_version_info(driver->ctx);
+ if (!info)
+ return 1; /* default to on */
+
+ ret = regcomp(®ex,
+ "(^| )dom0_mem=((|min:|max:)[0-9]+[bBkKmMgG]?,?)+($| )",
+ REG_NOSUB | REG_EXTENDED);
+ if (ret)
+ return 1;
+
+ ret = regexec(®ex, info->commandline, 0, NULL, 0);
+ regfree(®ex);
+ return ret == REG_NOMATCH;
+}
+
+
+static int
libxlStateInitialize(bool privileged,
virStateInhibitCallback callback ATTRIBUTE_UNUSED,
void *opaque ATTRIBUTE_UNUSED)
@@ -1154,6 +1178,7 @@ libxlStateInitialize(bool privileged,
char *log_file = NULL;
virCommandPtr cmd;
int status, ret = 0;
+ unsigned int free_mem;
char ebuf[1024];
/* Disable libxl driver if non-root */
@@ -1278,6 +1303,16 @@ libxlStateInitialize(bool privileged,
NULL)))
goto error;
+ /* This will fill xenstore info about free and dom0 memory if missing,
+ * should be called before starting first domain */
+ if (libxl_get_free_memory(libxl_driver->ctx, &free_mem)) {
+ VIR_ERROR(_("Unable to configure libxl's memory management
parameters"));
+ goto error;
+ }
+
+ /* setup autoballoon */
+ libxl_driver->autoballoon = libxlGetAutoballoon(libxl_driver);
+
/* Load running domains first. */
if (virDomainObjListLoadAllConfigs(libxl_driver->domains,
libxl_driver->stateDir,
8:54 p.m.
New subject: [libvirt] [PATCH v2 03/14] conf: support backend domain name in disk and network devices
At least Xen supports backend drivers in another domain (aka "driver
domain"). This patch introduces XML config option for such setting as
'domain' element with 'name' attribute. Verification its content is left
for the driver.
In the future some option will be needed for USB devices (hostdev
objects), but for now libxl doesn't have support for PVUSB.
Changes in v2:
- describe in docs/formatdomain.html.in
- enforce empty domain tag (only 'name' attribute allowed)
Signed-off-by: Marek Marczykowski-Górecki <marmarek(a)invisiblethingslab.com>
---
docs/formatdomain.html.in | 29 +++++++++++++++++++++++++++++
docs/schemas/domaincommon.rng | 16 ++++++++++++++++
src/conf/domain_conf.c | 28 ++++++++++++++++++++++++++++
src/conf/domain_conf.h | 2 ++
4 files changed, 75 insertions(+)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 755d084..1ca8ae0 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -1751,6 +1751,13 @@
</li>
</ul>
</dd>
+ <dt><code>domain</code></dt>
+ <dd>The optional <code>domain</code> element allows specifying
backend
+ domain (aka driver domain) for the device. If real device/file resides
+ in some other domain on the same host, you can use <code>name</code>
+ attribute to specify its name.
+ <span class="since">Since 1.0.7 (Xen only)</span>
+ </dd>
<dt><code>boot</code></dt>
<dd>Specifies that the disk is bootable. The <code>order</code>
attribute determines the order in which devices will be tried during
@@ -3554,6 +3561,28 @@ qemu-kvm -net nic,model=? /dev/null
element is unspecified is to have the link state <code>up</code>.
<span class="since">Since 0.9.5</span>
</p>
+ <h5><a name="elementDomain">Setting up network backend in
driver domain</a></h5>
+<pre>
+ ...
+ <devices>
+ ...
+ <interface type='bridge'>
+ <source bridge='br0'/>
+ <b><domain name='netvm'/></b>
+ </interface>
+ ...
+ </devices>
+ ...</pre>
+
+ <p>
+ The optional <code>domain</code> element allows specifying backend
+ domain (aka driver domain) for the device. Use <code>name</code>
attribute
+ to specify its name. You can use it to create direct network link between
+ domains (so data will not go through host system). Use with type
'ethernet'
+ to create plain network link, or with 'bridge' to connect to some bridge
+ inside driver domain.
+ <span class="since">Since 1.0.7 (Xen only)</span>
+ </p>
<h4><a name="elementsInput">Input devices</a></h4>
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 1eb2f68..6fe4d90 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -908,6 +908,14 @@
</optional>
<ref name="target"/>
<optional>
+ <element name="domain">
+ <attribute name="name">
+ <ref name="domainName"/>
+ </attribute>
+ <empty/>
+ </element>
+ </optional>
+ <optional>
<ref name="deviceBoot"/>
</optional>
<optional>
@@ -1981,6 +1989,14 @@
</element>
</optional>
<optional>
+ <element name="domain">
+ <attribute name="name">
+ <ref name="domainName"/>
+ </attribute>
+ <empty/>
+ </element>
+ </optional>
+ <optional>
<element name="model">
<attribute name="type">
<data type="string">
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 2373397..5350c56 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1183,6 +1183,7 @@ void virDomainDiskDefFree(virDomainDiskDefPtr def)
VIR_FREE(def->wwn);
VIR_FREE(def->vendor);
VIR_FREE(def->product);
+ VIR_FREE(def->domain_name);
if (def->auth.secretType == VIR_DOMAIN_DISK_SECRET_TYPE_USAGE)
VIR_FREE(def->auth.secret.usage);
virStorageEncryptionFree(def->encryption);
@@ -1308,6 +1309,7 @@ void virDomainNetDefFree(virDomainNetDefPtr def)
VIR_FREE(def->virtPortProfile);
VIR_FREE(def->script);
+ VIR_FREE(def->domain_name);
VIR_FREE(def->ifname);
virDomainDeviceInfoClear(&def->info);
@@ -4693,6 +4695,7 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
char *vendor = NULL;
char *product = NULL;
char *discard = NULL;
+ char *domain_name = NULL;
int expected_secret_usage = -1;
int auth_secret_usage = -1;
@@ -4853,6 +4856,9 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
if (target &&
STRPREFIX(target, "ioemu:"))
memmove(target, target+6, strlen(target)-5);
+ } else if (!domain_name &&
+ xmlStrEqual(cur->name, BAD_CAST "domain")) {
+ domain_name = virXMLPropString(cur, "name");
} else if (xmlStrEqual(cur->name, BAD_CAST "geometry")) {
if (virXPathUInt("string(./geometry/@cyls)",
ctxt, &def->geometry.cylinders) < 0) {
@@ -5148,6 +5154,11 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
ctxt->node = saved_node;
}
+ if (domain_name != NULL) {
+ def->domain_name = domain_name;
+ domain_name = NULL;
+ }
+
if (target == NULL) {
if (def->srcpool) {
char *tmp;
@@ -5504,6 +5515,7 @@ cleanup:
VIR_FREE(wwn);
VIR_FREE(vendor);
VIR_FREE(product);
+ VIR_FREE(domain_name);
ctxt->node = save_ctxt;
return def;
@@ -6109,6 +6121,7 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
char *mode = NULL;
char *linkstate = NULL;
char *addrtype = NULL;
+ char *domain_name = NULL;
virNWFilterHashTablePtr filterparams = NULL;
virDomainActualNetDefPtr actual = NULL;
xmlNodePtr oldnode = ctxt->node;
@@ -6207,6 +6220,9 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
} else if (!script &&
xmlStrEqual(cur->name, BAD_CAST "script")) {
script = virXMLPropString(cur, "path");
+ } else if (!domain_name &&
+ xmlStrEqual(cur->name, BAD_CAST "domain")) {
+ domain_name = virXMLPropString(cur, "name");
} else if (xmlStrEqual(cur->name, BAD_CAST "model")) {
model = virXMLPropString(cur, "type");
} else if (xmlStrEqual(cur->name, BAD_CAST "driver")) {
@@ -6437,6 +6453,10 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
def->script = script;
script = NULL;
}
+ if (domain_name != NULL) {
+ def->domain_name = domain_name;
+ domain_name = NULL;
+ }
if (ifname != NULL) {
def->ifname = ifname;
ifname = NULL;
@@ -6574,6 +6594,7 @@ cleanup:
VIR_FREE(mode);
VIR_FREE(linkstate);
VIR_FREE(addrtype);
+ VIR_FREE(domain_name);
virNWFilterHashTableFree(filterparams);
return def;
@@ -14018,6 +14039,11 @@ virDomainDiskDefFormat(virBufferPtr buf,
if (virDomainDiskSourceDefFormat(buf, def) < 0)
return -1;
+
+ if (def->domain_name) {
+ virBufferEscapeString(buf, " <domain name='%s'/>\n",
def->domain_name);
+ }
+
virDomainDiskGeometryDefFormat(buf, def);
virDomainDiskBlockIoDefFormat(buf, def);
@@ -14601,6 +14627,8 @@ virDomainNetDefFormat(virBufferPtr buf,
return -1;
virBufferEscapeString(buf, "<script path='%s'/>\n",
def->script);
+ if (def->domain_name)
+ virBufferEscapeString(buf, "<domain name='%s'/>\n",
def->domain_name);
if (def->ifname &&
!((flags & VIR_DOMAIN_XML_INACTIVE) &&
(STRPREFIX(def->ifname, VIR_NET_GENERATED_PREFIX)))) {
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 5b159ac..24d3809 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -718,6 +718,7 @@ struct _virDomainDiskDef {
int rawio; /* no = 0, yes = 1 */
int sgio; /* enum virDomainDeviceSGIO */
int discard; /* enum virDomainDiskDiscard */
+ char *domain_name; /* backend domain name */
size_t nseclabels;
virSecurityDeviceLabelDefPtr *seclabels;
@@ -981,6 +982,7 @@ struct _virDomainNetDef {
unsigned long sndbuf;
} tune;
char *script;
+ char *domain_name; /* backend domain name */
char *ifname;
virDomainDeviceInfo info;
char *filter;
--
1.8.1.4
1:14 p.m.
New subject: [libvirt] [PATCH v2 03/14] conf: support backend domain name in disk and network devices
Marek Marczykowski-Górecki wrote:
At least Xen supports backend drivers in another domain (aka
"driver
domain"). This patch introduces XML config option for such setting as
'domain' element with 'name' attribute. Verification its content is left
for the driver.
In the future some option will be needed for USB devices (hostdev
objects), but for now libxl doesn't have support for PVUSB.
Changes in v2:
- describe in docs/formatdomain.html.in
- enforce empty domain tag (only 'name' attribute allowed)
Remove patch change history.
Signed-off-by: Marek Marczykowski-Górecki
<marmarek(a)invisiblethingslab.com>
---
docs/formatdomain.html.in | 29 +++++++++++++++++++++++++++++
docs/schemas/domaincommon.rng | 16 ++++++++++++++++
src/conf/domain_conf.c | 28 ++++++++++++++++++++++++++++
src/conf/domain_conf.h | 2 ++
4 files changed, 75 insertions(+)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 755d084..1ca8ae0 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
Some grammar nits below, since this is the website documentation...
@@ -1751,6 +1751,13 @@
</li>
</ul>
</dd>
+ <dt><code>domain</code></dt>
+ <dd>The optional <code>domain</code> element allows specifying
backend
s/specifying backend/specifying a backend/
+ domain (aka driver domain) for the device. If real
device/file resides
s/If real/If the real/
+ in some other domain on the same host, you can use
<code>name</code>
s/you can use/use the/
+ attribute to specify its name.
+ <span class="since">Since 1.0.7 (Xen only)</span>
+ </dd>
<dt><code>boot</code></dt>
<dd>Specifies that the disk is bootable. The <code>order</code>
attribute determines the order in which devices will be tried during
@@ -3554,6 +3561,28 @@ qemu-kvm -net nic,model=? /dev/null
element is unspecified is to have the link state <code>up</code>.
<span class="since">Since 0.9.5</span>
</p>
+ <h5><a name="elementDomain">Setting up network backend in
driver domain</a></h5>
Should read, "Setting up a network backend in a driver domain"
+<pre>
+ ...
+ <devices>
+ ...
+ <interface type='bridge'>
+ <source bridge='br0'/>
+ <b><domain name='netvm'/></b>
+ </interface>
+ ...
+ </devices>
+ ...</pre>
+
+ <p>
+ The optional <code>domain</code> element allows specifying backend
s/specifying backend/specifying a backend/
+ domain (aka driver domain) for the device. Use
<code>name</code> attribute
s/Use/Use the/
+ to specify its name. You can use it to create direct network
link between
s/create direct/create a direct/
+ domains (so data will not go through host system). Use with
type 'ethernet'
+ to create plain network link, or with 'bridge' to connect to some bridge
+ inside driver domain.
s/inside driver/inside the driver/
+ <span class="since">Since 1.0.7 (Xen
only)</span>
+ </p>
<h4><a name="elementsInput">Input
devices</a></h4>
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 1eb2f68..6fe4d90 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -908,6 +908,14 @@
</optional>
<ref name="target"/>
<optional>
+ <element name="domain">
+ <attribute name="name">
+ <ref name="domainName"/>
+ </attribute>
+ <empty/>
+ </element>
+ </optional>
+ <optional>
<ref name="deviceBoot"/>
</optional>
<optional>
@@ -1981,6 +1989,14 @@
</element>
</optional>
<optional>
+ <element name="domain">
+ <attribute name="name">
+ <ref name="domainName"/>
+ </attribute>
+ <empty/>
+ </element>
+ </optional>
+ <optional>
<element name="model">
<attribute name="type">
<data type="string">
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 2373397..5350c56 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1183,6 +1183,7 @@ void virDomainDiskDefFree(virDomainDiskDefPtr def)
VIR_FREE(def->wwn);
VIR_FREE(def->vendor);
VIR_FREE(def->product);
+ VIR_FREE(def->domain_name);
if (def->auth.secretType == VIR_DOMAIN_DISK_SECRET_TYPE_USAGE)
VIR_FREE(def->auth.secret.usage);
virStorageEncryptionFree(def->encryption);
@@ -1308,6 +1309,7 @@ void virDomainNetDefFree(virDomainNetDefPtr def)
VIR_FREE(def->virtPortProfile);
VIR_FREE(def->script);
+ VIR_FREE(def->domain_name);
VIR_FREE(def->ifname);
virDomainDeviceInfoClear(&def->info);
@@ -4693,6 +4695,7 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
char *vendor = NULL;
char *product = NULL;
char *discard = NULL;
+ char *domain_name = NULL;
int expected_secret_usage = -1;
int auth_secret_usage = -1;
@@ -4853,6 +4856,9 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
if (target &&
STRPREFIX(target, "ioemu:"))
memmove(target, target+6, strlen(target)-5);
+ } else if (!domain_name &&
+ xmlStrEqual(cur->name, BAD_CAST "domain")) {
+ domain_name = virXMLPropString(cur, "name");
} else if (xmlStrEqual(cur->name, BAD_CAST "geometry")) {
if (virXPathUInt("string(./geometry/@cyls)",
ctxt, &def->geometry.cylinders) < 0) {
@@ -5148,6 +5154,11 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
ctxt->node = saved_node;
}
+ if (domain_name != NULL) {
+ def->domain_name = domain_name;
+ domain_name = NULL;
+ }
+
if (target == NULL) {
if (def->srcpool) {
char *tmp;
@@ -5504,6 +5515,7 @@ cleanup:
VIR_FREE(wwn);
VIR_FREE(vendor);
VIR_FREE(product);
+ VIR_FREE(domain_name);
ctxt->node = save_ctxt;
return def;
@@ -6109,6 +6121,7 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
char *mode = NULL;
char *linkstate = NULL;
char *addrtype = NULL;
+ char *domain_name = NULL;
virNWFilterHashTablePtr filterparams = NULL;
virDomainActualNetDefPtr actual = NULL;
xmlNodePtr oldnode = ctxt->node;
@@ -6207,6 +6220,9 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
} else if (!script &&
xmlStrEqual(cur->name, BAD_CAST "script")) {
script = virXMLPropString(cur, "path");
+ } else if (!domain_name &&
+ xmlStrEqual(cur->name, BAD_CAST "domain")) {
+ domain_name = virXMLPropString(cur, "name");
} else if (xmlStrEqual(cur->name, BAD_CAST "model")) {
model = virXMLPropString(cur, "type");
} else if (xmlStrEqual(cur->name, BAD_CAST "driver")) {
@@ -6437,6 +6453,10 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
def->script = script;
script = NULL;
}
+ if (domain_name != NULL) {
+ def->domain_name = domain_name;
+ domain_name = NULL;
+ }
if (ifname != NULL) {
def->ifname = ifname;
ifname = NULL;
@@ -6574,6 +6594,7 @@ cleanup:
VIR_FREE(mode);
VIR_FREE(linkstate);
VIR_FREE(addrtype);
+ VIR_FREE(domain_name);
virNWFilterHashTableFree(filterparams);
return def;
@@ -14018,6 +14039,11 @@ virDomainDiskDefFormat(virBufferPtr buf,
if (virDomainDiskSourceDefFormat(buf, def) < 0)
return -1;
+
+ if (def->domain_name) {
+ virBufferEscapeString(buf, " <domain
name='%s'/>\n", def->domain_name);
+ }
+
virDomainDiskGeometryDefFormat(buf, def);
virDomainDiskBlockIoDefFormat(buf, def);
@@ -14601,6 +14627,8 @@ virDomainNetDefFormat(virBufferPtr buf,
return -1;
virBufferEscapeString(buf, "<script path='%s'/>\n",
def->script);
+ if (def->domain_name)
+ virBufferEscapeString(buf, "<domain name='%s'/>\n",
def->domain_name);
if (def->ifname &&
!((flags & VIR_DOMAIN_XML_INACTIVE) &&
(STRPREFIX(def->ifname, VIR_NET_GENERATED_PREFIX)))) {
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 5b159ac..24d3809 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -718,6 +718,7 @@ struct _virDomainDiskDef {
int rawio; /* no = 0, yes = 1 */
int sgio; /* enum virDomainDeviceSGIO */
int discard; /* enum virDomainDiskDiscard */
+ char *domain_name; /* backend domain name */
size_t nseclabels;
virSecurityDeviceLabelDefPtr *seclabels;
@@ -981,6 +982,7 @@ struct _virDomainNetDef {
unsigned long sndbuf;
} tune;
char *script;
+ char *domain_name; /* backend domain name */
char *ifname;
virDomainDeviceInfo info;
char *filter;
Looks good to me with exception of the few nits, but would be nice to
have another set of eyes review the schema changes.
Regards,
Jim
9:48 p.m.
New subject: [libvirt] [PATCH v3] conf: support backend domain name in disk and network devices
At least Xen supports backend drivers in another domain (aka "driver
domain"). This patch introduces XML config option for such setting as
'domain' element with 'name' attribute. Verification its content is left
for the driver.
In the future some option will be needed for USB devices (hostdev
objects), but for now libxl doesn't have support for PVUSB.
Signed-off-by: Marek Marczykowski-Górecki <marmarek(a)invisiblethingslab.com>
---
Changes in v2:
- describe in docs/formatdomain.html.in
- enforce empty domain tag (only 'name' attribute allowed)
Changes in v3:
- fix grammar of the documentation
docs/formatdomain.html.in | 29 +++++++++++++++++++++++++++++
docs/schemas/domaincommon.rng | 16 ++++++++++++++++
src/conf/domain_conf.c | 28 ++++++++++++++++++++++++++++
src/conf/domain_conf.h | 2 ++
4 files changed, 75 insertions(+)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 755d084..9c67b5b 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -1751,6 +1751,13 @@
</li>
</ul>
</dd>
+ <dt><code>domain</code></dt>
+ <dd>The optional <code>domain</code> element allows to specify a
+ backend domain (aka driver domain) for the device. If actual device/file
+ resides in some other domain on the same host, use the
<code>name</code>
+ attribute to specify its name.
+ <span class="since">Since 1.0.7 (Xen only)</span>
+ </dd>
<dt><code>boot</code></dt>
<dd>Specifies that the disk is bootable. The <code>order</code>
attribute determines the order in which devices will be tried during
@@ -3554,6 +3561,28 @@ qemu-kvm -net nic,model=? /dev/null
element is unspecified is to have the link state <code>up</code>.
<span class="since">Since 0.9.5</span>
</p>
+ <h5><a name="elementDomain">Setting up a network backend in a
driver domain</a></h5>
+<pre>
+ ...
+ <devices>
+ ...
+ <interface type='bridge'>
+ <source bridge='br0'/>
+ <b><domain name='netvm'/></b>
+ </interface>
+ ...
+ </devices>
+ ...</pre>
+
+ <p>
+ The optional <code>domain</code> element allows to specify a backend
+ domain (aka driver domain) for the device. Use the <code>name</code>
+ attribute to specify its name. You can use it to create a direct network
+ link between domains (so data will not go through the host system). Use
+ with type 'ethernet' to create a plain network link, or with
'bridge' to
+ connect to some bridge inside the driver domain.
+ <span class="since">Since 1.0.7 (Xen only)</span>
+ </p>
<h4><a name="elementsInput">Input devices</a></h4>
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 1eb2f68..6fe4d90 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -908,6 +908,14 @@
</optional>
<ref name="target"/>
<optional>
+ <element name="domain">
+ <attribute name="name">
+ <ref name="domainName"/>
+ </attribute>
+ <empty/>
+ </element>
+ </optional>
+ <optional>
<ref name="deviceBoot"/>
</optional>
<optional>
@@ -1981,6 +1989,14 @@
</element>
</optional>
<optional>
+ <element name="domain">
+ <attribute name="name">
+ <ref name="domainName"/>
+ </attribute>
+ <empty/>
+ </element>
+ </optional>
+ <optional>
<element name="model">
<attribute name="type">
<data type="string">
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 2373397..5350c56 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1183,6 +1183,7 @@ void virDomainDiskDefFree(virDomainDiskDefPtr def)
VIR_FREE(def->wwn);
VIR_FREE(def->vendor);
VIR_FREE(def->product);
+ VIR_FREE(def->domain_name);
if (def->auth.secretType == VIR_DOMAIN_DISK_SECRET_TYPE_USAGE)
VIR_FREE(def->auth.secret.usage);
virStorageEncryptionFree(def->encryption);
@@ -1308,6 +1309,7 @@ void virDomainNetDefFree(virDomainNetDefPtr def)
VIR_FREE(def->virtPortProfile);
VIR_FREE(def->script);
+ VIR_FREE(def->domain_name);
VIR_FREE(def->ifname);
virDomainDeviceInfoClear(&def->info);
@@ -4693,6 +4695,7 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
char *vendor = NULL;
char *product = NULL;
char *discard = NULL;
+ char *domain_name = NULL;
int expected_secret_usage = -1;
int auth_secret_usage = -1;
@@ -4853,6 +4856,9 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
if (target &&
STRPREFIX(target, "ioemu:"))
memmove(target, target+6, strlen(target)-5);
+ } else if (!domain_name &&
+ xmlStrEqual(cur->name, BAD_CAST "domain")) {
+ domain_name = virXMLPropString(cur, "name");
} else if (xmlStrEqual(cur->name, BAD_CAST "geometry")) {
if (virXPathUInt("string(./geometry/@cyls)",
ctxt, &def->geometry.cylinders) < 0) {
@@ -5148,6 +5154,11 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
ctxt->node = saved_node;
}
+ if (domain_name != NULL) {
+ def->domain_name = domain_name;
+ domain_name = NULL;
+ }
+
if (target == NULL) {
if (def->srcpool) {
char *tmp;
@@ -5504,6 +5515,7 @@ cleanup:
VIR_FREE(wwn);
VIR_FREE(vendor);
VIR_FREE(product);
+ VIR_FREE(domain_name);
ctxt->node = save_ctxt;
return def;
@@ -6109,6 +6121,7 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
char *mode = NULL;
char *linkstate = NULL;
char *addrtype = NULL;
+ char *domain_name = NULL;
virNWFilterHashTablePtr filterparams = NULL;
virDomainActualNetDefPtr actual = NULL;
xmlNodePtr oldnode = ctxt->node;
@@ -6207,6 +6220,9 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
} else if (!script &&
xmlStrEqual(cur->name, BAD_CAST "script")) {
script = virXMLPropString(cur, "path");
+ } else if (!domain_name &&
+ xmlStrEqual(cur->name, BAD_CAST "domain")) {
+ domain_name = virXMLPropString(cur, "name");
} else if (xmlStrEqual(cur->name, BAD_CAST "model")) {
model = virXMLPropString(cur, "type");
} else if (xmlStrEqual(cur->name, BAD_CAST "driver")) {
@@ -6437,6 +6453,10 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
def->script = script;
script = NULL;
}
+ if (domain_name != NULL) {
+ def->domain_name = domain_name;
+ domain_name = NULL;
+ }
if (ifname != NULL) {
def->ifname = ifname;
ifname = NULL;
@@ -6574,6 +6594,7 @@ cleanup:
VIR_FREE(mode);
VIR_FREE(linkstate);
VIR_FREE(addrtype);
+ VIR_FREE(domain_name);
virNWFilterHashTableFree(filterparams);
return def;
@@ -14018,6 +14039,11 @@ virDomainDiskDefFormat(virBufferPtr buf,
if (virDomainDiskSourceDefFormat(buf, def) < 0)
return -1;
+
+ if (def->domain_name) {
+ virBufferEscapeString(buf, " <domain name='%s'/>\n",
def->domain_name);
+ }
+
virDomainDiskGeometryDefFormat(buf, def);
virDomainDiskBlockIoDefFormat(buf, def);
@@ -14601,6 +14627,8 @@ virDomainNetDefFormat(virBufferPtr buf,
return -1;
virBufferEscapeString(buf, "<script path='%s'/>\n",
def->script);
+ if (def->domain_name)
+ virBufferEscapeString(buf, "<domain name='%s'/>\n",
def->domain_name);
if (def->ifname &&
!((flags & VIR_DOMAIN_XML_INACTIVE) &&
(STRPREFIX(def->ifname, VIR_NET_GENERATED_PREFIX)))) {
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 5b159ac..24d3809 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -718,6 +718,7 @@ struct _virDomainDiskDef {
int rawio; /* no = 0, yes = 1 */
int sgio; /* enum virDomainDeviceSGIO */
int discard; /* enum virDomainDiskDiscard */
+ char *domain_name; /* backend domain name */
size_t nseclabels;
virSecurityDeviceLabelDefPtr *seclabels;
@@ -981,6 +982,7 @@ struct _virDomainNetDef {
unsigned long sndbuf;
} tune;
char *script;
+ char *domain_name; /* backend domain name */
char *ifname;
virDomainDeviceInfo info;
char *filter;
--
1.8.1.4
8:54 p.m.
New subject: [libvirt] [PATCH v2 04/14] libxl: support backend domain setting for disk and net devices
This implement handling of <domain name=''/> parameter introduced in
previous patch.
Lookup on domain name (to get domain ID) requires libxlDriverPrivate
object, so it must be passed down to libxlMakeDisk and libxlMakeNet from
top level callers.
Changes in v2:
- rebase on 1.0.6+
- fix indentation
- make libxl_name_to_domid switch more defensive
Signed-off-by: Marek Marczykowski-Górecki <marmarek(a)invisiblethingslab.com>
---
src/libxl/libxl_conf.c | 76 +++++++++++++++++++++++++++++++++++++++++++-----
src/libxl/libxl_conf.h | 4 +--
src/libxl/libxl_driver.c | 51 +++++++++++++++++++-------------
3 files changed, 101 insertions(+), 30 deletions(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index b9cb61e..623956e 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -441,7 +441,9 @@ error:
}
int
-libxlMakeDisk(virDomainDiskDefPtr l_disk, libxl_device_disk *x_disk)
+libxlMakeDisk(libxlDriverPrivatePtr driver,
+ virDomainDiskDefPtr l_disk,
+ libxl_device_disk *x_disk)
{
if (VIR_STRDUP(x_disk->pdev_path, l_disk->src) < 0)
return -1;
@@ -509,11 +511,39 @@ libxlMakeDisk(virDomainDiskDefPtr l_disk, libxl_device_disk
*x_disk)
return -1;
}
+ if (l_disk->domain_name) {
+ uint32_t domid;
+ /* Do not use virDomainObjListFindByName as it causes deadlock here -
+ * we already have lock on this domain object, but
+ * virDomainObjListFindByName will try to take it again.
+ */
+ switch (libxl_name_to_domid(driver->ctx, l_disk->domain_name, &domid))
{
+ case 0:
+ x_disk->backend_domid = domid;
+ break;
+ case ERROR_INVAL:
+ virReportError(VIR_ERR_XML_DETAIL,
+ _("Backend domain '%s' does not exists for disk
'%s'"),
+ l_disk->domain_name, l_disk->dst);
+ return -1;
+ case ERROR_NOMEM:
+ virReportOOMError();
+ return -1;
+ default:
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Failed to get ID of domain '%s'"),
+ l_disk->domain_name);
+ return -1;
+ }
+ }
+
return 0;
}
static int
-libxlMakeDiskList(virDomainDefPtr def, libxl_domain_config *d_config)
+libxlMakeDiskList(libxlDriverPrivatePtr driver,
+ virDomainDefPtr def,
+ libxl_domain_config *d_config)
{
virDomainDiskDefPtr *l_disks = def->disks;
int ndisks = def->ndisks;
@@ -526,7 +556,7 @@ libxlMakeDiskList(virDomainDefPtr def, libxl_domain_config *d_config)
}
for (i = 0; i < ndisks; i++) {
- if (libxlMakeDisk(l_disks[i], &x_disks[i]) < 0)
+ if (libxlMakeDisk(driver, l_disks[i], &x_disks[i]) < 0)
goto error;
}
@@ -543,7 +573,9 @@ error:
}
int
-libxlMakeNic(virDomainNetDefPtr l_nic, libxl_device_nic *x_nic)
+libxlMakeNic(libxlDriverPrivatePtr driver,
+ virDomainNetDefPtr l_nic,
+ libxl_device_nic *x_nic)
{
/* TODO: Where is mtu stored?
*
@@ -579,11 +611,39 @@ libxlMakeNic(virDomainNetDefPtr l_nic, libxl_device_nic *x_nic)
return -1;
}
+ if (l_nic->domain_name) {
+ uint32_t domid;
+ /* Do not use virDomainObjListFindByName as it causes deadlock here -
+ * we already have lock on this domain object, but
+ * virDomainObjListFindByName will try to take it again.
+ */
+ switch (libxl_name_to_domid(driver->ctx, l_nic->domain_name, &domid))
{
+ case 0:
+ x_nic->backend_domid = domid;
+ break;
+ case ERROR_INVAL:
+ virReportError(VIR_ERR_XML_DETAIL,
+ _("Backend domain '%s' does not exists for nic
'%s'"),
+ l_nic->domain_name, l_nic->ifname);
+ return -1;
+ case ERROR_NOMEM:
+ virReportOOMError();
+ return -1;
+ default:
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Failed to get ID of domain '%s'"),
+ l_nic->domain_name);
+ return -1;
+ }
+ }
+
return 0;
}
static int
-libxlMakeNicList(virDomainDefPtr def, libxl_domain_config *d_config)
+libxlMakeNicList(libxlDriverPrivatePtr driver,
+ virDomainDefPtr def,
+ libxl_domain_config *d_config)
{
virDomainNetDefPtr *l_nics = def->nets;
int nnics = def->nnets;
@@ -598,7 +658,7 @@ libxlMakeNicList(virDomainDefPtr def, libxl_domain_config *d_config)
for (i = 0; i < nnics; i++) {
x_nics[i].devid = i;
- if (libxlMakeNic(l_nics[i], &x_nics[i]))
+ if (libxlMakeNic(driver, l_nics[i], &x_nics[i]))
goto error;
}
@@ -756,11 +816,11 @@ libxlBuildDomainConfig(libxlDriverPrivatePtr driver,
return -1;
}
- if (libxlMakeDiskList(def, d_config) < 0) {
+ if (libxlMakeDiskList(driver, def, d_config) < 0) {
return -1;
}
- if (libxlMakeNicList(def, d_config) < 0) {
+ if (libxlMakeNicList(driver, def, d_config) < 0) {
return -1;
}
diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h
index e8fb9c4..33126f3 100644
--- a/src/libxl/libxl_conf.h
+++ b/src/libxl/libxl_conf.h
@@ -115,9 +115,9 @@ virCapsPtr
libxlMakeCapabilities(libxl_ctx *ctx);
int
-libxlMakeDisk(virDomainDiskDefPtr l_dev, libxl_device_disk *x_dev);
+libxlMakeDisk(libxlDriverPrivatePtr driver, virDomainDiskDefPtr l_dev, libxl_device_disk
*x_dev);
int
-libxlMakeNic(virDomainNetDefPtr l_nic, libxl_device_nic *x_nic);
+libxlMakeNic(libxlDriverPrivatePtr driver, virDomainNetDefPtr l_nic, libxl_device_nic
*x_nic);
int
libxlMakeVfb(libxlDriverPrivatePtr driver,
virDomainGraphicsDefPtr l_vfb, libxl_device_vfb *x_vfb);
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 0a0690d..df31001 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -3233,8 +3233,10 @@ libxlDomainUndefine(virDomainPtr dom)
}
static int
-libxlDomainChangeEjectableMedia(libxlDomainObjPrivatePtr priv,
- virDomainObjPtr vm, virDomainDiskDefPtr disk)
+libxlDomainChangeEjectableMedia(libxlDriverPrivatePtr driver,
+ libxlDomainObjPrivatePtr priv,
+ virDomainObjPtr vm,
+ virDomainDiskDefPtr disk)
{
virDomainDiskDefPtr origdisk = NULL;
libxl_device_disk x_disk;
@@ -3263,7 +3265,7 @@ libxlDomainChangeEjectableMedia(libxlDomainObjPrivatePtr priv,
return -1;
}
- if (libxlMakeDisk(disk, &x_disk) < 0)
+ if (libxlMakeDisk(driver, disk, &x_disk) < 0)
goto cleanup;
if ((ret = libxl_cdrom_insert(priv->ctx, vm->def->id, &x_disk, NULL))
< 0) {
@@ -3288,8 +3290,10 @@ cleanup:
}
static int
-libxlDomainAttachDeviceDiskLive(libxlDomainObjPrivatePtr priv,
- virDomainObjPtr vm, virDomainDeviceDefPtr dev)
+libxlDomainAttachDeviceDiskLive(libxlDriverPrivatePtr driver,
+ libxlDomainObjPrivatePtr priv,
+ virDomainObjPtr vm,
+ virDomainDeviceDefPtr dev)
{
virDomainDiskDefPtr l_disk = dev->data.disk;
libxl_device_disk x_disk;
@@ -3297,7 +3301,7 @@ libxlDomainAttachDeviceDiskLive(libxlDomainObjPrivatePtr priv,
switch (l_disk->device) {
case VIR_DOMAIN_DISK_DEVICE_CDROM:
- ret = libxlDomainChangeEjectableMedia(priv, vm, l_disk);
+ ret = libxlDomainChangeEjectableMedia(driver, priv, vm, l_disk);
break;
case VIR_DOMAIN_DISK_DEVICE_DISK:
if (l_disk->bus == VIR_DOMAIN_DISK_BUS_XEN) {
@@ -3318,7 +3322,7 @@ libxlDomainAttachDeviceDiskLive(libxlDomainObjPrivatePtr priv,
goto cleanup;
}
- if (libxlMakeDisk(l_disk, &x_disk) < 0)
+ if (libxlMakeDisk(driver, l_disk, &x_disk) < 0)
goto cleanup;
if ((ret = libxl_device_disk_add(priv->ctx, vm->def->id,
@@ -3349,8 +3353,10 @@ cleanup:
}
static int
-libxlDomainDetachDeviceDiskLive(libxlDomainObjPrivatePtr priv,
- virDomainObjPtr vm, virDomainDeviceDefPtr dev)
+libxlDomainDetachDeviceDiskLive(libxlDriverPrivatePtr driver,
+ libxlDomainObjPrivatePtr priv,
+ virDomainObjPtr vm,
+ virDomainDeviceDefPtr dev)
{
virDomainDiskDefPtr l_disk = NULL;
libxl_device_disk x_disk;
@@ -3371,7 +3377,7 @@ libxlDomainDetachDeviceDiskLive(libxlDomainObjPrivatePtr priv,
l_disk = vm->def->disks[i];
- if (libxlMakeDisk(l_disk, &x_disk) < 0)
+ if (libxlMakeDisk(driver, l_disk, &x_disk) < 0)
goto cleanup;
if ((ret = libxl_device_disk_remove(priv->ctx, vm->def->id,
@@ -3403,14 +3409,15 @@ cleanup:
}
static int
-libxlDomainAttachDeviceLive(libxlDomainObjPrivatePtr priv, virDomainObjPtr vm,
+libxlDomainAttachDeviceLive(libxlDriverPrivatePtr driver,
+ libxlDomainObjPrivatePtr priv, virDomainObjPtr vm,
virDomainDeviceDefPtr dev)
{
int ret = -1;
switch (dev->type) {
case VIR_DOMAIN_DEVICE_DISK:
- ret = libxlDomainAttachDeviceDiskLive(priv, vm, dev);
+ ret = libxlDomainAttachDeviceDiskLive(driver, priv, vm, dev);
if (!ret)
dev->data.disk = NULL;
break;
@@ -3455,14 +3462,16 @@ libxlDomainAttachDeviceConfig(virDomainDefPtr vmdef,
virDomainDeviceDefPtr dev)
}
static int
-libxlDomainDetachDeviceLive(libxlDomainObjPrivatePtr priv, virDomainObjPtr vm,
+libxlDomainDetachDeviceLive(libxlDriverPrivatePtr driver,
+ libxlDomainObjPrivatePtr priv,
+ virDomainObjPtr vm,
virDomainDeviceDefPtr dev)
{
int ret = -1;
switch (dev->type) {
case VIR_DOMAIN_DEVICE_DISK:
- ret = libxlDomainDetachDeviceDiskLive(priv, vm, dev);
+ ret = libxlDomainDetachDeviceDiskLive(driver, priv, vm, dev);
break;
default:
@@ -3502,8 +3511,10 @@ libxlDomainDetachDeviceConfig(virDomainDefPtr vmdef,
virDomainDeviceDefPtr dev)
}
static int
-libxlDomainUpdateDeviceLive(libxlDomainObjPrivatePtr priv,
- virDomainObjPtr vm, virDomainDeviceDefPtr dev)
+libxlDomainUpdateDeviceLive(libxlDriverPrivatePtr driver,
+ libxlDomainObjPrivatePtr priv,
+ virDomainObjPtr vm,
+ virDomainDeviceDefPtr dev)
{
virDomainDiskDefPtr disk;
int ret = -1;
@@ -3513,7 +3524,7 @@ libxlDomainUpdateDeviceLive(libxlDomainObjPrivatePtr priv,
disk = dev->data.disk;
switch (disk->device) {
case VIR_DOMAIN_DISK_DEVICE_CDROM:
- ret = libxlDomainChangeEjectableMedia(priv, vm, disk);
+ ret = libxlDomainChangeEjectableMedia(driver, priv, vm, disk);
if (ret == 0)
dev->data.disk = NULL;
break;
@@ -3650,7 +3661,7 @@ libxlDomainAttachDeviceFlags(virDomainPtr dom, const char *xml,
VIR_DOMAIN_XML_INACTIVE)))
goto cleanup;
- if ((ret = libxlDomainAttachDeviceLive(priv, vm, dev)) < 0)
+ if ((ret = libxlDomainAttachDeviceLive(driver, priv, vm, dev)) < 0)
goto cleanup;
/*
@@ -3755,7 +3766,7 @@ libxlDomainDetachDeviceFlags(virDomainPtr dom, const char *xml,
VIR_DOMAIN_XML_INACTIVE)))
goto cleanup;
- if ((ret = libxlDomainDetachDeviceLive(priv, vm, dev)) < 0)
+ if ((ret = libxlDomainDetachDeviceLive(driver, priv, vm, dev)) < 0)
goto cleanup;
/*
@@ -3860,7 +3871,7 @@ libxlDomainUpdateDeviceFlags(virDomainPtr dom, const char *xml,
VIR_DOMAIN_XML_INACTIVE)))
goto cleanup;
- if ((ret = libxlDomainUpdateDeviceLive(priv, vm, dev)) < 0)
+ if ((ret = libxlDomainUpdateDeviceLive(driver, priv, vm, dev)) < 0)
goto cleanup;
/*
--
1.8.1.4
3:45 p.m.
New subject: [libvirt] [PATCH v2 04/14] libxl: support backend domain setting for disk and net devices
Marek Marczykowski-Górecki wrote:
This implement handling of <domain name=''/> parameter
introduced in
previous patch.
Lookup on domain name (to get domain ID) requires libxlDriverPrivate
object, so it must be passed down to libxlMakeDisk and libxlMakeNet from
top level callers.
Changes in v2:
- rebase on 1.0.6+
- fix indentation
- make libxl_name_to_domid switch more defensive
Signed-off-by: Marek Marczykowski-Górecki <marmarek(a)invisiblethingslab.com>
---
src/libxl/libxl_conf.c | 76 +++++++++++++++++++++++++++++++++++++++++++-----
src/libxl/libxl_conf.h | 4 +--
src/libxl/libxl_driver.c | 51 +++++++++++++++++++-------------
3 files changed, 101 insertions(+), 30 deletions(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index b9cb61e..623956e 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -441,7 +441,9 @@ error:
}
int
-libxlMakeDisk(virDomainDiskDefPtr l_disk, libxl_device_disk *x_disk)
+libxlMakeDisk(libxlDriverPrivatePtr driver,
After looking at this again, I wonder if it is better to just pass the
libxl_ctx to libxlMakeDisk and libxlMakeNic. All the callers already
have a libxl_ctx, making for a much smaller patch. What do you think?
Regards,
Jim
+ virDomainDiskDefPtr l_disk,
+ libxl_device_disk *x_disk)
{
if (VIR_STRDUP(x_disk->pdev_path, l_disk->src) < 0)
return -1;
@@ -509,11 +511,39 @@ libxlMakeDisk(virDomainDiskDefPtr l_disk, libxl_device_disk
*x_disk)
return -1;
}
+ if (l_disk->domain_name) {
+ uint32_t domid;
+ /* Do not use virDomainObjListFindByName as it causes deadlock here -
+ * we already have lock on this domain object, but
+ * virDomainObjListFindByName will try to take it again.
+ */
+ switch (libxl_name_to_domid(driver->ctx, l_disk->domain_name, &domid))
{
+ case 0:
+ x_disk->backend_domid = domid;
+ break;
+ case ERROR_INVAL:
+ virReportError(VIR_ERR_XML_DETAIL,
+ _("Backend domain '%s' does not exists for disk
'%s'"),
+ l_disk->domain_name, l_disk->dst);
+ return -1;
+ case ERROR_NOMEM:
+ virReportOOMError();
+ return -1;
+ default:
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Failed to get ID of domain '%s'"),
+ l_disk->domain_name);
+ return -1;
+ }
+ }
+
return 0;
}
static int
-libxlMakeDiskList(virDomainDefPtr def, libxl_domain_config *d_config)
+libxlMakeDiskList(libxlDriverPrivatePtr driver,
+ virDomainDefPtr def,
+ libxl_domain_config *d_config)
{
virDomainDiskDefPtr *l_disks = def->disks;
int ndisks = def->ndisks;
@@ -526,7 +556,7 @@ libxlMakeDiskList(virDomainDefPtr def, libxl_domain_config
*d_config)
}
for (i = 0; i < ndisks; i++) {
- if (libxlMakeDisk(l_disks[i], &x_disks[i]) < 0)
+ if (libxlMakeDisk(driver, l_disks[i], &x_disks[i]) < 0)
goto error;
}
@@ -543,7 +573,9 @@ error:
}
int
-libxlMakeNic(virDomainNetDefPtr l_nic, libxl_device_nic *x_nic)
+libxlMakeNic(libxlDriverPrivatePtr driver,
+ virDomainNetDefPtr l_nic,
+ libxl_device_nic *x_nic)
{
/* TODO: Where is mtu stored?
*
@@ -579,11 +611,39 @@ libxlMakeNic(virDomainNetDefPtr l_nic, libxl_device_nic *x_nic)
return -1;
}
+ if (l_nic->domain_name) {
+ uint32_t domid;
+ /* Do not use virDomainObjListFindByName as it causes deadlock here -
+ * we already have lock on this domain object, but
+ * virDomainObjListFindByName will try to take it again.
+ */
+ switch (libxl_name_to_domid(driver->ctx, l_nic->domain_name, &domid))
{
+ case 0:
+ x_nic->backend_domid = domid;
+ break;
+ case ERROR_INVAL:
+ virReportError(VIR_ERR_XML_DETAIL,
+ _("Backend domain '%s' does not exists for nic
'%s'"),
+ l_nic->domain_name, l_nic->ifname);
+ return -1;
+ case ERROR_NOMEM:
+ virReportOOMError();
+ return -1;
+ default:
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Failed to get ID of domain '%s'"),
+ l_nic->domain_name);
+ return -1;
+ }
+ }
+
return 0;
}
static int
-libxlMakeNicList(virDomainDefPtr def, libxl_domain_config *d_config)
+libxlMakeNicList(libxlDriverPrivatePtr driver,
+ virDomainDefPtr def,
+ libxl_domain_config *d_config)
{
virDomainNetDefPtr *l_nics = def->nets;
int nnics = def->nnets;
@@ -598,7 +658,7 @@ libxlMakeNicList(virDomainDefPtr def, libxl_domain_config
*d_config)
for (i = 0; i < nnics; i++) {
x_nics[i].devid = i;
- if (libxlMakeNic(l_nics[i], &x_nics[i]))
+ if (libxlMakeNic(driver, l_nics[i], &x_nics[i]))
goto error;
}
@@ -756,11 +816,11 @@ libxlBuildDomainConfig(libxlDriverPrivatePtr driver,
return -1;
}
- if (libxlMakeDiskList(def, d_config) < 0) {
+ if (libxlMakeDiskList(driver, def, d_config) < 0) {
return -1;
}
- if (libxlMakeNicList(def, d_config) < 0) {
+ if (libxlMakeNicList(driver, def, d_config) < 0) {
return -1;
}
diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h
index e8fb9c4..33126f3 100644
--- a/src/libxl/libxl_conf.h
+++ b/src/libxl/libxl_conf.h
@@ -115,9 +115,9 @@ virCapsPtr
libxlMakeCapabilities(libxl_ctx *ctx);
int
-libxlMakeDisk(virDomainDiskDefPtr l_dev, libxl_device_disk *x_dev);
+libxlMakeDisk(libxlDriverPrivatePtr driver, virDomainDiskDefPtr l_dev, libxl_device_disk
*x_dev);
int
-libxlMakeNic(virDomainNetDefPtr l_nic, libxl_device_nic *x_nic);
+libxlMakeNic(libxlDriverPrivatePtr driver, virDomainNetDefPtr l_nic, libxl_device_nic
*x_nic);
int
libxlMakeVfb(libxlDriverPrivatePtr driver,
virDomainGraphicsDefPtr l_vfb, libxl_device_vfb *x_vfb);
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 0a0690d..df31001 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -3233,8 +3233,10 @@ libxlDomainUndefine(virDomainPtr dom)
}
static int
-libxlDomainChangeEjectableMedia(libxlDomainObjPrivatePtr priv,
- virDomainObjPtr vm, virDomainDiskDefPtr disk)
+libxlDomainChangeEjectableMedia(libxlDriverPrivatePtr driver,
+ libxlDomainObjPrivatePtr priv,
+ virDomainObjPtr vm,
+ virDomainDiskDefPtr disk)
{
virDomainDiskDefPtr origdisk = NULL;
libxl_device_disk x_disk;
@@ -3263,7 +3265,7 @@ libxlDomainChangeEjectableMedia(libxlDomainObjPrivatePtr priv,
return -1;
}
- if (libxlMakeDisk(disk, &x_disk) < 0)
+ if (libxlMakeDisk(driver, disk, &x_disk) < 0)
goto cleanup;
if ((ret = libxl_cdrom_insert(priv->ctx, vm->def->id, &x_disk, NULL))
< 0) {
@@ -3288,8 +3290,10 @@ cleanup:
}
static int
-libxlDomainAttachDeviceDiskLive(libxlDomainObjPrivatePtr priv,
- virDomainObjPtr vm, virDomainDeviceDefPtr dev)
+libxlDomainAttachDeviceDiskLive(libxlDriverPrivatePtr driver,
+ libxlDomainObjPrivatePtr priv,
+ virDomainObjPtr vm,
+ virDomainDeviceDefPtr dev)
{
virDomainDiskDefPtr l_disk = dev->data.disk;
libxl_device_disk x_disk;
@@ -3297,7 +3301,7 @@ libxlDomainAttachDeviceDiskLive(libxlDomainObjPrivatePtr priv,
switch (l_disk->device) {
case VIR_DOMAIN_DISK_DEVICE_CDROM:
- ret = libxlDomainChangeEjectableMedia(priv, vm, l_disk);
+ ret = libxlDomainChangeEjectableMedia(driver, priv, vm, l_disk);
break;
case VIR_DOMAIN_DISK_DEVICE_DISK:
if (l_disk->bus == VIR_DOMAIN_DISK_BUS_XEN) {
@@ -3318,7 +3322,7 @@ libxlDomainAttachDeviceDiskLive(libxlDomainObjPrivatePtr priv,
goto cleanup;
}
- if (libxlMakeDisk(l_disk, &x_disk) < 0)
+ if (libxlMakeDisk(driver, l_disk, &x_disk) < 0)
goto cleanup;
if ((ret = libxl_device_disk_add(priv->ctx, vm->def->id,
@@ -3349,8 +3353,10 @@ cleanup:
}
static int
-libxlDomainDetachDeviceDiskLive(libxlDomainObjPrivatePtr priv,
- virDomainObjPtr vm, virDomainDeviceDefPtr dev)
+libxlDomainDetachDeviceDiskLive(libxlDriverPrivatePtr driver,
+ libxlDomainObjPrivatePtr priv,
+ virDomainObjPtr vm,
+ virDomainDeviceDefPtr dev)
{
virDomainDiskDefPtr l_disk = NULL;
libxl_device_disk x_disk;
@@ -3371,7 +3377,7 @@ libxlDomainDetachDeviceDiskLive(libxlDomainObjPrivatePtr priv,
l_disk = vm->def->disks[i];
- if (libxlMakeDisk(l_disk, &x_disk) < 0)
+ if (libxlMakeDisk(driver, l_disk, &x_disk) < 0)
goto cleanup;
if ((ret = libxl_device_disk_remove(priv->ctx, vm->def->id,
@@ -3403,14 +3409,15 @@ cleanup:
}
static int
-libxlDomainAttachDeviceLive(libxlDomainObjPrivatePtr priv, virDomainObjPtr vm,
+libxlDomainAttachDeviceLive(libxlDriverPrivatePtr driver,
+ libxlDomainObjPrivatePtr priv, virDomainObjPtr vm,
virDomainDeviceDefPtr dev)
{
int ret = -1;
switch (dev->type) {
case VIR_DOMAIN_DEVICE_DISK:
- ret = libxlDomainAttachDeviceDiskLive(priv, vm, dev);
+ ret = libxlDomainAttachDeviceDiskLive(driver, priv, vm, dev);
if (!ret)
dev->data.disk = NULL;
break;
@@ -3455,14 +3462,16 @@ libxlDomainAttachDeviceConfig(virDomainDefPtr vmdef,
virDomainDeviceDefPtr dev)
}
static int
-libxlDomainDetachDeviceLive(libxlDomainObjPrivatePtr priv, virDomainObjPtr vm,
+libxlDomainDetachDeviceLive(libxlDriverPrivatePtr driver,
+ libxlDomainObjPrivatePtr priv,
+ virDomainObjPtr vm,
virDomainDeviceDefPtr dev)
{
int ret = -1;
switch (dev->type) {
case VIR_DOMAIN_DEVICE_DISK:
- ret = libxlDomainDetachDeviceDiskLive(priv, vm, dev);
+ ret = libxlDomainDetachDeviceDiskLive(driver, priv, vm, dev);
break;
default:
@@ -3502,8 +3511,10 @@ libxlDomainDetachDeviceConfig(virDomainDefPtr vmdef,
virDomainDeviceDefPtr dev)
}
static int
-libxlDomainUpdateDeviceLive(libxlDomainObjPrivatePtr priv,
- virDomainObjPtr vm, virDomainDeviceDefPtr dev)
+libxlDomainUpdateDeviceLive(libxlDriverPrivatePtr driver,
+ libxlDomainObjPrivatePtr priv,
+ virDomainObjPtr vm,
+ virDomainDeviceDefPtr dev)
{
virDomainDiskDefPtr disk;
int ret = -1;
@@ -3513,7 +3524,7 @@ libxlDomainUpdateDeviceLive(libxlDomainObjPrivatePtr priv,
disk = dev->data.disk;
switch (disk->device) {
case VIR_DOMAIN_DISK_DEVICE_CDROM:
- ret = libxlDomainChangeEjectableMedia(priv, vm, disk);
+ ret = libxlDomainChangeEjectableMedia(driver, priv, vm, disk);
if (ret == 0)
dev->data.disk = NULL;
break;
@@ -3650,7 +3661,7 @@ libxlDomainAttachDeviceFlags(virDomainPtr dom, const char *xml,
VIR_DOMAIN_XML_INACTIVE)))
goto cleanup;
- if ((ret = libxlDomainAttachDeviceLive(priv, vm, dev)) < 0)
+ if ((ret = libxlDomainAttachDeviceLive(driver, priv, vm, dev)) < 0)
goto cleanup;
/*
@@ -3755,7 +3766,7 @@ libxlDomainDetachDeviceFlags(virDomainPtr dom, const char *xml,
VIR_DOMAIN_XML_INACTIVE)))
goto cleanup;
- if ((ret = libxlDomainDetachDeviceLive(priv, vm, dev)) < 0)
+ if ((ret = libxlDomainDetachDeviceLive(driver, priv, vm, dev)) < 0)
goto cleanup;
/*
@@ -3860,7 +3871,7 @@ libxlDomainUpdateDeviceFlags(virDomainPtr dom, const char *xml,
VIR_DOMAIN_XML_INACTIVE)))
goto cleanup;
- if ((ret = libxlDomainUpdateDeviceLive(priv, vm, dev)) < 0)
+ if ((ret = libxlDomainUpdateDeviceLive(driver, priv, vm, dev)) < 0)
goto cleanup;
/*
8:54 p.m.
New subject: [libvirt] [PATCH v2 05/14] libxl: fill HVM SDL and VNC settings based on <graphics/> entries
Vfb entries in domain config are used only by PV drivers. Qemu
parameters are build based on b_info struct. So fill it with the same
data as vfb entries (actually the first one).
This will additionally allow graphic-less domain, when no <graphics/>
entries are present in domain XML (previously VNC was always enabled).
Signed-off-by: Marek Marczykowski-Górecki <marmarek(a)invisiblethingslab.com>
---
src/libxl/libxl_conf.c | 114 +++++++++++++++++++++++++++++++++++--------------
1 file changed, 82 insertions(+), 32 deletions(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 623956e..e5d8dc5 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -332,7 +332,56 @@ error:
}
static int
-libxlMakeDomBuildInfo(virDomainDefPtr def, libxl_domain_config *d_config)
+libxlMakeVNCInfo(libxlDriverPrivatePtr driver,
+ virDomainGraphicsDefPtr l_vfb,
+ libxl_vnc_info *x_vnc)
+{
+ unsigned short port;
+ const char *listenAddr;
+
+ libxl_defbool_set(&x_vnc->enable, 1);
+ /* driver handles selection of free port */
+ libxl_defbool_set(&x_vnc->findunused, 0);
+ if (l_vfb->data.vnc.autoport) {
+
+ if (virPortAllocatorAcquire(driver->reservedVNCPorts, &port) < 0)
+ return -1;
+ if (port == 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ "%s", _("Unable to find an unused VNC port"));
+ return -1;
+ }
+ l_vfb->data.vnc.port = port;
+ }
+ x_vnc->display = l_vfb->data.vnc.port - LIBXL_VNC_PORT_MIN;
+
+ listenAddr = virDomainGraphicsListenGetAddress(l_vfb, 0);
+ if (listenAddr) {
+ /* libxl_device_vfb_init() does strdup("127.0.0.1") */
+ VIR_FREE(x_vnc->listen);
+ if (VIR_STRDUP(x_vnc->listen, listenAddr) < 0) {
+ return -1;
+ }
+ }
+ return 0;
+}
+
+static int
+libxlMakeSDLInfo(virDomainGraphicsDefPtr l_vfb,
+ libxl_sdl_info *x_sdl)
+{
+ libxl_defbool_set(&x_sdl->enable, 1);
+ if (VIR_STRDUP(x_sdl->display, l_vfb->data.sdl.display) < 0)
+ return -1;
+ if (VIR_STRDUP(x_sdl->xauthority, l_vfb->data.sdl.xauth) < 0)
+ return -1;
+ return 0;
+}
+
+static int
+libxlMakeDomBuildInfo(libxlDriverPrivatePtr driver,
+ virDomainDefPtr def,
+ libxl_domain_config *d_config)
{
libxl_domain_build_info *b_info = &d_config->b_info;
int hvm = STREQ(def->os.type, "hvm");
@@ -404,6 +453,34 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, libxl_domain_config
*d_config)
if (VIR_STRDUP(b_info->u.hvm.boot, bootorder) < 0)
goto error;
+ /* Disable VNC and SDL until explicitly enabled */
+ libxl_defbool_set(&b_info->u.hvm.vnc.enable, 0);
+ libxl_defbool_set(&b_info->u.hvm.sdl.enable, 0);
+
+ for (i = 0; i < def->ngraphics; i++) {
+ switch (def->graphics[i]->type) {
+ case VIR_DOMAIN_GRAPHICS_TYPE_VNC:
+ if (libxl_defbool_val(b_info->u.hvm.vnc.enable))
+ continue;
+ if (libxlMakeVNCInfo(driver, def->graphics[i],
+ &b_info->u.hvm.vnc) < 0)
+ goto error;
+ if (def->graphics[i]->data.vnc.keymap &&
+ (b_info->u.hvm.keymap =
+ strdup(def->graphics[i]->data.vnc.keymap)) == NULL) {
+ virReportOOMError();
+ goto error;
+ }
+ break;
+ case VIR_DOMAIN_GRAPHICS_TYPE_SDL:
+ if (libxl_defbool_val(b_info->u.hvm.sdl.enable))
+ continue;
+ if (libxlMakeSDLInfo(def->graphics[i], &b_info->u.hvm.sdl)
< 0)
+ goto error;
+ break;
+ }
+ }
+
/*
* The following comment and calculation were taken directly from
* libxenlight's internal function libxl_get_required_shadow_memory():
@@ -679,41 +756,14 @@ libxlMakeVfb(libxlDriverPrivatePtr driver,
virDomainGraphicsDefPtr l_vfb,
libxl_device_vfb *x_vfb)
{
- unsigned short port;
- const char *listenAddr;
-
switch (l_vfb->type) {
case VIR_DOMAIN_GRAPHICS_TYPE_SDL:
- libxl_defbool_set(&x_vfb->sdl.enable, 1);
- if (VIR_STRDUP(x_vfb->sdl.display, l_vfb->data.sdl.display) < 0)
- return -1;
- if (VIR_STRDUP(x_vfb->sdl.xauthority, l_vfb->data.sdl.xauth) < 0)
+ if (libxlMakeSDLInfo(l_vfb, &x_vfb->sdl) < 0)
return -1;
break;
case VIR_DOMAIN_GRAPHICS_TYPE_VNC:
- libxl_defbool_set(&x_vfb->vnc.enable, 1);
- /* driver handles selection of free port */
- libxl_defbool_set(&x_vfb->vnc.findunused, 0);
- if (l_vfb->data.vnc.autoport) {
-
- if (virPortAllocatorAcquire(driver->reservedVNCPorts, &port) <
0)
- return -1;
- if (port == 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- "%s", _("Unable to find an unused VNC
port"));
- return -1;
- }
- l_vfb->data.vnc.port = port;
- }
- x_vfb->vnc.display = l_vfb->data.vnc.port - LIBXL_VNC_PORT_MIN;
-
- listenAddr = virDomainGraphicsListenGetAddress(l_vfb, 0);
- if (listenAddr) {
- /* libxl_device_vfb_init() does VIR_STRDUP("127.0.0.1") */
- VIR_FREE(x_vfb->vnc.listen);
- if (VIR_STRDUP(x_vfb->vnc.listen, listenAddr) < 0)
- return -1;
- }
+ if (libxlMakeVNCInfo(driver, l_vfb, &x_vfb->vnc) < 0)
+ return -1;
if (VIR_STRDUP(x_vfb->keymap, l_vfb->data.vnc.keymap) < 0)
return -1;
break;
@@ -812,7 +862,7 @@ libxlBuildDomainConfig(libxlDriverPrivatePtr driver,
if (libxlMakeDomCreateInfo(driver, def, &d_config->c_info) < 0)
return -1;
- if (libxlMakeDomBuildInfo(def, d_config) < 0) {
+ if (libxlMakeDomBuildInfo(driver, def, d_config) < 0) {
return -1;
}
--
1.8.1.4
4:28 p.m.
New subject: [libvirt] [PATCH v2 05/14] libxl: fill HVM SDL and VNC settings based on <graphics/> entries
Marek Marczykowski-Górecki wrote:
Vfb entries in domain config are used only by PV drivers. Qemu
parameters are build based on b_info struct. So fill it with the same
data as vfb entries (actually the first one).
This will additionally allow graphic-less domain, when no <graphics/>
entries are present in domain XML (previously VNC was always enabled).
Signed-off-by: Marek Marczykowski-Górecki <marmarek(a)invisiblethingslab.com>
---
src/libxl/libxl_conf.c | 114 +++++++++++++++++++++++++++++++++++--------------
1 file changed, 82 insertions(+), 32 deletions(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 623956e..e5d8dc5 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -332,7 +332,56 @@ error:
}
static int
-libxlMakeDomBuildInfo(virDomainDefPtr def, libxl_domain_config *d_config)
+libxlMakeVNCInfo(libxlDriverPrivatePtr driver,
+ virDomainGraphicsDefPtr l_vfb,
+ libxl_vnc_info *x_vnc)
For HVM guests, I noticed this function is called twice, first from
libxlMakeDomBuildInfo, then again via libxlMakeVfbList.
+{
+ unsigned short port;
+ const char *listenAddr;
+
+ libxl_defbool_set(&x_vnc->enable, 1);
+ /* driver handles selection of free port */
+ libxl_defbool_set(&x_vnc->findunused, 0);
+ if (l_vfb->data.vnc.autoport) {
+
+ if (virPortAllocatorAcquire(driver->reservedVNCPorts, &port) < 0)
Another port gets allocated on the second invocation
+ return -1;
+ if (port == 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ "%s", _("Unable to find an unused VNC port"));
+ return -1;
+ }
+ l_vfb->data.vnc.port = port;
which updates libvirt's port info with incorrect info, since qemu gets
launched with the port info in the b_info struct.
+ }
+ x_vnc->display = l_vfb->data.vnc.port - LIBXL_VNC_PORT_MIN;
+
+ listenAddr = virDomainGraphicsListenGetAddress(l_vfb, 0);
+ if (listenAddr) {
+ /* libxl_device_vfb_init() does strdup("127.0.0.1") */
You'll need to change the strdup to VIR_STRDUP, even in the comment, to
satisfy make syntax-check. Or tweak the syntax-check rule :).
+ VIR_FREE(x_vnc->listen);
+ if (VIR_STRDUP(x_vnc->listen, listenAddr) < 0) {
+ return -1;
+ }
+ }
+ return 0;
+}
+
+static int
+libxlMakeSDLInfo(virDomainGraphicsDefPtr l_vfb,
+ libxl_sdl_info *x_sdl)
+{
+ libxl_defbool_set(&x_sdl->enable, 1);
+ if (VIR_STRDUP(x_sdl->display, l_vfb->data.sdl.display) < 0)
+ return -1;
+ if (VIR_STRDUP(x_sdl->xauthority, l_vfb->data.sdl.xauth) < 0)
+ return -1;
+ return 0;
+}
+
+static int
+libxlMakeDomBuildInfo(libxlDriverPrivatePtr driver,
+ virDomainDefPtr def,
+ libxl_domain_config *d_config)
{
libxl_domain_build_info *b_info = &d_config->b_info;
int hvm = STREQ(def->os.type, "hvm");
@@ -404,6 +453,34 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, libxl_domain_config
*d_config)
if (VIR_STRDUP(b_info->u.hvm.boot, bootorder) < 0)
goto error;
+ /* Disable VNC and SDL until explicitly enabled */
+ libxl_defbool_set(&b_info->u.hvm.vnc.enable, 0);
+ libxl_defbool_set(&b_info->u.hvm.sdl.enable, 0);
+
+ for (i = 0; i < def->ngraphics; i++) {
+ switch (def->graphics[i]->type) {
+ case VIR_DOMAIN_GRAPHICS_TYPE_VNC:
+ if (libxl_defbool_val(b_info->u.hvm.vnc.enable))
+ continue;
+ if (libxlMakeVNCInfo(driver, def->graphics[i],
+ &b_info->u.hvm.vnc) < 0)
+ goto error;
+ if (def->graphics[i]->data.vnc.keymap &&
+ (b_info->u.hvm.keymap =
+ strdup(def->graphics[i]->data.vnc.keymap)) == NULL)
{
+ virReportOOMError();
Needs to be converted to VIR_STRDUP.
Regards,
Jim
+ goto error;
+ }
+ break;
+ case VIR_DOMAIN_GRAPHICS_TYPE_SDL:
+ if (libxl_defbool_val(b_info->u.hvm.sdl.enable))
+ continue;
+ if (libxlMakeSDLInfo(def->graphics[i], &b_info->u.hvm.sdl)
< 0)
+ goto error;
+ break;
+ }
+ }
+
/*
* The following comment and calculation were taken directly from
* libxenlight's internal function libxl_get_required_shadow_memory():
@@ -679,41 +756,14 @@ libxlMakeVfb(libxlDriverPrivatePtr driver,
virDomainGraphicsDefPtr l_vfb,
libxl_device_vfb *x_vfb)
{
- unsigned short port;
- const char *listenAddr;
-
switch (l_vfb->type) {
case VIR_DOMAIN_GRAPHICS_TYPE_SDL:
- libxl_defbool_set(&x_vfb->sdl.enable, 1);
- if (VIR_STRDUP(x_vfb->sdl.display, l_vfb->data.sdl.display) < 0)
- return -1;
- if (VIR_STRDUP(x_vfb->sdl.xauthority, l_vfb->data.sdl.xauth) < 0)
+ if (libxlMakeSDLInfo(l_vfb, &x_vfb->sdl) < 0)
return -1;
break;
case VIR_DOMAIN_GRAPHICS_TYPE_VNC:
- libxl_defbool_set(&x_vfb->vnc.enable, 1);
- /* driver handles selection of free port */
- libxl_defbool_set(&x_vfb->vnc.findunused, 0);
- if (l_vfb->data.vnc.autoport) {
-
- if (virPortAllocatorAcquire(driver->reservedVNCPorts, &port) <
0)
- return -1;
- if (port == 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- "%s", _("Unable to find an unused VNC
port"));
- return -1;
- }
- l_vfb->data.vnc.port = port;
- }
- x_vfb->vnc.display = l_vfb->data.vnc.port - LIBXL_VNC_PORT_MIN;
-
- listenAddr = virDomainGraphicsListenGetAddress(l_vfb, 0);
- if (listenAddr) {
- /* libxl_device_vfb_init() does VIR_STRDUP("127.0.0.1") */
- VIR_FREE(x_vfb->vnc.listen);
- if (VIR_STRDUP(x_vfb->vnc.listen, listenAddr) < 0)
- return -1;
- }
+ if (libxlMakeVNCInfo(driver, l_vfb, &x_vfb->vnc) < 0)
+ return -1;
if (VIR_STRDUP(x_vfb->keymap, l_vfb->data.vnc.keymap) < 0)
return -1;
break;
@@ -812,7 +862,7 @@ libxlBuildDomainConfig(libxlDriverPrivatePtr driver,
if (libxlMakeDomCreateInfo(driver, def, &d_config->c_info) < 0)
return -1;
- if (libxlMakeDomBuildInfo(def, d_config) < 0) {
+ if (libxlMakeDomBuildInfo(driver, def, d_config) < 0) {
return -1;
}
8:54 p.m.
New subject: [libvirt] [PATCH v2 06/14] conf: virDomainObjListRemoveLocked function
While iterating with virDomainObjListForEach it is safe to remove
current element. But while iterating, 'doms' lock is already taken, so
can't use standard virDomainObjListRemove. So introduce
virDomainObjListRemoveLocked for this purpose.
Changes in v2:
- fix indentation
Signed-off-by: Marek Marczykowski-Górecki <marmarek(a)invisiblethingslab.com>
---
src/conf/domain_conf.c | 17 +++++++++++++++++
src/conf/domain_conf.h | 2 ++
src/libvirt_private.syms | 1 +
3 files changed, 20 insertions(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 5350c56..a4010da 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2336,6 +2336,23 @@ void virDomainObjListRemove(virDomainObjListPtr doms,
virObjectUnlock(doms);
}
+/* The caller must hold lock on 'doms' in addition to
'virDomainObjListRemove'
+ * requirements
+ *
+ * Can be used to remove current element while iterating with
+ * virDomainObjListForEach
+ */
+void virDomainObjListRemoveLocked(virDomainObjListPtr doms,
+ virDomainObjPtr dom)
+{
+ char uuidstr[VIR_UUID_STRING_BUFLEN];
+
+ virUUIDFormat(dom->def->uuid, uuidstr);
+ virObjectUnlock(dom);
+
+ virHashRemoveEntry(doms->objs, uuidstr);
+}
+
static int
virDomainDeviceCCWAddressIsValid(virDomainDeviceCCWAddressPtr addr)
{
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 24d3809..b8dfe19 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2234,6 +2234,8 @@ virDomainDefPtr virDomainObjCopyPersistentDef(virDomainObjPtr dom,
void virDomainObjListRemove(virDomainObjListPtr doms,
virDomainObjPtr dom);
+void virDomainObjListRemoveLocked(virDomainObjListPtr doms,
+ virDomainObjPtr dom);
virDomainDeviceDefPtr virDomainDeviceDefParse(const char *xmlStr,
virDomainDefPtr def,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 042081f..e21a14d 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -274,6 +274,7 @@ virDomainObjListLoadAllConfigs;
virDomainObjListNew;
virDomainObjListNumOfDomains;
virDomainObjListRemove;
+virDomainObjListRemoveLocked;
virDomainObjNew;
virDomainObjSetDefTransient;
virDomainObjSetState;
--
1.8.1.4
10:34 a.m.
New subject: [libvirt] [PATCH v2 06/14] conf: virDomainObjListRemoveLocked function
Marek Marczykowski-Górecki wrote:
While iterating with virDomainObjListForEach it is safe to remove
current element. But while iterating, 'doms' lock is already taken, so
can't use standard virDomainObjListRemove. So introduce
virDomainObjListRemoveLocked for this purpose.
Changes in v2:
- fix indentation
Signed-off-by: Marek Marczykowski-Górecki <marmarek(a)invisiblethingslab.com>
---
src/conf/domain_conf.c | 17 +++++++++++++++++
src/conf/domain_conf.h | 2 ++
src/libvirt_private.syms | 1 +
3 files changed, 20 insertions(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 5350c56..a4010da 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2336,6 +2336,23 @@ void virDomainObjListRemove(virDomainObjListPtr doms,
virObjectUnlock(doms);
}
+/* The caller must hold lock on 'doms' in addition to
'virDomainObjListRemove'
+ * requirements
+ *
+ * Can be used to remove current element while iterating with
+ * virDomainObjListForEach
+ */
+void virDomainObjListRemoveLocked(virDomainObjListPtr doms,
+ virDomainObjPtr dom)
+{
+ char uuidstr[VIR_UUID_STRING_BUFLEN];
+
+ virUUIDFormat(dom->def->uuid, uuidstr);
+ virObjectUnlock(dom);
+
+ virHashRemoveEntry(doms->objs, uuidstr);
Peter fixed a race in virDomainObjListRemove with commit b7c98329, and
although this function expects the domain obj list to be locked on
entry, it still seems some aspect of the race might exist here. Is it
possible for another thread to lock the dom and execute some code on it
after the freeing function has been triggered?
Regards,
Jim
+}
+
static int
virDomainDeviceCCWAddressIsValid(virDomainDeviceCCWAddressPtr addr)
{
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 24d3809..b8dfe19 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2234,6 +2234,8 @@ virDomainDefPtr virDomainObjCopyPersistentDef(virDomainObjPtr dom,
void virDomainObjListRemove(virDomainObjListPtr doms,
virDomainObjPtr dom);
+void virDomainObjListRemoveLocked(virDomainObjListPtr doms,
+ virDomainObjPtr dom);
virDomainDeviceDefPtr virDomainDeviceDefParse(const char *xmlStr,
virDomainDefPtr def,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 042081f..e21a14d 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -274,6 +274,7 @@ virDomainObjListLoadAllConfigs;
virDomainObjListNew;
virDomainObjListNumOfDomains;
virDomainObjListRemove;
+virDomainObjListRemoveLocked;
virDomainObjNew;
virDomainObjSetDefTransient;
virDomainObjSetState;
11:24 a.m.
New subject: [libvirt] [PATCH v2 06/14] conf: virDomainObjListRemoveLocked function
On 20.06.2013 17:34, Jim Fehlig wrote:
Marek Marczykowski-Górecki wrote:
> While iterating with virDomainObjListForEach it is safe to remove
> current element. But while iterating, 'doms' lock is already taken, so
> can't use standard virDomainObjListRemove. So introduce
> virDomainObjListRemoveLocked for this purpose.
>
> Changes in v2:
> - fix indentation
>
> Signed-off-by: Marek Marczykowski-Górecki <marmarek(a)invisiblethingslab.com>
> ---
> src/conf/domain_conf.c | 17 +++++++++++++++++
> src/conf/domain_conf.h | 2 ++
> src/libvirt_private.syms | 1 +
> 3 files changed, 20 insertions(+)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 5350c56..a4010da 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -2336,6 +2336,23 @@ void virDomainObjListRemove(virDomainObjListPtr doms,
> virObjectUnlock(doms);
> }
>
> +/* The caller must hold lock on 'doms' in addition to
'virDomainObjListRemove'
> + * requirements
> + *
> + * Can be used to remove current element while iterating with
> + * virDomainObjListForEach
> + */
> +void virDomainObjListRemoveLocked(virDomainObjListPtr doms,
> + virDomainObjPtr dom)
> +{
> + char uuidstr[VIR_UUID_STRING_BUFLEN];
> +
> + virUUIDFormat(dom->def->uuid, uuidstr);
> + virObjectUnlock(dom);
> +
> + virHashRemoveEntry(doms->objs, uuidstr);
>
Peter fixed a race in virDomainObjListRemove with commit b7c98329, and
although this function expects the domain obj list to be locked on
entry, it still seems some aspect of the race might exist here. Is it
possible for another thread to lock the dom and execute some code on it
after the freeing function has been triggered?
The next patch uses it only in libxlReconnectDomain (at driver
initialization), with driver lock hold whole the time from the vm object
creation. So in this particular case no other thread can access this vm object.
In general case I think it is still safe. The other thread need to have lock
on 'doms' list to get access to 'dom' object. Because the caller also
must
have a lock on the list before a) getting lock on a 'dom' object, b) calling
virDomainObjListRemoveLocked, it isn't possible for the other thread to access
'dom' object before caller release lock on the list.
Hmm, this description isn't clear... So let me try to enumerate calls:
Thread A (which removes the entry from the list):
1. virObjectLock(doms)
2. virObjectLock(dom)
3. virDomainObjListRemoveLocked(doms, dom)
4. virObjectUnlock(dom)
5. virHashRemoveEntry
6. virObjectUnlock(doms)
Above sequence can happen during virDomainObjListForEach.
If thread B tries to get lock on dom object, it should call:
1. virObjectLock(doms) (e.g. as part of *Lookup function)
1a. (lookup here)
2. virObjectLock(dom)
At no point thread B can get lock on 'doms' while thread A is in the middle of
above code, so it can't reach dom object to get lock on it and use it. If
thread B have lock on 'doms' or 'dom' before thread A starts above code,
thread A will block until B finish.
--
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
3:50 p.m.
New subject: [libvirt] [PATCH v2 06/14] conf: virDomainObjListRemoveLocked function
On 06/20/2013 10:24 AM, Marek Marczykowski-Górecki wrote:
On 20.06.2013 17:34, Jim Fehlig wrote:
> Marek Marczykowski-Górecki wrote:
>> While iterating with virDomainObjListForEach it is safe to remove
>> current element. But while iterating, 'doms' lock is already taken, so
>> can't use standard virDomainObjListRemove. So introduce
>> virDomainObjListRemoveLocked for this purpose.
>>
>> Changes in v2:
>> - fix indentation
>>
>> Signed-off-by: Marek Marczykowski-Górecki
<marmarek(a)invisiblethingslab.com>
>> ---
>> src/conf/domain_conf.c | 17 +++++++++++++++++
>> src/conf/domain_conf.h | 2 ++
>> src/libvirt_private.syms | 1 +
>> 3 files changed, 20 insertions(+)
>>
>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>> index 5350c56..a4010da 100644
>> --- a/src/conf/domain_conf.c
>> +++ b/src/conf/domain_conf.c
>> @@ -2336,6 +2336,23 @@ void virDomainObjListRemove(virDomainObjListPtr doms,
>> virObjectUnlock(doms);
>> }
>>
>> +/* The caller must hold lock on 'doms' in addition to
'virDomainObjListRemove'
>> + * requirements
>> + *
>> + * Can be used to remove current element while iterating with
>> + * virDomainObjListForEach
>> + */
>> +void virDomainObjListRemoveLocked(virDomainObjListPtr doms,
>> + virDomainObjPtr dom)
>> +{
>> + char uuidstr[VIR_UUID_STRING_BUFLEN];
>> +
>> + virUUIDFormat(dom->def->uuid, uuidstr);
>> + virObjectUnlock(dom);
>> +
>> + virHashRemoveEntry(doms->objs, uuidstr);
>>
> Peter fixed a race in virDomainObjListRemove with commit b7c98329, and
> although this function expects the domain obj list to be locked on
> entry, it still seems some aspect of the race might exist here. Is it
> possible for another thread to lock the dom and execute some code on it
> after the freeing function has been triggered?
The next patch uses it only in libxlReconnectDomain (at driver
initialization), with driver lock hold whole the time from the vm object
creation. So in this particular case no other thread can access this vm object.
In general case I think it is still safe. The other thread need to have lock
on 'doms' list to get access to 'dom' object. Because the caller also
must
have a lock on the list before a) getting lock on a 'dom' object, b) calling
virDomainObjListRemoveLocked, it isn't possible for the other thread to access
'dom' object before caller release lock on the list.
Ah, I think I got it...
Hmm, this description isn't clear... So let me try to enumerate calls:
Thread A (which removes the entry from the list):
1. virObjectLock(doms)
2. virObjectLock(dom)
3. virDomainObjListRemoveLocked(doms, dom)
4. virObjectUnlock(dom)
5. virHashRemoveEntry
6. virObjectUnlock(doms)
Above sequence can happen during virDomainObjListForEach.
If thread B tries to get lock on dom object, it should call:
1. virObjectLock(doms) (e.g. as part of *Lookup function)
1a. (lookup here)
2. virObjectLock(dom)
At no point thread B can get lock on 'doms' while thread A is in the middle of
above code, so it can't reach dom object to get lock on it and use it. If
thread B have lock on 'doms' or 'dom' before thread A starts above code,
thread A will block until B finish.
and now I do, given the clarification. Thanks.
This has been on the list for a while with no additional comments, so ACK and
pushed.
Regards,
Jim
8:54 p.m.
New subject: [libvirt] [PATCH v2 07/14] libxl: fix deadlock in libxlReconnectDomain
Use virDomainObjListRemoveLocked instead of virDomainObjListRemove, as
driver->domains is already taken by virDomainObjListForEach.
Above deadlock can be triggered when libvirtd is started after some
domain have been started by hand (in which case driver will not find
libvirt-xml domain config).
Signed-off-by: Marek Marczykowski-Górecki <marmarek(a)invisiblethingslab.com>
---
src/libxl/libxl_driver.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index df31001..b2cff5b 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -1100,7 +1100,7 @@ libxlReconnectDomain(virDomainObjPtr vm,
out:
libxlVmCleanup(driver, vm, VIR_DOMAIN_SHUTOFF_UNKNOWN);
if (!vm->persistent)
- virDomainObjListRemove(driver->domains, vm);
+ virDomainObjListRemoveLocked(driver->domains, vm);
else
virObjectUnlock(vm);
--
1.8.1.4
3:51 p.m.
New subject: [libvirt] [PATCH v2 07/14] libxl: fix deadlock in libxlReconnectDomain
On 06/12/2013 07:54 PM, Marek Marczykowski-Górecki wrote:
Use virDomainObjListRemoveLocked instead of virDomainObjListRemove,
as
driver->domains is already taken by virDomainObjListForEach.
Above deadlock can be triggered when libvirtd is started after some
domain have been started by hand (in which case driver will not find
libvirt-xml domain config).
Signed-off-by: Marek Marczykowski-Górecki <marmarek(a)invisiblethingslab.com>
---
src/libxl/libxl_driver.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index df31001..b2cff5b 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -1100,7 +1100,7 @@ libxlReconnectDomain(virDomainObjPtr vm,
out:
libxlVmCleanup(driver, vm, VIR_DOMAIN_SHUTOFF_UNKNOWN);
if (!vm->persistent)
- virDomainObjListRemove(driver->domains, vm);
+ virDomainObjListRemoveLocked(driver->domains, vm);
else
virObjectUnlock(vm);
Trivial once 6/14 ACK'ed.
ACK and pushed.
Regards,
Jim
8:54 p.m.
New subject: [libvirt] [PATCH v2 08/14] libxl: initialize device structures
Do not leave uninitialized variables, not all parameters are set in
libxlMake*.
Signed-off-by: Marek Marczykowski-Górecki <marmarek(a)invisiblethingslab.com>
---
src/libxl/libxl_conf.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index e5d8dc5..d654ace 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -522,6 +522,8 @@ libxlMakeDisk(libxlDriverPrivatePtr driver,
virDomainDiskDefPtr l_disk,
libxl_device_disk *x_disk)
{
+ libxl_device_disk_init(x_disk);
+
if (VIR_STRDUP(x_disk->pdev_path, l_disk->src) < 0)
return -1;
@@ -659,6 +661,8 @@ libxlMakeNic(libxlDriverPrivatePtr driver,
* x_nics[i].mtu = 1492;
*/
+ libxl_device_nic_init(x_nic);
+
virMacAddrGetRaw(&l_nic->mac, x_nic->mac);
if (l_nic->model && !STREQ(l_nic->model, "netfront")) {
@@ -756,6 +760,8 @@ libxlMakeVfb(libxlDriverPrivatePtr driver,
virDomainGraphicsDefPtr l_vfb,
libxl_device_vfb *x_vfb)
{
+ libxl_device_vfb_init(x_vfb);
+
switch (l_vfb->type) {
case VIR_DOMAIN_GRAPHICS_TYPE_SDL:
if (libxlMakeSDLInfo(l_vfb, &x_vfb->sdl) < 0)
@@ -859,6 +865,8 @@ libxlBuildDomainConfig(libxlDriverPrivatePtr driver,
virDomainDefPtr def, libxl_domain_config *d_config)
{
+ libxl_domain_config_init(d_config);
+
if (libxlMakeDomCreateInfo(driver, def, &d_config->c_info) < 0)
return -1;
--
1.8.1.4
11:14 a.m.
New subject: [libvirt] [PATCH v2 08/14] libxl: initialize device structures
Marek Marczykowski-Górecki wrote:
Do not leave uninitialized variables, not all parameters are set in
libxlMake*.
Signed-off-by: Marek Marczykowski-Górecki <marmarek(a)invisiblethingslab.com>
---
src/libxl/libxl_conf.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index e5d8dc5..d654ace 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -522,6 +522,8 @@ libxlMakeDisk(libxlDriverPrivatePtr driver,
virDomainDiskDefPtr l_disk,
libxl_device_disk *x_disk)
{
+ libxl_device_disk_init(x_disk);
+
if (VIR_STRDUP(x_disk->pdev_path, l_disk->src) < 0)
return -1;
@@ -659,6 +661,8 @@ libxlMakeNic(libxlDriverPrivatePtr driver,
* x_nics[i].mtu = 1492;
*/
+ libxl_device_nic_init(x_nic);
+
virMacAddrGetRaw(&l_nic->mac, x_nic->mac);
if (l_nic->model && !STREQ(l_nic->model, "netfront")) {
@@ -756,6 +760,8 @@ libxlMakeVfb(libxlDriverPrivatePtr driver,
virDomainGraphicsDefPtr l_vfb,
libxl_device_vfb *x_vfb)
{
+ libxl_device_vfb_init(x_vfb);
This was already being called in libxlMakeVfbList, but agreed it is
better called here. I rebased the patch, removed the extra
libxl_device_vfb_init call in libxlMakeVfbList, and pushed the patch.
Regards,
Jim
+
switch (l_vfb->type) {
case VIR_DOMAIN_GRAPHICS_TYPE_SDL:
if (libxlMakeSDLInfo(l_vfb, &x_vfb->sdl) < 0)
@@ -859,6 +865,8 @@ libxlBuildDomainConfig(libxlDriverPrivatePtr driver,
virDomainDefPtr def, libxl_domain_config *d_config)
{
+ libxl_domain_config_init(d_config);
+
if (libxlMakeDomCreateInfo(driver, def, &d_config->c_info) < 0)
return -1;
12:36 p.m.
New subject: [libvirt] [PATCH v2 08/14] libxl: initialize device structures
Jim Fehlig wrote:
Marek Marczykowski-Górecki wrote:
> Do not leave uninitialized variables, not all parameters are set in
> libxlMake*.
>
> Signed-off-by: Marek Marczykowski-Górecki <marmarek(a)invisiblethingslab.com>
> ---
> src/libxl/libxl_conf.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
> index e5d8dc5..d654ace 100644
> --- a/src/libxl/libxl_conf.c
> +++ b/src/libxl/libxl_conf.c
> @@ -522,6 +522,8 @@ libxlMakeDisk(libxlDriverPrivatePtr driver,
> virDomainDiskDefPtr l_disk,
> libxl_device_disk *x_disk)
> {
> + libxl_device_disk_init(x_disk);
> +
> if (VIR_STRDUP(x_disk->pdev_path, l_disk->src) < 0)
> return -1;
>
> @@ -659,6 +661,8 @@ libxlMakeNic(libxlDriverPrivatePtr driver,
> * x_nics[i].mtu = 1492;
> */
>
> + libxl_device_nic_init(x_nic);
>
Hmm, after committing this I realized the init here clears the devid set
in libxlMakeNicList. It wasn't spotted in my testing since libxl will
provide a sane devid when not specified. In fact, for all the other
devices we allow libxl to determine devid. I'll send a patch to remove
the nic devid assignment. It's trivial, but maybe you can review it -
quick sanity check to ensure I'm not overlooking something.
Regards,
Jim
> +
> virMacAddrGetRaw(&l_nic->mac, x_nic->mac);
>
> if (l_nic->model && !STREQ(l_nic->model, "netfront")) {
> @@ -756,6 +760,8 @@ libxlMakeVfb(libxlDriverPrivatePtr driver,
> virDomainGraphicsDefPtr l_vfb,
> libxl_device_vfb *x_vfb)
> {
> + libxl_device_vfb_init(x_vfb);
>
>
This was already being called in libxlMakeVfbList, but agreed it is
better called here. I rebased the patch, removed the extra
libxl_device_vfb_init call in libxlMakeVfbList, and pushed the patch.
Regards,
Jim
> +
> switch (l_vfb->type) {
> case VIR_DOMAIN_GRAPHICS_TYPE_SDL:
> if (libxlMakeSDLInfo(l_vfb, &x_vfb->sdl) < 0)
> @@ -859,6 +865,8 @@ libxlBuildDomainConfig(libxlDriverPrivatePtr driver,
> virDomainDefPtr def, libxl_domain_config *d_config)
> {
>
> + libxl_domain_config_init(d_config);
> +
> if (libxlMakeDomCreateInfo(driver, def, &d_config->c_info) < 0)
> return -1;
>
>
>
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
8:54 p.m.
New subject: [libvirt] [PATCH v2 09/14] libxl: support paused domain restore in virDomainRestoreFlags
Signed-off-by: Marek Marczykowski-Górecki <marmarek(a)invisiblethingslab.com>
---
src/libxl/libxl_driver.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index b2cff5b..8dec70b 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -2305,7 +2305,7 @@ libxlDomainRestoreFlags(virConnectPtr conn, const char *from,
int fd = -1;
int ret = -1;
- virCheckFlags(0, -1);
+ virCheckFlags(VIR_DOMAIN_SAVE_PAUSED, -1);
if (dxml) {
virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
_("xml modification unsupported"));
@@ -2327,7 +2327,7 @@ libxlDomainRestoreFlags(virConnectPtr conn, const char *from,
def = NULL;
- if ((ret = libxlVmStart(driver, vm, false, fd)) < 0 &&
+ if ((ret = libxlVmStart(driver, vm, (flags & VIR_DOMAIN_SAVE_PAUSED) != 0, fd))
< 0 &&
!vm->persistent) {
virDomainObjListRemove(driver->domains, vm);
vm = NULL;
--
1.8.1.4
1:15 p.m.
New subject: [libvirt] [PATCH v2 09/14] libxl: support paused domain restore in virDomainRestoreFlags
Marek Marczykowski-Górecki wrote:
Signed-off-by: Marek Marczykowski-Górecki
<marmarek(a)invisiblethingslab.com>
---
src/libxl/libxl_driver.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index b2cff5b..8dec70b 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -2305,7 +2305,7 @@ libxlDomainRestoreFlags(virConnectPtr conn, const char *from,
int fd = -1;
int ret = -1;
- virCheckFlags(0, -1);
+ virCheckFlags(VIR_DOMAIN_SAVE_PAUSED, -1);
if (dxml) {
virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
_("xml modification unsupported"));
@@ -2327,7 +2327,7 @@ libxlDomainRestoreFlags(virConnectPtr conn, const char *from,
def = NULL;
- if ((ret = libxlVmStart(driver, vm, false, fd)) < 0 &&
+ if ((ret = libxlVmStart(driver, vm, (flags & VIR_DOMAIN_SAVE_PAUSED) != 0, fd))
< 0 &&
I changed this a bit to fit within 80 columns and pushed.
Regards,
Jim
!vm->persistent) {
virDomainObjListRemove(driver->domains, vm);
vm = NULL;
8:54 p.m.
New subject: [libvirt] [PATCH v2 10/14] libxl: support domain config modification in virDomainRestoreFlags
Signed-off-by: Marek Marczykowski-Górecki <marmarek(a)invisiblethingslab.com>
---
src/libxl/libxl_driver.c | 18 +++++++++++++-----
1 file changed, 13 insertions(+), 5 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 8dec70b..7b50853 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -2306,11 +2306,6 @@ libxlDomainRestoreFlags(virConnectPtr conn, const char *from,
int ret = -1;
virCheckFlags(VIR_DOMAIN_SAVE_PAUSED, -1);
- if (dxml) {
- virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
- _("xml modification unsupported"));
- return -1;
- }
libxlDriverLock(driver);
@@ -2318,6 +2313,19 @@ libxlDomainRestoreFlags(virConnectPtr conn, const char *from,
if (fd < 0)
goto cleanup;
+ if (dxml) {
+ virDomainDefPtr def2 = NULL;
+
+ if (!(def2 = virDomainDefParseString(dxml, driver->caps, driver->xmlopt,
+ 1 << VIR_DOMAIN_VIRT_XEN,
+ VIR_DOMAIN_XML_INACTIVE))) {
+ goto cleanup;
+ }
+ virDomainDefFree(def);
+ def = def2;
+ }
+
+
if (!(vm = virDomainObjListAdd(driver->domains, def,
driver->xmlopt,
VIR_DOMAIN_OBJ_LIST_ADD_LIVE |
--
1.8.1.4
4:25 p.m.
New subject: [libvirt] [PATCH v2 10/14] libxl: support domain config modification in virDomainRestoreFlags
Marek Marczykowski-Górecki wrote:
Signed-off-by: Marek Marczykowski-Górecki
<marmarek(a)invisiblethingslab.com>
---
src/libxl/libxl_driver.c | 18 +++++++++++++-----
1 file changed, 13 insertions(+), 5 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 8dec70b..7b50853 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -2306,11 +2306,6 @@ libxlDomainRestoreFlags(virConnectPtr conn, const char *from,
int ret = -1;
virCheckFlags(VIR_DOMAIN_SAVE_PAUSED, -1);
- if (dxml) {
- virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
- _("xml modification unsupported"));
- return -1;
- }
libxlDriverLock(driver);
@@ -2318,6 +2313,19 @@ libxlDomainRestoreFlags(virConnectPtr conn, const char *from,
if (fd < 0)
goto cleanup;
+ if (dxml) {
What kind of changes are supported in dxml? I tried passing xml that
contained another vif, which caused libxl to fail the restore
libxl: error: libxl_device.c:878:device_backend_callback: unable to add
device with path /local/domain/0/backend/vif/25/0
+ virDomainDefPtr def2 = NULL;
+
+ if (!(def2 = virDomainDefParseString(dxml, driver->caps, driver->xmlopt,
+ 1 << VIR_DOMAIN_VIRT_XEN,
+ VIR_DOMAIN_XML_INACTIVE))) {
+ goto cleanup;
+ }
+ virDomainDefFree(def);
+ def = def2;
But we've already thrown away the original config and replaced it with
config that subsequently fails.
The qemu driver calls virDomainDefCheckABIStability to ensure the new
config is compatible, and IMO we should do the same here.
Regards,
Jim
+ }
+
+
if (!(vm = virDomainObjListAdd(driver->domains, def,
driver->xmlopt,
VIR_DOMAIN_OBJ_LIST_ADD_LIVE |
4:39 p.m.
New subject: [libvirt] [PATCH v2 10/14] libxl: support domain config modification in virDomainRestoreFlags
On 20.06.2013 23:25, Jim Fehlig wrote:
Marek Marczykowski-Górecki wrote:
> Signed-off-by: Marek Marczykowski-Górecki <marmarek(a)invisiblethingslab.com>
> ---
> src/libxl/libxl_driver.c | 18 +++++++++++++-----
> 1 file changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index 8dec70b..7b50853 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -2306,11 +2306,6 @@ libxlDomainRestoreFlags(virConnectPtr conn, const char *from,
> int ret = -1;
>
> virCheckFlags(VIR_DOMAIN_SAVE_PAUSED, -1);
> - if (dxml) {
> - virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
> - _("xml modification unsupported"));
> - return -1;
> - }
>
> libxlDriverLock(driver);
>
> @@ -2318,6 +2313,19 @@ libxlDomainRestoreFlags(virConnectPtr conn, const char *from,
> if (fd < 0)
> goto cleanup;
>
> + if (dxml) {
>
What kind of changes are supported in dxml? I tried passing xml that
contained another vif, which caused libxl to fail the restore
What I use it for is (and is working):
- change name (use the same savefile to create many VMs)
- change IP
- change backend disk image
libxl: error: libxl_device.c:878:device_backend_callback: unable to
add
device with path /local/domain/0/backend/vif/25/0
> + virDomainDefPtr def2 = NULL;
> +
> + if (!(def2 = virDomainDefParseString(dxml, driver->caps,
driver->xmlopt,
> + 1 << VIR_DOMAIN_VIRT_XEN,
> + VIR_DOMAIN_XML_INACTIVE))) {
> + goto cleanup;
> + }
> + virDomainDefFree(def);
> + def = def2;
>
But we've already thrown away the original config and replaced it with
config that subsequently fails.
The qemu driver calls virDomainDefCheckABIStability to ensure the new
config is compatible, and IMO we should do the same here.
I'll take a look at it after my vacation (10.07)...
Regards,
Jim
> + }
> +
> +
> if (!(vm = virDomainObjListAdd(driver->domains, def,
> driver->xmlopt,
> VIR_DOMAIN_OBJ_LIST_ADD_LIVE |
>
--
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
8:54 p.m.
New subject: [libvirt] [PATCH v2 11/14] libxl: support network device attach/detach
Both live and config.
Signed-off-by: Marek Marczykowski-Górecki <marmarek(a)invisiblethingslab.com>
---
src/libxl/libxl_driver.c | 164 ++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 161 insertions(+), 3 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 7b50853..ae0d4f7 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -3417,6 +3417,111 @@ cleanup:
}
static int
+libxlDomainAttachDeviceNetLive(libxlDriverPrivatePtr driver,
+ libxlDomainObjPrivatePtr priv,
+ virDomainObjPtr vm,
+ virDomainDeviceDefPtr dev)
+{
+ virDomainNetDefPtr l_net = dev->data.net;
+ libxl_device_nic x_nic;
+ char mac[VIR_MAC_STRING_BUFLEN];
+ int ret = -1;
+
+ switch (dev->data.net->type) {
+ case VIR_DOMAIN_NET_TYPE_ETHERNET:
+ case VIR_DOMAIN_NET_TYPE_BRIDGE:
+ /* -2 means "multiple matches" so then fail also */
+ if (virDomainNetFindIdx(vm->def, dev->data.net) != -1) {
+ virReportError(VIR_ERR_OPERATION_FAILED,
+ _("device matching mac address %s already exists"),
+ virMacAddrFormat(&dev->data.net->mac, mac));
+ goto cleanup;
+ }
+
+ if (libxlMakeNic(driver, l_net, &x_nic) < 0)
+ goto cleanup;
+
+ if ((ret = libxl_device_nic_add(priv->ctx, vm->def->id,
+ &x_nic, NULL)) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("libxenlight failed to attach net '%s'"),
+ virMacAddrFormat(&dev->data.net->mac, mac));
+ goto cleanup;
+ }
+
+ virDomainNetInsert(vm->def, l_net);
+
+ break;
+ default:
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("net device type '%s' cannot be hotplugged"),
+ virDomainNetTypeToString(l_net->type));
+ break;
+ }
+
+cleanup:
+ return ret;
+}
+
+static int
+libxlDomainDetachDeviceNetLive(libxlDriverPrivatePtr driver,
+ libxlDomainObjPrivatePtr priv,
+ virDomainObjPtr vm,
+ virDomainDeviceDefPtr dev)
+{
+ virDomainNetDefPtr l_net = NULL;
+ libxl_device_nic x_nic;
+ char mac[VIR_MAC_STRING_BUFLEN];
+ int i;
+ int ret = -1;
+
+ switch (dev->data.net->type) {
+ case VIR_DOMAIN_NET_TYPE_ETHERNET:
+ case VIR_DOMAIN_NET_TYPE_BRIDGE:
+ if ((i = virDomainNetFindIdx(vm->def, dev->data.net)) < 0) {
+ if (i == -2) {
+
+ virReportError(VIR_ERR_OPERATION_FAILED,
+ _("multiple devices matching mac address %s
found"),
+ virMacAddrFormat(&dev->data.net->mac, mac));
+ }
+ else {
+ virReportError(VIR_ERR_OPERATION_FAILED,
+ _("network device %s not found"),
+ virMacAddrFormat(&dev->data.net->mac, mac));
+ }
+ goto cleanup;
+ }
+
+ l_net = vm->def->nets[i];
+
+ if (libxlMakeNic(driver, l_net, &x_nic) < 0)
+ goto cleanup;
+
+ if ((ret = libxl_device_nic_remove(priv->ctx, vm->def->id,
+ &x_nic, NULL)) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("libxenlight failed to detach nic '%d'"),
+ i);
+ goto cleanup;
+ }
+
+ virDomainNetRemove(vm->def, i);
+ virDomainNetDefFree(l_net);
+
+ break;
+ default:
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("net device type '%s' cannot hot
unplugged"),
+ virDomainNetTypeToString(dev->data.net->type));
+ break;
+ }
+
+cleanup:
+ return ret;
+}
+
+static int
libxlDomainAttachDeviceLive(libxlDriverPrivatePtr driver,
libxlDomainObjPrivatePtr priv, virDomainObjPtr vm,
virDomainDeviceDefPtr dev)
@@ -3430,6 +3535,12 @@ libxlDomainAttachDeviceLive(libxlDriverPrivatePtr driver,
dev->data.disk = NULL;
break;
+ case VIR_DOMAIN_DEVICE_NET:
+ ret = libxlDomainAttachDeviceNetLive(driver, priv, vm, dev);
+ if (!ret)
+ dev->data.net = NULL;
+ break;
+
default:
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("device type '%s' cannot be attached"),
@@ -3444,6 +3555,8 @@ static int
libxlDomainAttachDeviceConfig(virDomainDefPtr vmdef, virDomainDeviceDefPtr dev)
{
virDomainDiskDefPtr disk;
+ virDomainNetDefPtr net;
+ char mac[VIR_MAC_STRING_BUFLEN];
switch (dev->type) {
case VIR_DOMAIN_DEVICE_DISK:
@@ -3461,6 +3574,22 @@ libxlDomainAttachDeviceConfig(virDomainDefPtr vmdef,
virDomainDeviceDefPtr dev)
dev->data.disk = NULL;
break;
+ case VIR_DOMAIN_DEVICE_NET:
+ net = dev->data.net;
+ if (virDomainNetFindIdx(vmdef, net) >= 0) {
+ virReportError(VIR_ERR_INVALID_ARG,
+ _("net device with mac %s already exists."),
+ virMacAddrFormat(&net->mac, mac));
+ return -1;
+ }
+ if (virDomainNetInsert(vmdef, net)) {
+ virReportOOMError();
+ return -1;
+ }
+ /* vmdef has the pointer. Generic codes for vmdef will do all jobs */
+ dev->data.net = NULL;
+ break;
+
default:
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("persistent attach of device is not supported"));
@@ -3482,6 +3611,10 @@ libxlDomainDetachDeviceLive(libxlDriverPrivatePtr driver,
ret = libxlDomainDetachDeviceDiskLive(driver, priv, vm, dev);
break;
+ case VIR_DOMAIN_DEVICE_NET:
+ ret = libxlDomainDetachDeviceNetLive(driver, priv, vm, dev);
+ break;
+
default:
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("device type '%s' cannot be detached"),
@@ -3495,20 +3628,45 @@ libxlDomainDetachDeviceLive(libxlDriverPrivatePtr driver,
static int
libxlDomainDetachDeviceConfig(virDomainDefPtr vmdef, virDomainDeviceDefPtr dev)
{
- virDomainDiskDefPtr disk, detach;
+ virDomainDiskDefPtr disk, disk_detach;
+ virDomainNetDefPtr net, net_detach;
+ char mac[VIR_MAC_STRING_BUFLEN];
+ int net_idx;
int ret = -1;
switch (dev->type) {
case VIR_DOMAIN_DEVICE_DISK:
disk = dev->data.disk;
- if (!(detach = virDomainDiskRemoveByName(vmdef, disk->dst))) {
+ if (!(disk_detach = virDomainDiskRemoveByName(vmdef, disk->dst))) {
virReportError(VIR_ERR_INVALID_ARG,
_("no target device %s"), disk->dst);
break;
}
- virDomainDiskDefFree(detach);
+ virDomainDiskDefFree(disk_detach);
+ ret = 0;
+ break;
+
+ case VIR_DOMAIN_DEVICE_NET:
+ net = dev->data.net;
+ if ((net_idx = virDomainNetFindIdx(vmdef, net)) < 0) {
+ if (net_idx == -2) {
+
+ virReportError(VIR_ERR_OPERATION_FAILED,
+ _("multiple devices matching mac address %s
found"),
+ virMacAddrFormat(&dev->data.net->mac, mac));
+ }
+ else {
+ virReportError(VIR_ERR_OPERATION_FAILED,
+ _("network device %s not found"),
+ virMacAddrFormat(&dev->data.net->mac, mac));
+ }
+ return -1;
+ }
+ net_detach = virDomainNetRemove(vmdef, net_idx);
+ virDomainNetDefFree(net_detach);
ret = 0;
break;
+
default:
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("persistent detach of device is not supported"));
--
1.8.1.4
4:43 p.m.
New subject: [libvirt] [PATCH v2 11/14] libxl: support network device attach/detach
On 06/12/2013 07:54 PM, Marek Marczykowski-Górecki wrote:
Both live and config.
Signed-off-by: Marek Marczykowski-Górecki <marmarek(a)invisiblethingslab.com>
---
src/libxl/libxl_driver.c | 164 ++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 161 insertions(+), 3 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 7b50853..ae0d4f7 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -3417,6 +3417,111 @@ cleanup:
}
static int
+libxlDomainAttachDeviceNetLive(libxlDriverPrivatePtr driver,
+ libxlDomainObjPrivatePtr priv,
+ virDomainObjPtr vm,
+ virDomainDeviceDefPtr dev)
+{
+ virDomainNetDefPtr l_net = dev->data.net;
+ libxl_device_nic x_nic;
+ char mac[VIR_MAC_STRING_BUFLEN];
+ int ret = -1;
+
+ switch (dev->data.net->type) {
l_net->type ?
+ case VIR_DOMAIN_NET_TYPE_ETHERNET:
+ case VIR_DOMAIN_NET_TYPE_BRIDGE:
+ /* -2 means "multiple matches" so then fail also */
+ if (virDomainNetFindIdx(vm->def, dev->data.net) != -1) {
And just pass l_net here. Also, you should check for -2. I think this is
better coded as
idx = virDomainNetFindIdx(vm->def, l_net);
if (idx == -2) {
virReportError(VIR_ERR_OPERATION_FAILED,
_("multiple devices matching mac address %s
found"),
virMacAddrFormat(&net->mac, mac));
return -1;
} else if (idx < 0) {
virReportError(VIR_ERR_OPERATION_FAILED, "%s",
_("no matching network device was found"));
return -1;
}
+ virReportError(VIR_ERR_OPERATION_FAILED,
+ _("device matching mac address %s already exists"),
We should use the same error message as the qemu driver, which is included in my
snippet above.
+
virMacAddrFormat(&dev->data.net->mac, mac));
+ goto cleanup;
+ }
+
+ if (libxlMakeNic(driver, l_net, &x_nic) < 0)
+ goto cleanup;
+
+ if ((ret = libxl_device_nic_add(priv->ctx, vm->def->id,
+ &x_nic, NULL)) < 0) {
That indentation looks a little odd. Perhaps best to split into 2 lines
ret = libxl_deivce_nic_add(...);
if (ret < 0)
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("libxenlight failed to attach net '%s'"),
+ virMacAddrFormat(&dev->data.net->mac, mac));
+ goto cleanup;
+ }
+
+ virDomainNetInsert(vm->def, l_net);
+
+ break;
+ default:
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("net device type '%s' cannot be hotplugged"),
s/net/network/
+ virDomainNetTypeToString(l_net->type));
+ break;
+ }
+
+cleanup:
+ return ret;
Nothing to cleanup. Might as well just return errors where they occur or
success when done.
+}
+
+static int
+libxlDomainDetachDeviceNetLive(libxlDriverPrivatePtr driver,
+ libxlDomainObjPrivatePtr priv,
+ virDomainObjPtr vm,
+ virDomainDeviceDefPtr dev)
+{
+ virDomainNetDefPtr l_net = NULL;
+ libxl_device_nic x_nic;
+ char mac[VIR_MAC_STRING_BUFLEN];
+ int i;
+ int ret = -1;
+
+ switch (dev->data.net->type) {
+ case VIR_DOMAIN_NET_TYPE_ETHERNET:
+ case VIR_DOMAIN_NET_TYPE_BRIDGE:
+ if ((i = virDomainNetFindIdx(vm->def, dev->data.net)) < 0) {
+ if (i == -2) {
+
+ virReportError(VIR_ERR_OPERATION_FAILED,
+ _("multiple devices matching mac address %s
found"),
+ virMacAddrFormat(&dev->data.net->mac, mac));
+ }
+ else {
+ virReportError(VIR_ERR_OPERATION_FAILED,
+ _("network device %s not found"),
+ virMacAddrFormat(&dev->data.net->mac, mac));
+ }
+ goto cleanup;
+ }
For consistency, the error checking logic here should be changed to match the
Attach function.
+
+ l_net = vm->def->nets[i];
+
+ if (libxlMakeNic(driver, l_net, &x_nic) < 0)
+ goto cleanup;
+
+ if ((ret = libxl_device_nic_remove(priv->ctx, vm->def->id,
+ &x_nic, NULL)) < 0) {
Split into 2 lines?
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("libxenlight failed to detach nic '%d'"),
+ i);
+ goto cleanup;
+ }
+
+ virDomainNetRemove(vm->def, i);
+ virDomainNetDefFree(l_net);
+
+ break;
+ default:
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("net device type '%s' cannot hot
unplugged"),
s/net/network/
+
virDomainNetTypeToString(dev->data.net->type));
+ break;
+ }
+
+cleanup:
+ return ret;
Nothing to cleanup.
+}
+
+static int
libxlDomainAttachDeviceLive(libxlDriverPrivatePtr driver,
libxlDomainObjPrivatePtr priv, virDomainObjPtr vm,
virDomainDeviceDefPtr dev)
@@ -3430,6 +3535,12 @@ libxlDomainAttachDeviceLive(libxlDriverPrivatePtr driver,
dev->data.disk = NULL;
break;
+ case VIR_DOMAIN_DEVICE_NET:
+ ret = libxlDomainAttachDeviceNetLive(driver, priv, vm, dev);
+ if (!ret)
+ dev->data.net = NULL;
+ break;
+
default:
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("device type '%s' cannot be attached"),
@@ -3444,6 +3555,8 @@ static int
libxlDomainAttachDeviceConfig(virDomainDefPtr vmdef, virDomainDeviceDefPtr dev)
{
virDomainDiskDefPtr disk;
+ virDomainNetDefPtr net;
+ char mac[VIR_MAC_STRING_BUFLEN];
switch (dev->type) {
case VIR_DOMAIN_DEVICE_DISK:
@@ -3461,6 +3574,22 @@ libxlDomainAttachDeviceConfig(virDomainDefPtr vmdef,
virDomainDeviceDefPtr dev)
dev->data.disk = NULL;
break;
+ case VIR_DOMAIN_DEVICE_NET:
+ net = dev->data.net;
+ if (virDomainNetFindIdx(vmdef, net) >= 0) {
+ virReportError(VIR_ERR_INVALID_ARG,
+ _("net device with mac %s already exists."),
Reuse existing error message.
+ virMacAddrFormat(&net->mac,
mac));
+ return -1;
+ }
+ if (virDomainNetInsert(vmdef, net)) {
+ virReportOOMError();
+ return -1;
+ }
+ /* vmdef has the pointer. Generic codes for vmdef will do all jobs */
+ dev->data.net = NULL;
+ break;
+
default:
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("persistent attach of device is not
supported"));
@@ -3482,6 +3611,10 @@ libxlDomainDetachDeviceLive(libxlDriverPrivatePtr driver,
ret = libxlDomainDetachDeviceDiskLive(driver, priv, vm, dev);
break;
+ case VIR_DOMAIN_DEVICE_NET:
+ ret = libxlDomainDetachDeviceNetLive(driver, priv, vm, dev);
+ break;
+
default:
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("device type '%s' cannot be detached"),
@@ -3495,20 +3628,45 @@ libxlDomainDetachDeviceLive(libxlDriverPrivatePtr driver,
static int
libxlDomainDetachDeviceConfig(virDomainDefPtr vmdef, virDomainDeviceDefPtr dev)
{
- virDomainDiskDefPtr disk, detach;
+ virDomainDiskDefPtr disk, disk_detach;
+ virDomainNetDefPtr net, net_detach;
+ char mac[VIR_MAC_STRING_BUFLEN];
+ int net_idx;
int ret = -1;
switch (dev->type) {
case VIR_DOMAIN_DEVICE_DISK:
disk = dev->data.disk;
- if (!(detach = virDomainDiskRemoveByName(vmdef, disk->dst))) {
+ if (!(disk_detach = virDomainDiskRemoveByName(vmdef, disk->dst))) {
virReportError(VIR_ERR_INVALID_ARG,
_("no target device %s"), disk->dst);
break;
}
- virDomainDiskDefFree(detach);
+ virDomainDiskDefFree(disk_detach);
+ ret = 0;
+ break;
+
+ case VIR_DOMAIN_DEVICE_NET:
+ net = dev->data.net;
+ if ((net_idx = virDomainNetFindIdx(vmdef, net)) < 0) {
+ if (net_idx == -2) {
+
+ virReportError(VIR_ERR_OPERATION_FAILED,
+ _("multiple devices matching mac address %s
found"),
+ virMacAddrFormat(&dev->data.net->mac, mac));
+ }
+ else {
+ virReportError(VIR_ERR_OPERATION_FAILED,
+ _("network device %s not found"),
+ virMacAddrFormat(&dev->data.net->mac, mac));
+ }
+ return -1;
+ }
Again for consistency, same error checking logic here.
Regards,
Jim
+ net_detach = virDomainNetRemove(vmdef, net_idx);
+ virDomainNetDefFree(net_detach);
ret = 0;
break;
+
default:
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("persistent detach of device is not
supported"));
8:54 p.m.
New subject: [libvirt] [PATCH v2 12/14] libxl: pass ipaddr to libxl toolstack
Do not silently ignore its value.
Signed-off-by: Marek Marczykowski-Górecki <marmarek(a)invisiblethingslab.com>
---
src/libxl/libxl_conf.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index d654ace..278c5e5 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -684,6 +684,8 @@ libxlMakeNic(libxlDriverPrivatePtr driver,
case VIR_DOMAIN_NET_TYPE_ETHERNET:
if (VIR_STRDUP(x_nic->script, l_nic->script) < 0)
return -1;
+ if (VIR_STRDUP(x_nic->ip, l_nic->data.bridge.ipaddr) < 0)
+ return -1;
break;
default:
virReportError(VIR_ERR_INTERNAL_ERROR,
--
1.8.1.4
5:26 p.m.
New subject: [libvirt] [PATCH v2 12/14] libxl: pass ipaddr to libxl toolstack
On 06/12/2013 07:54 PM, Marek Marczykowski-Górecki wrote:
Do not silently ignore its value.
Signed-off-by: Marek Marczykowski-Górecki <marmarek(a)invisiblethingslab.com>
---
src/libxl/libxl_conf.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index d654ace..278c5e5 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -684,6 +684,8 @@ libxlMakeNic(libxlDriverPrivatePtr driver,
case VIR_DOMAIN_NET_TYPE_ETHERNET:
if (VIR_STRDUP(x_nic->script, l_nic->script) < 0)
return -1;
+ if (VIR_STRDUP(x_nic->ip, l_nic->data.bridge.ipaddr) < 0)
If the type is ETHERNET, shouldn't this be l_nic->data.ethernet.ipaddr ? Does
this apply to type BRIDGE as well?
Regards,
Jim
+ return -1;
break;
default:
virReportError(VIR_ERR_INTERNAL_ERROR,
8:54 p.m.
New subject: [libvirt] [PATCH v2 13/14] libxl: implement lifecycle actions from domain config
Instead of hardcoded actions for poweroff, reboot and crash.
Known limitations:
1. Crash actions not fully implemented (namely no coredump done).
2. VIR_DOMAIN_LIFECYCLE_PRESERVE emit VIR_DOMAIN_EVENT_STOPPED_SHUTDOWN
event but do not mark domain object as stopped. So effectively use of
'preserve' can cause Weird Effects (tm).
Signed-off-by: Marek Marczykowski-Górecki <marmarek(a)invisiblethingslab.com>
---
src/libxl/libxl_driver.c | 54 +++++++++++++++++++++++++++++++++++++++---------
1 file changed, 44 insertions(+), 10 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 448db73..04142bb 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -721,7 +721,8 @@ libxlEventHandler(void *data, VIR_LIBXL_EVENT_CONST libxl_event
*event)
libxl_shutdown_reason xl_reason = event->u.domain_shutdown.shutdown_reason;
if (event->type == LIBXL_EVENT_TYPE_DOMAIN_SHUTDOWN) {
- virDomainShutoffReason reason;
+ virDomainShutoffReason reason = VIR_DOMAIN_SHUTOFF_SHUTDOWN;
+ int action = VIR_DOMAIN_LIFECYCLE_DESTROY;
/*
* Similar to the xl implementation, ignore SUSPEND. Any actions needed
@@ -739,27 +740,60 @@ libxlEventHandler(void *data, VIR_LIBXL_EVENT_CONST libxl_event
*event)
switch (xl_reason) {
case LIBXL_SHUTDOWN_REASON_POWEROFF:
+ reason = VIR_DOMAIN_SHUTOFF_SHUTDOWN;
+ action = vm->def->onPoweroff;
+ break;
case LIBXL_SHUTDOWN_REASON_CRASH:
- if (xl_reason == LIBXL_SHUTDOWN_REASON_CRASH) {
- dom_event = virDomainEventNewFromObj(vm,
- VIR_DOMAIN_EVENT_STOPPED,
- VIR_DOMAIN_EVENT_STOPPED_CRASHED);
- reason = VIR_DOMAIN_SHUTOFF_CRASHED;
- } else {
- reason = VIR_DOMAIN_SHUTOFF_SHUTDOWN;
+ dom_event = virDomainEventNewFromObj(vm,
+ VIR_DOMAIN_EVENT_STOPPED,
+ VIR_DOMAIN_EVENT_STOPPED_CRASHED);
+ reason = VIR_DOMAIN_SHUTOFF_CRASHED;
+ switch (vm->def->onCrash) {
+ case VIR_DOMAIN_LIFECYCLE_CRASH_COREDUMP_DESTROY:
+ /* TODO: coredump here */
+ VIR_ERROR("Coredump on crash not supported yet");
+ action = VIR_DOMAIN_LIFECYCLE_DESTROY;
+ break;
+ case VIR_DOMAIN_LIFECYCLE_CRASH_COREDUMP_RESTART:
+ /* TODO: coredump here */
+ VIR_ERROR("Coredump on crash not supported yet");
+ action = VIR_DOMAIN_LIFECYCLE_RESTART;
+ break;
+ default:
+ action = vm->def->onCrash;
+ break;
}
+ break;
+ case LIBXL_SHUTDOWN_REASON_REBOOT:
+ reason = VIR_DOMAIN_SHUTOFF_SHUTDOWN;
+ action = vm->def->onReboot;
+ break;
+ default:
+ VIR_INFO("Unhandled shutdown_reason %d", xl_reason);
+ break;
+ }
+ switch (action) {
+ case VIR_DOMAIN_LIFECYCLE_DESTROY:
libxlVmReap(driver, vm, reason);
if (!vm->persistent) {
virDomainObjListRemove(driver->domains, vm);
vm = NULL;
}
break;
- case LIBXL_SHUTDOWN_REASON_REBOOT:
+ case VIR_DOMAIN_LIFECYCLE_RESTART:
+ case VIR_DOMAIN_LIFECYCLE_RESTART_RENAME:
libxlVmReap(driver, vm, VIR_DOMAIN_SHUTOFF_SHUTDOWN);
libxlVmStart(driver, vm, 0, -1);
break;
+ case VIR_DOMAIN_LIFECYCLE_PRESERVE:
+ if (xl_reason != LIBXL_SHUTDOWN_REASON_CRASH) {
+ dom_event = virDomainEventNewFromObj(vm,
+ VIR_DOMAIN_EVENT_STOPPED,
+ VIR_DOMAIN_EVENT_STOPPED_SHUTDOWN);
+ }
+ break;
default:
- VIR_INFO("Unhandled shutdown_reason %d", xl_reason);
+ VIR_INFO("Unsupported shutdown action: %d", action);
break;
}
}
--
1.8.1.4
8:54 p.m.
New subject: [libvirt] [PATCH v2 14/14] libxl: add tablet/mouse input device support
Signed-off-by: Marek Marczykowski-Górecki <marmarek(a)invisiblethingslab.com>
---
src/libxl/libxl_conf.c | 30 ++++++++++++++++++++++++++++++
1 file changed, 30 insertions(+)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 278c5e5..61c370f 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -481,6 +481,36 @@ libxlMakeDomBuildInfo(libxlDriverPrivatePtr driver,
}
}
+ /* Only the first one is used - b_info->u.hvm.usbdevice doesn't have
+ * space for more (until xen 4.3) */
+ if (def->ninputs) {
+ if (def->ninputs > 1) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("This driver supports only one input device"));
+ goto error;
+ }
+ if (def->inputs[0]->bus != VIR_DOMAIN_INPUT_BUS_USB) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("libxenlight support only USB input for now"));
+ goto error;
+ }
+
+ switch (def->inputs[0]->type) {
+ case VIR_DOMAIN_INPUT_TYPE_MOUSE:
+ free(b_info->u.hvm.usbdevice);
+ b_info->u.hvm.usbdevice = strdup("mouse");
+ break;
+ case VIR_DOMAIN_INPUT_TYPE_TABLET:
+ free(b_info->u.hvm.usbdevice);
+ b_info->u.hvm.usbdevice = strdup("tablet");
+ break;
+ default:
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("Unknown input device type"));
+ goto error;
+ }
+ }
+
/*
* The following comment and calculation were taken directly from
* libxenlight's internal function libxl_get_required_shadow_memory():
--
1.8.1.4
5:38 p.m.
New subject: [libvirt] [PATCH v2 14/14] libxl: add tablet/mouse input device support
On 06/12/2013 07:54 PM, Marek Marczykowski-Górecki wrote:
Signed-off-by: Marek Marczykowski-Górecki
<marmarek(a)invisiblethingslab.com>
---
src/libxl/libxl_conf.c | 30 ++++++++++++++++++++++++++++++
1 file changed, 30 insertions(+)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 278c5e5..61c370f 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -481,6 +481,36 @@ libxlMakeDomBuildInfo(libxlDriverPrivatePtr driver,
}
}
+ /* Only the first one is used - b_info->u.hvm.usbdevice doesn't have
+ * space for more (until xen 4.3) */
Do you mean xen 4.4? 4.3 is done.
+ if (def->ninputs) {
+ if (def->ninputs > 1) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("This driver supports only one input device"));
I suggest "libxenlight only supports one input device".
+ goto error;
+ }
+ if (def->inputs[0]->bus != VIR_DOMAIN_INPUT_BUS_USB) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("libxenlight support only USB input for now"));
And here, "libxenlight only supports USB input devices".
+ goto error;
+ }
+
+ switch (def->inputs[0]->type) {
+ case VIR_DOMAIN_INPUT_TYPE_MOUSE:
+ free(b_info->u.hvm.usbdevice);
VIR_FREE().
+ b_info->u.hvm.usbdevice =
strdup("mouse");
VIR_STRDUP().
+ break;
+ case VIR_DOMAIN_INPUT_TYPE_TABLET:
+ free(b_info->u.hvm.usbdevice);
+ b_info->u.hvm.usbdevice = strdup("tablet");
Same here.
Regards,
Jim
+ break;
+ default:
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("Unknown input device type"));
+ goto error;
+ }
+ }
+
/*
* The following comment and calculation were taken directly from
* libxenlight's internal function libxl_get_required_shadow_memory():
4192
days inactive
4212
days old
34 comments
2 participants
participants (2)
-
Jim Fehlig -
Marek Marczykowski-Górecki