| Message ID | 20221118170213.2872-1-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [for-4.17] tools/libxl: Correct error message units in libxl__domain_set_paging_mempool_size() | expand |
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,
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
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
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
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; }
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(-)