Re: [Qemu-devel] [PATCH v6 11/33] hostmem-file: use whole file size if possible

[Xiao Guangrong]

On 10/30/2015 11:27 PM, Vladimir Sementsov-Ogievskiy wrote:
> On 30.10.2015 08:56, Xiao Guangrong wrote:
> > Use the whole file size if @size is not specified which is useful
> > if we want to directly pass a file to guest
> >
> > Signed-off-by: Xiao Guangrong <address@hidden>
> > ---
> >   backends/hostmem-file.c | 48 ++++++++++++++++++++++++++++++++++++++++++++----
> >   1 file changed, 44 insertions(+), 4 deletions(-)
> >
> > diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c
> > index 9097a57..e1bc9ff 100644
> > --- a/backends/hostmem-file.c
> > +++ b/backends/hostmem-file.c
> > @@ -9,6 +9,9 @@
> >    * This work is licensed under the terms of the GNU GPL, version 2 or later.
> >    * See the COPYING file in the top-level directory.
> >    */
> > +#include <sys/ioctl.h>
> > +#include <linux/fs.h>
> > +
> >   #include "qemu-common.h"
> >   #include "sysemu/hostmem.h"
> >   #include "sysemu/sysemu.h"
> > @@ -33,20 +36,57 @@ struct HostMemoryBackendFile {
> >       char *mem_path;
> >   };
> > +static uint64_t get_file_size(const char *file)
> > +{
> > +    struct stat stat_buf;
> > +    uint64_t size = 0;
> > +    int fd;
> > +
> > +    fd = open(file, O_RDONLY);
> > +    if (fd < 0) {
> > +        return 0;
> > +    }
> > +
> > +    if (stat(file, &stat_buf) < 0) {
> > +        goto exit;
> > +    }
> > +
> > +    if ((S_ISBLK(stat_buf.st_mode)) && !ioctl(fd, BLKGETSIZE64, &size)) {
> > +        goto exit;
> > +    }
> > +
> > +    size = lseek(fd, 0, SEEK_END);
> > +    if (size == -1) {
> > +        size = 0;
> > +    }
> > +exit:
> > +    close(fd);
> > +    return size;
> > +}
> > +
> >   static void
> >   file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
> >   {
> >       HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(backend);
> > -    if (!backend->size) {
> > -        error_setg(errp, "can't create backend with size 0");
> > -        return;
> > -    }
> >       if (!fb->mem_path) {
> >           error_setg(errp, "mem-path property not set");
> >           return;
> >       }
> > +    if (!backend->size) {
> > +        /*
> > +         * use the whole file size if @size is not specified.
> > +         */
> > +        backend->size = get_file_size(fb->mem_path);
> > +    }
> > +
> > +    if (!backend->size) {
> > +        error_setg(errp, "failed to get file size for %s, can't create "
> > +                         "backend on it", mem_path);
> > +        return;
> > +    }
> > +
> >       backend->force_prealloc = mem_prealloc;
> >       memory_region_init_ram_from_file(&backend->mr, OBJECT(backend),
> >                                    object_get_canonical_path(OBJECT(backend)),
>
> You can use error_setg_errno to get an error descriptioin for free, as all 
> possible errors from
> get_file_size comes with errno. Just zero size should be separated for this, 
> for example by usiing
> int64_t as return type and -1 for an error.

Okay.

> Sorry for this nit-picking)

Your comments/suggestion is valuable to me. :)