[libvirt] [PATCH 0/3] libxl: a few minor patches
This series contains a few simple changes to the libxl driver. Patches 1 and 2 were included in https://www.redhat.com/archives/libvir-list/2015-February/msg00611.html To make an upcoming V2 of that series easier to review, I removed the mostly unrelated patches and include them here, along with one other small fix. Jim Fehlig (3): libxl: remove redundant calls to libxl_evdisable_domain_death libxl: use libxl_ctx passed to libxlConsoleCallback libxl: remove unneeded cleanup_unlock label src/libxl/libxl_domain.c | 13 ++----------- src/libxl/libxl_driver.c | 10 +++------- 2 files changed, 5 insertions(+), 18 deletions(-) -- 1.8.4.5
Domain death watch is already disabled in libxlDomainCleanup. No need to disable it a second and third time. Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- src/libxl/libxl_domain.c | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index 21c41d7..e186c53 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -439,9 +439,6 @@ libxlDomainObjPrivateDispose(void *obj) { libxlDomainObjPrivatePtr priv = obj; - if (priv->deathW) - libxl_evdisable_domain_death(priv->ctx, priv->deathW); - libxlDomainObjFreeJob(priv); virChrdevFree(priv->devs); libxl_ctx_free(priv->ctx); @@ -456,11 +453,6 @@ libxlDomainObjPrivateFree(void *data) { libxlDomainObjPrivatePtr priv = data; - if (priv->deathW) { - libxl_evdisable_domain_death(priv->ctx, priv->deathW); - priv->deathW = NULL; - } - virObjectUnref(priv); } -- 1.8.4.5
Instead of using the libxl_ctx in the libxlDomainObjPrivatePtr, use the ctx passed to the callback. Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- src/libxl/libxl_domain.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index e186c53..9af5758 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -1158,10 +1158,9 @@ libxlDomainFreeMem(libxlDomainObjPrivatePtr priv, libxl_domain_config *d_config) } static void -libxlConsoleCallback(libxl_ctx *ctx, libxl_event* ev, void *for_callback) +libxlConsoleCallback(libxl_ctx *ctx, libxl_event *ev, void *for_callback) { virDomainObjPtr vm = for_callback; - libxlDomainObjPrivatePtr priv = vm->privateData; size_t i; virObjectLock(vm); @@ -1175,7 +1174,7 @@ libxlConsoleCallback(libxl_ctx *ctx, libxl_event* ev, void *for_callback) console_type = (chr->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL ? LIBXL_CONSOLE_TYPE_SERIAL : LIBXL_CONSOLE_TYPE_PV); - ret = libxl_console_get_tty(priv->ctx, ev->domid, + ret = libxl_console_get_tty(ctx, ev->domid, chr->target.port, console_type, &console); if (!ret) { -- 1.8.4.5
In the old days of a global driver lock, it was necessary to unlock the driver after a domain restore operation. When the global lock was removed from the driver, some remnants were left behind in libxlDomainRestoreFlags. Remove this unneeded (and incorrect) code. Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- src/libxl/libxl_driver.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index ce3a99b..55fa64f 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -1464,17 +1464,17 @@ libxlDomainRestoreFlags(virConnectPtr conn, const char *from, fd = libxlDomainSaveImageOpen(driver, cfg, from, &def, &hdr); if (fd < 0) - goto cleanup_unlock; + return -1; if (virDomainRestoreFlagsEnsureACL(conn, def) < 0) - goto cleanup_unlock; + return -1; if (!(vm = virDomainObjListAdd(driver->domains, def, driver->xmlopt, VIR_DOMAIN_OBJ_LIST_ADD_LIVE | VIR_DOMAIN_OBJ_LIST_ADD_CHECK_LIVE, NULL))) - goto cleanup_unlock; + goto cleanup; def = NULL; @@ -1492,10 +1492,6 @@ libxlDomainRestoreFlags(virConnectPtr conn, const char *from, virObjectUnlock(vm); virObjectUnref(cfg); return ret; - - cleanup_unlock: - libxlDriverUnlock(driver); - goto cleanup; } static int -- 1.8.4.5
On 04.03.2015 01:09, Jim Fehlig wrote:
In the old days of a global driver lock, it was necessary to unlock the driver after a domain restore operation. When the global lock was removed from the driver, some remnants were left behind in libxlDomainRestoreFlags. Remove this unneeded (and incorrect) code.
Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- src/libxl/libxl_driver.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index ce3a99b..55fa64f 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -1464,17 +1464,17 @@ libxlDomainRestoreFlags(virConnectPtr conn, const char *from,
fd = libxlDomainSaveImageOpen(driver, cfg, from, &def, &hdr); if (fd < 0) - goto cleanup_unlock; + return -1;
While this one is okay, ...
if (virDomainRestoreFlagsEnsureACL(conn, def) < 0) - goto cleanup_unlock; + return -1;
.. this one's not. If EnsureACL fails, the @fd opened just above is leaked. ACKed with s/return -1/goto cleanup/. Michal
On 04.03.2015 01:09, Jim Fehlig wrote:
This series contains a few simple changes to the libxl driver. Patches 1 and 2 were included in
https://www.redhat.com/archives/libvir-list/2015-February/msg00611.html
To make an upcoming V2 of that series easier to review, I removed the mostly unrelated patches and include them here, along with one other small fix.
Jim Fehlig (3): libxl: remove redundant calls to libxl_evdisable_domain_death libxl: use libxl_ctx passed to libxlConsoleCallback libxl: remove unneeded cleanup_unlock label
src/libxl/libxl_domain.c | 13 ++----------- src/libxl/libxl_driver.c | 10 +++------- 2 files changed, 5 insertions(+), 18 deletions(-)
ACK to all, but see my comment to 3/3. Michal
Michal Privoznik wrote:
On 04.03.2015 01:09, Jim Fehlig wrote:
This series contains a few simple changes to the libxl driver. Patches 1 and 2 were included in
https://www.redhat.com/archives/libvir-list/2015-February/msg00611.html
To make an upcoming V2 of that series easier to review, I removed the mostly unrelated patches and include them here, along with one other small fix.
Jim Fehlig (3): libxl: remove redundant calls to libxl_evdisable_domain_death libxl: use libxl_ctx passed to libxlConsoleCallback libxl: remove unneeded cleanup_unlock label
src/libxl/libxl_domain.c | 13 ++----------- src/libxl/libxl_driver.c | 10 +++------- 2 files changed, 5 insertions(+), 18 deletions(-)
ACK to all, but see my comment to 3/3.
Thanks for catching that! I fixed it and pushed the series. Regards, Jim
participants (2)
-
Jim Fehlig -
Michal Privoznik