From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from [140.186.70.92] (port=33026 helo=eggs.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1ORfWi-0002z2-Lm
	for qemu-devel@nongnu.org; Thu, 24 Jun 2010 02:03:06 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69)
	(envelope-from <paolo.bonzini@gmail.com>) id 1ORfJi-0001Sr-3H
	for qemu-devel@nongnu.org; Thu, 24 Jun 2010 01:49:41 -0400
Received: from mail-fx0-f45.google.com ([209.85.161.45]:47371)
	by eggs.gnu.org with esmtp (Exim 4.69)
	(envelope-from <paolo.bonzini@gmail.com>) id 1ORfJh-0001Sk-VE
	for qemu-devel@nongnu.org; Thu, 24 Jun 2010 01:49:38 -0400
Received: by fxm9 with SMTP id 9so4163322fxm.4
	for <qemu-devel@nongnu.org>; Wed, 23 Jun 2010 22:49:36 -0700 (PDT)
Sender: Paolo Bonzini <paolo.bonzini@gmail.com>
Message-ID: <4C22F1ED.4000904@redhat.com>
Date: Thu, 24 Jun 2010 07:49:33 +0200
From: Paolo Bonzini <pbonzini@redhat.com>
MIME-Version: 1.0
References: <20100624044046.16168.32804.stgit@localhost.localdomain>
	<20100624044202.16168.45991.stgit@localhost.localdomain>
In-Reply-To: <20100624044202.16168.45991.stgit@localhost.localdomain>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Subject: [Qemu-devel] Re: [PATCH 10/15] savevm: Migrate RAM based on
	name/offset
List-Id: qemu-devel.nongnu.org
List-Unsubscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Alex Williamson <alex.williamson@redhat.com>
Cc: jan.kiszka@siemens.com, qemu-devel@nongnu.org, armbru@redhat.com, paul@codesourcery.com, cam@cs.ualberta.ca, kraxel@redhat.com

On 06/24/2010 06:42 AM, Alex Williamson wrote:
>           if (flags&  RAM_SAVE_FLAG_COMPRESS) {
> -            uint8_t ch = qemu_get_byte(f);
> -            memset(qemu_get_ram_ptr(addr), ch, TARGET_PAGE_SIZE);
> +            void *host;
> +            uint8_t ch;
> +
> +            if (version_id == 3) {
> +                host = qemu_get_ram_ptr(addr);
> +            } else {
> +                RAMBlock *block;
> +                char id[256];
> +                uint8_t len;
> +
> +                len = qemu_get_byte(f);
> +                qemu_get_buffer(f, (uint8_t *)id, len);
> +                id[len] = 0;

Can you measure the amount of bytes transferred before and now?  It 
seems pretty expensive.  Maybe you could add another flag so that the 
memory name is passed on the wire only if it differs from the previous 
one.  Alternatively (perhaps better) define a new kind of RAM save data 
just for the RAM block, and assume all pages after it are for that 
block.  You can even encode the RAM length in the flags, so that you 
only have to pass "len | RAM_SAVE_FLAG_BLOCK_NAME" followed by the 
length+characters of the name.

Also, code like this:

+                len = qemu_get_byte(f);
+                qemu_get_buffer(f, (uint8_t *)id, len);
+                id[len] = 0;
+
+                QLIST_FOREACH(block, &ram_list.blocks, next) {
+                    if (!strncmp(id, block->idstr, sizeof(id)))
+                        break;
+                }
+                if (!block)
+                    return -EINVAL;

is present three times.  It should be split in two functions and these 
functions be called (the first function can also be used elsewhere, I 
think, though this of course can be done in a followup).

Paolo