From patchwork Fri May 30 05:58:54 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alexey Kardashevskiy X-Patchwork-Id: 353880 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id C6F3A1400FA for ; Fri, 30 May 2014 15:59:38 +1000 (EST) Received: from localhost ([::1]:51582 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WqFqq-000145-I4 for incoming@patchwork.ozlabs.org; Fri, 30 May 2014 01:59:36 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37029) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WqFqO-0000aG-58 for qemu-devel@nongnu.org; Fri, 30 May 2014 01:59:16 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WqFqG-0002Xr-GT for qemu-devel@nongnu.org; Fri, 30 May 2014 01:59:08 -0400 Received: from mail-pb0-f43.google.com ([209.85.160.43]:57935) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WqFqG-0002Xk-8k for qemu-devel@nongnu.org; Fri, 30 May 2014 01:59:00 -0400 Received: by mail-pb0-f43.google.com with SMTP id up15so1352605pbc.2 for ; Thu, 29 May 2014 22:58:58 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:message-id:date:from:user-agent:mime-version:to :cc:subject:references:in-reply-to:content-type :content-transfer-encoding; bh=uJyMBthnIs5C/hqphVcCN/ozrtjzbZsh914CGk3lyfo=; b=egOmIqDOXOCiqsXq23c9O0lOaq+0srFgEq0hiyGozZnHrSxJvB1BpqwIAHJWil0wuh SOSyZSYF/q1TVAXy5uHZ1gCFyrVEi8MKYnTxwLLfrAjbtJeJ8iOedVlAj4EqhJ5aFedA AvR8C9mMkEBut6lt8jE3wxxxbB4nQ/hQCl9xu6UvfHzNNcO8lpthaxhFLlqH7JooiK4j JxJ20EAUlgEtRO/ET2ehBK/Q3e/4iPWrgDgzn9KkfVl6Xk4hO3TFMilKCnqjKdassC41 xOn+0BVHIf6toJviSvuHlXZSqgIEx3Egk1NAreOtgSMLrFwQ5y0dNNdN4+3dZrjetri9 S5lA== X-Gm-Message-State: ALoCoQnv3bGXEqRmvaK0bi0i4kUtOF0WCzGK22fEBASWlF/uYDHy6icuK/4zO0w2aK8rvU2SPAES X-Received: by 10.66.180.168 with SMTP id dp8mr15794370pac.100.1401429538361; Thu, 29 May 2014 22:58:58 -0700 (PDT) Received: from aik.ozlabs.ibm.com (ibmaus65.lnk.telstra.net. [165.228.126.9]) by mx.google.com with ESMTPSA id yw3sm4306917pbc.69.2014.05.29.22.58.56 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Thu, 29 May 2014 22:58:57 -0700 (PDT) Message-ID: <53881E1E.7050009@ozlabs.ru> Date: Fri, 30 May 2014 15:58:54 +1000 From: Alexey Kardashevskiy User-Agent: Mozilla/5.0 (X11; Linux i686 on x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.5.0 MIME-Version: 1.0 To: Alexander Graf , qemu-devel@nongnu.org References: <1400756006-26297-1-git-send-email-aik@ozlabs.ru> <537DD82F.4020402@suse.de> <537E08EE.1050501@ozlabs.ru> <538419D0.9030300@ozlabs.ru> <5385260B.2060108@suse.de> <53852F22.1070900@ozlabs.ru> <538530A5.2010903@suse.de> <53853967.5090905@ozlabs.ru> <5385CA0C.7040109@suse.de> In-Reply-To: <5385CA0C.7040109@suse.de> X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 209.85.160.43 Cc: qemu-ppc@nongnu.org, Juan Quintela Subject: Re: [Qemu-devel] [PATCH v2 8/8] spapr_pci: Use XICS interrupt allocator and do not cache interrupts in PHB X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org On 05/28/2014 09:35 PM, Alexander Graf wrote: > > On 28.05.14 03:18, Alexey Kardashevskiy wrote: >> On 05/28/2014 10:41 AM, Alexander Graf wrote: >>> On 28.05.14 02:34, Alexey Kardashevskiy wrote: >>>> On 05/28/2014 09:55 AM, Alexander Graf wrote: > > ... > >>>>> How do I migrate GHashTable? If I am allowed to use custom and bit more >>>>> polished get/put from "[PATCH 1/2] vmstate: Add helper to enable >>>>> GHashTable >>>>> migration", I'll be fine. >>>>> Yeah, I think it's ok to be custom in this case. Or another crazy idea - >>>>> could you flatten the hash table into an array of structs that you can >>>>> describe using VMState? You could then convert from the flat array >>>>> to/from >>>>> the GHashTable with pre_load/post_load hooks. >>>> Array is exactly what I am trying to get rid of. Ok, I'll remove >>>> hashmap at >>>> all and implement dynamic flat array (yay, yet another bicycle!). >>> Huh? The array would only live during the migration. It would be size=0 >>> during normal execution, but in a pre_save hook we could make size = >>> hash.length() and reuse the existing, working VMState infrastructure. >> When would I free that array? What if I continue the source guest and then >> migrate again? > > Something like > > void pre_save(...) { > free(s->array); > s->array_len = s->hash.number_of_keys(); > s->array = g_malloc(s->array_len * sizeof(struct array_elem)); > for (i = 0; i < s->array_len; i++) { > s->array[i].key = s->hash.key[i]; > s->array[i].value = s->hash.value[i]; > } > } > > That would waste a few bytes when we continue after migration, but it > should at least keep that overhead to a minimum. Ok. Fine. When do I allocate an array on the destination then? Remember, I do not know the number of device being transferred in advance because of PCI hotplug so I cannot guess. sPAPRPHBState::pre_load is too early - I do not know the size yet. I can: 1. transfer size separately as part of sPAPRPHBState 2. move this temporary array into a subsection 3. allocate array in the subsection's pre_load() in a hope that QEMU will call subsection's pre_load() AFTER the size of array is transferred. This is true for now but what if one day someone decides that all pre_load() callbacks from all subsections must be called at once at the beginning of the object migration? I am screwed then. Oooor I can make a patch as below (did not test it at all, just an idea). Basically define VMS_ALLOC flag which will allocate necessary amount of memory for that thing. I am definitely missing somewhere here, as usual, and there must be already some cool hack which I do not see, so what is it? >> I mean I can solve all of this for sure but duplicating data >> just to make existing migration happy is bit weird. But - I'll do what you >> say here, it is no big deal :) > > I don't find the concept of duplicating data for migration too odd - it > sounds like a good compromise between introspectability and abstraction. If > you have a better suggestion I'm all open :). diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h index 6af599d..7a14d26 100644 --- a/include/migration/vmstate.h +++ b/include/migration/vmstate.h @@ -101,6 +101,7 @@ enum VMStateFlags { VMS_VARRAY_UINT8 = 0x400, /* Array with size in uint8_t field*/ VMS_VARRAY_UINT32 = 0x800, /* Array with size in uint32_t field*/ VMS_MUST_EXIST = 0x1000, /* Field must exist in input */ + VMS_ALLOC = 0x2000, /* Alloc a buffer on the destination */ }; typedef struct { @@ -750,6 +751,16 @@ static const VMStateInfo vmstate_info_hash; .offset = vmstate_offset_value(_state, _field, qemu_hash), \ } +#define VMSTATE_VARRAY_STRUCT_ALLOC(_field, _state, _field_num, _version, _info, _type) {\ + .name = (stringify(_field)), \ + .version_id = (_version), \ + .num_offset = vmstate_offset_value(_state, _field_num, int32_t), \ + .info = &(_info), \ + .size = sizeof(_type), \ + .flags = VMS_VARRAY_INT32|VMS_POINTER|VMS_ALLOC, \ + .offset = vmstate_offset_pointer(_state, _field, _type), \ +} + #define VMSTATE_UNUSED_V(_v, _size) \ VMSTATE_UNUSED_BUFFER(NULL, _v, _size) diff --git a/vmstate.c b/vmstate.c index e1518da..7d6b0b9 100644 --- a/vmstate.c +++ b/vmstate.c @@ -48,6 +48,10 @@ static void *vmstate_base_addr(void *opaque, VMStateField *field) void *base_addr = opaque + field->offset; if (field->flags & VMS_POINTER) { + if (field->flags & VMS_ALLOC) { + n_elems = vmstate_n_elems(opaque, field); + *base_addr = g_malloc_n(n_elems, field->size); + } base_addr = *(void **)base_addr + field->start; }