Message ID | 4548d1078beba2f65b3aacf0438a6d4c958cea6e.1407402040.git.hutao@cn.fujitsu.com |
---|---|
State | New |
Headers | show |
On Thu, Aug 7, 2014 at 7:10 PM, Hu Tao <hutao@cn.fujitsu.com> wrote: > Report error when memory < hpagesize in file_ram_alloc() so callers can "an error" > handle the error. > > This patch fix a problem that if user adds a memory-backend-file object Long sentence. I would drop the "This patch fixes a problem that" > using object_add command, specifying a size that is less than huge page > size, qemu will core dump with message: > > Bad ram offset fffffffffffff000 > Aborted (core dumped) > Then say here "This patch fixes the problem." > with this patch, qemu reports error message like: > > qemu-system-x86_64: -object memory-backend-file,mem-path=/hugepages,id=mem-file0,size=1M: memory > size 0x100000 must be equal to or larger than huge page size 0x200000 > > Signed-off-by: Hu Tao <hutao@cn.fujitsu.com> > --- > exec.c | 13 ++++++++----- > 1 file changed, 8 insertions(+), 5 deletions(-) > > diff --git a/exec.c b/exec.c > index accba00..50cd510 100644 > --- a/exec.c > +++ b/exec.c > @@ -1024,9 +1024,9 @@ static void *file_ram_alloc(RAMBlock *block, > char *filename; > char *sanitized_name; > char *c; > - void *area; > + void *area = NULL; > int fd; > - unsigned long hpagesize; > + uint64_t hpagesize; > > hpagesize = gethugepagesize(path); > if (!hpagesize) { > @@ -1034,7 +1034,10 @@ static void *file_ram_alloc(RAMBlock *block, > } > > if (memory < hpagesize) { > - return NULL; > + error_setg(errp, "memory size 0x" RAM_ADDR_FMT " must be equal to " > + "or larger than huge page size 0x%" PRIx64, > + memory, hpagesize); > + goto error; > } > > if (kvm_enabled() && !kvm_has_sync_mmu()) { > @@ -1094,8 +1097,8 @@ static void *file_ram_alloc(RAMBlock *block, > return area; > > error: > - if (mem_prealloc) { > - exit(1); So I get the movitation behind getting rid of the core dump and abort(). But this seems like a different change. You are demoting an explicit fatal error (which looks to me to be unhelpfully silent) to a propagating error. What's the reasoning? Is there any awareness of the must-exit on the mem_prealloc failure case in the higher levels? Regards, Peter > + if (area && area != MAP_FAILED) { > + munmap(area, memory); > } > return NULL; > } > -- > 1.9.3 > >
On Thu, Aug 07, 2014 at 09:45:04PM +1000, Peter Crosthwaite wrote: > On Thu, Aug 7, 2014 at 7:10 PM, Hu Tao <hutao@cn.fujitsu.com> wrote: > > Report error when memory < hpagesize in file_ram_alloc() so callers can > > "an error" > > > handle the error. > > > > This patch fix a problem that if user adds a memory-backend-file object > > Long sentence. I would drop the "This patch fixes a problem that" > > > using object_add command, specifying a size that is less than huge page > > size, qemu will core dump with message: > > > > Bad ram offset fffffffffffff000 > > Aborted (core dumped) > > > > Then say here "This patch fixes the problem." I'll reword if respin. > > > with this patch, qemu reports error message like: > > > > qemu-system-x86_64: -object memory-backend-file,mem-path=/hugepages,id=mem-file0,size=1M: memory > > size 0x100000 must be equal to or larger than huge page size 0x200000 > > > > Signed-off-by: Hu Tao <hutao@cn.fujitsu.com> > > --- > > exec.c | 13 ++++++++----- > > 1 file changed, 8 insertions(+), 5 deletions(-) > > > > diff --git a/exec.c b/exec.c > > index accba00..50cd510 100644 > > --- a/exec.c > > +++ b/exec.c > > @@ -1024,9 +1024,9 @@ static void *file_ram_alloc(RAMBlock *block, > > char *filename; > > char *sanitized_name; > > char *c; > > - void *area; > > + void *area = NULL; > > int fd; > > - unsigned long hpagesize; > > + uint64_t hpagesize; > > > > hpagesize = gethugepagesize(path); > > if (!hpagesize) { > > @@ -1034,7 +1034,10 @@ static void *file_ram_alloc(RAMBlock *block, > > } > > > > if (memory < hpagesize) { > > - return NULL; > > + error_setg(errp, "memory size 0x" RAM_ADDR_FMT " must be equal to " > > + "or larger than huge page size 0x%" PRIx64, > > + memory, hpagesize); > > + goto error; > > } > > > > if (kvm_enabled() && !kvm_has_sync_mmu()) { > > @@ -1094,8 +1097,8 @@ static void *file_ram_alloc(RAMBlock *block, > > return area; > > > > error: > > - if (mem_prealloc) { > > - exit(1); > > So I get the movitation behind getting rid of the core dump and > abort(). But this seems like a different change. You are demoting an > explicit fatal error (which looks to me to be unhelpfully silent) to a > propagating error. What's the reasoning? Is there any awareness of the > must-exit on the mem_prealloc failure case in the higher levels? Indeed this change is irrelevant to this patch, but relevant(not quite) to patch 5. The change fixes a similar problem described in patch 5 but at the time preallocating failed. Steps to reproduce this problem are similar to the ones in patch 5, except that adds -mem-prealloc to qemu command line. Regards, Hu > > Regards, > Peter > > > + if (area && area != MAP_FAILED) { > > + munmap(area, memory); > > } > > return NULL; > > } > > -- > > 1.9.3 > > > >
diff --git a/exec.c b/exec.c index accba00..50cd510 100644 --- a/exec.c +++ b/exec.c @@ -1024,9 +1024,9 @@ static void *file_ram_alloc(RAMBlock *block, char *filename; char *sanitized_name; char *c; - void *area; + void *area = NULL; int fd; - unsigned long hpagesize; + uint64_t hpagesize; hpagesize = gethugepagesize(path); if (!hpagesize) { @@ -1034,7 +1034,10 @@ static void *file_ram_alloc(RAMBlock *block, } if (memory < hpagesize) { - return NULL; + error_setg(errp, "memory size 0x" RAM_ADDR_FMT " must be equal to " + "or larger than huge page size 0x%" PRIx64, + memory, hpagesize); + goto error; } if (kvm_enabled() && !kvm_has_sync_mmu()) { @@ -1094,8 +1097,8 @@ static void *file_ram_alloc(RAMBlock *block, return area; error: - if (mem_prealloc) { - exit(1); + if (area && area != MAP_FAILED) { + munmap(area, memory); } return NULL; }
Report error when memory < hpagesize in file_ram_alloc() so callers can handle the error. This patch fix a problem that if user adds a memory-backend-file object using object_add command, specifying a size that is less than huge page size, qemu will core dump with message: Bad ram offset fffffffffffff000 Aborted (core dumped) with this patch, qemu reports error message like: qemu-system-x86_64: -object memory-backend-file,mem-path=/hugepages,id=mem-file0,size=1M: memory size 0x100000 must be equal to or larger than huge page size 0x200000 Signed-off-by: Hu Tao <hutao@cn.fujitsu.com> --- exec.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-)