qemu-ppc
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v3 11/11] machine: Improve error message when using default R


From: David Hildenbrand
Subject: Re: [PATCH v3 11/11] machine: Improve error message when using default RAM backend id
Date: Fri, 25 Aug 2023 11:13:02 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.13.0

On 25.08.23 11:10, Markus Armbruster wrote:
David Hildenbrand <david@redhat.com> writes:

On 25.08.23 08:57, ThinerLogoer wrote:
Hello,

At 2023-08-23 23:34:11, "David Hildenbrand" <david@redhat.com> wrote:
For migration purposes, users might want to reuse the default RAM
backend id, but specify a different memory backend.

For example, to reuse "pc.ram" on q35, one has to set
     -machine q35,memory-backend=pc.ram
Only then, can a memory backend with the id "pc.ram" be created
manually.

Let's improve the error message.

Unfortuantely, we cannot use error_append_hint(), because the caller
passes &error_fatal.

Suggested-by: ThinerLogoer <logoerthiner1@163.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
hw/core/machine.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index f0d35c6401..dbcd124d45 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -1382,7 +1382,9 @@ void machine_run_board_init(MachineState *machine, const 
char *mem_path, Error *
                                   machine_class->default_ram_id)) {
              error_setg(errp, "object name '%s' is reserved for the default"
                  " RAM backend, it can't be used for any other purposes."
-                " Change the object's 'id' to something else",
+                " Change the object's 'id' to something else or disable"
+                " automatic creation of the default RAM backend by setting"
+                " the 'memory-backend' machine property",
                  machine_class->default_ram_id);
              return;
          }

I'd suggest a more explicit version:

                  " Change the object's 'id' to something else or disable"
                  " automatic creation of the default RAM backend by appending"
                  "  'memory-backend={machine_class->default_ram_id}' in '-machine' 
arguments",


Thanks, I'll do:

diff --git a/hw/core/machine.c b/hw/core/machine.c
index f0d35c6401..cd0fd6cdd1 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -1382,8 +1382,10 @@ void machine_run_board_init(MachineState *machine, const 
char *mem_path, Error *
                                    machine_class->default_ram_id)) {
               error_setg(errp, "object name '%s' is reserved for the default"
                   " RAM backend, it can't be used for any other purposes."
-                " Change the object's 'id' to something else",
-                machine_class->default_ram_id);
+                " Change the object's 'id' to something else or disable"
+                " automatic creation of the default RAM backend by appending"
+                " 'memory-backend=%s' in '-machine' arguments",
+                machine_class->default_ram_id, machine_class->default_ram_id);
               return;
           }
           if (!create_default_memdev(current_machine, mem_path, errp)) {


Hi Markus,

error_setg()'s function comment specifies:

  * The resulting message should be a single phrase, with no newline or
  * trailing punctuation.

Please use error_append_hint(), like so

Please see the patch description: "Unfortunately, we cannot use error_append_hint(), because the caller passes &error_fatal."

How should I deal with that?


              error_setg(errp, "object name '%s' is reserved for the default"
                  " RAM backend, it can't be used for any other purposes",
                  machine_class->default_ram_id);
              error_append_hint(errp,
                  "Change the object's 'id' to something else or disable"
                  " automatic creation of the default RAM backend by appending"
                  " 'memory-backend=%s' in '-machine' arguments\n",
                  machine_class->default_ram_id);

Moreover:

* "object name" feels off, we're talking about IDs, aren't we?

Yes, I think so.


* "appending X in Y" should be "appending X to Y".  Consider "setting
   'memory-backend=%s' with -machine".


Can do, thanks.

--
Cheers,

David / dhildenb




reply via email to

[Prev in Thread] Current Thread [Next in Thread]