Re: [libvirt PATCH v7 25/35] qemu: Taint domain if nbdkit restart fails

2023-08-29 Thread Peter Krempa
On Mon, Aug 28, 2023 at 16:45:00 -0500, Jonathon Jongsma wrote:
> Since the restart handler will trigger at an arbitrary time (when the
> nbdkit process crashes, for instance), it's difficult to provide
> feedback to the user if the restart is unsuccessful. Rather than just
> relying on a warning in the log, taint the domain so that there will be
> a slightly more user-visible notification.
> 
> Signed-off-by: Jonathon Jongsma 
> ---
>  src/conf/domain_conf.c | 2 ++
>  src/conf/domain_conf.h | 1 +
>  src/qemu/qemu_nbdkit.c | 4 +++-
>  3 files changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index bb4f1fdb94..8feaf5d055 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -87,6 +87,7 @@ VIR_ENUM_IMPL(virDomainTaint,
>"custom-hypervisor-feature",
>"deprecated-config",
>"custom-device",
> +  "nbdkit-restart",
>  );
>  
>  VIR_ENUM_IMPL(virDomainTaintMessage,
> @@ -105,6 +106,7 @@ VIR_ENUM_IMPL(virDomainTaintMessage,
>N_("hypervisor feature autodetection override"),
>N_("use of deprecated configuration settings"),
>N_("custom device configuration"),
> +  N_("nbdkit restart failed"),
>  );
>  
>  VIR_ENUM_IMPL(virDomainVirt,
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index ca195a52d2..c0729905a8 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -3194,6 +3194,7 @@ typedef enum {
>  VIR_DOMAIN_TAINT_CUSTOM_HYPERVISOR_FEATURE, /* custom hypervisor feature 
> control */
>  VIR_DOMAIN_TAINT_DEPRECATED_CONFIG,  /* Configuration that is marked 
> deprecated */
>  VIR_DOMAIN_TAINT_CUSTOM_DEVICE, /* hypervisor device config customized */
> +VIR_DOMAIN_TAINT_NBDKIT_RESTART,/* nbdkit could not be restarted */
>  
>  VIR_DOMAIN_TAINT_LAST
>  } virDomainTaintFlags;

As mentioned in review to 24/35, these two patches should be swapped, so
that this can be used directly.

Reviewed-by: Peter Krempa 


> diff --git a/src/qemu/qemu_nbdkit.c b/src/qemu/qemu_nbdkit.c
> index c3fa349922..ff5f1c0d05 100644
> --- a/src/qemu/qemu_nbdkit.c
> +++ b/src/qemu/qemu_nbdkit.c
> @@ -623,8 +623,10 @@ qemuNbdkitProcessRestart(qemuNbdkitProcess *proc,
>  /* clean up resources associated with process */
>  qemuNbdkitProcessStop(proc);
>  
> -if (qemuNbdkitProcessStart(proc, vm, driver) < 0)
> +if (qemuNbdkitProcessStart(proc, vm, driver) < 0) {
>  VIR_WARN("Unable to restart nbkdit process");
> +virDomainObjTaint(vm, VIR_DOMAIN_TAINT_NBDKIT_RESTART);
> +}

This then obviously belongs to the patch adding the watching impl



[libvirt PATCH v7 25/35] qemu: Taint domain if nbdkit restart fails

2023-08-28 Thread Jonathon Jongsma
Since the restart handler will trigger at an arbitrary time (when the
nbdkit process crashes, for instance), it's difficult to provide
feedback to the user if the restart is unsuccessful. Rather than just
relying on a warning in the log, taint the domain so that there will be
a slightly more user-visible notification.

Signed-off-by: Jonathon Jongsma 
---
 src/conf/domain_conf.c | 2 ++
 src/conf/domain_conf.h | 1 +
 src/qemu/qemu_nbdkit.c | 4 +++-
 3 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index bb4f1fdb94..8feaf5d055 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -87,6 +87,7 @@ VIR_ENUM_IMPL(virDomainTaint,
   "custom-hypervisor-feature",
   "deprecated-config",
   "custom-device",
+  "nbdkit-restart",
 );
 
 VIR_ENUM_IMPL(virDomainTaintMessage,
@@ -105,6 +106,7 @@ VIR_ENUM_IMPL(virDomainTaintMessage,
   N_("hypervisor feature autodetection override"),
   N_("use of deprecated configuration settings"),
   N_("custom device configuration"),
+  N_("nbdkit restart failed"),
 );
 
 VIR_ENUM_IMPL(virDomainVirt,
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index ca195a52d2..c0729905a8 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -3194,6 +3194,7 @@ typedef enum {
 VIR_DOMAIN_TAINT_CUSTOM_HYPERVISOR_FEATURE, /* custom hypervisor feature 
control */
 VIR_DOMAIN_TAINT_DEPRECATED_CONFIG,  /* Configuration that is marked 
deprecated */
 VIR_DOMAIN_TAINT_CUSTOM_DEVICE, /* hypervisor device config customized */
+VIR_DOMAIN_TAINT_NBDKIT_RESTART,/* nbdkit could not be restarted */
 
 VIR_DOMAIN_TAINT_LAST
 } virDomainTaintFlags;
diff --git a/src/qemu/qemu_nbdkit.c b/src/qemu/qemu_nbdkit.c
index c3fa349922..ff5f1c0d05 100644
--- a/src/qemu/qemu_nbdkit.c
+++ b/src/qemu/qemu_nbdkit.c
@@ -623,8 +623,10 @@ qemuNbdkitProcessRestart(qemuNbdkitProcess *proc,
 /* clean up resources associated with process */
 qemuNbdkitProcessStop(proc);
 
-if (qemuNbdkitProcessStart(proc, vm, driver) < 0)
+if (qemuNbdkitProcessStart(proc, vm, driver) < 0) {
 VIR_WARN("Unable to restart nbkdit process");
+virDomainObjTaint(vm, VIR_DOMAIN_TAINT_NBDKIT_RESTART);
+}
 }
 
 
-- 
2.41.0