[PATCH v2 1/6] qemu_process.c: check migrateURI when setting VIR_QEMU_PROCESS_START_NEW

Daniel Henrique Barboza posted 6 patches 2 months ago

[PATCH v2 1/6] qemu_process.c: check migrateURI when setting VIR_QEMU_PROCESS_START_NEW

Posted by Daniel Henrique Barboza 2 months ago
qemuProcessCreatePretendCmdPrepare() is setting the
VIR_QEMU_PROCESS_START_NEW regardless of whether this is
a migration case or not. This behavior differs from what we're
doing in qemuProcessStart(), where the flag is set only
if !migrate && !snapshot.

Fix it by making the flag setting consistent with what we're
doing in qemuProcessStart().

Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
 src/qemu/qemu_process.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 579b3c3713..3677da635c 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -7436,7 +7436,10 @@ qemuProcessCreatePretendCmdPrepare(virQEMUDriverPtr driver,
                   VIR_QEMU_PROCESS_START_AUTODESTROY, -1);
 
     flags |= VIR_QEMU_PROCESS_START_PRETEND;
-    flags |= VIR_QEMU_PROCESS_START_NEW;
+
+    if (!migrateURI)
+        flags |= VIR_QEMU_PROCESS_START_NEW;
+
     if (standalone)
         flags |= VIR_QEMU_PROCESS_START_STANDALONE;
 
-- 
2.26.2

Re: [PATCH v2 1/6] qemu_process.c: check migrateURI when setting VIR_QEMU_PROCESS_START_NEW

Posted by Andrea Bolognani 1 month, 2 weeks ago
On Wed, 2020-11-18 at 16:58 -0300, Daniel Henrique Barboza wrote:
> qemuProcessCreatePretendCmdPrepare() is setting the
> VIR_QEMU_PROCESS_START_NEW regardless of whether this is
> a migration case or not. This behavior differs from what we're
> doing in qemuProcessStart(), where the flag is set only
> if !migrate && !snapshot.
> 
> Fix it by making the flag setting consistent with what we're
> doing in qemuProcessStart().
> 
> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> ---
>  src/qemu/qemu_process.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)

Reviewed-by: Andrea Bolognani <abologna@redhat.com>

-- 
Andrea Bolognani / Red Hat / Virtualization

Re: [PATCH v2 1/6] qemu_process.c: check migrateURI when setting VIR_QEMU_PROCESS_START_NEW

Posted by Michal Privoznik 1 month, 2 weeks ago
On 11/18/20 8:58 PM, Daniel Henrique Barboza wrote:
> qemuProcessCreatePretendCmdPrepare() is setting the
> VIR_QEMU_PROCESS_START_NEW regardless of whether this is
> a migration case or not. This behavior differs from what we're
> doing in qemuProcessStart(), where the flag is set only
> if !migrate && !snapshot.
> 
> Fix it by making the flag setting consistent with what we're
> doing in qemuProcessStart().
> 
> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> ---
>   src/qemu/qemu_process.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 579b3c3713..3677da635c 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -7436,7 +7436,10 @@ qemuProcessCreatePretendCmdPrepare(virQEMUDriverPtr driver,
>                     VIR_QEMU_PROCESS_START_AUTODESTROY, -1);
>   
>       flags |= VIR_QEMU_PROCESS_START_PRETEND;
> -    flags |= VIR_QEMU_PROCESS_START_NEW;
> +
> +    if (!migrateURI)
> +        flags |= VIR_QEMU_PROCESS_START_NEW;
> +
>       if (standalone)
>           flags |= VIR_QEMU_PROCESS_START_STANDALONE;
>   
> 

Le sigh, this lead me down the rabbit hole of VIR_QEMU_PROCESS_START_NEW 
flag and where it's used and what consequences this can have. Then I 
read the commit message properly and realized this is only for "pretend" 
case. And of course it's what we do for qemuProcessStart(). Lesson 
learned :-)

Michal