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

[Eduardo Habkost]

On Fri, Oct 30, 2015 at 01:56:05PM +0800, 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>

This code needs to build on other platforms too. e.g. using mingw32:

  CC    backends/hostmem-file.o
/home/ehabkost/rh/proj/virt/qemu/backends/hostmem-file.c:12:23: fatal error: 
sys/ioctl.h: No such file or directory
 #include <sys/ioctl.h>
                       ^
compilation terminated.
/home/ehabkost/rh/proj/virt/qemu/rules.mak:57: recipe for target 
'backends/hostmem-file.o' failed
make: *** [backends/hostmem-file.o] Error 1


> +
>  #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;
> +}

This code seems to duplicate what block/raw-posix.c:raw_getlength() does
(except for the BLKGETSIZE64 part). Have you considered using the same
code for both?

We can probably move all the raw-posix.c raw_getlength(BlockDriverState
*bs) code to fd_getlength(int fd) functions (on osdep.c?), and just
implement raw-posix.c:raw_getlength(s) as fd_getlength(s->fd).

> +
>  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)),
> -- 
> 1.8.3.1
> 
> 

-- 
Eduardo