Re: [Qemu-devel] [PATCH for 2.1 2/2] memory-backend-file: improve error handling

[Hu Tao]

On Thu, Jul 03, 2014 at 10:05:56PM -0600, Eric Blake wrote:
> On 07/03/2014 08:56 PM, Hu Tao wrote:
> > On Thu, Jul 03, 2014 at 06:33:28AM -0600, Eric Blake wrote:
> >> On 07/03/2014 12:10 AM, Hu Tao wrote:
> >>> This patch fixes two problems of memory-backend-file:
> >>>
> >>> 1. If user adds a memory-backend-file object using object_add command,
> >>>    specifying a non-existing directory for property mem-path, qemu
> >>>    will core dump with message:
> >>>
> >>>      /nonexistingdir: No such file or directory
> >>>      Bad ram offset fffffffffffff000
> >>>      Aborted (core dumped)
> >>>
> >>> 2. 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)
> >>>
> >>
> >> Might be nice if the commit message also shows the new message issued
> >> for the same cases after the patch is applied.
> > 
> > OK.
> > 
> 
> >>>  
> >>>      if (memory < hpagesize) {
> >>> -        return NULL;
> >>> +        error_setg(errp, "memory size 0x" RAM_ADDR_FMT " should be 
> >>> larger "
> >>> +                   "than huge page size 0x%" PRIx64, memory, hpagesize);
> >>> +        goto error;
> >>
> >> Isn't exactly equal also allowed?  Maybe a better wording is "should be
> >> a multiple of the huge page size"
> > 
> > Yes. I'll change the error message.
> > 
> 
> Thinking about it more, should you also enforce that it is a multiple?
> 
> As in: if (!memory || memory % hpageszie) { error... }

The memory size is rounded up to hpagesize when allocating memory, we
can waste at most hpagesize-1 bytes memory. I don't think it's a
problem.

In the other side, do you think we should give an error in the case like
-object memory-backend-ram,size=1111M(given 2M hpagesize)?

Regards,
Hu