7:35 p.m.
While hacking qemuMigrationFinish I found it pretty hard to follow and
revealed few bugs (patches 3 to 5) in the code.
Jiri Denemark (7):
qemu: Split qemuMigrationFinish
qemu: Simplify qemuMigrationFinish
qemu: Don't fail migration on save status failure
qemu: Kill domain when migration finish fails
qemu: Don't report false errors in migration protocol v2
qemuDomainEventQueue: Check if event is non-NULL
qemu: Queue events in migration Finish phase ASAP
src/qemu/qemu_blockjob.c | 6 +-
src/qemu/qemu_cgroup.c | 3 +-
src/qemu/qemu_domain.c | 6 +-
src/qemu/qemu_driver.c | 87 +++++++-------------
src/qemu/qemu_hotplug.c | 26 +++---
src/qemu/qemu_migration.c | 199 ++++++++++++++++++++++------------------------
src/qemu/qemu_process.c | 72 ++++++-----------
7 files changed, 167 insertions(+), 232 deletions(-)
--
2.4.5
7:36 p.m.
New subject: [libvirt] [PATCH 1/7] qemu: Split qemuMigrationFinish
Separate code which makes incoming domain persistent into
qemuMigrationPersist.
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
src/qemu/qemu_migration.c | 75 ++++++++++++++++++++++++++++++-----------------
1 file changed, 48 insertions(+), 27 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 97901c6..6b06e79 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -5526,6 +5526,51 @@ qemuMigrationVPAssociatePortProfiles(virDomainDefPtr def)
}
+static int
+qemuMigrationPersist(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ qemuMigrationCookiePtr mig)
+{
+ virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+ virCapsPtr caps = NULL;
+ virDomainDefPtr vmdef;
+ virObjectEventPtr event;
+ bool newVM;
+ int ret = -1;
+
+ if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
+ goto cleanup;
+
+ newVM = !vm->persistent;
+ vm->persistent = 1;
+
+ if (mig->persistent)
+ vm->newDef = mig->persistent;
+
+ vmdef = virDomainObjGetPersistentDef(caps, driver->xmlopt, vm);
+ if (!vmdef)
+ goto cleanup;
+
+ if (virDomainSaveConfig(cfg->configDir, vmdef) < 0)
+ goto cleanup;
+
+ event = virDomainEventLifecycleNewFromObj(vm,
+ VIR_DOMAIN_EVENT_DEFINED,
+ newVM ?
+ VIR_DOMAIN_EVENT_DEFINED_ADDED :
+ VIR_DOMAIN_EVENT_DEFINED_UPDATED);
+ if (event)
+ qemuDomainEventQueue(driver, event);
+
+ ret = 0;
+
+ cleanup:
+ virObjectUnref(caps);
+ virObjectUnref(cfg);
+ return ret;
+}
+
+
virDomainPtr
qemuMigrationFinish(virQEMUDriverPtr driver,
virConnectPtr dconn,
@@ -5540,13 +5585,11 @@ qemuMigrationFinish(virQEMUDriverPtr driver,
{
virDomainPtr dom = NULL;
virObjectEventPtr event = NULL;
- bool newVM = true;
qemuMigrationCookiePtr mig = NULL;
virErrorPtr orig_err = NULL;
int cookie_flags = 0;
qemuDomainObjPrivatePtr priv = vm->privateData;
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
- virCapsPtr caps = NULL;
unsigned short port;
VIR_DEBUG("driver=%p, dconn=%p, vm=%p, cookiein=%s, cookieinlen=%d, "
@@ -5557,9 +5600,6 @@ qemuMigrationFinish(virQEMUDriverPtr driver,
port = priv->migrationPort;
priv->migrationPort = 0;
- if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
- goto cleanup;
-
if (!qemuMigrationJobIsActive(vm, QEMU_ASYNC_JOB_MIGRATION_IN)) {
qemuMigrationErrorReport(driver, vm->def->name);
goto cleanup;
@@ -5622,15 +5662,9 @@ qemuMigrationFinish(virQEMUDriverPtr driver,
goto endjob;
if (flags & VIR_MIGRATE_PERSIST_DEST) {
- virDomainDefPtr vmdef;
- if (vm->persistent)
- newVM = false;
- vm->persistent = 1;
- if (mig->persistent)
- vm->newDef = vmdef = mig->persistent;
- else
- vmdef = virDomainObjGetPersistentDef(caps, driver->xmlopt, vm);
- if (!vmdef || virDomainSaveConfig(cfg->configDir, vmdef) < 0) {
+ bool newVM = !vm->persistent;
+
+ if (qemuMigrationPersist(driver, vm, mig) < 0) {
/* Hmpf. Migration was successful, but making it persistent
* was not. If we report successful, then when this domain
* shuts down, management tools are in for a surprise. On the
@@ -5654,20 +5688,8 @@ qemuMigrationFinish(virQEMUDriverPtr driver,
if (newVM)
vm->persistent = 0;
}
- if (!vmdef)
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("can't get vmdef"));
goto endjob;
}
-
- event = virDomainEventLifecycleNewFromObj(vm,
- VIR_DOMAIN_EVENT_DEFINED,
- newVM ?
- VIR_DOMAIN_EVENT_DEFINED_ADDED :
- VIR_DOMAIN_EVENT_DEFINED_UPDATED);
- if (event)
- qemuDomainEventQueue(driver, event);
- event = NULL;
}
if (!(flags & VIR_MIGRATE_PAUSED) && !(flags &
VIR_MIGRATE_OFFLINE)) {
@@ -5775,7 +5797,6 @@ qemuMigrationFinish(virQEMUDriverPtr driver,
virSetError(orig_err);
virFreeError(orig_err);
}
- virObjectUnref(caps);
virObjectUnref(cfg);
/* Set a special error if Finish is expected to return NULL as a result of
--
2.4.5
3:06 p.m.
New subject: [libvirt] [PATCH 1/7] qemu: Split qemuMigrationFinish
On Wed, Jul 08, 2015 at 19:36:00 +0200, Jiri Denemark wrote:
Separate code which makes incoming domain persistent into
qemuMigrationPersist.
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
src/qemu/qemu_migration.c | 75 ++++++++++++++++++++++++++++++-----------------
1 file changed, 48 insertions(+), 27 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 97901c6..6b06e79 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -5526,6 +5526,51 @@ qemuMigrationVPAssociatePortProfiles(virDomainDefPtr def)
}
+static int
+qemuMigrationPersist(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ qemuMigrationCookiePtr mig)
+{
+ virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+ virCapsPtr caps = NULL;
+ virDomainDefPtr vmdef;
+ virObjectEventPtr event;
+ bool newVM;
+ int ret = -1;
+
+ if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
+ goto cleanup;
+
+ newVM = !vm->persistent;
+ vm->persistent = 1;
+
+ if (mig->persistent)
+ vm->newDef = mig->persistent;
This could be done unconditionally since vm->newDef will apperently be
unset.
+
+ vmdef = virDomainObjGetPersistentDef(caps, driver->xmlopt, vm);
+ if (!vmdef)
+ goto cleanup;
+
+ if (virDomainSaveConfig(cfg->configDir, vmdef) < 0)
+ goto cleanup;
+
+ event = virDomainEventLifecycleNewFromObj(vm,
+ VIR_DOMAIN_EVENT_DEFINED,
+ newVM ?
+ VIR_DOMAIN_EVENT_DEFINED_ADDED :
+ VIR_DOMAIN_EVENT_DEFINED_UPDATED);
+ if (event)
+ qemuDomainEventQueue(driver, event);
+
+ ret = 0;
+
+ cleanup:
+ virObjectUnref(caps);
+ virObjectUnref(cfg);
+ return ret;
+}
+
+
virDomainPtr
qemuMigrationFinish(virQEMUDriverPtr driver,
virConnectPtr dconn,
ACK as-is.
Peter
7:36 p.m.
New subject: [libvirt] [PATCH 2/7] qemu: Simplify qemuMigrationFinish
Offline migration migration is quite special because we don't really
need to do anything but make the domain persistent. Let's do it
separately from normal migration to avoid cluttering the code with
!(flags & VIR_MIGRATE_OFFLINE).
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
src/qemu/qemu_migration.c | 72 +++++++++++++++++++++++------------------------
1 file changed, 36 insertions(+), 36 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 6b06e79..6a3e2e6 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -5625,8 +5625,14 @@ qemuMigrationFinish(virQEMUDriverPtr driver,
/* Did the migration go as planned? If yes, return the domain
* object, but if no, clean up the empty qemu process.
*/
- if (retcode == 0) {
- if (!virDomainObjIsActive(vm) && !(flags & VIR_MIGRATE_OFFLINE)) {
+ if (flags & VIR_MIGRATE_OFFLINE) {
+ if (retcode != 0 ||
+ qemuMigrationPersist(driver, vm, mig) < 0)
+ goto endjob;
+
+ dom = virGetDomain(dconn, vm->def->name, vm->def->uuid);
+ } else if (retcode == 0) {
+ if (!virDomainObjIsActive(vm)) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("guest unexpectedly quit"));
qemuMigrationErrorReport(driver, vm->def->name);
@@ -5643,20 +5649,17 @@ qemuMigrationFinish(virQEMUDriverPtr driver,
}
}
- if (!(flags & VIR_MIGRATE_OFFLINE)) {
- if (qemuMigrationVPAssociatePortProfiles(vm->def) < 0) {
- qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED,
- VIR_QEMU_PROCESS_STOP_MIGRATED);
- virDomainAuditStop(vm, "failed");
- event = virDomainEventLifecycleNewFromObj(vm,
- VIR_DOMAIN_EVENT_STOPPED,
- VIR_DOMAIN_EVENT_STOPPED_FAILED);
- goto endjob;
- }
- if (mig->network)
- if (qemuDomainMigrateOPDRelocate(driver, vm, mig) < 0)
- VIR_WARN("unable to provide network data for relocation");
+ if (qemuMigrationVPAssociatePortProfiles(vm->def) < 0) {
+ qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED,
+ VIR_QEMU_PROCESS_STOP_MIGRATED);
+ virDomainAuditStop(vm, "failed");
+ event = virDomainEventLifecycleNewFromObj(vm,
+ VIR_DOMAIN_EVENT_STOPPED,
+ VIR_DOMAIN_EVENT_STOPPED_FAILED);
+ goto endjob;
}
+ if (mig->network && qemuDomainMigrateOPDRelocate(driver, vm, mig) <
0)
+ VIR_WARN("unable to provide network data for relocation");
if (qemuMigrationStopNBDServer(driver, vm, mig) < 0)
goto endjob;
@@ -5680,11 +5683,9 @@ qemuMigrationFinish(virQEMUDriverPtr driver,
* to restart during confirm() step, so we kill it off now.
*/
if (v3proto) {
- if (!(flags & VIR_MIGRATE_OFFLINE)) {
- qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED,
- VIR_QEMU_PROCESS_STOP_MIGRATED);
- virDomainAuditStop(vm, "failed");
- }
+ qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED,
+ VIR_QEMU_PROCESS_STOP_MIGRATED);
+ virDomainAuditStop(vm, "failed");
if (newVM)
vm->persistent = 0;
}
@@ -5692,7 +5693,7 @@ qemuMigrationFinish(virQEMUDriverPtr driver,
}
}
- if (!(flags & VIR_MIGRATE_PAUSED) && !(flags &
VIR_MIGRATE_OFFLINE)) {
+ if (!(flags & VIR_MIGRATE_PAUSED)) {
/* run 'cont' on the destination, which allows migration on qemu
* >= 0.10.6 to work properly. This isn't strictly necessary on
* older qemu's, but it also doesn't hurt anything there
@@ -5734,19 +5735,17 @@ qemuMigrationFinish(virQEMUDriverPtr driver,
dom = virGetDomain(dconn, vm->def->name, vm->def->uuid);
- if (!(flags & VIR_MIGRATE_OFFLINE)) {
+ event = virDomainEventLifecycleNewFromObj(vm,
+ VIR_DOMAIN_EVENT_RESUMED,
+ VIR_DOMAIN_EVENT_RESUMED_MIGRATED);
+ if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_PAUSED) {
+ virDomainObjSetState(vm, VIR_DOMAIN_PAUSED,
+ VIR_DOMAIN_PAUSED_USER);
+ if (event)
+ qemuDomainEventQueue(driver, event);
event = virDomainEventLifecycleNewFromObj(vm,
- VIR_DOMAIN_EVENT_RESUMED,
- VIR_DOMAIN_EVENT_RESUMED_MIGRATED);
- if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_PAUSED) {
- virDomainObjSetState(vm, VIR_DOMAIN_PAUSED,
- VIR_DOMAIN_PAUSED_USER);
- if (event)
- qemuDomainEventQueue(driver, event);
- event = virDomainEventLifecycleNewFromObj(vm,
- VIR_DOMAIN_EVENT_SUSPENDED,
- VIR_DOMAIN_EVENT_SUSPENDED_PAUSED);
- }
+ VIR_DOMAIN_EVENT_SUSPENDED,
+ VIR_DOMAIN_EVENT_SUSPENDED_PAUSED);
}
if (virDomainObjIsActive(vm) &&
@@ -5757,7 +5756,7 @@ qemuMigrationFinish(virQEMUDriverPtr driver,
/* Guest is successfully running, so cancel previous auto destroy */
qemuProcessAutoDestroyRemove(driver, vm);
- } else if (!(flags & VIR_MIGRATE_OFFLINE)) {
+ } else {
qemuDomainJobInfo info;
/* Check for a possible error on the monitor in case Finish was called
@@ -5775,11 +5774,12 @@ qemuMigrationFinish(virQEMUDriverPtr driver,
VIR_DOMAIN_EVENT_STOPPED_FAILED);
}
- if (qemuMigrationBakeCookie(mig, driver, vm, cookieout, cookieoutlen,
+ endjob:
+ if (dom &&
+ qemuMigrationBakeCookie(mig, driver, vm, cookieout, cookieoutlen,
QEMU_MIGRATION_COOKIE_STATS) < 0)
VIR_WARN("Unable to encode migration cookie");
- endjob:
qemuMigrationJobFinish(driver, vm);
if (!vm->persistent && !virDomainObjIsActive(vm))
qemuDomainRemoveInactive(driver, vm);
--
2.4.5
3:13 p.m.
New subject: [libvirt] [PATCH 2/7] qemu: Simplify qemuMigrationFinish
On Wed, Jul 08, 2015 at 19:36:01 +0200, Jiri Denemark wrote:
Offline migration migration is quite special because we don't
really
need to do anything but make the domain persistent. Let's do it
separately from normal migration to avoid cluttering the code with
!(flags & VIR_MIGRATE_OFFLINE).
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
src/qemu/qemu_migration.c | 72 +++++++++++++++++++++++------------------------
1 file changed, 36 insertions(+), 36 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 6b06e79..6a3e2e6 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -5625,8 +5625,14 @@ qemuMigrationFinish(virQEMUDriverPtr driver,
/* Did the migration go as planned? If yes, return the domain
* object, but if no, clean up the empty qemu process.
*/
- if (retcode == 0) {
- if (!virDomainObjIsActive(vm) && !(flags & VIR_MIGRATE_OFFLINE)) {
+ if (flags & VIR_MIGRATE_OFFLINE) {
+ if (retcode != 0 ||
+ qemuMigrationPersist(driver, vm, mig) < 0)
+ goto endjob;
This isn't entirely equivalend to the previous impl, since if
qemuMigrationPersist fails at a later stage the vm->persistent flag does
get set. The code below cleared it in some cases, but in this case it
would not get cleared.
Either qemuMigrationPersist should get updated so that the flag is set
only when everything went well, or you should clear if afterwards.
Later on in the endjob section there is a check if the object is active
so you'd get a persistent VM object with no definition in some cases.
+
+ dom = virGetDomain(dconn, vm->def->name, vm->def->uuid);
+ } else if (retcode == 0) {
+ if (!virDomainObjIsActive(vm)) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("guest unexpectedly quit"));
qemuMigrationErrorReport(driver, vm->def->name);
ACK if you fix the offline migration error handling.
Peter
7:36 p.m.
New subject: [libvirt] [PATCH 3/7] qemu: Don't fail migration on save status failure
When we save status XML at the point during migration where we have
already started the domain on destination, we can't really go back and
abort migration. Thus the only thing we can do is to log a warning and
report success.
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
src/qemu/qemu_migration.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 6a3e2e6..9439954 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -3794,10 +3794,8 @@ qemuMigrationConfirmPhase(virQEMUDriverPtr driver,
VIR_DOMAIN_EVENT_RESUMED_MIGRATED);
}
- if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0) {
+ if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0)
VIR_WARN("Failed to save status on vm %s", vm->def->name);
- goto cleanup;
- }
}
done:
@@ -5749,10 +5747,8 @@ qemuMigrationFinish(virQEMUDriverPtr driver,
}
if (virDomainObjIsActive(vm) &&
- virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0) {
+ virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0)
VIR_WARN("Failed to save status on vm %s", vm->def->name);
- goto endjob;
- }
/* Guest is successfully running, so cancel previous auto destroy */
qemuProcessAutoDestroyRemove(driver, vm);
--
2.4.5
2:20 p.m.
New subject: [libvirt] [PATCH 3/7] qemu: Don't fail migration on save status failure
On Wed, Jul 08, 2015 at 19:36:02 +0200, Jiri Denemark wrote:
When we save status XML at the point during migration where we have
already started the domain on destination, we can't really go back and
abort migration. Thus the only thing we can do is to log a warning and
report success.
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
src/qemu/qemu_migration.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
ACK,
Peter
7:36 p.m.
New subject: [libvirt] [PATCH 4/7] qemu: Kill domain when migration finish fails
Whenever something fails during incoming migration in Finish phase
before we started guest CPUs, we need to kill the domain in addition to
reporting the failure.
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
src/qemu/qemu_migration.c | 32 ++++++++++++--------------------
1 file changed, 12 insertions(+), 20 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 9439954..576b32d 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -5589,6 +5589,7 @@ qemuMigrationFinish(virQEMUDriverPtr driver,
qemuDomainObjPrivatePtr priv = vm->privateData;
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
unsigned short port;
+ bool keep = false;
VIR_DEBUG("driver=%p, dconn=%p, vm=%p, cookiein=%s, cookieinlen=%d, "
"cookieout=%p, cookieoutlen=%p, flags=%lx, retcode=%d",
@@ -5647,15 +5648,9 @@ qemuMigrationFinish(virQEMUDriverPtr driver,
}
}
- if (qemuMigrationVPAssociatePortProfiles(vm->def) < 0) {
- qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED,
- VIR_QEMU_PROCESS_STOP_MIGRATED);
- virDomainAuditStop(vm, "failed");
- event = virDomainEventLifecycleNewFromObj(vm,
- VIR_DOMAIN_EVENT_STOPPED,
- VIR_DOMAIN_EVENT_STOPPED_FAILED);
+ if (qemuMigrationVPAssociatePortProfiles(vm->def) < 0)
goto endjob;
- }
+
if (mig->network && qemuDomainMigrateOPDRelocate(driver, vm, mig) <
0)
VIR_WARN("unable to provide network data for relocation");
@@ -5681,11 +5676,10 @@ qemuMigrationFinish(virQEMUDriverPtr driver,
* to restart during confirm() step, so we kill it off now.
*/
if (v3proto) {
- qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED,
- VIR_QEMU_PROCESS_STOP_MIGRATED);
- virDomainAuditStop(vm, "failed");
if (newVM)
vm->persistent = 0;
+ } else {
+ keep = true;
}
goto endjob;
}
@@ -5715,14 +5709,8 @@ qemuMigrationFinish(virQEMUDriverPtr driver,
* target in paused state, in case admin can fix
* things up
*/
- if (v3proto) {
- qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED,
- VIR_QEMU_PROCESS_STOP_MIGRATED);
- virDomainAuditStop(vm, "failed");
- event = virDomainEventLifecycleNewFromObj(vm,
- VIR_DOMAIN_EVENT_STOPPED,
- VIR_DOMAIN_EVENT_STOPPED_FAILED);
- }
+ if (!v3proto)
+ keep = true;
goto endjob;
}
if (priv->job.completed) {
@@ -5761,7 +5749,12 @@ qemuMigrationFinish(virQEMUDriverPtr driver,
qemuMigrationFetchJobStatus(driver, vm,
QEMU_ASYNC_JOB_MIGRATION_IN,
&info);
+ }
+ endjob:
+ if (!dom && !keep &&
+ !(flags & VIR_MIGRATE_OFFLINE) &&
+ virDomainObjIsActive(vm)) {
qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED,
VIR_QEMU_PROCESS_STOP_MIGRATED);
virDomainAuditStop(vm, "failed");
@@ -5770,7 +5763,6 @@ qemuMigrationFinish(virQEMUDriverPtr driver,
VIR_DOMAIN_EVENT_STOPPED_FAILED);
}
- endjob:
if (dom &&
qemuMigrationBakeCookie(mig, driver, vm, cookieout, cookieoutlen,
QEMU_MIGRATION_COOKIE_STATS) < 0)
--
2.4.5
3:19 p.m.
New subject: [libvirt] [PATCH 4/7] qemu: Kill domain when migration finish fails
On Wed, Jul 08, 2015 at 19:36:03 +0200, Jiri Denemark wrote:
Whenever something fails during incoming migration in Finish phase
before we started guest CPUs, we need to kill the domain in addition to
reporting the failure.
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
src/qemu/qemu_migration.c | 32 ++++++++++++--------------------
1 file changed, 12 insertions(+), 20 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 9439954..576b32d 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -5589,6 +5589,7 @@ qemuMigrationFinish(virQEMUDriverPtr driver,
qemuDomainObjPrivatePtr priv = vm->privateData;
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
unsigned short port;
+ bool keep = false;
VIR_DEBUG("driver=%p, dconn=%p, vm=%p, cookiein=%s, cookieinlen=%d, "
"cookieout=%p, cookieoutlen=%p, flags=%lx, retcode=%d",
@@ -5647,15 +5648,9 @@ qemuMigrationFinish(virQEMUDriverPtr driver,
}
}
- if (qemuMigrationVPAssociatePortProfiles(vm->def) < 0) {
- qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED,
- VIR_QEMU_PROCESS_STOP_MIGRATED);
- virDomainAuditStop(vm, "failed");
- event = virDomainEventLifecycleNewFromObj(vm,
- VIR_DOMAIN_EVENT_STOPPED,
- VIR_DOMAIN_EVENT_STOPPED_FAILED);
+ if (qemuMigrationVPAssociatePortProfiles(vm->def) < 0)
goto endjob;
- }
+
if (mig->network && qemuDomainMigrateOPDRelocate(driver, vm, mig)
< 0)
VIR_WARN("unable to provide network data for relocation");
@@ -5681,11 +5676,10 @@ qemuMigrationFinish(virQEMUDriverPtr driver,
* to restart during confirm() step, so we kill it off now.
*/
if (v3proto) {
- qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED,
- VIR_QEMU_PROCESS_STOP_MIGRATED);
- virDomainAuditStop(vm, "failed");
if (newVM)
vm->persistent = 0;
If you choose to fix qemuMigrationPersist so that it sets the persistent
flag only on succes to fix the previous patch then this will create a
conflict since the above statement shouldn't be necessary.
+ } else {
+ keep = true;
}
goto endjob;
}
ACK,
Peter
7:36 p.m.
New subject: [libvirt] [PATCH 5/7] qemu: Don't report false errors in migration protocol v2
Finish is the final state in v2 of our migration protocol. If something
fails, we have no option to abort the migration and resume the original
domain. Non fatal errors (such as failure to start guest CPUs or make
the domain persistent) has to be treated as success. Keeping the domain
running while reporting the failure was just asking for troubles.
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
src/qemu/qemu_migration.c | 20 +++++++-------------
1 file changed, 7 insertions(+), 13 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 576b32d..bca5ad1 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -5589,7 +5589,6 @@ qemuMigrationFinish(virQEMUDriverPtr driver,
qemuDomainObjPrivatePtr priv = vm->privateData;
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
unsigned short port;
- bool keep = false;
VIR_DEBUG("driver=%p, dconn=%p, vm=%p, cookiein=%s, cookieinlen=%d, "
"cookieout=%p, cookieoutlen=%p, flags=%lx, retcode=%d",
@@ -5667,21 +5666,17 @@ qemuMigrationFinish(virQEMUDriverPtr driver,
* other hand, if we report failure, then the management tools
* might try to restart the domain on the source side, even
* though the domain is actually running on the destination.
- * Return a NULL dom pointer, and hope that this is a rare
- * situation and management tools are smart.
- */
-
- /*
+ * Pretend success and hope that this is a rare situation and
+ * management tools are smart.
+ *
* However, in v3 protocol, the source VM is still available
* to restart during confirm() step, so we kill it off now.
*/
if (v3proto) {
if (newVM)
vm->persistent = 0;
- } else {
- keep = true;
+ goto endjob;
}
- goto endjob;
}
}
@@ -5709,9 +5704,8 @@ qemuMigrationFinish(virQEMUDriverPtr driver,
* target in paused state, in case admin can fix
* things up
*/
- if (!v3proto)
- keep = true;
- goto endjob;
+ if (v3proto)
+ goto endjob;
}
if (priv->job.completed) {
qemuDomainJobInfoUpdateTime(priv->job.completed);
@@ -5752,7 +5746,7 @@ qemuMigrationFinish(virQEMUDriverPtr driver,
}
endjob:
- if (!dom && !keep &&
+ if (!dom &&
!(flags & VIR_MIGRATE_OFFLINE) &&
virDomainObjIsActive(vm)) {
qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED,
--
2.4.5
3:21 p.m.
New subject: [libvirt] [PATCH 5/7] qemu: Don't report false errors in migration protocol v2
On Wed, Jul 08, 2015 at 19:36:04 +0200, Jiri Denemark wrote:
Finish is the final state in v2 of our migration protocol. If
something
fails, we have no option to abort the migration and resume the original
domain. Non fatal errors (such as failure to start guest CPUs or make
the domain persistent) has to be treated as success. Keeping the domain
running while reporting the failure was just asking for troubles.
s/es/e/
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
src/qemu/qemu_migration.c | 20 +++++++-------------
1 file changed, 7 insertions(+), 13 deletions(-)
ACK,
Peter
7:36 p.m.
New subject: [libvirt] [PATCH 6/7] qemuDomainEventQueue: Check if event is non-NULL
Every single call to qemuDomainEventQueue() uses the following pattern:
if (event)
qemuDomainEventQueue(driver, event);
Let's move the check for valid event to qemuDomainEventQueue and
simplify all callers.
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
src/qemu/qemu_blockjob.c | 6 ++--
src/qemu/qemu_cgroup.c | 3 +-
src/qemu/qemu_domain.c | 6 ++--
src/qemu/qemu_driver.c | 87 ++++++++++++++++-------------------------------
src/qemu/qemu_hotplug.c | 26 ++++++--------
src/qemu/qemu_migration.c | 24 +++++--------
src/qemu/qemu_process.c | 72 +++++++++++++--------------------------
7 files changed, 78 insertions(+), 146 deletions(-)
diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c
index 8849850..1d5b7ce 100644
--- a/src/qemu/qemu_blockjob.c
+++ b/src/qemu/qemu_blockjob.c
@@ -194,10 +194,8 @@ qemuBlockJobEventProcess(virQEMUDriverPtr driver,
"after block job", vm->def->name);
}
- if (event)
- qemuDomainEventQueue(driver, event);
- if (event2)
- qemuDomainEventQueue(driver, event2);
+ qemuDomainEventQueue(driver, event);
+ qemuDomainEventQueue(driver, event2);
virObjectUnref(cfg);
}
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index 8ed74ee..733a899 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -696,8 +696,7 @@ qemuSetupCpuCgroup(virQEMUDriverPtr driver,
event = virDomainEventTunableNewFromObj(vm, eventParams, eventNparams);
}
- if (event)
- qemuDomainEventQueue(driver, event);
+ qemuDomainEventQueue(driver, event);
}
return 0;
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 8b050a0..6b500c1 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -126,7 +126,8 @@ qemuDomainAsyncJobPhaseFromString(qemuDomainAsyncJob job,
void qemuDomainEventQueue(virQEMUDriverPtr driver,
virObjectEventPtr event)
{
- virObjectEventStateQueue(driver->domainEventState, event);
+ if (event)
+ virObjectEventStateQueue(driver->domainEventState, event);
}
@@ -2652,8 +2653,7 @@ qemuDomainCheckRemoveOptionalDisk(virQEMUDriverPtr driver,
virDomainDiskDefFree(disk);
}
- if (event)
- qemuDomainEventQueue(driver, event);
+ qemuDomainEventQueue(driver, event);
return 0;
}
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index ea4e545..c59834a 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -976,8 +976,7 @@ static void qemuNotifyLoadDomain(virDomainObjPtr vm, int newVM, void
*opaque)
virDomainEventLifecycleNewFromObj(vm,
VIR_DOMAIN_EVENT_DEFINED,
VIR_DOMAIN_EVENT_DEFINED_ADDED);
- if (event)
- qemuDomainEventQueue(driver, event);
+ qemuDomainEventQueue(driver, event);
}
}
@@ -1791,8 +1790,7 @@ static virDomainPtr qemuDomainCreateXML(virConnectPtr conn,
virDomainObjEndAPI(&vm);
if (event) {
qemuDomainEventQueue(driver, event);
- if (event2)
- qemuDomainEventQueue(driver, event2);
+ qemuDomainEventQueue(driver, event2);
}
virObjectUnref(caps);
virObjectUnref(qemuCaps);
@@ -1873,8 +1871,7 @@ static int qemuDomainSuspend(virDomainPtr dom)
cleanup:
virDomainObjEndAPI(&vm);
- if (event)
- qemuDomainEventQueue(driver, event);
+ qemuDomainEventQueue(driver, event);
virObjectUnref(cfg);
return ret;
}
@@ -1933,8 +1930,7 @@ static int qemuDomainResume(virDomainPtr dom)
cleanup:
virDomainObjEndAPI(&vm);
- if (event)
- qemuDomainEventQueue(driver, event);
+ qemuDomainEventQueue(driver, event);
virObjectUnref(cfg);
return ret;
}
@@ -2251,8 +2247,7 @@ qemuDomainDestroyFlags(virDomainPtr dom,
cleanup:
virDomainObjEndAPI(&vm);
- if (event)
- qemuDomainEventQueue(driver, event);
+ qemuDomainEventQueue(driver, event);
return ret;
}
@@ -3275,8 +3270,7 @@ qemuDomainSaveInternal(virQEMUDriverPtr driver, virDomainPtr dom,
cleanup:
VIR_FREE(xml);
- if (event)
- qemuDomainEventQueue(driver, event);
+ qemuDomainEventQueue(driver, event);
virObjectUnref(caps);
return ret;
}
@@ -3777,8 +3771,7 @@ qemuDomainCoreDumpWithFormat(virDomainPtr dom,
cleanup:
virDomainObjEndAPI(&vm);
- if (event)
- qemuDomainEventQueue(driver, event);
+ qemuDomainEventQueue(driver, event);
return ret;
}
@@ -4016,8 +4009,7 @@ processGuestPanicEvent(virQEMUDriverPtr driver,
VIR_DOMAIN_EVENT_CRASHED,
VIR_DOMAIN_EVENT_CRASHED_PANICKED);
- if (event)
- qemuDomainEventQueue(driver, event);
+ qemuDomainEventQueue(driver, event);
if (virDomainLockProcessPause(driver->lockManager, vm, &priv->lockState)
< 0)
VIR_WARN("Unable to release lease on %s", vm->def->name);
@@ -4050,8 +4042,7 @@ processGuestPanicEvent(virQEMUDriverPtr driver,
VIR_DOMAIN_EVENT_STOPPED,
VIR_DOMAIN_EVENT_STOPPED_CRASHED);
- if (event)
- qemuDomainEventQueue(driver, event);
+ qemuDomainEventQueue(driver, event);
virDomainAuditStop(vm, "destroyed");
@@ -4494,9 +4485,9 @@ processSerialChangedEvent(virQEMUDriverPtr driver,
}
}
- if ((event = virDomainEventAgentLifecycleNewFromObj(vm, newstate,
-
VIR_CONNECT_DOMAIN_EVENT_AGENT_LIFECYCLE_REASON_CHANNEL)))
- qemuDomainEventQueue(driver, event);
+ event = virDomainEventAgentLifecycleNewFromObj(vm, newstate,
+
VIR_CONNECT_DOMAIN_EVENT_AGENT_LIFECYCLE_REASON_CHANNEL);
+ qemuDomainEventQueue(driver, event);
}
dev.data.chr->state = newstate;
@@ -5194,8 +5185,7 @@ qemuDomainPinVcpuFlags(virDomainPtr dom,
if (cgroup_vcpu)
virCgroupFree(&cgroup_vcpu);
virDomainObjEndAPI(&vm);
- if (event)
- qemuDomainEventQueue(driver, event);
+ qemuDomainEventQueue(driver, event);
VIR_FREE(str);
virBitmapFree(pcpumap);
virObjectUnref(cfg);
@@ -5391,8 +5381,7 @@ qemuDomainPinEmulator(virDomainPtr dom,
cleanup:
if (cgroup_emulator)
virCgroupFree(&cgroup_emulator);
- if (event)
- qemuDomainEventQueue(driver, event);
+ qemuDomainEventQueue(driver, event);
VIR_FREE(str);
virBitmapFree(pcpumap);
virDomainObjEndAPI(&vm);
@@ -5886,8 +5875,7 @@ qemuDomainPinIOThread(virDomainPtr dom,
cleanup:
if (cgroup_iothread)
virCgroupFree(&cgroup_iothread);
- if (event)
- qemuDomainEventQueue(driver, event);
+ qemuDomainEventQueue(driver, event);
VIR_FREE(str);
virBitmapFree(pcpumap);
virDomainObjEndAPI(&vm);
@@ -6689,8 +6677,7 @@ qemuDomainSaveImageStartVM(virConnectPtr conn,
VIR_DOMAIN_EVENT_STARTED,
VIR_DOMAIN_EVENT_STARTED_RESTORED);
virDomainAuditStart(vm, "restored", true);
- if (event)
- qemuDomainEventQueue(driver, event);
+ qemuDomainEventQueue(driver, event);
/* If it was running before, resume it now unless caller requested pause. */
@@ -6713,8 +6700,7 @@ qemuDomainSaveImageStartVM(virConnectPtr conn,
event = virDomainEventLifecycleNewFromObj(vm,
VIR_DOMAIN_EVENT_SUSPENDED,
detail);
- if (event)
- qemuDomainEventQueue(driver, event);
+ qemuDomainEventQueue(driver, event);
}
ret = 0;
@@ -7382,8 +7368,7 @@ qemuDomainObjStart(virConnectPtr conn,
event = virDomainEventLifecycleNewFromObj(vm,
VIR_DOMAIN_EVENT_SUSPENDED,
VIR_DOMAIN_EVENT_SUSPENDED_PAUSED);
- if (event)
- qemuDomainEventQueue(driver, event);
+ qemuDomainEventQueue(driver, event);
}
}
}
@@ -7532,8 +7517,7 @@ static virDomainPtr qemuDomainDefineXMLFlags(virConnectPtr conn,
const char *xml
virDomainDefFree(oldDef);
virDomainDefFree(def);
virDomainObjEndAPI(&vm);
- if (event)
- qemuDomainEventQueue(driver, event);
+ qemuDomainEventQueue(driver, event);
virObjectUnref(qemuCaps);
virObjectUnref(caps);
virObjectUnref(cfg);
@@ -7650,8 +7634,7 @@ qemuDomainUndefineFlags(virDomainPtr dom,
cleanup:
VIR_FREE(name);
virDomainObjEndAPI(&vm);
- if (event)
- qemuDomainEventQueue(driver, event);
+ qemuDomainEventQueue(driver, event);
virObjectUnref(cfg);
return ret;
}
@@ -7799,8 +7782,7 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm,
* is in monitor */
virObjectEventPtr event;
event = virDomainEventDeviceAddedNewFromObj(vm, alias);
- if (event)
- qemuDomainEventQueue(driver, event);
+ qemuDomainEventQueue(driver, event);
}
if (ret == 0)
@@ -10466,8 +10448,7 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom,
if (eventNparams) {
event = virDomainEventTunableNewFromDom(dom, eventParams, eventNparams);
eventNparams = 0;
- if (event)
- qemuDomainEventQueue(driver, event);
+ qemuDomainEventQueue(driver, event);
}
if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
@@ -13677,8 +13658,7 @@ qemuDomainSnapshotCreateActiveInternal(virConnectPtr conn,
}
}
- if (event)
- qemuDomainEventQueue(driver, event);
+ qemuDomainEventQueue(driver, event);
return ret;
}
@@ -14499,8 +14479,7 @@ qemuDomainSnapshotCreateActiveExternal(virConnectPtr conn,
virDomainAuditStop(vm, "from-snapshot");
resume = false;
thaw = 0;
- if (event)
- qemuDomainEventQueue(driver, event);
+ qemuDomainEventQueue(driver, event);
} else if (memory && pmsuspended) {
/* qemu 1.3 is unable to save a domain in pm-suspended (S3)
* state; so we must emit an event stating that it was
@@ -14509,8 +14488,7 @@ qemuDomainSnapshotCreateActiveExternal(virConnectPtr conn,
VIR_DOMAIN_PAUSED_FROM_SNAPSHOT);
event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_SUSPENDED,
VIR_DOMAIN_EVENT_SUSPENDED_FROM_SNAPSHOT);
- if (event)
- qemuDomainEventQueue(driver, event);
+ qemuDomainEventQueue(driver, event);
}
ret = 0;
@@ -14523,8 +14501,7 @@ qemuDomainSnapshotCreateActiveExternal(virConnectPtr conn,
event = virDomainEventLifecycleNewFromObj(vm,
VIR_DOMAIN_EVENT_SUSPENDED,
VIR_DOMAIN_EVENT_SUSPENDED_API_ERROR);
- if (event)
- qemuDomainEventQueue(driver, event);
+ qemuDomainEventQueue(driver, event);
if (virGetLastError() == NULL) {
virReportError(VIR_ERR_OPERATION_FAILED, "%s",
_("resuming after snapshot failed"));
@@ -15363,8 +15340,7 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
event = virDomainEventLifecycleNewFromObj(vm,
VIR_DOMAIN_EVENT_STOPPED,
detail);
- if (event)
- qemuDomainEventQueue(driver, event);
+ qemuDomainEventQueue(driver, event);
goto load;
}
@@ -15502,8 +15478,7 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
start_flags |= paused ? VIR_QEMU_PROCESS_START_PAUSED : 0;
- if (event)
- qemuDomainEventQueue(driver, event);
+ qemuDomainEventQueue(driver, event);
rc = qemuProcessStart(snapshot->domain->conn, driver, vm,
QEMU_ASYNC_JOB_NONE, NULL, -1, NULL, NULL,
VIR_NETDEV_VPORT_PROFILE_OP_CREATE,
@@ -15563,8 +15538,7 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
}
if (event) {
qemuDomainEventQueue(driver, event);
- if (event2)
- qemuDomainEventQueue(driver, event2);
+ qemuDomainEventQueue(driver, event2);
}
virDomainObjEndAPI(&vm);
virObjectUnref(caps);
@@ -17635,8 +17609,7 @@ qemuDomainSetBlockIoTune(virDomainPtr dom,
if (eventNparams) {
event = virDomainEventTunableNewFromDom(dom, eventParams, eventNparams);
eventNparams = 0;
- if (event)
- qemuDomainEventQueue(driver, event);
+ qemuDomainEventQueue(driver, event);
}
}
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 79338cf..5436be9 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1809,8 +1809,7 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver,
}
event = virDomainEventDeviceAddedNewFromObj(vm, objalias);
- if (event)
- qemuDomainEventQueue(driver, event);
+ qemuDomainEventQueue(driver, event);
/* fix the balloon size if it was set to maximum */
if (fix_balloon)
@@ -2843,8 +2842,7 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver,
virDomainAuditDisk(vm, disk->src, NULL, "detach", true);
event = virDomainEventDeviceRemovedNewFromObj(vm, disk->info.alias);
- if (event)
- qemuDomainEventQueue(driver, event);
+ qemuDomainEventQueue(driver, event);
for (i = 0; i < vm->def->ndisks; i++) {
if (vm->def->disks[i] == disk) {
@@ -2886,8 +2884,7 @@ qemuDomainRemoveControllerDevice(virQEMUDriverPtr driver,
controller->info.alias, vm, vm->def->name);
event = virDomainEventDeviceRemovedNewFromObj(vm, controller->info.alias);
- if (event)
- qemuDomainEventQueue(driver, event);
+ qemuDomainEventQueue(driver, event);
for (i = 0; i < vm->def->ncontrollers; i++) {
if (vm->def->controllers[i] == controller) {
@@ -2918,8 +2915,8 @@ qemuDomainRemoveMemoryDevice(virQEMUDriverPtr driver,
VIR_DEBUG("Removing memory device %s from domain %p %s",
mem->info.alias, vm, vm->def->name);
- if ((event = virDomainEventDeviceRemovedNewFromObj(vm, mem->info.alias)))
- qemuDomainEventQueue(driver, event);
+ event = virDomainEventDeviceRemovedNewFromObj(vm, mem->info.alias);
+ qemuDomainEventQueue(driver, event);
if (virAsprintf(&backendAlias, "mem%s", mem->info.alias) < 0)
goto cleanup;
@@ -3002,8 +2999,7 @@ qemuDomainRemoveHostDevice(virQEMUDriverPtr driver,
}
event = virDomainEventDeviceRemovedNewFromObj(vm, hostdev->info->alias);
- if (event)
- qemuDomainEventQueue(driver, event);
+ qemuDomainEventQueue(driver, event);
if (hostdev->parent.type == VIR_DOMAIN_DEVICE_NET) {
net = hostdev->parent.data.net;
@@ -3117,8 +3113,7 @@ qemuDomainRemoveNetDevice(virQEMUDriverPtr driver,
virDomainAuditNet(vm, net, NULL, "detach", true);
event = virDomainEventDeviceRemovedNewFromObj(vm, net->info.alias);
- if (event)
- qemuDomainEventQueue(driver, event);
+ qemuDomainEventQueue(driver, event);
for (i = 0; i < vm->def->nnets; i++) {
if (vm->def->nets[i] == net) {
@@ -3195,8 +3190,7 @@ qemuDomainRemoveChrDevice(virQEMUDriverPtr driver,
goto cleanup;
event = virDomainEventDeviceRemovedNewFromObj(vm, chr->info.alias);
- if (event)
- qemuDomainEventQueue(driver, event);
+ qemuDomainEventQueue(driver, event);
qemuDomainChrRemove(vm->def, chr);
virDomainChrDefFree(chr);
@@ -3244,8 +3238,8 @@ qemuDomainRemoveRNGDevice(virQEMUDriverPtr driver,
if (rc < 0)
goto cleanup;
- if ((event = virDomainEventDeviceRemovedNewFromObj(vm, rng->info.alias)))
- qemuDomainEventQueue(driver, event);
+ event = virDomainEventDeviceRemovedNewFromObj(vm, rng->info.alias);
+ qemuDomainEventQueue(driver, event);
if ((idx = virDomainRNGFind(vm->def, rng)) >= 0)
virDomainRNGRemove(vm->def, idx);
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index bca5ad1..ae7433e 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -2289,8 +2289,7 @@ qemuMigrationSetOffline(virQEMUDriverPtr driver,
event = virDomainEventLifecycleNewFromObj(vm,
VIR_DOMAIN_EVENT_SUSPENDED,
VIR_DOMAIN_EVENT_SUSPENDED_MIGRATED);
- if (event)
- qemuDomainEventQueue(driver, event);
+ qemuDomainEventQueue(driver, event);
}
return ret;
@@ -3454,8 +3453,7 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
qemuDomainRemoveInactive(driver, vm);
}
virDomainObjEndAPI(&vm);
- if (event)
- qemuDomainEventQueue(driver, event);
+ qemuDomainEventQueue(driver, event);
qemuMigrationCookieFree(mig);
virObjectUnref(caps);
virNWFilterUnlockFilterUpdates();
@@ -3803,8 +3801,7 @@ qemuMigrationConfirmPhase(virQEMUDriverPtr driver,
rv = 0;
cleanup:
- if (event)
- qemuDomainEventQueue(driver, event);
+ qemuDomainEventQueue(driver, event);
virObjectUnref(cfg);
return rv;
}
@@ -5330,8 +5327,7 @@ qemuMigrationPerformJob(virQEMUDriverPtr driver,
cleanup:
virDomainObjEndAPI(&vm);
- if (event)
- qemuDomainEventQueue(driver, event);
+ qemuDomainEventQueue(driver, event);
virObjectUnref(cfg);
return ret;
}
@@ -5399,8 +5395,7 @@ qemuMigrationPerformPhase(virQEMUDriverPtr driver,
cleanup:
virDomainObjEndAPI(&vm);
- if (event)
- qemuDomainEventQueue(driver, event);
+ qemuDomainEventQueue(driver, event);
return ret;
}
@@ -5557,8 +5552,7 @@ qemuMigrationPersist(virQEMUDriverPtr driver,
newVM ?
VIR_DOMAIN_EVENT_DEFINED_ADDED :
VIR_DOMAIN_EVENT_DEFINED_UPDATED);
- if (event)
- qemuDomainEventQueue(driver, event);
+ qemuDomainEventQueue(driver, event);
ret = 0;
@@ -5721,8 +5715,7 @@ qemuMigrationFinish(virQEMUDriverPtr driver,
if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_PAUSED) {
virDomainObjSetState(vm, VIR_DOMAIN_PAUSED,
VIR_DOMAIN_PAUSED_USER);
- if (event)
- qemuDomainEventQueue(driver, event);
+ qemuDomainEventQueue(driver, event);
event = virDomainEventLifecycleNewFromObj(vm,
VIR_DOMAIN_EVENT_SUSPENDED,
VIR_DOMAIN_EVENT_SUSPENDED_PAUSED);
@@ -5772,8 +5765,7 @@ qemuMigrationFinish(virQEMUDriverPtr driver,
qemuMonitorSetDomainLog(priv->mon, -1);
VIR_FREE(priv->origname);
virDomainObjEndAPI(&vm);
- if (event)
- qemuDomainEventQueue(driver, event);
+ qemuDomainEventQueue(driver, event);
qemuMigrationCookieFree(mig);
if (orig_err) {
virSetError(orig_err);
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 14d3db6..3f7a9dc 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -329,8 +329,7 @@ qemuProcessHandleMonitorEOF(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
virObjectUnlock(vm);
cleanup:
- if (event)
- qemuDomainEventQueue(driver, event);
+ qemuDomainEventQueue(driver, event);
}
@@ -354,8 +353,7 @@ qemuProcessHandleMonitorError(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
((qemuDomainObjPrivatePtr) vm->privateData)->monError = true;
event = virDomainEventControlErrorNewFromObj(vm);
- if (event)
- qemuDomainEventQueue(driver, event);
+ qemuDomainEventQueue(driver, event);
virObjectUnlock(vm);
}
@@ -518,8 +516,7 @@ qemuProcessHandleReset(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
virObjectUnlock(vm);
- if (event)
- qemuDomainEventQueue(driver, event);
+ qemuDomainEventQueue(driver, event);
virObjectUnref(cfg);
return 0;
@@ -595,8 +592,7 @@ qemuProcessFakeReboot(void *opaque)
if (ret == -1)
ignore_value(qemuProcessKill(vm, VIR_QEMU_PROCESS_KILL_FORCE));
virDomainObjEndAPI(&vm);
- if (event)
- qemuDomainEventQueue(driver, event);
+ qemuDomainEventQueue(driver, event);
virObjectUnref(cfg);
}
@@ -645,8 +641,7 @@ qemuProcessHandleEvent(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
seconds, micros, details);
virObjectUnlock(vm);
- if (event)
- qemuDomainEventQueue(driver, event);
+ qemuDomainEventQueue(driver, event);
return 0;
}
@@ -699,8 +694,7 @@ qemuProcessHandleShutdown(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
unlock:
virObjectUnlock(vm);
- if (event)
- qemuDomainEventQueue(driver, event);
+ qemuDomainEventQueue(driver, event);
virObjectUnref(cfg);
return 0;
@@ -749,8 +743,7 @@ qemuProcessHandleStop(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
unlock:
virObjectUnlock(vm);
- if (event)
- qemuDomainEventQueue(driver, event);
+ qemuDomainEventQueue(driver, event);
virObjectUnref(cfg);
return 0;
@@ -803,8 +796,7 @@ qemuProcessHandleResume(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
unlock:
virObjectUnlock(vm);
- if (event)
- qemuDomainEventQueue(driver, event);
+ qemuDomainEventQueue(driver, event);
virObjectUnref(cfg);
return 0;
}
@@ -848,8 +840,7 @@ qemuProcessHandleRTCChange(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
virObjectUnlock(vm);
- if (event)
- qemuDomainEventQueue(driver, event);
+ qemuDomainEventQueue(driver, event);
virObjectUnref(cfg);
return 0;
}
@@ -910,10 +901,8 @@ qemuProcessHandleWatchdog(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
if (vm)
virObjectUnlock(vm);
- if (watchdogEvent)
- qemuDomainEventQueue(driver, watchdogEvent);
- if (lifecycleEvent)
- qemuDomainEventQueue(driver, lifecycleEvent);
+ qemuDomainEventQueue(driver, watchdogEvent);
+ qemuDomainEventQueue(driver, lifecycleEvent);
virObjectUnref(cfg);
return 0;
@@ -974,12 +963,9 @@ qemuProcessHandleIOError(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
}
virObjectUnlock(vm);
- if (ioErrorEvent)
- qemuDomainEventQueue(driver, ioErrorEvent);
- if (ioErrorEvent2)
- qemuDomainEventQueue(driver, ioErrorEvent2);
- if (lifecycleEvent)
- qemuDomainEventQueue(driver, lifecycleEvent);
+ qemuDomainEventQueue(driver, ioErrorEvent);
+ qemuDomainEventQueue(driver, ioErrorEvent2);
+ qemuDomainEventQueue(driver, lifecycleEvent);
virObjectUnref(cfg);
return 0;
}
@@ -1102,8 +1088,7 @@ qemuProcessHandleGraphics(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
event = virDomainEventGraphicsNewFromObj(vm, phase, localAddr, remoteAddr,
authScheme, subject);
virObjectUnlock(vm);
- if (event)
- qemuDomainEventQueue(driver, event);
+ qemuDomainEventQueue(driver, event);
return 0;
@@ -1162,8 +1147,7 @@ qemuProcessHandleTrayChange(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
}
virObjectUnlock(vm);
- if (event)
- qemuDomainEventQueue(driver, event);
+ qemuDomainEventQueue(driver, event);
virObjectUnref(cfg);
return 0;
}
@@ -1201,10 +1185,8 @@ qemuProcessHandlePMWakeup(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
}
virObjectUnlock(vm);
- if (event)
- qemuDomainEventQueue(driver, event);
- if (lifecycleEvent)
- qemuDomainEventQueue(driver, lifecycleEvent);
+ qemuDomainEventQueue(driver, event);
+ qemuDomainEventQueue(driver, lifecycleEvent);
virObjectUnref(cfg);
return 0;
}
@@ -1245,10 +1227,8 @@ qemuProcessHandlePMSuspend(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
virObjectUnlock(vm);
- if (event)
- qemuDomainEventQueue(driver, event);
- if (lifecycleEvent)
- qemuDomainEventQueue(driver, lifecycleEvent);
+ qemuDomainEventQueue(driver, event);
+ qemuDomainEventQueue(driver, lifecycleEvent);
virObjectUnref(cfg);
return 0;
}
@@ -1275,8 +1255,7 @@ qemuProcessHandleBalloonChange(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
virObjectUnlock(vm);
- if (event)
- qemuDomainEventQueue(driver, event);
+ qemuDomainEventQueue(driver, event);
virObjectUnref(cfg);
return 0;
}
@@ -1317,10 +1296,8 @@ qemuProcessHandlePMSuspendDisk(qemuMonitorPtr mon
ATTRIBUTE_UNUSED,
virObjectUnlock(vm);
- if (event)
- qemuDomainEventQueue(driver, event);
- if (lifecycleEvent)
- qemuDomainEventQueue(driver, lifecycleEvent);
+ qemuDomainEventQueue(driver, event);
+ qemuDomainEventQueue(driver, lifecycleEvent);
virObjectUnref(cfg);
return 0;
@@ -5696,8 +5673,7 @@ qemuProcessAutoDestroy(virDomainObjPtr dom,
dom = NULL;
}
- if (event)
- qemuDomainEventQueue(driver, event);
+ qemuDomainEventQueue(driver, event);
cleanup:
return dom;
--
2.4.5
9:58 a.m.
New subject: [libvirt] [PATCH 6/7] qemuDomainEventQueue: Check if event is non-NULL
On Wed, Jul 08, 2015 at 19:36:05 +0200, Jiri Denemark wrote:
Every single call to qemuDomainEventQueue() uses the following
pattern:
if (event)
qemuDomainEventQueue(driver, event);
Let's move the check for valid event to qemuDomainEventQueue and
simplify all callers.
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
src/qemu/qemu_blockjob.c | 6 ++--
src/qemu/qemu_cgroup.c | 3 +-
src/qemu/qemu_domain.c | 6 ++--
src/qemu/qemu_driver.c | 87 ++++++++++++++++-------------------------------
src/qemu/qemu_hotplug.c | 26 ++++++--------
src/qemu/qemu_migration.c | 24 +++++--------
src/qemu/qemu_process.c | 72 +++++++++++++--------------------------
7 files changed, 78 insertions(+), 146 deletions(-)
Yay \o/, finally this is taken care of.
ACK
Peter
7:36 p.m.
New subject: [libvirt] [PATCH 7/7] qemu: Queue events in migration Finish phase ASAP
For quite a long time we don't need to postpone queueing events until
the end of the function since we no longer have the big driver lock.
Let's make the code of qemuMigrationFinish simpler by queuing events at
the time we generate them.
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
src/qemu/qemu_migration.c | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index ae7433e..cc7754c 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -5576,7 +5576,6 @@ qemuMigrationFinish(virQEMUDriverPtr driver,
bool v3proto)
{
virDomainPtr dom = NULL;
- virObjectEventPtr event = NULL;
qemuMigrationCookiePtr mig = NULL;
virErrorPtr orig_err = NULL;
int cookie_flags = 0;
@@ -5709,16 +5708,17 @@ qemuMigrationFinish(virQEMUDriverPtr driver,
dom = virGetDomain(dconn, vm->def->name, vm->def->uuid);
- event = virDomainEventLifecycleNewFromObj(vm,
- VIR_DOMAIN_EVENT_RESUMED,
- VIR_DOMAIN_EVENT_RESUMED_MIGRATED);
+ qemuDomainEventQueue(driver,
+ virDomainEventLifecycleNewFromObj(vm,
+ VIR_DOMAIN_EVENT_RESUMED,
+ VIR_DOMAIN_EVENT_RESUMED_MIGRATED));
if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_PAUSED) {
virDomainObjSetState(vm, VIR_DOMAIN_PAUSED,
VIR_DOMAIN_PAUSED_USER);
- qemuDomainEventQueue(driver, event);
- event = virDomainEventLifecycleNewFromObj(vm,
- VIR_DOMAIN_EVENT_SUSPENDED,
- VIR_DOMAIN_EVENT_SUSPENDED_PAUSED);
+ qemuDomainEventQueue(driver,
+ virDomainEventLifecycleNewFromObj(vm,
+ VIR_DOMAIN_EVENT_SUSPENDED,
+ VIR_DOMAIN_EVENT_SUSPENDED_PAUSED));
}
if (virDomainObjIsActive(vm) &&
@@ -5745,9 +5745,10 @@ qemuMigrationFinish(virQEMUDriverPtr driver,
qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED,
VIR_QEMU_PROCESS_STOP_MIGRATED);
virDomainAuditStop(vm, "failed");
- event = virDomainEventLifecycleNewFromObj(vm,
- VIR_DOMAIN_EVENT_STOPPED,
- VIR_DOMAIN_EVENT_STOPPED_FAILED);
+ qemuDomainEventQueue(driver,
+ virDomainEventLifecycleNewFromObj(vm,
+ VIR_DOMAIN_EVENT_STOPPED,
+ VIR_DOMAIN_EVENT_STOPPED_FAILED));
}
if (dom &&
@@ -5765,7 +5766,6 @@ qemuMigrationFinish(virQEMUDriverPtr driver,
qemuMonitorSetDomainLog(priv->mon, -1);
VIR_FREE(priv->origname);
virDomainObjEndAPI(&vm);
- qemuDomainEventQueue(driver, event);
qemuMigrationCookieFree(mig);
if (orig_err) {
virSetError(orig_err);
--
2.4.5
10:14 a.m.
New subject: [libvirt] [PATCH 7/7] qemu: Queue events in migration Finish phase ASAP
On Wed, Jul 08, 2015 at 19:36:06 +0200, Jiri Denemark wrote:
For quite a long time we don't need to postpone queueing events
until
the end of the function since we no longer have the big driver lock.
Let's make the code of qemuMigrationFinish simpler by queuing events at
the time we generate them.
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
src/qemu/qemu_migration.c | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index ae7433e..cc7754c 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
...
@@ -5709,16 +5708,17 @@ qemuMigrationFinish(virQEMUDriverPtr driver,
dom = virGetDomain(dconn, vm->def->name, vm->def->uuid);
- event = virDomainEventLifecycleNewFromObj(vm,
- VIR_DOMAIN_EVENT_RESUMED,
- VIR_DOMAIN_EVENT_RESUMED_MIGRATED);
+ qemuDomainEventQueue(driver,
+ virDomainEventLifecycleNewFromObj(vm,
+ VIR_DOMAIN_EVENT_RESUMED,
+ VIR_DOMAIN_EVENT_RESUMED_MIGRATED));
I dislike this formatting.
if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_PAUSED) {
virDomainObjSetState(vm, VIR_DOMAIN_PAUSED,
VIR_DOMAIN_PAUSED_USER);
- qemuDomainEventQueue(driver, event);
- event = virDomainEventLifecycleNewFromObj(vm,
- VIR_DOMAIN_EVENT_SUSPENDED,
- VIR_DOMAIN_EVENT_SUSPENDED_PAUSED);
+ qemuDomainEventQueue(driver,
+ virDomainEventLifecycleNewFromObj(vm,
+ VIR_DOMAIN_EVENT_SUSPENDED,
+ VIR_DOMAIN_EVENT_SUSPENDED_PAUSED));
}
if (virDomainObjIsActive(vm) &&
ACK,
Peter
3357
days inactive
3358
days old
14 comments
2 participants
participants (2)
-
Jiri Denemark -
Peter Krempa