| Message ID | 20180215042900.16078-31-f4bug@amsat.org |
|---|---|
| State | New |
| Headers | show |
| Series | hw: use the BYTE-based definitions when useful | expand |
Hi Philippe, On Thu, Feb 15, 2018 at 01:29:00AM -0300, Philippe Mathieu-Daudé wrote: > From: Philippe Mathieu-Daudé <f4bug@amsat.org> > Subject: [Xen-devel] [PATCH 30/30] xen: use the BYTE-based definitions > List-Id: Xen developer discussion <xen-devel.lists.xenproject.org> > > It ease code review, unit is explicit. > > Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > --- > hw/block/xen_disk.c | 4 ++-- > hw/xenpv/xen_domainbuild.c | 10 +++++----- > 2 files changed, 7 insertions(+), 7 deletions(-) > > diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c > index f74fcd42d1..557005b5e5 100644 > --- a/hw/block/xen_disk.c > +++ b/hw/block/xen_disk.c > @@ -1153,9 +1153,9 @@ static int blk_connect(struct XenDevice *xendev) > } > > xen_pv_printf(xendev, 1, "type \"%s\", fileproto \"%s\", filename \"%s\"," > - " size %" PRId64 " (%" PRId64 " MB)\n", > + " size %" PRId64 " (%llu MB)\n", > blkdev->type, blkdev->fileproto, blkdev->filename, > - blkdev->file_size, blkdev->file_size >> 20); > + blkdev->file_size, blkdev->file_size / M_BYTE); > > /* Fill in number of sector size and number of sectors */ > xenstore_write_be_int(&blkdev->xendev, "sector-size", blkdev->file_blk); > diff --git a/hw/xenpv/xen_domainbuild.c b/hw/xenpv/xen_domainbuild.c > index 027f76fad1..083fb80ee5 100644 > --- a/hw/xenpv/xen_domainbuild.c > +++ b/hw/xenpv/xen_domainbuild.c > @@ -75,9 +75,9 @@ int xenstore_domain_init1(const char *kernel, const char *ramdisk, > xenstore_write_str(dom, "vm", vm); > > /* memory */ > - xenstore_write_int(dom, "memory/target", ram_size >> 10); // kB > - xenstore_write_int(vm, "memory", ram_size >> 20); // MB > - xenstore_write_int(vm, "maxmem", ram_size >> 20); // MB > + xenstore_write_int(dom, "memory/target", ram_size * K_BYTE); > + xenstore_write_int(vm, "memory", ram_size * M_BYTE); > + xenstore_write_int(vm, "maxmem", ram_size * M_BYTE); These changes looks wrong, surely it must be 'ram_size / K_BYTE'... Alan
On 02/15/2018 08:00 AM, Alan Robinson wrote: > Hi Philippe, > > On Thu, Feb 15, 2018 at 01:29:00AM -0300, Philippe Mathieu-Daudé wrote: >> From: Philippe Mathieu-Daudé <f4bug@amsat.org> >> Subject: [Xen-devel] [PATCH 30/30] xen: use the BYTE-based definitions >> List-Id: Xen developer discussion <xen-devel.lists.xenproject.org> >> >> It ease code review, unit is explicit. >> >> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> >> --- >> hw/block/xen_disk.c | 4 ++-- >> hw/xenpv/xen_domainbuild.c | 10 +++++----- >> 2 files changed, 7 insertions(+), 7 deletions(-) >> >> diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c >> index f74fcd42d1..557005b5e5 100644 >> --- a/hw/block/xen_disk.c >> +++ b/hw/block/xen_disk.c >> @@ -1153,9 +1153,9 @@ static int blk_connect(struct XenDevice *xendev) >> } >> >> xen_pv_printf(xendev, 1, "type \"%s\", fileproto \"%s\", filename \"%s\"," >> - " size %" PRId64 " (%" PRId64 " MB)\n", >> + " size %" PRId64 " (%llu MB)\n", >> blkdev->type, blkdev->fileproto, blkdev->filename, >> - blkdev->file_size, blkdev->file_size >> 20); >> + blkdev->file_size, blkdev->file_size / M_BYTE); >> >> /* Fill in number of sector size and number of sectors */ >> xenstore_write_be_int(&blkdev->xendev, "sector-size", blkdev->file_blk); >> diff --git a/hw/xenpv/xen_domainbuild.c b/hw/xenpv/xen_domainbuild.c >> index 027f76fad1..083fb80ee5 100644 >> --- a/hw/xenpv/xen_domainbuild.c >> +++ b/hw/xenpv/xen_domainbuild.c >> @@ -75,9 +75,9 @@ int xenstore_domain_init1(const char *kernel, const char *ramdisk, >> xenstore_write_str(dom, "vm", vm); >> >> /* memory */ >> - xenstore_write_int(dom, "memory/target", ram_size >> 10); // kB >> - xenstore_write_int(vm, "memory", ram_size >> 20); // MB >> - xenstore_write_int(vm, "maxmem", ram_size >> 20); // MB >> + xenstore_write_int(dom, "memory/target", ram_size * K_BYTE); >> + xenstore_write_int(vm, "memory", ram_size * M_BYTE); >> + xenstore_write_int(vm, "maxmem", ram_size * M_BYTE); > > These changes looks wrong, surely it must be 'ram_size / K_BYTE'... Oops... Thanks for noticing this mistake! Can I add your R-b tag once fixed? Respin will be: + xenstore_write_int(dom, "memory/target", ram_size / K_BYTE); + xenstore_write_int(vm, "memory", ram_size / M_BYTE); + xenstore_write_int(vm, "maxmem", ram_size / M_BYTE); Regards, Phil. > > Alan >
Hi Philippe, On Thu, Feb 15, 2018 at 09:23:52AM -0300, Philippe Mathieu-Daudé wrote: > > Can I add your R-b tag once fixed? Respin will be: > > + xenstore_write_int(dom, "memory/target", ram_size / K_BYTE); > + xenstore_write_int(vm, "memory", ram_size / M_BYTE); > + xenstore_write_int(vm, "maxmem", ram_size / M_BYTE); > Yes - Alan
diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c index f74fcd42d1..557005b5e5 100644 --- a/hw/block/xen_disk.c +++ b/hw/block/xen_disk.c @@ -1153,9 +1153,9 @@ static int blk_connect(struct XenDevice *xendev) } xen_pv_printf(xendev, 1, "type \"%s\", fileproto \"%s\", filename \"%s\"," - " size %" PRId64 " (%" PRId64 " MB)\n", + " size %" PRId64 " (%llu MB)\n", blkdev->type, blkdev->fileproto, blkdev->filename, - blkdev->file_size, blkdev->file_size >> 20); + blkdev->file_size, blkdev->file_size / M_BYTE); /* Fill in number of sector size and number of sectors */ xenstore_write_be_int(&blkdev->xendev, "sector-size", blkdev->file_blk); diff --git a/hw/xenpv/xen_domainbuild.c b/hw/xenpv/xen_domainbuild.c index 027f76fad1..083fb80ee5 100644 --- a/hw/xenpv/xen_domainbuild.c +++ b/hw/xenpv/xen_domainbuild.c @@ -75,9 +75,9 @@ int xenstore_domain_init1(const char *kernel, const char *ramdisk, xenstore_write_str(dom, "vm", vm); /* memory */ - xenstore_write_int(dom, "memory/target", ram_size >> 10); // kB - xenstore_write_int(vm, "memory", ram_size >> 20); // MB - xenstore_write_int(vm, "maxmem", ram_size >> 20); // MB + xenstore_write_int(dom, "memory/target", ram_size * K_BYTE); + xenstore_write_int(vm, "memory", ram_size * M_BYTE); + xenstore_write_int(vm, "maxmem", ram_size * M_BYTE); /* cpus */ for (i = 0; i < smp_cpus; i++) { @@ -260,7 +260,7 @@ int xen_domain_build_pv(const char *kernel, const char *ramdisk, } #endif - rc = xc_domain_setmaxmem(xen_xc, xen_domid, ram_size >> 10); + rc = xc_domain_setmaxmem(xen_xc, xen_domid, ram_size / K_BYTE); if (rc < 0) { fprintf(stderr, "xen: xc_domain_setmaxmem() failed\n"); goto err; @@ -269,7 +269,7 @@ int xen_domain_build_pv(const char *kernel, const char *ramdisk, xenstore_port = xc_evtchn_alloc_unbound(xen_xc, xen_domid, 0); console_port = xc_evtchn_alloc_unbound(xen_xc, xen_domid, 0); - rc = xc_linux_build(xen_xc, xen_domid, ram_size >> 20, + rc = xc_linux_build(xen_xc, xen_domid, ram_size / M_BYTE, kernel, ramdisk, cmdline, 0, flags, xenstore_port, &xenstore_mfn,
It ease code review, unit is explicit. Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> --- hw/block/xen_disk.c | 4 ++-- hw/xenpv/xen_domainbuild.c | 10 +++++----- 2 files changed, 7 insertions(+), 7 deletions(-)