From mboxrd@z Thu Jan  1 00:00:00 1970
From: Paolo Bonzini <pbonzini@redhat.com>
Subject: Re: [PATCH 15/19] savevm: introduce qemu_savevm_trans_{begin,commit}.
Date: Fri, 28 Jan 2011 14:26:53 +0100
Message-ID: <4D42C41D.7090705@redhat.com>
References: <1296199312-26334-1-git-send-email-tamura.yoshiaki@lab.ntt.co.jp> <1296199312-26334-16-git-send-email-tamura.yoshiaki@lab.ntt.co.jp>
Mime-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Cc: kvm@vger.kernel.org, qemu-devel@nongnu.org, avi@redhat.com,
	anthony@codemonkey.ws, aliguori@us.ibm.com, mtosatti@redhat.com,
	dlaor@redhat.com, mst@redhat.com, kwolf@redhat.com,
	ananth@in.ibm.com, psuriset@linux.vnet.ibm.com,
	vatsa@linux.vnet.ibm.com, stefanha@linux.vnet.ibm.com,
	blauwirbel@gmail.com, ohmura.kei@lab.ntt.co.jp
To: Yoshiaki Tamura <tamura.yoshiaki@lab.ntt.co.jp>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mail-yw0-f46.google.com ([209.85.213.46]:56360 "EHLO
	mail-yw0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1751874Ab1A1N1F (ORCPT <rfc822;kvm@vger.kernel.org>);
	Fri, 28 Jan 2011 08:27:05 -0500
Received: by ywe10 with SMTP id 10so1049301ywe.19
        for <kvm@vger.kernel.org>; Fri, 28 Jan 2011 05:27:04 -0800 (PST)
In-Reply-To: <1296199312-26334-16-git-send-email-tamura.yoshiaki@lab.ntt.co.jp>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On 01/28/2011 08:21 AM, Yoshiaki Tamura wrote:
> +int qemu_savevm_trans_begin(Monitor *mon, QEMUFile *f, int init)
> +{
> +    SaveStateEntry *se;
> +    int skipped = 0;
> +
> +    QTAILQ_FOREACH(se,&savevm_handlers, entry) {
> +        int len, stage, ret;
> +
> +        if (se->save_live_state == NULL) {
> +            continue;
> +        }
> +
> +        /* Section type */
> +        qemu_put_byte(f, QEMU_VM_SECTION_START);
> +        qemu_put_be32(f, se->section_id);
> +
> +        /* ID string */
> +        len = strlen(se->idstr);
> +        qemu_put_byte(f, len);
> +        qemu_put_buffer(f, (uint8_t *)se->idstr, len);
> +
> +        qemu_put_be32(f, se->instance_id);
> +        qemu_put_be32(f, se->version_id);
> +
> +        stage = init ? QEMU_VM_SECTION_START : QEMU_VM_SECTION_PART;
> +        ret = se->save_live_state(mon, f, stage, se->opaque);
> +        if (!ret) {
> +            skipped++;
> +        }
> +    }
> +
> +    if (qemu_file_has_error(f)) {
> +        return -EIO;
> +    }
> +
> +    return skipped;
> +}
> +

Right now, this is very similar to qemu_savevm_state_begin and _iterate, 
but not quite.  Perhaps you could abstract it to a single function that 
could be used everywhere live handlers are used.  For example,

/* section says which header to write; incremental == true forces to
    pass SECTION_PART instead of SECTION_START. In code:

if (section == QEMU_VM_SECTION_START) {
     stage = incremental ? QEMU_VM_SECTION_PART : QEMU_VM_SECTION_START;
} else {
     assert(incremental);
     stage = section;
}

    */
int qemu_savevm_state_live(Monitor *mon, QEMUFile *f, int section,
			   int incremental)

Likewise,

> +    QTAILQ_FOREACH(se, &savevm_handlers, entry) {
> +        int len;
> +
> +        if (se->save_state == NULL && se->vmsd == NULL) {
> +            continue;
> +        }
> +
> +        /* Section type */
> +        qemu_put_byte(f, QEMU_VM_SECTION_FULL);
> +        qemu_put_be32(f, se->section_id);
> +
> +        /* ID string */
> +        len = strlen(se->idstr);
> +        qemu_put_byte(f, len);
> +        qemu_put_buffer(f, (uint8_t *)se->idstr, len);
> +
> +        qemu_put_be32(f, se->instance_id);
> +        qemu_put_be32(f, se->version_id);
> +
> +        vmstate_save(f, se);
> +    }

this code is straight from qemu_savevm_state_complete and should be 
moved into its own function.

Paolo