Re: [Qemu-devel] [PATCH v2 2/8] bitmap dump code via QAPI framework

[Dr. David Alan Gilbert]

* Sanidhya Kashyap (address@hidden) wrote:
> Following are the changes made with respect to the previous version:
> Chen's advice

> +    if (qemu_write_full(fd, &ram_bitmap_pages, sizeof(int64_t)) < 0) {
> +        b->state = LOG_BITMAP_STATE_ERROR;
> +        goto log_thread_end;
> +    }
> +
> +    /*
> +     * sync the dirty bitmap along with saving it
> +     * using the FILE pointer f.
> +     */
> +    while (epoch_count < total_epochs) {
> +        if (!runstate_is_running() || b->state != LOG_BITMAP_STATE_ACTIVE) {
> +            goto log_thread_end;
> +        }
> +        bitmap_zero(logging_bitmap, ram_bitmap_pages);
> +        logging_lock();
> +        dirty_bitmap_sync();
> +        logging_unlock();
> +        if (qemu_write_full(fd, logging_bitmap, bitmap_size) < 0) {
> +            b->state = LOG_BITMAP_STATE_ERROR;
> +            goto log_thread_end;
> +        }
> +        g_usleep(b->current_frequency * 1000);
> +        epoch_count++;
> +    }

I wonder about adding two extra things to the file format:
  1) The block names/length/offset information - so that you can tell
that bitmap entry 'n' is from main ram or from video ram.
  2) A marker word between/after each bitmap with a known value - it would
help spot any error where the wrong length is being read in the scripts;
otherwise it would be easy to get misaligned bitmaps without really noticing.

Dave

> +
> +    /*
> +     * stop the logging period.
> +     */
> + log_thread_end:
> +    logging_bitmap_close(b);
> +    if (b->state == LOG_BITMAP_STATE_ACTIVE) {
> +        logging_state_set_status(b, LOG_BITMAP_STATE_ACTIVE,
> +                                    LOG_BITMAP_STATE_COMPLETED);
> +    } else if (b->state == LOG_BITMAP_STATE_CANCELING) {
> +        logging_state_set_status(b, LOG_BITMAP_STATE_CANCELING,
> +                                    LOG_BITMAP_STATE_COMPLETED);
> +    } else if (b->state == LOG_BITMAP_STATE_ERROR) {
> +        logging_state_set_status(b, LOG_BITMAP_STATE_ERROR,
> +                                    LOG_BITMAP_STATE_COMPLETED);
> +    }
> +    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 (b->state == LOG_BITMAP_STATE_ACTIVE ||
> +            b->state == LOG_BITMAP_STATE_SETUP ||
> +            b->state == LOG_BITMAP_STATE_CANCELING) {
> +        b = NULL;
> +        error_setg(errp, "dirty bitmap dump in progress");
> +        return;
> +    }
> +
> +    if (b->state == LOG_BITMAP_STATE_COMPLETED) {
> +        b->state = LOG_BITMAP_STATE_NONE;
> +    }
> +
> +    if (!has_epochs) {
> +        epochs = MIN_EPOCH_VALUE;
> +    }
> +    if (!has_frequency) {
> +        frequency = MIN_FREQUENCY_VALUE;
> +    }
> +
> +    if (!check_value(epochs, MIN_EPOCH_VALUE, "epoch", &local_err) ||
> +        !check_value(frequency, MIN_FREQUENCY_VALUE, "frequency", 
> &local_err)) {
> +        if (local_err) {
> +            b = NULL;
> +            error_propagate(errp, local_err);
> +            return;
> +        }
> +    }
> +
> +    fd = qemu_open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, 
> S_IRUSR);
> +    if (fd < 0) {
> +        error_setg_file_open(errp, errno, filename);
> +        b = NULL;
> +        return;
> +    }
> +
> +    b->total_epochs = epochs;
> +    b->current_frequency = frequency;
> +    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.8.3.1
> 
> 
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK