[PATCH for-4.17] tools/libxl: Correct error message units in libxl__domain_set_paging_mempool_size()

Andrew Cooper posted 1 patch 2 weeks, 6 days ago
Failed in applying to current master (apply log)
tools/libs/light/libxl_dom.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
[PATCH for-4.17] tools/libxl: Correct error message units in libxl__domain_set_paging_mempool_size()
Posted by Andrew Cooper 2 weeks, 6 days ago
The error message accidentally printed the bytes value as if it were kB.

Fixes: 7c3bbd940dd8 ("xen/arm, libxl: Revert XEN_DOMCTL_shadow_op; use p2m mempool hypercalls")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Anthony PERARD <anthony.perard@citrix.com>
CC: Henry Wang <Henry.Wang@arm.com>

For 4.17.  This is a low risk change, and makes an error message accurate.
---
 tools/libs/light/libxl_dom.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/libs/light/libxl_dom.c b/tools/libs/light/libxl_dom.c
index b59bbe00bb30..68ad9763b6ba 100644
--- a/tools/libs/light/libxl_dom.c
+++ b/tools/libs/light/libxl_dom.c
@@ -1459,7 +1459,8 @@ int libxl__domain_set_paging_mempool_size(
     int r = xc_set_paging_mempool_size(CTX->xch, domid, shadow_mem);
     if (r) {
         LOGED(ERROR, domid,
-              "Failed to set paging mempool size to %"PRIu64"kB", shadow_mem);
+              "Failed to set paging mempool size to %lukB",
+              d_config->b_info.shadow_memkb);
         return ERROR_FAIL;
     }
 
-- 
2.11.0
Re: [PATCH for-4.17] tools/libxl: Correct error message units in libxl__domain_set_paging_mempool_size()
Posted by Anthony PERARD 2 weeks, 6 days ago
On Fri, Nov 18, 2022 at 05:02:13PM +0000, Andrew Cooper wrote:
> diff --git a/tools/libs/light/libxl_dom.c b/tools/libs/light/libxl_dom.c
> index b59bbe00bb30..68ad9763b6ba 100644
> --- a/tools/libs/light/libxl_dom.c
> +++ b/tools/libs/light/libxl_dom.c
> @@ -1459,7 +1459,8 @@ int libxl__domain_set_paging_mempool_size(
>      int r = xc_set_paging_mempool_size(CTX->xch, domid, shadow_mem);
>      if (r) {
>          LOGED(ERROR, domid,
> -              "Failed to set paging mempool size to %"PRIu64"kB", shadow_mem);
> +              "Failed to set paging mempool size to %lukB",
> +              d_config->b_info.shadow_memkb);

Unless I miss read, `shadow_memkb` is also "uint64_t", so I'm pretty
sure the format doesn't need to be changed, and we should keep using
PRIu64.

With that changed: Acked-by: Anthony PERARD <anthony.perard@citrix.com>

Thanks,

-- 
Anthony PERARD
RE: [PATCH for-4.17] tools/libxl: Correct error message units in libxl__domain_set_paging_mempool_size()
Posted by Henry Wang 2 weeks, 3 days ago
Hi Andrew,

> -----Original Message-----
> Subject: Re: [PATCH for-4.17] tools/libxl: Correct error message units in
> libxl__domain_set_paging_mempool_size()
> 
> On Fri, Nov 18, 2022 at 05:02:13PM +0000, Andrew Cooper wrote:
> > diff --git a/tools/libs/light/libxl_dom.c b/tools/libs/light/libxl_dom.c
> > index b59bbe00bb30..68ad9763b6ba 100644
> > --- a/tools/libs/light/libxl_dom.c
> > +++ b/tools/libs/light/libxl_dom.c
> > @@ -1459,7 +1459,8 @@ int libxl__domain_set_paging_mempool_size(
> >      int r = xc_set_paging_mempool_size(CTX->xch, domid, shadow_mem);
> >      if (r) {
> >          LOGED(ERROR, domid,
> > -              "Failed to set paging mempool size to %"PRIu64"kB",
> shadow_mem);
> > +              "Failed to set paging mempool size to %lukB",
> > +              d_config->b_info.shadow_memkb);
> 
> Unless I miss read, `shadow_memkb` is also "uint64_t", so I'm pretty
> sure the format doesn't need to be changed, and we should keep using
> PRIu64.
> 
> With that changed: Acked-by: Anthony PERARD <anthony.perard@citrix.com>

Release-acked-by: Henry Wang <Henry.Wang@arm.com>

Kind regards,
Henry
RE: [PATCH for-4.17] tools/libxl: Correct error message units in libxl__domain_set_paging_mempool_size()
Posted by Henry Wang 2 weeks, 5 days ago
Hi Anthony and Andrew,

> -----Original Message-----
> From: Anthony PERARD <anthony.perard@citrix.com>
> Subject: Re: [PATCH for-4.17] tools/libxl: Correct error message units in
> libxl__domain_set_paging_mempool_size()
> 
> On Fri, Nov 18, 2022 at 05:02:13PM +0000, Andrew Cooper wrote:
> > diff --git a/tools/libs/light/libxl_dom.c b/tools/libs/light/libxl_dom.c
> > index b59bbe00bb30..68ad9763b6ba 100644
> > --- a/tools/libs/light/libxl_dom.c
> > +++ b/tools/libs/light/libxl_dom.c
> > @@ -1459,7 +1459,8 @@ int libxl__domain_set_paging_mempool_size(
> >      int r = xc_set_paging_mempool_size(CTX->xch, domid, shadow_mem);
> >      if (r) {
> >          LOGED(ERROR, domid,
> > -              "Failed to set paging mempool size to %"PRIu64"kB",
> shadow_mem);
> > +              "Failed to set paging mempool size to %lukB",
> > +              d_config->b_info.shadow_memkb);
> 
> Unless I miss read, `shadow_memkb` is also "uint64_t", so I'm pretty
> sure the format doesn't need to be changed, and we should keep using
> PRIu64.

I did a grep in current code, and:
In libs/light/libxl_types.idl, "shadow_memkb" is defined as MemKB, which
is MemKB = UInt(64, init_val = "LIBXL_MEMKB_DEFAULT", json_gen_fn = "libxl__uint64_gen_json")
so yes it is 64bit indeed. Using PRIu64 seems correct.

Kind regards,
Henry

> 
> With that changed: Acked-by: Anthony PERARD <anthony.perard@citrix.com>
> 
> Thanks,
> 
> --
> Anthony PERARD
Re: [PATCH for-4.17] tools/libxl: Correct error message units in libxl__domain_set_paging_mempool_size()
Posted by Andrew Cooper 2 weeks, 3 days ago
On 19/11/2022 01:45, Henry Wang wrote:
> Hi Anthony and Andrew,
>
>> -----Original Message-----
>> From: Anthony PERARD <anthony.perard@citrix.com>
>> Subject: Re: [PATCH for-4.17] tools/libxl: Correct error message units in
>> libxl__domain_set_paging_mempool_size()
>>
>> On Fri, Nov 18, 2022 at 05:02:13PM +0000, Andrew Cooper wrote:
>>> diff --git a/tools/libs/light/libxl_dom.c b/tools/libs/light/libxl_dom.c
>>> index b59bbe00bb30..68ad9763b6ba 100644
>>> --- a/tools/libs/light/libxl_dom.c
>>> +++ b/tools/libs/light/libxl_dom.c
>>> @@ -1459,7 +1459,8 @@ int libxl__domain_set_paging_mempool_size(
>>>      int r = xc_set_paging_mempool_size(CTX->xch, domid, shadow_mem);
>>>      if (r) {
>>>          LOGED(ERROR, domid,
>>> -              "Failed to set paging mempool size to %"PRIu64"kB",
>> shadow_mem);
>>> +              "Failed to set paging mempool size to %lukB",
>>> +              d_config->b_info.shadow_memkb);
>> Unless I miss read, `shadow_memkb` is also "uint64_t", so I'm pretty
>> sure the format doesn't need to be changed, and we should keep using
>> PRIu64.
> I did a grep in current code, and:
> In libs/light/libxl_types.idl, "shadow_memkb" is defined as MemKB, which
> is MemKB = UInt(64, init_val = "LIBXL_MEMKB_DEFAULT", json_gen_fn = "libxl__uint64_gen_json")
> so yes it is 64bit indeed. Using PRIu64 seems correct.

It highlights that there's yet another overflow bug, pre-existing from
the old implementation.

~Andrew