* Sanidhya Kashyap (sanidhya.ii...@gmail.com) wrote:
> Reformatted the code and added the functionality of dumping the blocks' info
> which can be read by the user when required. I have also made the block name
> length global.
> Some minor modification to the structure which is now storing all the
> information.
One thought, I wonder how much of the savevm.c changes could move into
a separate file rather than making savevm even bigger?
>
> Signed-off-by: Sanidhya Kashyap <sanidhya.ii...@gmail.com>
> ---
> include/exec/cpu-all.h | 4 +-
> migration.c | 7 ++
> qapi-schema.json | 18 +++
> qmp-commands.hx | 30 +++++
> savevm.c | 325
> +++++++++++++++++++++++++++++++++++++++++++++++++
> 5 files changed, 383 insertions(+), 1 deletion(-)
>
> diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
> index f91581f..b459301 100644
> --- a/include/exec/cpu-all.h
> +++ b/include/exec/cpu-all.h
> @@ -297,13 +297,15 @@ CPUArchState *cpu_copy(CPUArchState *env);
>
> /* memory API */
>
> +#define RAMBLOCK_NAME_LENGTH (1<<8)
Be careful; making this bigger would break migration formats,
making it smaller would probably break migration loading.
> typedef struct RAMBlock {
> struct MemoryRegion *mr;
> uint8_t *host;
> ram_addr_t offset;
> ram_addr_t length;
> uint32_t flags;
> - char idstr[256];
> + char idstr[RAMBLOCK_NAME_LENGTH];
> /* Reads can take either the iothread or the ramlist lock.
> * Writes must take both locks.
> */
> diff --git a/migration.c b/migration.c
> index 8d675b3..e2e313c 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -436,6 +436,13 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
> return;
> }
>
> + if (runstate_check(RUN_STATE_DUMP_BITMAP)) {
> + error_setg(errp, "bitmap dump in progress");
> + return;
> + }
> +
> + runstate_set(RUN_STATE_MIGRATE);
> +
> s = migrate_init(¶ms);
>
> if (strstart(uri, "tcp:", &p)) {
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 501b8d0..924d6bc 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -3485,3 +3485,21 @@
> # Since: 2.1
> ##
> { 'command': 'rtc-reset-reinjection' }
> +
> +##
> +# @log-dirty-bitmap
> +#
> +# dumps the dirty bitmap to a file by logging the
> +# memory for a specified number of times with a
> +# a defined time differnce
> +#
> +# @filename: name of the file in which the bitmap will be saved.
> +# @epochs: number of times the memory will be logged (optional).
> +# @frequency: time difference in milliseconds between each epoch (optional).
> +#
> +# Since 2.2
> +##
> +{ 'command' : 'log-dirty-bitmap',
> + 'data' : { 'filename' : 'str',
> + '*epochs' : 'int',
> + '*frequency' : 'int' } }
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index 4be4765..200f57e 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -3753,5 +3753,35 @@ Example:
>
> -> { "execute": "rtc-reset-reinjection" }
> <- { "return": {} }
> +EQMP
> +
> + {
> + .name = "log-dirty-bitmap",
> + .args_type = "filename:s,epochs:i?,frequency:i?,readable:-r?",
> + .mhandler.cmd_new = qmp_marshal_input_log_dirty_bitmap,
> + },
> +
> +SQMP
> +log-dirty-bitmap
> +--------------------
> +
> +start logging the memory of the VM for writable working set
> +
> +Arguments:
> +
> +- "filename": name of the file, in which the bitmap will be saved
> +- "epochs": number of times, the memory will be logged
> +- "frequency": time difference in milliseconds between each epoch
> +
> +Examples:
> +-> { "execute" : "log-dirty-bitmap",
> + "arguments" : {
> + "filename" : "/tmp/fileXXX",
> + "epochs" : 3,
> + "frequency" : 10 } }
> +
> +<- { "return": {} }
>
> +Note: The epochs, frequency and readable are optional. epochs default
> +value is 3 while that of frequency is 10.
> EQMP
> diff --git a/savevm.c b/savevm.c
> index e19ae0a..ecb334e 100644
> --- a/savevm.c
> +++ b/savevm.c
> @@ -42,6 +42,9 @@
> #include "qemu/iov.h"
> #include "block/snapshot.h"
> #include "block/qapi.h"
> +#include "exec/address-spaces.h"
> +#include "exec/ram_addr.h"
> +#include "qemu/bitmap.h"
>
>
> #ifndef ETH_P_RARP
> @@ -1137,6 +1140,328 @@ void do_savevm(Monitor *mon, const QDict *qdict)
> }
> }
>
> +/*
> + * Adding the functionality of continuous logging of the
> + * dirty bitmap which is almost similar to the migration
> + * thread
> + */
> +
> +enum {
> + LOG_BITMAP_STATE_ERROR = -1,
> + LOG_BITMAP_STATE_NONE,
> + LOG_BITMAP_STATE_SETUP,
> + LOG_BITMAP_STATE_ACTIVE,
> + LOG_BITMAP_STATE_CANCELING,
> + LOG_BITMAP_STATE_COMPLETED
> +};
I'd be tempted to give that enum a name and use it as the type
for functions that pass the state around (although I realise
you have to be careful with the variable having to be an int
for the atomic).
> +
> +typedef struct BitmapLogState BitmapLogState;
> +static unsigned long *logging_bitmap;
> +static int64_t MIN_EPOCH_VALUE = 3;
> +static int64_t MIN_FREQUENCY_VALUE = 10;
> +static int64_t MAX_EPOCH_VALUE = 100000;
> +static int64_t MAX_FREQUENCY_VALUE = 100000;
> +
> +struct BitmapLogState {
> + int state;
> + int fd;
> + int64_t current_frequency;
> + int64_t current_epoch;
> + int64_t total_epochs;
> + QemuThread thread;
> +};
> +
> +/*
> + * helper functions
> + */
> +
> +static inline void logging_lock(void)
> +{
> + qemu_mutex_lock_iothread();
> + qemu_mutex_lock_ramlist();
> +}
I wonder how often you can really not have the ramlist locked; if stuff
is added/removed the last_ram_offset would change, so to really be safe
you probably need to hold it for much longer than you currently do -
but that might not be practical.
> +
> +static inline void logging_unlock(void)
> +{
> + qemu_mutex_unlock_ramlist();
> + qemu_mutex_unlock_iothread();
> +}
> +
> +static inline void logging_bitmap_set_dirty(ram_addr_t addr)
> +{
> + int nr = addr >> TARGET_PAGE_BITS;
Be careful; int is too small; long is probably what's
needed (which I think is the type of the parameter to set_bit).
> + set_bit(nr, logging_bitmap);
> +}
> +
> +static bool logging_state_set_status(BitmapLogState *b,
> + int old_state,
> + int new_state)
> +{
> + return atomic_cmpxchg(&b->state, old_state, new_state);
> +}
> +
> +static inline bool value_in_range(int64_t value, int64_t min_value,
> + int64_t max_value, const char *str,
> + Error **errp)
> +{
> + if (value < min_value) {
> + error_setg(errp, "%s's value must be greater than %ld",
> + str, min_value);
> + return false;
> + }
> + if (value > max_value) {
> + error_setg(errp, "%s's value must be less than %ld",
> + str, max_value);
> + return false;
> + }
> + return true;
> +}
This seems a pretty generic function; could it go somewhere
in util or the like?
> +
> +/*
> + * inspired from migration mechanism
> + */
> +
> +static BitmapLogState *logging_current_state(void)
> +{
> + static BitmapLogState current_bitmaplogstate = {
> + .state = LOG_BITMAP_STATE_NONE,
> + };
> +
> + return ¤t_bitmaplogstate;
> +}
> +
> +/*
> + * syncing the logging_bitmap with the ram_list dirty bitmap
> + */
> +
> +static void dirty_bitmap_sync(void)
> +{
> + RAMBlock *block;
> + address_space_sync_dirty_bitmap(&address_space_memory);
> + QTAILQ_FOREACH(block, &ram_list.blocks, next) {
> + qemu_bitmap_sync_range(block->mr->ram_addr, block->length,
> + logging_bitmap, false);
> + }
> +}
I think that should be logging_dirty_bitmap_sync since it's
specific to logging_bitmap.
> +
> +static inline void logging_bitmap_close(BitmapLogState *b)
> +{
> + logging_lock();
> + memory_global_dirty_log_stop();
> + logging_unlock();
> +
> + g_free(logging_bitmap);
> + logging_bitmap = NULL;
> + qemu_close(b->fd);
> + b->fd = -1;
> +}
> +
> +static bool ram_block_info_dump(int fd)
> +{
> + int64_t ram_bitmap_pages = last_ram_offset() >> TARGET_PAGE_BITS;
> + int block_count = 0;
> + RAMBlock *block;
> + int ret;
> +
> + if (qemu_write_full(fd, &ram_bitmap_pages, sizeof(int64_t)) < 0) {
> + return true;
> + }
> +
> + QTAILQ_FOREACH(block, &ram_list.blocks, next) {
> + block_count++;
> + }
> +
> + ret = qemu_write_full(fd, &block_count, sizeof(int));
> + if (ret < sizeof(int)) {
> + return true;
> + }
> +
> + QTAILQ_FOREACH(block, &ram_list.blocks, next) {
> + ret = qemu_write_full(fd, &(block->idstr), sizeof(char) *
> + RAMBLOCK_NAME_LENGTH);
> + if (ret < sizeof(char) * RAMBLOCK_NAME_LENGTH) {
> + return true;
> + }
> + ret = qemu_write_full(fd, &(block->offset), sizeof(ram_addr_t));
> + if (ret < sizeof(ram_addr_t)) {
> + return true;
> + }
> + ret = qemu_write_full(fd, &(block->length), sizeof(ram_addr_t));
> + if (ret < sizeof(ram_addr_t)) {
> + return true;
> + }
> + }
> + return false;
> +}
> +
> +static void logging_state_update_status(BitmapLogState *b)
> +{
> + switch (b->state) {
> + case LOG_BITMAP_STATE_ACTIVE:
> + logging_state_set_status(b, LOG_BITMAP_STATE_ACTIVE,
> + LOG_BITMAP_STATE_COMPLETED);
> + return;
> + case LOG_BITMAP_STATE_CANCELING:
> + logging_state_set_status(b, LOG_BITMAP_STATE_CANCELING,
> + LOG_BITMAP_STATE_COMPLETED);
> + return;
> + case LOG_BITMAP_STATE_ERROR:
> + logging_state_set_status(b, LOG_BITMAP_STATE_ERROR,
> + LOG_BITMAP_STATE_COMPLETED);
> + }
> + return;
> +}
I didn't really see the point of this at first, but I guess
it always moves to 'COMPLETED' unless you're already at completed
or in NONE; but then perhaps:
int s = b->state;
switch (s) {
case LOG_BITMAP_STATE_ACTIVE:
case LOG_BITMAP_STATE_CANCELING:
case LOG_BITMAP_STATE_ERROR:
logging_state_set_status(b, s,
LOG_BITMAP_STATE_COMPLETED);
return;
}
return;
would be more obvious (note I only read the state once)
Dave
> +static void *bitmap_logging_thread(void *opaque)
> +{
> + /*
> + * setup basic structures
> + */
> +
> + BitmapLogState *b = opaque;
> + int fd = b->fd;
> + b->current_epoch = 1;
> + int64_t ram_bitmap_pages = last_ram_offset() >> TARGET_PAGE_BITS;
> + size_t bitmap_size = BITS_TO_LONGS(ram_bitmap_pages) *
> + sizeof(unsigned long);
> + int ret;
> + char marker = 'M';
> +
> + logging_state_set_status(b, LOG_BITMAP_STATE_NONE,
> + LOG_BITMAP_STATE_SETUP);
> +
> + logging_bitmap = bitmap_new(ram_bitmap_pages);
> +
> + if (logging_bitmap == NULL) {
> + b->state = LOG_BITMAP_STATE_ERROR;
> + goto log_thread_end;
> + }
> +
> + logging_state_set_status(b, LOG_BITMAP_STATE_SETUP,
> + LOG_BITMAP_STATE_ACTIVE);
> + /*
> + * start the logging period
> + */
> + logging_lock();
> + memory_global_dirty_log_start();
> + dirty_bitmap_sync();
> + bitmap_zero(logging_bitmap, ram_bitmap_pages);
> + logging_unlock();
> +
> + if (ram_block_info_dump(fd)) {
> + b->state = LOG_BITMAP_STATE_ERROR;
> + goto log_thread_end;
> + }
> +
> + /*
> + * sync the dirty bitmap along with saving it
> + * using the FILE pointer f.
> + */
> + while (b->current_epoch <= b->total_epochs) {
> + if (!runstate_check(RUN_STATE_DUMP_BITMAP) ||
> + b->state != LOG_BITMAP_STATE_ACTIVE) {
> + goto log_thread_end;
> + }
> + bitmap_zero(logging_bitmap, ram_bitmap_pages);
> + logging_lock();
> + dirty_bitmap_sync();
> + logging_unlock();
> +
> + ret = qemu_write_full(fd, logging_bitmap, bitmap_size);
> + if (ret < bitmap_size) {
> + b->state = LOG_BITMAP_STATE_ERROR;
> + goto log_thread_end;
> + }
> +
> + ret = qemu_write_full(fd, &marker, sizeof(char));
> + if (ret < sizeof(char)) {
> + b->state = LOG_BITMAP_STATE_ERROR;
> + goto log_thread_end;
> + }
> + g_usleep(b->current_frequency * 1000);
> + b->current_epoch++;
> + }
> +
> + /*
> + * stop the logging period.
> + */
> + log_thread_end:
> + logging_bitmap_close(b);
> + logging_state_update_status(b);
> + runstate_set(RUN_STATE_RUNNING);
> + return NULL;
> +}
> +
> +void qmp_log_dirty_bitmap(const char *filename, bool has_epochs,
> + int64_t epochs, bool has_frequency,
> + int64_t frequency, Error **errp)
> +{
> + int fd = -1;
> + BitmapLogState *b = logging_current_state();
> + Error *local_err = NULL;
> + if (runstate_check(RUN_STATE_DUMP_BITMAP) ||
> + b->state == LOG_BITMAP_STATE_ACTIVE ||
> + b->state == LOG_BITMAP_STATE_SETUP ||
> + b->state == LOG_BITMAP_STATE_CANCELING) {
> + error_setg(errp, "dirty bitmap dump in progress");
> + return;
> + }
> +
> + if (!runstate_is_running()) {
> + error_setg(errp, "Guest is not in a running state");
> + return;
> + }
> +
> + runstate_set(RUN_STATE_DUMP_BITMAP);
> + b->state = LOG_BITMAP_STATE_NONE;
> +
> + /*
> + * checking the epoch range
> + */
> + if (!has_epochs) {
> + b->total_epochs = MIN_EPOCH_VALUE;
> + } else if (!value_in_range(epochs, MIN_EPOCH_VALUE,
> + MAX_EPOCH_VALUE, "epoch", &local_err)) {
> + if (local_err) {
> + error_propagate(errp, local_err);
> + }
> + runstate_set(RUN_STATE_RUNNING);
> + return;
> + } else {
> + b->total_epochs = epochs;
> + }
> +
> + /*
> + * checking the frequency range
> + */
> + if (!has_frequency) {
> + b->current_frequency = MIN_FREQUENCY_VALUE;
> + } else if (!value_in_range(frequency, MIN_FREQUENCY_VALUE,
> + MAX_FREQUENCY_VALUE, "frequency",
> &local_err)) {
> + if (local_err) {
> + error_propagate(errp, local_err);
> + }
> + runstate_set(RUN_STATE_RUNNING);
> + return;
> + } else {
> + b->current_frequency = frequency;
> + }
> +
> + fd = qemu_open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY,
> S_IRUSR);
> + if (fd < 0) {
> + error_setg_file_open(errp, errno, filename);
> + runstate_set(RUN_STATE_RUNNING);
> + return;
> + }
> +
> + b->fd = fd;
> + qemu_thread_create(&b->thread, "dirty-bitmap-dump",
> + bitmap_logging_thread, b,
> + QEMU_THREAD_JOINABLE);
> +
> + return;
> +}
> +
> void qmp_xen_save_devices_state(const char *filename, Error **errp)
> {
> QEMUFile *f;
> --
> 1.9.3
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
