On Tue, Nov 24, 2015 at 10:39:20 +0100, Pavel Hrdina wrote:
> On Thu, Nov 19, 2015 at 01:09:19PM +0100, Jiri Denemark wrote:
> > Some failure paths in qemuMigrationPrepareAny forgot to kill the just
> > started QEMU process. This patch fixes this by combining 'stop' and
> > 'endjob' label into a new label 'stopjob'. This name was chosen to avoid
> > confusion with the most common semantics of 'endjob'. Normally, 'endjob'
> > is always called at the end of an API to stop the job we entered at the
> > beginning. In qemuMigrationPrepareAny we only want to stop the job in
> > failure path; on success we need to carry the job over to the Finish
> > phase.
> >
> > Signed-off-by: Jiri Denemark <jdenemar@xxxxxxxxxx>
> > ---
> >
> > Notes:
> > ACKed in version 1
> >
> > Version 2:
> > - no change
> >
> > src/qemu/qemu_migration.c | 31 ++++++++++++++++---------------
> > 1 file changed, 16 insertions(+), 15 deletions(-)
> >
> > diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> > index 4ab6ab7..9a4f19f 100644
> > --- a/src/qemu/qemu_migration.c
> > +++ b/src/qemu/qemu_migration.c
...
> > @@ -3593,7 +3593,7 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
> > if (qemuMigrationStartNBDServer(driver, vm, listenAddress,
> > nmigrate_disks, migrate_disks) < 0) {
> > /* error already reported */
> > - goto endjob;
> > + goto stopjob;
> > }
> > cookieFlags |= QEMU_MIGRATION_COOKIE_NBD;
> > }
> > @@ -3608,7 +3608,7 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
> > }
> >
> > if (qemuDomainCleanupAdd(vm, qemuMigrationPrepareCleanup) < 0)
> > - goto endjob;
> > + goto stopjob;
> >
> > if (!(flags & VIR_MIGRATE_OFFLINE)) {
> > virDomainAuditStart(vm, "migrated", true);
...
> I wouldn't join those two labels together regarding the change in 1/7 patch.
> Just change all goto after qemuProcessStart to jump to stop. About the naming
> we've discussed, what about endmigjob? It's maybe to long, but tels, that we are
> about to stop the qemuMigrationJob.
The bug was in the two hunks above. There are two paths going through
the code there, one path would need to stop the job and the process
while the other path should only stop the job. So instead of having the
logic there, I merged the two labels and decide what to do in one place.
Jirka
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list