[libvirt] [PATCH v2] qemuDomainUndefineFlags: Fix error message

Michal Privoznik posted 1 patch 6 years, 4 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/eb40cb8aa06d442b8538befd62a88976813e8731.1512989743.git.mprivozn@redhat.com
There is a newer version of this series
src/qemu/qemu_driver.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
[libvirt] [PATCH v2] qemuDomainUndefineFlags: Fix error message
Posted by Michal Privoznik 6 years, 4 months ago
https://bugzilla.redhat.com/show_bug.cgi?id=1522706

If domain is active, but the undefine API was called without the
VIR_DOMAIN_UNDEFINE_KEEP_NVRAM flag set, the following incorrect
error message is produced:

error: Requested operation is not valid: cannot delete inactive domain with nvram

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---

Diff to v1:
- switched to different code pattern to make it easier for translators

 src/qemu/qemu_driver.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index aa30b119a..cc5520c54 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7537,8 +7537,13 @@ qemuDomainUndefineFlags(virDomainPtr dom,
                 goto endjob;
             }
         } else if (!(flags & VIR_DOMAIN_UNDEFINE_KEEP_NVRAM)) {
-            virReportError(VIR_ERR_OPERATION_INVALID, "%s",
-                           _("cannot delete inactive domain with nvram"));
+            if (virDomainObjIsActive(vm)) {
+                virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+                               _("cannot delete active domain with nvram"));
+            } else {
+                virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+                               _("cannot delete inactive domain with nvram"));
+            }
             goto endjob;
         }
     }
-- 
2.13.6

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] qemuDomainUndefineFlags: Fix error message
Posted by Jiri Denemark 6 years, 4 months ago
On Mon, Dec 11, 2017 at 11:56:19 +0100, Michal Privoznik wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1522706
> 
> If domain is active, but the undefine API was called without the
> VIR_DOMAIN_UNDEFINE_KEEP_NVRAM flag set, the following incorrect
> error message is produced:
> 
> error: Requested operation is not valid: cannot delete inactive domain with nvram
> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
> 
> Diff to v1:
> - switched to different code pattern to make it easier for translators
> 
>  src/qemu/qemu_driver.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index aa30b119a..cc5520c54 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -7537,8 +7537,13 @@ qemuDomainUndefineFlags(virDomainPtr dom,
>                  goto endjob;
>              }
>          } else if (!(flags & VIR_DOMAIN_UNDEFINE_KEEP_NVRAM)) {
> -            virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> -                           _("cannot delete inactive domain with nvram"));
> +            if (virDomainObjIsActive(vm)) {
> +                virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> +                               _("cannot delete active domain with nvram"));
> +            } else {
> +                virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> +                               _("cannot delete inactive domain with nvram"));
> +            }

Sigh, I should have noticed this earlier... it's in the undefine API so
"delete" is probably not the best word. And it doesn't really matter
that much whether the domain is active or inactive. So how about having
just one simple error message "cannot undefine domain with nvram"? :-)

Jirka

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] qemuDomainUndefineFlags: Fix error message
Posted by Andrea Bolognani 6 years, 4 months ago
On Mon, 2017-12-11 at 11:56 +0100, Michal Privoznik wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1522706
> 
> If domain is active, but the undefine API was called without the
> VIR_DOMAIN_UNDEFINE_KEEP_NVRAM flag set, the following incorrect
> error message is produced:
> 
> error: Requested operation is not valid: cannot delete inactive domain with nvram
> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
> 
> Diff to v1:
> - switched to different code pattern to make it easier for translators
> 
>  src/qemu/qemu_driver.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index aa30b119a..cc5520c54 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -7537,8 +7537,13 @@ qemuDomainUndefineFlags(virDomainPtr dom,
>                  goto endjob;
>              }
>          } else if (!(flags & VIR_DOMAIN_UNDEFINE_KEEP_NVRAM)) {
> -            virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> -                           _("cannot delete inactive domain with nvram"));
> +            if (virDomainObjIsActive(vm)) {
> +                virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> +                               _("cannot delete active domain with nvram"));
> +            } else {
> +                virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> +                               _("cannot delete inactive domain with nvram"));
> +            }
>              goto endjob;
>          }
>      }

I'm probably missing something, but can't we just avoid including
the domain state in the error message?

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list