[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v5 2/6] BitmapLog: bitmap dump code
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH v5 2/6] BitmapLog: bitmap dump code |
Date: |
Tue, 12 Aug 2014 07:12:28 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.7.0 |
On 07/31/2014 09:12 PM, Sanidhya Kashyap wrote:
> In this patch, I have incorporated an enum named QemuProcess
> which defines what kind of process is being executed i.e.
> none --> no other process except the VM execution
> migration --> migration is being executed
> bitmap-dump --> bitmap dump process is undergoing
>
> Besides this, I have tried to incorporate the dynamic change of
> the last_ram_offset, if it gets change. The downside is that I am
> holding lock for a longer period of time and I don't know whether
> that should be done or not. I am also doing some allocation when
> locked.
> I am not sure whether last_ram_offset gets changed when a device
> is hot plugged or hot unplugged.
>
> I have modified the variables name as:
> current-iteration: for the current iteration under process
> iterations: total iterations that will be done which is constant
> period: the delay in each iteration.
>
> Signed-off-by: Sanidhya Kashyap <address@hidden>
> ---
> hmp-commands.hx | 16 ++
> hmp.c | 18 +++
> hmp.h | 1 +
> include/exec/cpu-all.h | 5 +-
> include/sysemu/sysemu.h | 5 +
> migration.c | 12 ++
> qapi-schema.json | 35 +++++
> qmp-commands.hx | 34 +++++
> savevm.c | 378
> ++++++++++++++++++++++++++++++++++++++++++++++++
> vl.c | 24 +++
> 10 files changed, 527 insertions(+), 1 deletion(-)
>
> +++ b/hmp-commands.hx
> @@ -1788,6 +1788,22 @@ STEXI
> show available trace events and their state
> ETEXI
>
> + {
> + .name = "ldb|log_dirty_bitmap",
The only other instances of a .name with an | are for giving a
single-letter option name synonym to a long option, but ldb is not a
single-letter option. I don't think you can create 'ldb' in the HMP
interface. Just stick with the long name.
> + .args_type = "filename:s,iterations:i?,period:i?",
> + .params = "filename iterations period",
> + .help = "dumps the memory's dirty bitmap to file\n\t\t\t"
> + "filename: name of the file in which the bitmap will
> be saved\n\t\t\t"
> + "iterations: number of times, the memory will be
> logged\n\t\t\t"
s/times,/times/
> +++ b/qapi-schema.json
> @@ -3480,3 +3480,38 @@
> # Since: 2.1
> ##
> { 'command': 'rtc-reset-reinjection' }
> +
> +##
> +# QemuProcess
I agree with David's advice to name this QemuBitmapUser.
> +##
> +# @log-dirty-bitmap
> +#
> +# This command will dump the dirty bitmap to a file by logging the
> +# memory for a specified number of times with a defined time difference
> +#
> +# @filename: name of the file in which the bitmap will be saved.
> +#
> +# @iterations: number of times the memory will be logged (optional). The
> +# and max values are 3 and 100000 respectively.
s/and/min and/
For consistency, the optional markup should look like:
@iterations: #optional number of times the memory will be logged.
Also, you should document what value is used when the option is omitted.
> +#
> +# @period: time difference in milliseconds between each iteration (optional).
> +# The min and max values are 10 and 100000 respectively.
Again, wrong markup for #optional, and missing the default value.
> +Arguments:
> +
> +- "filename": name of the file, in which the bitmap will be saved.
> +
> +- "iterations": number of times, the memory will be logged (optional).
> + The min and max values are 3 and 100000 respectively.
> +
> +- "period": time difference in milliseconds between each iteration
> (optional).
> + The min and max values are 10 and 100000 respectively.
> +
[1]
> +Examples:
> +-> { "execute": "log-dirty-bitmap",
> + "arguments": {
> + "filename": "/tmp/fileXXX",
> + "iterations": 3,
> + "period": 10 } }
> +
> +<- { "return": {} }
>
> +Note: The iterations, and period parameters are optional. iterations default
> +value is 3 while that of period is 10.
This note is redundant, and it is not good to split information.
Instead, drop the note paragraph, and mention the optional values of 3
and 10 up above at the point [1] where you mention the arguments are
optional.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature