[PATCH] qemu: validate: Allow 'preserve' action for on_crash lifecycle action

Peter Krempa posted 1 patch 2 years, 6 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/a43755c81ab9ecd745ed348e39bc4d512d00f900.1631704449.git.pkrempa@redhat.com
src/qemu/qemu_driver.c   | 1 +
src/qemu/qemu_validate.c | 5 ++---
2 files changed, 3 insertions(+), 3 deletions(-)
[PATCH] qemu: validate: Allow 'preserve' action for on_crash lifecycle action
Posted by Peter Krempa 2 years, 6 months ago
In fact keeping the VM around for debugging is a desirable configuration
and actually the implementation has no code as we keep the VM around.

Remove the validation and add a note that it's actually used.

Fixes: b1b85a475fb251b9068b75f629479f5c452f1b43
Reported-by: Christian Borntraeger <borntraeger@de.ibm.com>
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
---
 src/qemu/qemu_driver.c   | 1 +
 src/qemu/qemu_validate.c | 5 ++---
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index dfc27572c4..6ae678b165 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -3647,6 +3647,7 @@ processGuestPanicEvent(virQEMUDriver *driver,
         break;

     case VIR_DOMAIN_LIFECYCLE_ACTION_PRESERVE:
+        /* the VM is kept around for debugging */
         break;

     default:
diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
index 9d93f373ab..6b685881a8 100644
--- a/src/qemu/qemu_validate.c
+++ b/src/qemu/qemu_validate.c
@@ -1083,10 +1083,9 @@ qemuValidateLifecycleAction(virDomainLifecycleAction onPoweroff,
     /* The qemu driver doesn't yet implement any meaningful handling for
      * VIR_DOMAIN_LIFECYCLE_ACTION_PRESERVE */
     if (onPoweroff == VIR_DOMAIN_LIFECYCLE_ACTION_PRESERVE ||
-        onReboot == VIR_DOMAIN_LIFECYCLE_ACTION_PRESERVE ||
-        onCrash == VIR_DOMAIN_LIFECYCLE_ACTION_PRESERVE) {
+        onReboot == VIR_DOMAIN_LIFECYCLE_ACTION_PRESERVE) {
         virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-                       _("qemu driver doesn't support the 'preserve' action for 'on_reboot'/'on_poweroff'/'on_crash'"));
+                       _("qemu driver doesn't support the 'preserve' action for 'on_reboot'/'on_poweroff'"));
         return -1;
     }

-- 
2.31.1

Re: [PATCH] qemu: validate: Allow 'preserve' action for on_crash lifecycle action
Posted by Christian Borntraeger 2 years, 6 months ago

On 15.09.21 13:14, Peter Krempa wrote:
> In fact keeping the VM around for debugging is a desirable configuration
> and actually the implementation has no code as we keep the VM around.
> 
> Remove the validation and add a note that it's actually used.
> 
> Fixes: b1b85a475fb251b9068b75f629479f5c452f1b43
> Reported-by: Christian Borntraeger <borntraeger@de.ibm.com>
Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>

> Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> ---
>   src/qemu/qemu_driver.c   | 1 +
>   src/qemu/qemu_validate.c | 5 ++---
>   2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index dfc27572c4..6ae678b165 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -3647,6 +3647,7 @@ processGuestPanicEvent(virQEMUDriver *driver,
>           break;
> 
>       case VIR_DOMAIN_LIFECYCLE_ACTION_PRESERVE:
> +        /* the VM is kept around for debugging */
>           break;
> 
>       default:
> diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
> index 9d93f373ab..6b685881a8 100644
> --- a/src/qemu/qemu_validate.c
> +++ b/src/qemu/qemu_validate.c
> @@ -1083,10 +1083,9 @@ qemuValidateLifecycleAction(virDomainLifecycleAction onPoweroff,
>       /* The qemu driver doesn't yet implement any meaningful handling for
>        * VIR_DOMAIN_LIFECYCLE_ACTION_PRESERVE */
>       if (onPoweroff == VIR_DOMAIN_LIFECYCLE_ACTION_PRESERVE ||
> -        onReboot == VIR_DOMAIN_LIFECYCLE_ACTION_PRESERVE ||
> -        onCrash == VIR_DOMAIN_LIFECYCLE_ACTION_PRESERVE) {
> +        onReboot == VIR_DOMAIN_LIFECYCLE_ACTION_PRESERVE) {
>           virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> -                       _("qemu driver doesn't support the 'preserve' action for 'on_reboot'/'on_poweroff'/'on_crash'"));
> +                       _("qemu driver doesn't support the 'preserve' action for 'on_reboot'/'on_poweroff'"));
>           return -1;
>       }
> 

Re: [PATCH] qemu: validate: Allow 'preserve' action for on_crash lifecycle action
Posted by Boris Fiuczynski 2 years, 6 months ago

On 9/15/21 1:14 PM, Peter Krempa wrote:
> In fact keeping the VM around for debugging is a desirable configuration
> and actually the implementation has no code as we keep the VM around.
> 
> Remove the validation and add a note that it's actually used.
> 
> Fixes: b1b85a475fb251b9068b75f629479f5c452f1b43
> Reported-by: Christian Borntraeger <borntraeger@de.ibm.com>
> Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> ---
>   src/qemu/qemu_driver.c   | 1 +
>   src/qemu/qemu_validate.c | 5 ++---
>   2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index dfc27572c4..6ae678b165 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -3647,6 +3647,7 @@ processGuestPanicEvent(virQEMUDriver *driver,
>           break;
> 
>       case VIR_DOMAIN_LIFECYCLE_ACTION_PRESERVE:
> +        /* the VM is kept around for debugging */
>           break;
> 
>       default:
> diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
> index 9d93f373ab..6b685881a8 100644
> --- a/src/qemu/qemu_validate.c
> +++ b/src/qemu/qemu_validate.c
> @@ -1083,10 +1083,9 @@ qemuValidateLifecycleAction(virDomainLifecycleAction onPoweroff,
>       /* The qemu driver doesn't yet implement any meaningful handling for
>        * VIR_DOMAIN_LIFECYCLE_ACTION_PRESERVE */
>       if (onPoweroff == VIR_DOMAIN_LIFECYCLE_ACTION_PRESERVE ||
> -        onReboot == VIR_DOMAIN_LIFECYCLE_ACTION_PRESERVE ||
> -        onCrash == VIR_DOMAIN_LIFECYCLE_ACTION_PRESERVE) {
> +        onReboot == VIR_DOMAIN_LIFECYCLE_ACTION_PRESERVE) {
>           virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> -                       _("qemu driver doesn't support the 'preserve' action for 'on_reboot'/'on_poweroff'/'on_crash'"));
> +                       _("qemu driver doesn't support the 'preserve' action for 'on_reboot'/'on_poweroff'"));
>           return -1;
>       }
> 

Looks good and works for me
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>

-- 
Mit freundlichen Grüßen/Kind regards
    Boris Fiuczynski

IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Gregor Pillen
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294