Re: [Qemu-devel] [PATCH v4 6/7] qmp: add pmemload command

[Eric Blake <eblake@redhat.com>]

On 08/16/2018 04:01 AM, Simon Ruderich wrote:
> Adapted patch from Baojun Wang [1] with the following commit message:
> 
>      I found this could be useful to have qemu-softmmu as a cross
>      debugger (launch with -s -S command line option), then if we can
>      have a command to load guest physical memory, we can use cross gdb
>      to do some target debug which gdb cannot do directly.
> 
> This patch contains only the qmp changes of the original patch.
> 
> pmemload is necessary to directly write physical memory which is not
> possible with gdb alone as it uses only logical addresses.
> 
> The QAPI for pmemload uses "val" as parameter name for the physical
> address. This name is not very descriptive but is consistent with the
> existing pmemsave. Changing the parameter name of pmemsave is not
> possible without breaking the existing API.
> 
> [1]: https://lists.gnu.org/archive/html/qemu-trivial/2014-04/msg00074.html
> 
> Based-on-patch-by: Baojun Wang <wangbj@gmail.com>
> Signed-off-by: Simon Ruderich <simon@ruderich.org>
> ---

> +    }
> +    if (!has_size) {
> +        struct stat s;
> +        if (fstat(fd, &s)) {
> +            error_setg_errno(errp, errno, "could not fstat fd to get size");
> +            goto exit;
> +        }
> +        size = s.st_size;
> +    }

This works for regular files, but not for block devices.

Do we have a helper function for easily determining the size of an 
arbitrary fd (whether file or block device)?  If not, should we?  As 
there are multiple spots in the code that grab this sort of information.

Otherwise looks okay to me.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org