[PATCH] hw/mem/pc-dimm: Fix error message if no slots were defined some more

Markus Armbruster posted 1 patch 2 years, 2 months ago
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20220222152211.1209292-1-armbru@redhat.com
Maintainers: Ani Sinha <ani@anisinha.ca>, "Michael S. Tsirkin" <mst@redhat.com>, Igor Mammedov <imammedo@redhat.com>
hw/mem/pc-dimm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] hw/mem/pc-dimm: Fix error message if no slots were defined some more
Posted by Markus Armbruster 2 years, 2 months ago
The error message added in commit 3ff333effa "pc-dimm: fix error
messages if no slots were defined" is misleading:

    $ qemu-system-x86_64 -object memory-backend-file,id=mem1,size=1M,mem-path=1G.img -device pc-dimm,id=dimm1,memdev=mem1
    qemu-system-x86_64: -device pc-dimm,id=dimm1,memdev=mem1: no slots where allocated, please specify the 'slots' option
    $ qemu-system-x86_64 -object memory-backend-file,id=mem1,size=1M,mem-path=1G.img -device pc-dimm,id=dimm1,memdev=mem1,slots=0
    qemu-system-x86_64: -device pc-dimm,id=dimm1,memdev=mem1,slots=0: Property 'pc-dimm.slots' not found

The property it called 'slot', not 'slots'.  With that fixed, we get
another bad error message:

    $ qemu-system-x86_64 -object memory-backend-file,id=mem1,size=1M,mem-path=1G.img -device pc-dimm,id=dimm1,memdev=mem1,slot=0
    qemu-system-x86_64: -device pc-dimm,id=dimm1,memdev=mem1,slot=0: invalid slot number 0, valid range is [0-18446744073709551615]

Left for another day.

Fixes: 3ff333effa319df6178f138d9cf32e3937419790
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/mem/pc-dimm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
index 48b913aba6..28fec00575 100644
--- a/hw/mem/pc-dimm.c
+++ b/hw/mem/pc-dimm.c
@@ -115,7 +115,7 @@ static int pc_dimm_get_free_slot(const int *hint, int max_slots, Error **errp)
 
     if (max_slots <= 0) {
         error_setg(errp, "no slots where allocated, please specify "
-                   "the 'slots' option");
+                   "the 'slot' option");
         return slot;
     }
 
-- 
2.35.1


Re: [PATCH] hw/mem/pc-dimm: Fix error message if no slots were defined some more
Posted by David Hildenbrand 2 years, 2 months ago
On 22.02.22 16:22, Markus Armbruster wrote:
> The error message added in commit 3ff333effa "pc-dimm: fix error
> messages if no slots were defined" is misleading:
> 
>     $ qemu-system-x86_64 -object memory-backend-file,id=mem1,size=1M,mem-path=1G.img -device pc-dimm,id=dimm1,memdev=mem1
>     qemu-system-x86_64: -device pc-dimm,id=dimm1,memdev=mem1: no slots where allocated, please specify the 'slots' option
>     $ qemu-system-x86_64 -object memory-backend-file,id=mem1,size=1M,mem-path=1G.img -device pc-dimm,id=dimm1,memdev=mem1,slots=0
>     qemu-system-x86_64: -device pc-dimm,id=dimm1,memdev=mem1,slots=0: Property 'pc-dimm.slots' not found
> 
> The property it called 'slot', not 'slots'.  With that fixed, we get
> another bad error message:
> 
>     $ qemu-system-x86_64 -object memory-backend-file,id=mem1,size=1M,mem-path=1G.img -device pc-dimm,id=dimm1,memdev=mem1,slot=0
>     qemu-system-x86_64: -device pc-dimm,id=dimm1,memdev=mem1,slot=0: invalid slot number 0, valid range is [0-18446744073709551615]
> 
> Left for another day.
> 

We're referring to the "-m 2g,maxmem=8g,slots=5" slots parameter. And I
agree that we can make that clearer somehow :)

> Fixes: 3ff333effa319df6178f138d9cf32e3937419790
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  hw/mem/pc-dimm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
> index 48b913aba6..28fec00575 100644
> --- a/hw/mem/pc-dimm.c
> +++ b/hw/mem/pc-dimm.c
> @@ -115,7 +115,7 @@ static int pc_dimm_get_free_slot(const int *hint, int max_slots, Error **errp)
>  
>      if (max_slots <= 0) {
>          error_setg(errp, "no slots where allocated, please specify "
> -                   "the 'slots' option");
> +                   "the 'slot' option");
>          return slot;
>      }
>  


-- 
Thanks,

David / dhildenb


Re: [PATCH] hw/mem/pc-dimm: Fix error message if no slots were defined some more
Posted by Markus Armbruster 2 years, 2 months ago
David Hildenbrand <david@redhat.com> writes:

> On 22.02.22 16:22, Markus Armbruster wrote:
>> The error message added in commit 3ff333effa "pc-dimm: fix error
>> messages if no slots were defined" is misleading:
>> 
>>     $ qemu-system-x86_64 -object memory-backend-file,id=mem1,size=1M,mem-path=1G.img -device pc-dimm,id=dimm1,memdev=mem1
>>     qemu-system-x86_64: -device pc-dimm,id=dimm1,memdev=mem1: no slots where allocated, please specify the 'slots' option
>>     $ qemu-system-x86_64 -object memory-backend-file,id=mem1,size=1M,mem-path=1G.img -device pc-dimm,id=dimm1,memdev=mem1,slots=0
>>     qemu-system-x86_64: -device pc-dimm,id=dimm1,memdev=mem1,slots=0: Property 'pc-dimm.slots' not found
>> 
>> The property it called 'slot', not 'slots'.  With that fixed, we get
>> another bad error message:
>> 
>>     $ qemu-system-x86_64 -object memory-backend-file,id=mem1,size=1M,mem-path=1G.img -device pc-dimm,id=dimm1,memdev=mem1,slot=0
>>     qemu-system-x86_64: -device pc-dimm,id=dimm1,memdev=mem1,slot=0: invalid slot number 0, valid range is [0-18446744073709551615]
>> 
>> Left for another day.
>> 
>
> We're referring to the "-m 2g,maxmem=8g,slots=5" slots parameter. And I
> agree that we can make that clearer somehow :)

Aha!

So this patch actually moves us sideways rather than forward.  Please
ignore it.

A patch that moves us forward would be nice :)