[libvirt] [PATCH 0/3] libxl: misc trivial cleanups

A few trivial patches for things I noticed while working on my upcoming patches that add job support in the libxl driver. Jim Fehlig (3): libxl: rename libxlCreateDomEvents to libxlDomEventsRegister libxl: register for domain events immediately after creation libxl: fix libxlDoDomainSave documentation src/libxl/libxl_driver.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) -- 1.8.1.4

libxlDomEventsRegister better reflects its purpose: register for domain events from libxl. Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- src/libxl/libxl_driver.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 03aa463..50fbe5c 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -464,16 +464,16 @@ static const struct libxl_event_hooks ev_hooks = { }; /* - * Register domain events with libxenlight and insert event handles - * in libvirt's event loop. + * Register for domain events emitted by libxl. */ static int -libxlCreateDomEvents(virDomainObjPtr vm) +libxlDomEventsRegister(virDomainObjPtr vm) { libxlDomainObjPrivatePtr priv = vm->privateData; libxl_event_register_callbacks(priv->ctx, &ev_hooks, vm); + /* Always enable domain death events */ if (libxl_evenable_domain_death(priv->ctx, vm->def->id, 0, &priv->deathW)) goto error; @@ -700,7 +700,7 @@ libxlVmStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm, goto error; } - if (libxlCreateDomEvents(vm) < 0) + if (libxlDomEventsRegister(vm) < 0) goto error; if (libxlDomainSetVcpuAffinities(driver, vm) < 0) @@ -791,8 +791,8 @@ libxlReconnectDomain(virDomainObjPtr vm, if (virAtomicIntInc(&driver->nactive) == 1 && driver->inhibitCallback) driver->inhibitCallback(true, driver->inhibitOpaque); - /* Recreate domain death et. al. events */ - libxlCreateDomEvents(vm); + /* Re-register domain death et. al. events */ + libxlDomEventsRegister(vm); virObjectUnlock(vm); return 0; -- 1.8.1.4

On Thu, Feb 06, 2014 at 06:21:41PM -0700, Jim Fehlig wrote:
libxlDomEventsRegister better reflects its purpose: register for domain events from libxl.
Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- src/libxl/libxl_driver.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
ACK Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

A small fix for the possiblitiy of jumping to an error path before registering for domain events, preventing receiving important ones like shutdown and death. Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- src/libxl/libxl_driver.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 50fbe5c..99643e3 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -690,6 +690,9 @@ libxlVmStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm, } vm->def->id = domid; + if (libxlDomEventsRegister(vm) < 0) + goto error; + if ((dom_xml = virDomainDefFormat(vm->def, 0)) == NULL) goto error; @@ -700,9 +703,6 @@ libxlVmStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm, goto error; } - if (libxlDomEventsRegister(vm) < 0) - goto error; - if (libxlDomainSetVcpuAffinities(driver, vm) < 0) goto error; -- 1.8.1.4

On Thu, Feb 06, 2014 at 06:21:42PM -0700, Jim Fehlig wrote:
A small fix for the possiblitiy of jumping to an error path before registering for domain events, preventing receiving important ones like shutdown and death.
Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- src/libxl/libxl_driver.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
ACK Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

Update the function's comment, which was missed when removing use of the driver lock everywhere. Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- src/libxl/libxl_driver.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 99643e3..8e4242a 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -1812,8 +1812,9 @@ libxlDomainGetState(virDomainPtr dom, return ret; } -/* This internal function expects the driver lock to already be held on - * entry and the vm must be active. */ +/* + * virDomainObjPtr must be locked on invocation + */ static int libxlDoDomainSave(libxlDriverPrivatePtr driver, virDomainObjPtr vm, const char *to) -- 1.8.1.4

On Thu, Feb 06, 2014 at 06:21:43PM -0700, Jim Fehlig wrote:
Update the function's comment, which was missed when removing use of the driver lock everywhere.
Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- src/libxl/libxl_driver.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
ACK Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On 07.02.2014 02:21, Jim Fehlig wrote:
A few trivial patches for things I noticed while working on my upcoming patches that add job support in the libxl driver.
Jim Fehlig (3): libxl: rename libxlCreateDomEvents to libxlDomEventsRegister libxl: register for domain events immediately after creation libxl: fix libxlDoDomainSave documentation
src/libxl/libxl_driver.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-)
ACK series. Michal

Michal Privoznik wrote:
On 07.02.2014 02:21, Jim Fehlig wrote:
A few trivial patches for things I noticed while working on my upcoming patches that add job support in the libxl driver.
Jim Fehlig (3): libxl: rename libxlCreateDomEvents to libxlDomEventsRegister libxl: register for domain events immediately after creation libxl: fix libxlDoDomainSave documentation
src/libxl/libxl_driver.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-)
ACK series.
Thanks for the reviews Michal and Daniel. This trivial series has been pushed now. Regards, Jim
participants (3)
-
Daniel P. Berrange
-
Jim Fehlig
-
Michal Privoznik