[libvirt] [PATCH v2 1/7] qemu: Introduce qemuProcessInit

Pavel Hrdina phrdina at redhat.com
Mon Nov 23 15:53:19 UTC 2015


On Thu, Nov 19, 2015 at 01:09:15PM +0100, Jiri Denemark wrote:
> qemuProcessStart is going to be split in three parts: qemuProcessInit,
> qemuProcessLaunch, and qemuProcessFinish so that migration Prepare phase
> can insert additional code in the process. qemuProcessStart will be a
> small wrapper for all other callers.
> 
> qemuProcessInit prepares the domain up to the point when priv->qemuCaps
> is initialized.
> 
> Signed-off-by: Jiri Denemark <jdenemar at redhat.com>
> ---
> 
> Notes:
>     Version 2:
>     - avoid calling qemuProcessStop when starting fails with
>       "VM is already active"
> 
>  src/qemu/qemu_process.c | 133 ++++++++++++++++++++++++++++++++----------------
>  src/qemu/qemu_process.h |   4 ++
>  2 files changed, 92 insertions(+), 45 deletions(-)
> 
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 2192ad8..784ae08 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -4476,6 +4476,86 @@ qemuProcessMakeDir(virQEMUDriverPtr driver,
>  }
>  
>  
> +/**
> + * qemuProcessInit:
> + *
> + * Prepares the domain up to the point when priv->qemuCaps is initialized.
> + *
> + * @returns 0 on success,
> + *          -1 on error requiring qemuProcessStop to be called,
> + *          -2 on error when qemuProcessStop must NOT be called.
> + */

I don't like this as there is no benefit from letting the caller to decide
whether qemuProcessStop needs to be called or not.  Why don't just do it inside
qemuProcessInit and add a comment that qemuProcessStop must not be called at all
if this function fails?

I know, that last patch updates qemuMigrationPrepareAny and that patch 5/7
joined 'stop' with 'endjob' but that's not necessary too.  Just jump to
'stop' instead to 'stopjob'.  Then it would be possible in case of
qemuProcessInit fails jump to 'endjob' in qemuMigrationPrepareAny.

Pavel

[...]




More information about the libvir-list mailing list