4:12 p.m.
My main objective with this series was to decompose libxlDomainStart a bit,
as it had become quite unwieldy. Along the way I made a few other small
improvements to the code.
Jim Fehlig (6):
libxl: Drop unused 'cfg' parameter from libxlDomainSaveImageOpen
libxl: Move managed save logic to libxlDomainStartNew
libxl: Add a helper function to unprepare network devices
libxl: Introduce libxlDomainStartPrepare
libxl: Introduce libxlDomainStartPerform
libxl: Add helper function for running the hook script
src/libxl/libxl_domain.c | 384 ++++++++++++++++++------------------
src/libxl/libxl_domain.h | 11 +-
src/libxl/libxl_driver.c | 30 +--
src/libxl/libxl_migration.c | 15 +-
4 files changed, 218 insertions(+), 222 deletions(-)
--
2.31.1
4:12 p.m.
New subject: [PATCH 1/6] libxl: Drop unused 'cfg' parameter from libxlDomainSaveImageOpen
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/libxl/libxl_domain.c | 4 +---
src/libxl/libxl_domain.h | 3 +--
src/libxl/libxl_driver.c | 4 +---
3 files changed, 3 insertions(+), 8 deletions(-)
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index af20434843..336b67b129 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -764,7 +764,6 @@ libxlDomainManagedSavePath(libxlDriverPrivate *driver, virDomainObj
*vm)
*/
int
libxlDomainSaveImageOpen(libxlDriverPrivate *driver,
- libxlDriverConfig *cfg G_GNUC_UNUSED,
const char *from,
virDomainDef **ret_def,
libxlSavefileHeader *ret_hdr)
@@ -1260,8 +1259,7 @@ libxlDomainStart(libxlDriverPrivate *driver,
if (virFileExists(managed_save_path)) {
- managed_save_fd = libxlDomainSaveImageOpen(driver, cfg,
- managed_save_path,
+ managed_save_fd = libxlDomainSaveImageOpen(driver, managed_save_path,
&def, &hdr);
if (managed_save_fd < 0)
goto cleanup;
diff --git a/src/libxl/libxl_domain.h b/src/libxl/libxl_domain.h
index 928ee8f5cd..526c8e7332 100644
--- a/src/libxl/libxl_domain.h
+++ b/src/libxl/libxl_domain.h
@@ -103,11 +103,10 @@ libxlDomainManagedSavePath(libxlDriverPrivate *driver,
int
libxlDomainSaveImageOpen(libxlDriverPrivate *driver,
- libxlDriverConfig *cfg,
const char *from,
virDomainDef **ret_def,
libxlSavefileHeader *ret_hdr)
- ATTRIBUTE_NONNULL(4) ATTRIBUTE_NONNULL(5);
+ ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4);
int
libxlDomainDestroyInternal(libxlDriverPrivate *driver,
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index c97b2fb485..838747a1e3 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -1942,7 +1942,6 @@ libxlDomainRestoreFlags(virConnectPtr conn, const char *from,
const char *dxml, unsigned int flags)
{
libxlDriverPrivate *driver = conn->privateData;
- libxlDriverConfig *cfg = libxlDriverConfigGet(driver);
virDomainObj *vm = NULL;
virDomainDef *def = NULL;
libxlSavefileHeader hdr;
@@ -1961,7 +1960,7 @@ libxlDomainRestoreFlags(virConnectPtr conn, const char *from,
return -1;
}
- fd = libxlDomainSaveImageOpen(driver, cfg, from, &def, &hdr);
+ fd = libxlDomainSaveImageOpen(driver, from, &def, &hdr);
if (fd < 0)
goto cleanup;
@@ -1995,7 +1994,6 @@ libxlDomainRestoreFlags(virConnectPtr conn, const char *from,
virReportSystemError(errno, "%s", _("cannot close file"));
virDomainDefFree(def);
virDomainObjEndAPI(&vm);
- virObjectUnref(cfg);
return ret;
}
--
2.31.1
4:12 p.m.
New subject: [PATCH 2/6] libxl: Move managed save logic to libxlDomainStartNew
the logic to check for existence of a managed save image and use it to
start the VM can be moved to libxlDomainStartNew. libxlDomainStart has
become unwieldy and this is a small step to make it more manageable.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/libxl/libxl_domain.c | 97 ++++++++++++++++++++--------------------
1 file changed, 49 insertions(+), 48 deletions(-)
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index 336b67b129..e906495b64 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -1230,14 +1230,10 @@ libxlDomainStart(libxlDriverPrivate *driver,
uint32_t restore_ver)
{
libxl_domain_config d_config;
- virDomainDef *def = NULL;
virObjectEvent *event = NULL;
- libxlSavefileHeader hdr;
int ret = -1;
uint32_t domid = 0;
g_autofree char *dom_xml = NULL;
- g_autofree char *managed_save_path = NULL;
- int managed_save_fd = -1;
libxlDomainObjPrivate *priv = vm->privateData;
g_autoptr(libxlDriverConfig) cfg = libxlDriverConfigGet(driver);
virHostdevManager *hostdev_mgr = driver->hostdevMgr;
@@ -1250,47 +1246,6 @@ libxlDomainStart(libxlDriverPrivate *driver,
libxl_domain_config_init(&d_config);
- /* If there is a managed saved state restore it instead of starting
- * from scratch. The old state is removed once the restoring succeeded. */
- if (restore_fd < 0) {
- managed_save_path = libxlDomainManagedSavePath(driver, vm);
- if (managed_save_path == NULL)
- goto cleanup;
-
- if (virFileExists(managed_save_path)) {
-
- managed_save_fd = libxlDomainSaveImageOpen(driver, managed_save_path,
- &def, &hdr);
- if (managed_save_fd < 0)
- goto cleanup;
-
- restore_fd = managed_save_fd;
- restore_ver = hdr.version;
-
- if (STRNEQ(vm->def->name, def->name) ||
- memcmp(vm->def->uuid, def->uuid, VIR_UUID_BUFLEN)) {
- char vm_uuidstr[VIR_UUID_STRING_BUFLEN];
- char def_uuidstr[VIR_UUID_STRING_BUFLEN];
- virUUIDFormat(vm->def->uuid, vm_uuidstr);
- virUUIDFormat(def->uuid, def_uuidstr);
- virReportError(VIR_ERR_OPERATION_FAILED,
- _("cannot restore domain '%s' uuid %s from a
file"
- " which belongs to domain '%s' uuid
%s"),
- vm->def->name, vm_uuidstr, def->name,
def_uuidstr);
- goto cleanup;
- }
-
- virDomainObjAssignDef(vm, def, true, NULL);
- def = NULL;
-
- if (unlink(managed_save_path) < 0)
- VIR_WARN("Failed to remove the managed state %s",
- managed_save_path);
-
- vm->hasManagedSave = false;
- }
- }
-
if (virDomainObjSetDefTransient(driver->xmlopt, vm, NULL) < 0)
goto cleanup;
@@ -1476,8 +1431,6 @@ libxlDomainStart(libxlDriverPrivate *driver,
cleanup:
libxl_domain_config_dispose(&d_config);
- virDomainDefFree(def);
- VIR_FORCE_CLOSE(managed_save_fd);
return ret;
}
@@ -1486,7 +1439,55 @@ libxlDomainStartNew(libxlDriverPrivate *driver,
virDomainObj *vm,
bool start_paused)
{
- return libxlDomainStart(driver, vm, start_paused, -1, LIBXL_SAVE_VERSION);
+ g_autofree char *managed_save_path = NULL;
+ int restore_fd = -1;
+ virDomainDef *def = NULL;
+ libxlSavefileHeader hdr;
+ uint32_t restore_ver = LIBXL_SAVE_VERSION;
+ int ret = -1;
+
+ /* If there is a managed saved state restore it instead of starting
+ * from scratch. The old state is removed once the restoring succeeded. */
+ managed_save_path = libxlDomainManagedSavePath(driver, vm);
+ if (managed_save_path == NULL)
+ return -1;
+
+ if (virFileExists(managed_save_path)) {
+ restore_fd = libxlDomainSaveImageOpen(driver, managed_save_path,
+ &def, &hdr);
+ if (restore_fd < 0)
+ goto cleanup;
+
+ restore_ver = hdr.version;
+
+ if (STRNEQ(vm->def->name, def->name) ||
+ memcmp(vm->def->uuid, def->uuid, VIR_UUID_BUFLEN)) {
+ char vm_uuidstr[VIR_UUID_STRING_BUFLEN];
+ char def_uuidstr[VIR_UUID_STRING_BUFLEN];
+ virUUIDFormat(vm->def->uuid, vm_uuidstr);
+ virUUIDFormat(def->uuid, def_uuidstr);
+ virReportError(VIR_ERR_OPERATION_FAILED,
+ _("cannot restore domain '%s' uuid %s from a file
which belongs to domain '%s' uuid %s"),
+ vm->def->name, vm_uuidstr, def->name, def_uuidstr);
+ goto cleanup;
+ }
+
+ virDomainObjAssignDef(vm, def, true, NULL);
+ def = NULL;
+
+ if (unlink(managed_save_path) < 0)
+ VIR_WARN("Failed to remove the managed state %s",
+ managed_save_path);
+
+ vm->hasManagedSave = false;
+ }
+
+ ret = libxlDomainStart(driver, vm, start_paused, restore_fd, restore_ver);
+
+ cleanup:
+ virDomainDefFree(def);
+ VIR_FORCE_CLOSE(restore_fd);
+ return ret;
}
int
--
2.31.1
4:12 p.m.
New subject: [PATCH 3/6] libxl: Add a helper function to unprepare network devices
Move network device cleanup code from libxlDomainCleanup to a helper
function for use in a subsequent patch.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/libxl/libxl_domain.c | 49 +++++++++++++++++++++++-----------------
1 file changed, 28 insertions(+), 21 deletions(-)
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index e906495b64..f957c29d0d 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -826,6 +826,33 @@ libxlDomainSaveImageOpen(libxlDriverPrivate *driver,
return -1;
}
+static void
+libxlNetworkUnwindDevices(virDomainDef *def)
+{
+ if (def->nnets) {
+ size_t i;
+
+ for (i = 0; i < def->nnets; i++) {
+ virDomainNetDef *net = def->nets[i];
+
+ if (net->ifname &&
+ STRPREFIX(net->ifname, LIBXL_GENERATED_PREFIX_XEN))
+ VIR_FREE(net->ifname);
+
+ /* cleanup actual device */
+ virDomainNetRemoveHostdev(def, net);
+ if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK) {
+ g_autoptr(virConnect) conn = virGetConnectNetwork();
+
+ if (conn)
+ virDomainNetReleaseActualDevice(conn, def, net);
+ else
+ VIR_WARN("Unable to release network device '%s'",
NULLSTR(net->ifname));
+ }
+ }
+ }
+}
+
/*
* Internal domain destroy function.
*
@@ -923,29 +950,9 @@ libxlDomainCleanup(libxlDriverPrivate *driver,
}
}
- if ((vm->def->nnets)) {
- size_t i;
-
- for (i = 0; i < vm->def->nnets; i++) {
- virDomainNetDef *net = vm->def->nets[i];
-
- if (net->ifname &&
- STRPREFIX(net->ifname, LIBXL_GENERATED_PREFIX_XEN))
- VIR_FREE(net->ifname);
-
- /* cleanup actual device */
- virDomainNetRemoveHostdev(vm->def, net);
- if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK) {
- if (conn || (conn = virGetConnectNetwork()))
- virDomainNetReleaseActualDevice(conn, vm->def, net);
- else
- VIR_WARN("Unable to release network device '%s'",
NULLSTR(net->ifname));
- }
- }
- }
+ libxlNetworkUnwindDevices(vm->def);
file = g_strdup_printf("%s/%s.xml", cfg->stateDir,
vm->def->name);
-
if (unlink(file) < 0 && errno != ENOENT && errno != ENOTDIR)
VIR_DEBUG("Failed to remove domain XML for %s", vm->def->name);
VIR_FREE(file);
--
2.31.1
8:53 a.m.
New subject: [PATCH 3/6] libxl: Add a helper function to unprepare network devices
On a %A in %Y, Jim Fehlig wrote:
Move network device cleanup code from libxlDomainCleanup to a helper
function for use in a subsequent patch.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/libxl/libxl_domain.c | 49 +++++++++++++++++++++++-----------------
1 file changed, 28 insertions(+), 21 deletions(-)
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index e906495b64..f957c29d0d 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -923,29 +950,9 @@ libxlDomainCleanup(libxlDriverPrivate *driver,
}
}
- if ((vm->def->nnets)) {
- size_t i;
-
- for (i = 0; i < vm->def->nnets; i++) {
- virDomainNetDef *net = vm->def->nets[i];
-
- if (net->ifname &&
- STRPREFIX(net->ifname, LIBXL_GENERATED_PREFIX_XEN))
- VIR_FREE(net->ifname);
-
- /* cleanup actual device */
- virDomainNetRemoveHostdev(vm->def, net);
- if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK) {
- if (conn || (conn = virGetConnectNetwork()))
- virDomainNetReleaseActualDevice(conn, vm->def, net);
This removes the last use of 'conn' from this function:
../src/libxl/libxl_domain.c:919:27: error: unused variable 'conn'
[-Werror,-Wunused-variable]
g_autoptr(virConnect) conn = NULL;
^
Jano
- else
- VIR_WARN("Unable to release network device '%s'",
NULLSTR(net->ifname));
- }
- }
- }
+ libxlNetworkUnwindDevices(vm->def);
file = g_strdup_printf("%s/%s.xml", cfg->stateDir,
vm->def->name);
-
if (unlink(file) < 0 && errno != ENOENT && errno != ENOTDIR)
VIR_DEBUG("Failed to remove domain XML for %s", vm->def->name);
VIR_FREE(file);
4:12 p.m.
New subject: [PATCH 4/6] libxl: Introduce libxlDomainStartPrepare
Introduce libxlDomainStartPrepare as part of decomposing libxlDomainStart.
Perform all prepratory operations such as hostdevs, network devs, etc.
Also ensure all such operations are properly unwound on error.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/libxl/libxl_domain.c | 99 ++++++++++++++++++++++------------------
1 file changed, 55 insertions(+), 44 deletions(-)
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index f957c29d0d..d564165807 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -1224,6 +1224,55 @@ libxlDomainCreateChannelPTY(virDomainDef *def, libxl_ctx *ctx)
libxl_device_channel_dispose(&x_channels[i]);
}
+static int
+libxlDomainStartPrepare(libxlDriverPrivate *driver,
+ virDomainObj *vm)
+{
+ virHostdevManager *hostdev_mgr = driver->hostdevMgr;
+ unsigned int hostdev_flags = VIR_HOSTDEV_SP_PCI | VIR_HOSTDEV_SP_USB;
+
+ if (virDomainObjSetDefTransient(driver->xmlopt, vm, NULL) < 0)
+ return -1;
+
+ /* Run an early hook to set-up missing devices */
+ if (virHookPresent(VIR_HOOK_DRIVER_LIBXL)) {
+ g_autofree char *xml = virDomainDefFormat(vm->def, driver->xmlopt, 0);
+ int hookret;
+
+ hookret = virHookCall(VIR_HOOK_DRIVER_LIBXL, vm->def->name,
+ VIR_HOOK_LIBXL_OP_PREPARE, VIR_HOOK_SUBOP_BEGIN,
+ NULL, xml, NULL);
+ /*
+ * If the script raised an error abort the launch
+ */
+ if (hookret < 0)
+ goto error;
+ }
+
+ if (virDomainLockProcessStart(driver->lockManager,
+ "xen:///system",
+ vm,
+ true,
+ NULL) < 0)
+ goto error;
+
+ if (libxlNetworkPrepareDevices(vm->def) < 0)
+ goto error;
+
+ if (virHostdevPrepareDomainDevices(hostdev_mgr, LIBXL_DRIVER_INTERNAL_NAME,
+ vm->def, hostdev_flags) < 0)
+ goto error;
+
+ return 0;
+
+ error:
+ libxlNetworkUnwindDevices(vm->def);
+ virHostdevReAttachDomainDevices(hostdev_mgr, LIBXL_DRIVER_INTERNAL_NAME,
+ vm->def, hostdev_flags);
+ virDomainObjRemoveTransientDef(vm);
+ return -1;
+}
+
/*
* Start a domain through libxenlight.
*
@@ -1243,46 +1292,15 @@ libxlDomainStart(libxlDriverPrivate *driver,
g_autofree char *dom_xml = NULL;
libxlDomainObjPrivate *priv = vm->privateData;
g_autoptr(libxlDriverConfig) cfg = libxlDriverConfigGet(driver);
- virHostdevManager *hostdev_mgr = driver->hostdevMgr;
libxl_asyncprogress_how aop_console_how;
libxl_domain_restore_params params;
- unsigned int hostdev_flags = VIR_HOSTDEV_SP_PCI;
g_autofree char *config_json = NULL;
- hostdev_flags |= VIR_HOSTDEV_SP_USB;
+ if (libxlDomainStartPrepare(driver, vm) < 0)
+ return -1;
libxl_domain_config_init(&d_config);
- if (virDomainObjSetDefTransient(driver->xmlopt, vm, NULL) < 0)
- goto cleanup;
-
- /* Run an early hook to set-up missing devices */
- if (virHookPresent(VIR_HOOK_DRIVER_LIBXL)) {
- char *xml = virDomainDefFormat(vm->def, driver->xmlopt, 0);
- int hookret;
-
- hookret = virHookCall(VIR_HOOK_DRIVER_LIBXL, vm->def->name,
- VIR_HOOK_LIBXL_OP_PREPARE, VIR_HOOK_SUBOP_BEGIN,
- NULL, xml, NULL);
- VIR_FREE(xml);
-
- /*
- * If the script raised an error abort the launch
- */
- if (hookret < 0)
- goto cleanup_dom;
- }
-
- if (virDomainLockProcessStart(driver->lockManager,
- "xen:///system",
- vm,
- true,
- NULL) < 0)
- goto cleanup;
-
- if (libxlNetworkPrepareDevices(vm->def) < 0)
- goto cleanup_dom;
-
if (libxlBuildDomainConfig(driver->reservedGraphicsPorts, vm->def,
cfg, &d_config) < 0)
goto cleanup_dom;
@@ -1290,10 +1308,6 @@ libxlDomainStart(libxlDriverPrivate *driver,
if (cfg->autoballoon && libxlDomainFreeMem(cfg->ctx, &d_config)
< 0)
goto cleanup_dom;
- if (virHostdevPrepareDomainDevices(hostdev_mgr, LIBXL_DRIVER_INTERNAL_NAME,
- vm->def, hostdev_flags) < 0)
- goto cleanup_dom;
-
/* now that we know it is about to start call the hook if present */
if (virHookPresent(VIR_HOOK_DRIVER_LIBXL)) {
char *xml = virDomainDefFormat(vm->def, driver->xmlopt, 0);
@@ -1415,7 +1429,7 @@ libxlDomainStart(libxlDriverPrivate *driver,
* If the script raised an error abort the launch
*/
if (hookret < 0)
- goto cleanup_dom;
+ goto destroy_dom;
}
event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_STARTED,
@@ -1424,21 +1438,18 @@ libxlDomainStart(libxlDriverPrivate *driver,
VIR_DOMAIN_EVENT_STARTED_RESTORED);
virObjectEventStateQueue(driver->domainEventState, event);
- ret = 0;
- goto cleanup;
+ libxl_domain_config_dispose(&d_config);
+ return 0;
destroy_dom:
- ret = -1;
libxlDomainDestroyInternal(driver, vm);
vm->def->id = -1;
virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF, VIR_DOMAIN_SHUTOFF_FAILED);
cleanup_dom:
libxlDomainCleanup(driver, vm);
-
- cleanup:
libxl_domain_config_dispose(&d_config);
- return ret;
+ return -1;
}
int
--
2.31.1
4:12 p.m.
New subject: [PATCH 5/6] libxl: Introduce libxlDomainStartPerform
Introduce libxlDomainStartPerform as part of decomposing libxlDomainStart.
Perform all operations that are part of starting a domain. On error the
domain is destroyed from libxl's perspective, but the operations perfomed
in libxlDomainStartPrepare must be unwound by libxlDomainStart.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/libxl/libxl_domain.c | 87 ++++++++++++++++++++++++----------------
1 file changed, 52 insertions(+), 35 deletions(-)
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index d564165807..c656cba3a3 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -1273,21 +1273,16 @@ libxlDomainStartPrepare(libxlDriverPrivate *driver,
return -1;
}
-/*
- * Start a domain through libxenlight.
- *
- * virDomainObj *must be locked and a job acquired on invocation
- */
static int
-libxlDomainStart(libxlDriverPrivate *driver,
- virDomainObj *vm,
- bool start_paused,
- int restore_fd,
- uint32_t restore_ver)
+libxlDomainStartPerform(libxlDriverPrivate *driver,
+ virDomainObj *vm,
+ bool start_paused,
+ int restore_fd,
+ uint32_t restore_ver)
{
libxl_domain_config d_config;
- virObjectEvent *event = NULL;
int ret = -1;
+ int libxlret = -1;
uint32_t domid = 0;
g_autofree char *dom_xml = NULL;
libxlDomainObjPrivate *priv = vm->privateData;
@@ -1296,17 +1291,14 @@ libxlDomainStart(libxlDriverPrivate *driver,
libxl_domain_restore_params params;
g_autofree char *config_json = NULL;
- if (libxlDomainStartPrepare(driver, vm) < 0)
- return -1;
-
libxl_domain_config_init(&d_config);
if (libxlBuildDomainConfig(driver->reservedGraphicsPorts, vm->def,
cfg, &d_config) < 0)
- goto cleanup_dom;
+ goto cleanup;
if (cfg->autoballoon && libxlDomainFreeMem(cfg->ctx, &d_config)
< 0)
- goto cleanup_dom;
+ goto cleanup;
/* now that we know it is about to start call the hook if present */
if (virHookPresent(VIR_HOOK_DRIVER_LIBXL)) {
@@ -1322,7 +1314,7 @@ libxlDomainStart(libxlDriverPrivate *driver,
* If the script raised an error abort the launch
*/
if (hookret < 0)
- goto cleanup_dom;
+ goto cleanup;
}
if (priv->hookRun) {
@@ -1341,19 +1333,19 @@ libxlDomainStart(libxlDriverPrivate *driver,
aop_console_how.for_callback = vm;
aop_console_how.callback = libxlConsoleCallback;
if (restore_fd < 0) {
- ret = libxl_domain_create_new(cfg->ctx, &d_config,
+ libxlret = libxl_domain_create_new(cfg->ctx, &d_config,
&domid, NULL, &aop_console_how);
} else {
libxl_domain_restore_params_init(¶ms);
params.stream_version = restore_ver;
- ret = libxlDomainCreateRestoreWrapper(cfg->ctx, &d_config, &domid,
+ libxlret = libxlDomainCreateRestoreWrapper(cfg->ctx, &d_config,
&domid,
restore_fd, ¶ms,
&aop_console_how);
libxl_domain_restore_params_dispose(¶ms);
}
virObjectLock(vm);
- if (ret) {
+ if (libxlret) {
if (restore_fd < 0)
virReportError(VIR_ERR_INTERNAL_ERROR,
_("libxenlight failed to create new domain
'%s'"),
@@ -1362,7 +1354,7 @@ libxlDomainStart(libxlDriverPrivate *driver,
virReportError(VIR_ERR_INTERNAL_ERROR,
_("libxenlight failed to restore domain
'%s'"),
d_config.c_info.name);
- goto cleanup_dom;
+ goto cleanup;
}
/*
@@ -1412,9 +1404,6 @@ libxlDomainStart(libxlDriverPrivate *driver,
if (virDomainObjSave(vm, driver->xmlopt, cfg->stateDir) < 0)
goto destroy_dom;
- if (g_atomic_int_add(&driver->nactive, 1) == 0 &&
driver->inhibitCallback)
- driver->inhibitCallback(true, driver->inhibitOpaque);
-
/* finally we can call the 'started' hook script if any */
if (virHookPresent(VIR_HOOK_DRIVER_LIBXL)) {
char *xml = virDomainDefFormat(vm->def, driver->xmlopt, 0);
@@ -1432,24 +1421,52 @@ libxlDomainStart(libxlDriverPrivate *driver,
goto destroy_dom;
}
- event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_STARTED,
- restore_fd < 0 ?
- VIR_DOMAIN_EVENT_STARTED_BOOTED :
- VIR_DOMAIN_EVENT_STARTED_RESTORED);
- virObjectEventStateQueue(driver->domainEventState, event);
-
- libxl_domain_config_dispose(&d_config);
- return 0;
+ ret = 0;
+ goto cleanup;
destroy_dom:
libxlDomainDestroyInternal(driver, vm);
vm->def->id = -1;
virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF, VIR_DOMAIN_SHUTOFF_FAILED);
- cleanup_dom:
- libxlDomainCleanup(driver, vm);
+ cleanup:
libxl_domain_config_dispose(&d_config);
- return -1;
+ return ret;
+}
+
+/*
+ * Start a domain through libxenlight.
+ *
+ * virDomainObj must be locked and a job acquired on invocation
+ */
+static int
+libxlDomainStart(libxlDriverPrivate *driver,
+ virDomainObj *vm,
+ bool start_paused,
+ int restore_fd,
+ uint32_t restore_ver)
+{
+ virObjectEvent *event = NULL;
+
+ if (libxlDomainStartPrepare(driver, vm) < 0)
+ return -1;
+
+ if (libxlDomainStartPerform(driver, vm, start_paused,
+ restore_fd, restore_ver) < 0) {
+ libxlDomainCleanup(driver, vm);
+ return -1;
+ }
+
+ if (g_atomic_int_add(&driver->nactive, 1) == 0 &&
driver->inhibitCallback)
+ driver->inhibitCallback(true, driver->inhibitOpaque);
+
+ event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_STARTED,
+ restore_fd < 0 ?
+ VIR_DOMAIN_EVENT_STARTED_BOOTED :
+ VIR_DOMAIN_EVENT_STARTED_RESTORED);
+ virObjectEventStateQueue(driver->domainEventState, event);
+
+ return 0;
}
int
--
2.31.1
4:12 p.m.
New subject: [PATCH 6/6] libxl: Add helper function for running the hook script
The same pattern of retrieving the domXML, running the hook script, and
checking for error is used throughout the libxl driver. Remove some
repetitive code by adding a helper function to perform these tasks.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/libxl/libxl_domain.c | 102 ++++++++++++++----------------------
src/libxl/libxl_domain.h | 8 +++
src/libxl/libxl_driver.c | 26 +++------
src/libxl/libxl_migration.c | 15 ++----
4 files changed, 58 insertions(+), 93 deletions(-)
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index c656cba3a3..e699c1daa9 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -853,6 +853,25 @@ libxlNetworkUnwindDevices(virDomainDef *def)
}
}
+int
+libxlDomainHookRun(libxlDriverPrivate *driver,
+ virDomainDef *def,
+ unsigned int def_fmtflags,
+ int hookop,
+ int hooksubop,
+ char **output)
+{
+ g_autofree char *xml = NULL;
+
+ if (!virHookPresent(VIR_HOOK_DRIVER_LIBXL))
+ return 0;
+
+ xml = virDomainDefFormat(def, driver->xmlopt, def_fmtflags);
+ return virHookCall(VIR_HOOK_DRIVER_LIBXL, def->name,
+ hookop, hooksubop,
+ NULL, xml, output);
+}
+
/*
* Internal domain destroy function.
*
@@ -904,16 +923,10 @@ libxlDomainCleanup(libxlDriverPrivate *driver,
hostdev_flags |= VIR_HOSTDEV_SP_USB;
- /* now that we know it's stopped call the hook if present */
- if (virHookPresent(VIR_HOOK_DRIVER_LIBXL)) {
- char *xml = virDomainDefFormat(vm->def, driver->xmlopt, 0);
-
- /* we can't stop the operation even if the script raised an error */
- ignore_value(virHookCall(VIR_HOOK_DRIVER_LIBXL, vm->def->name,
- VIR_HOOK_LIBXL_OP_STOPPED, VIR_HOOK_SUBOP_END,
- NULL, xml, NULL));
- VIR_FREE(xml);
- }
+ /* Call hook with stopped operation. Ignore error and continue with cleanup */
+ ignore_value(libxlDomainHookRun(driver, vm->def, 0,
+ VIR_HOOK_LIBXL_OP_STOPPED,
+ VIR_HOOK_SUBOP_END, NULL));
virHostdevReAttachDomainDevices(hostdev_mgr, LIBXL_DRIVER_INTERNAL_NAME,
vm->def, hostdev_flags);
@@ -957,16 +970,10 @@ libxlDomainCleanup(libxlDriverPrivate *driver,
VIR_DEBUG("Failed to remove domain XML for %s", vm->def->name);
VIR_FREE(file);
- /* The "release" hook cleans up additional resources */
- if (virHookPresent(VIR_HOOK_DRIVER_LIBXL)) {
- char *xml = virDomainDefFormat(vm->def, driver->xmlopt, 0);
-
- /* we can't stop the operation even if the script raised an error */
- ignore_value(virHookCall(VIR_HOOK_DRIVER_LIBXL, vm->def->name,
- VIR_HOOK_LIBXL_OP_RELEASE, VIR_HOOK_SUBOP_END,
- NULL, xml, NULL));
- VIR_FREE(xml);
- }
+ /* Call hook with release operation. Ignore error and continue with cleanup */
+ ignore_value(libxlDomainHookRun(driver, vm->def, 0,
+ VIR_HOOK_LIBXL_OP_RELEASE,
+ VIR_HOOK_SUBOP_END, NULL));
virDomainObjRemoveTransientDef(vm);
}
@@ -1235,19 +1242,10 @@ libxlDomainStartPrepare(libxlDriverPrivate *driver,
return -1;
/* Run an early hook to set-up missing devices */
- if (virHookPresent(VIR_HOOK_DRIVER_LIBXL)) {
- g_autofree char *xml = virDomainDefFormat(vm->def, driver->xmlopt, 0);
- int hookret;
-
- hookret = virHookCall(VIR_HOOK_DRIVER_LIBXL, vm->def->name,
- VIR_HOOK_LIBXL_OP_PREPARE, VIR_HOOK_SUBOP_BEGIN,
- NULL, xml, NULL);
- /*
- * If the script raised an error abort the launch
- */
- if (hookret < 0)
- goto error;
- }
+ if (libxlDomainHookRun(driver, vm->def, 0,
+ VIR_HOOK_LIBXL_OP_PREPARE,
+ VIR_HOOK_SUBOP_BEGIN, NULL) < 0)
+ goto error;
if (virDomainLockProcessStart(driver->lockManager,
"xen:///system",
@@ -1301,21 +1299,10 @@ libxlDomainStartPerform(libxlDriverPrivate *driver,
goto cleanup;
/* now that we know it is about to start call the hook if present */
- if (virHookPresent(VIR_HOOK_DRIVER_LIBXL)) {
- char *xml = virDomainDefFormat(vm->def, driver->xmlopt, 0);
- int hookret;
-
- hookret = virHookCall(VIR_HOOK_DRIVER_LIBXL, vm->def->name,
- VIR_HOOK_LIBXL_OP_START, VIR_HOOK_SUBOP_BEGIN,
- NULL, xml, NULL);
- VIR_FREE(xml);
-
- /*
- * If the script raised an error abort the launch
- */
- if (hookret < 0)
- goto cleanup;
- }
+ if (libxlDomainHookRun(driver, vm->def, 0,
+ VIR_HOOK_LIBXL_OP_START,
+ VIR_HOOK_SUBOP_BEGIN, NULL) < 0)
+ goto cleanup;
if (priv->hookRun) {
char uuidstr[VIR_UUID_STRING_BUFLEN];
@@ -1405,21 +1392,10 @@ libxlDomainStartPerform(libxlDriverPrivate *driver,
goto destroy_dom;
/* finally we can call the 'started' hook script if any */
- if (virHookPresent(VIR_HOOK_DRIVER_LIBXL)) {
- char *xml = virDomainDefFormat(vm->def, driver->xmlopt, 0);
- int hookret;
-
- hookret = virHookCall(VIR_HOOK_DRIVER_LIBXL, vm->def->name,
- VIR_HOOK_LIBXL_OP_STARTED, VIR_HOOK_SUBOP_BEGIN,
- NULL, xml, NULL);
- VIR_FREE(xml);
-
- /*
- * If the script raised an error abort the launch
- */
- if (hookret < 0)
- goto destroy_dom;
- }
+ if (libxlDomainHookRun(driver, vm->def, 0,
+ VIR_HOOK_LIBXL_OP_STARTED,
+ VIR_HOOK_SUBOP_BEGIN, NULL) < 0)
+ goto destroy_dom;
ret = 0;
goto cleanup;
diff --git a/src/libxl/libxl_domain.h b/src/libxl/libxl_domain.h
index 526c8e7332..1618c47ed5 100644
--- a/src/libxl/libxl_domain.h
+++ b/src/libxl/libxl_domain.h
@@ -108,6 +108,14 @@ libxlDomainSaveImageOpen(libxlDriverPrivate *driver,
libxlSavefileHeader *ret_hdr)
ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4);
+int
+libxlDomainHookRun(libxlDriverPrivate *driver,
+ virDomainDef *def,
+ unsigned int def_fmtflags,
+ int hookop,
+ int hooksubop,
+ char **output);
+
int
libxlDomainDestroyInternal(libxlDriverPrivate *driver,
virDomainObj *vm);
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 838747a1e3..0c3c53c1d1 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -450,26 +450,12 @@ libxlReconnectDomain(virDomainObj *vm,
if (virDomainObjSave(vm, driver->xmlopt, cfg->stateDir) < 0)
VIR_WARN("Cannot update XML for running Xen guest %s",
vm->def->name);
- /* now that we know it's reconnected call the hook if present */
- if (virHookPresent(VIR_HOOK_DRIVER_LIBXL) &&
- STRNEQ("Domain-0", vm->def->name)) {
- char *xml = virDomainDefFormat(vm->def, driver->xmlopt, 0);
- int hookret;
-
- /* we can't stop the operation even if the script raised an error */
- hookret = virHookCall(VIR_HOOK_DRIVER_LIBXL, vm->def->name,
- VIR_HOOK_LIBXL_OP_RECONNECT, VIR_HOOK_SUBOP_BEGIN,
- NULL, xml, NULL);
- VIR_FREE(xml);
- if (hookret < 0) {
- /* Stop the domain if the hook failed */
- if (virDomainObjIsActive(vm)) {
- libxlDomainDestroyInternal(driver, vm);
- virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF, VIR_DOMAIN_SHUTOFF_FAILED);
- }
- goto error;
- }
- }
+ /* now that we know it's reconnected call the hook */
+ if (STRNEQ("Domain-0", vm->def->name) &&
+ (libxlDomainHookRun(driver, vm->def, 0,
+ VIR_HOOK_LIBXL_OP_RECONNECT,
+ VIR_HOOK_SUBOP_BEGIN, NULL) < 0))
+ goto error;
ret = 0;
diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c
index 0af6e0d09a..4677f798fc 100644
--- a/src/libxl/libxl_migration.c
+++ b/src/libxl/libxl_migration.c
@@ -490,18 +490,13 @@ libxlDomainMigrationPrepareAny(virConnectPtr dconn,
/* Let migration hook filter domain XML */
if (virHookPresent(VIR_HOOK_DRIVER_LIBXL)) {
- char *xml;
int hookret;
- if (!(xml = virDomainDefFormat(*def, driver->xmlopt,
- VIR_DOMAIN_XML_SECURE |
- VIR_DOMAIN_XML_MIGRATABLE)))
- return -1;
-
- hookret = virHookCall(VIR_HOOK_DRIVER_LIBXL, (*def)->name,
- VIR_HOOK_LIBXL_OP_MIGRATE, VIR_HOOK_SUBOP_BEGIN,
- NULL, xml, xmlout);
- VIR_FREE(xml);
+ hookret = libxlDomainHookRun(driver, *def,
+ VIR_DOMAIN_XML_SECURE | VIR_DOMAIN_XML_MIGRATABLE,
+ VIR_HOOK_LIBXL_OP_MIGRATE,
+ VIR_HOOK_SUBOP_BEGIN,
+ xmlout);
if (hookret < 0) {
return -1;
--
2.31.1
8:45 a.m.
On a %A in %Y, Jim Fehlig wrote:
My main objective with this series was to decompose libxlDomainStart
a bit,
as it had become quite unwieldy. Along the way I made a few other small
improvements to the code.
Jim Fehlig (6):
libxl: Drop unused 'cfg' parameter from libxlDomainSaveImageOpen
libxl: Move managed save logic to libxlDomainStartNew
libxl: Add a helper function to unprepare network devices
libxl: Introduce libxlDomainStartPrepare
libxl: Introduce libxlDomainStartPerform
libxl: Add helper function for running the hook script
src/libxl/libxl_domain.c | 384 ++++++++++++++++++------------------
src/libxl/libxl_domain.h | 11 +-
src/libxl/libxl_driver.c | 30 +--
src/libxl/libxl_migration.c | 15 +-
4 files changed, 218 insertions(+), 222 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
1234
days inactive
1235
days old
8 comments
2 participants
participants (2)
-
Jano Tomko -
Jim Fehlig