From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43221) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1W5aMn-0001uN-SO for qemu-devel@nongnu.org; Tue, 21 Jan 2014 07:23:46 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1W5aMi-0005Bq-S0 for qemu-devel@nongnu.org; Tue, 21 Jan 2014 07:23:41 -0500 Message-ID: <52DE66E8.4040003@redhat.com> Date: Tue, 21 Jan 2014 14:24:08 +0200 From: Orit Wasserman MIME-Version: 1.0 References: <33183CC9F5247A488A2544077AF19020815BB9E1@SZXEMA503-MBS.china.huawei.com> In-Reply-To: <33183CC9F5247A488A2544077AF19020815BB9E1@SZXEMA503-MBS.china.huawei.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2] migration:fix free XBZRLE decoded_buf wrong List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Gonglei (Arei)" , "qemu-devel@nongnu.org" , "qemu-stable@nongnu.org" , Peter Maydell , "anthony@codemonkey.ws" , "pbonzini@redhat.com" Cc: "chenliang (T)" , Luonengjun , "Huangweidong (Hardware)" On 01/21/2014 02:11 PM, Gonglei (Arei) wrote: > Hi, > > This is an update of my patch. > Modifications in v2: > * Removing excess check for g_free > * The structure of XBZRLE is divided into two halves.One is for > * src side, another is for dest side. > What is the benefit of splitting the structure? decode_buf is only allocated (and freed) in the destination any way. Orit > Signed-off-by: chenliang > --- > arch_init.c | 23 ++++++++++++++--------- > include/migration/migration.h | 1 + > migration.c | 1 + > 3 files changed, 16 insertions(+), 9 deletions(-) > > diff --git a/arch_init.c b/arch_init.c > index 77912e7..9a28a43 100644 > --- a/arch_init.c > +++ b/arch_init.c > @@ -164,17 +164,15 @@ static struct { > uint8_t *encoded_buf; > /* buffer for storing page content */ > uint8_t *current_buf; > - /* buffer used for XBZRLE decoding */ > - uint8_t *decoded_buf; > /* Cache for XBZRLE */ > PageCache *cache; > } XBZRLE = { > .encoded_buf = NULL, > .current_buf = NULL, > - .decoded_buf = NULL, > .cache = NULL, > }; > - > +/* buffer used for XBZRLE decoding */ > +static uint8_t *xbzrle_decoded_buf = NULL; > > int64_t xbzrle_cache_resize(int64_t new_size) > { > @@ -602,6 +600,12 @@ uint64_t ram_bytes_total(void) > return total; > } > > +void free_xbzrle_decoded_buf(void) > +{ > + g_free(xbzrle_decoded_buf); > + xbzrle_decoded_buf = NULL; > +} > + > static void migration_end(void) > { > if (migration_bitmap) { > @@ -615,8 +619,9 @@ static void migration_end(void) > g_free(XBZRLE.cache); > g_free(XBZRLE.encoded_buf); > g_free(XBZRLE.current_buf); > - g_free(XBZRLE.decoded_buf); > XBZRLE.cache = NULL; > + XBZRLE.encoded_buf = NULL; > + XBZRLE.current_buf = NULL; > } > } > > @@ -807,8 +812,8 @@ static int load_xbzrle(QEMUFile *f, ram_addr_t addr, void *host) > unsigned int xh_len; > int xh_flags; > > - if (!XBZRLE.decoded_buf) { > - XBZRLE.decoded_buf = g_malloc(TARGET_PAGE_SIZE); > + if (!xbzrle_decoded_buf) { > + xbzrle_decoded_buf = g_malloc(TARGET_PAGE_SIZE); > } > > /* extract RLE header */ > @@ -825,10 +830,10 @@ static int load_xbzrle(QEMUFile *f, ram_addr_t addr, void *host) > return -1; > } > /* load data and decode */ > - qemu_get_buffer(f, XBZRLE.decoded_buf, xh_len); > + qemu_get_buffer(f, xbzrle_decoded_buf, xh_len); > > /* decode RLE */ > - ret = xbzrle_decode_buffer(XBZRLE.decoded_buf, xh_len, host, > + ret = xbzrle_decode_buffer(xbzrle_decoded_buf, xh_len, host, > TARGET_PAGE_SIZE); > if (ret == -1) { > fprintf(stderr, "Failed to load XBZRLE page - decode error!\n"); > diff --git a/include/migration/migration.h b/include/migration/migration.h > index bfa3951..3e1e6c7 100644 > --- a/include/migration/migration.h > +++ b/include/migration/migration.h > @@ -109,6 +109,7 @@ MigrationState *migrate_get_current(void); > uint64_t ram_bytes_remaining(void); > uint64_t ram_bytes_transferred(void); > uint64_t ram_bytes_total(void); > +void free_xbzrle_decoded_buf(void); > > void acct_update_position(QEMUFile *f, size_t size, bool zero); > > diff --git a/migration.c b/migration.c > index 7235c23..3d46804 100644 > --- a/migration.c > +++ b/migration.c > @@ -105,6 +105,7 @@ static void process_incoming_migration_co(void *opaque) > > ret = qemu_loadvm_state(f); > qemu_fclose(f); > + free_xbzrle_decoded_buf(); > if (ret < 0) { > fprintf(stderr, "load of migration failed\n"); > exit(EXIT_FAILURE); >