On Fri, Nov 13, 2015 at 10:17:16 -0500, John Ferlan wrote:
On 11/12/2015 01:37 PM, Jiri Denemark wrote:
Once qemuProcessInit was called, qemuProcessLaunch will launch a new QEMU process with stopped virtual CPUs.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_process.c | 162 ++++++++++++++++++++++++++++++++---------------- src/qemu/qemu_process.h | 9 +++ 2 files changed, 118 insertions(+), 53 deletions(-)
Been following along with the review so far - have a question though...
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 5735935..0314c4a 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c
[..]
+int +qemuProcessLaunch(virConnectPtr conn, + virQEMUDriverPtr driver, + virDomainObjPtr vm, + qemuDomainAsyncJob asyncJob, + qemuProcessIncomingDefPtr incoming, + virDomainSnapshotObjPtr snapshot, + virNetDevVPortProfileOp vmop, + unsigned int flags)
[...]
VIR_DEBUG("Setting domain security labels"); - if (virSecurityManagerSetAllLabel(driver->securityManager, - vm->def, migratePath) < 0) + if (incoming && + virSecurityManagerSetAllLabel(driver->securityManager, + vm->def, incoming->path) < 0)
shouldn't this be
if (virSecurityManagerSetAllLabel(driver->securityManager, vm->def, incoming ? incoming->path : NULL) < 0)
Previously we'd call SetAllLabel with migratePath as NULL anyway...
goto error;
Hmm, right you are. Thanks for catching this.
[...]
+ + error: + /* We jump here if we failed to start the VM for any reason, or + * if we failed to initialize the now running VM. kill it off and + * pretend we never started it */ + qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED, stop_flags);
Doesn't this happen in qemuProcessStart too? IOW would this happen twice?
Well, yes, although running it twice does nothing except of logging a "VM not active" debug message. However, a much bigger problem is in the previous patch, which causes qemuProcessStop in case we fail to start a domain because it is already running, not something you'd expect :-) Jirka