Re: [Qemu-devel] [PATCH 1/2] qmp/hmp: add writeconfig

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1448 bytes --]

On 02/15/2017 04:14 AM, Eduardo Otubo wrote:
> This patch adds support for the command `writeconfig' on the QMP and HMP
> consoles. This is a simple way to keep track of current state of VM
> after series of hotplugs and/or hotunplugs of different devices:
> 
>   (qemu) writeconfig qemu.conf
> 
> Signed-off-by: Eduardo Otubo <eduardo.otubo@profitbricks.com>
> ---

> +++ b/qapi-schema.json
> @@ -4696,6 +4696,26 @@
>    'data': { 'keys': ['KeyValue'], '*hold-time': 'int' } }
>  
>  ##
> +# @writeconfig:
> +#
> +# Write config to file.
> +#
> +# @filename: the path of a new file to store the current config
> +#
> +# Returns: Nothing on success
> +#
> +# Since: 2.7.0
> +#

You've missed 2.7 by two releases.  This should be 2.9.


> +++ b/ui/console.c

>  
> +void qmp_writeconfig(const char *filename, Error **errp)
> +{
> +    if (filename == NULL) {
> +        error_setg(errp, "You must specify a filename.");
> +        return;
> +    }

Dead code; QAPI ensures that filename is non-NULL.
Even if it weren't dead code, error_setg() messages should not end in '.'.

> +
> +    FILE *fp;
> +    fp = fopen(filename, "w");

Please use qemu_fopen() - that way, the caller can also use qemu's magic
/dev/fdset to write to a pre-opened fd even when qemu itself can't
open() the file.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]