From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52818) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XX5aY-0003uh-Os for qemu-devel@nongnu.org; Thu, 25 Sep 2014 05:43:56 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XX5aP-0003TV-Je for qemu-devel@nongnu.org; Thu, 25 Sep 2014 05:43:49 -0400 Message-ID: <5423E3C6.4070708@suse.de> Date: Thu, 25 Sep 2014 11:43:34 +0200 From: Alexander Graf MIME-Version: 1.0 References: <1411628523-3498-1-git-send-email-aik@ozlabs.ru> <1411628523-3498-3-git-send-email-aik@ozlabs.ru> In-Reply-To: <1411628523-3498-3-git-send-email-aik@ozlabs.ru> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 2/2] spapr_nvram: Enable migration List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexey Kardashevskiy , qemu-devel@nongnu.org Cc: Paolo Bonzini , qemu-ppc@nongnu.org On 25.09.14 09:02, Alexey Kardashevskiy wrote: > The only case when sPAPR NVRAM migrates now is if is backed by a file and > copy-storage migration is performed. > > This enables RAM copy of NVRAM even if NVRAM is backed by a file. > > This defines a VMSTATE descriptor for NVRAM device so the memory copy > of NVRAM can migrate and be written to a backing file on the destination > if one is provided. > > Signed-off-by: Alexey Kardashevskiy > --- > hw/nvram/spapr_nvram.c | 68 +++++++++++++++++++++++++++++++++++++++++++------- > 1 file changed, 59 insertions(+), 9 deletions(-) > > diff --git a/hw/nvram/spapr_nvram.c b/hw/nvram/spapr_nvram.c > index 6a72ef4..254009e 100644 > --- a/hw/nvram/spapr_nvram.c > +++ b/hw/nvram/spapr_nvram.c > @@ -76,15 +76,20 @@ static void rtas_nvram_fetch(PowerPCCPU *cpu, sPAPREnvironment *spapr, > return; > } > > + assert(nvram->buf); > + > membuf = cpu_physical_memory_map(buffer, &len, 1); > + > + alen = len; > if (nvram->drive) { > alen = bdrv_pread(nvram->drive, offset, membuf, len); > + if (alen > 0) { > + memcpy(nvram->buf + offset, membuf, alen); Why? > + } > } else { > - assert(nvram->buf); > - > memcpy(membuf, nvram->buf + offset, len); > - alen = len; > } > + > cpu_physical_memory_unmap(membuf, len, 1, len); > > rtas_st(rets, 0, (alen < len) ? RTAS_OUT_HW_ERROR : RTAS_OUT_SUCCESS); > @@ -122,14 +127,15 @@ static void rtas_nvram_store(PowerPCCPU *cpu, sPAPREnvironment *spapr, > } > > membuf = cpu_physical_memory_map(buffer, &len, 0); > + > + alen = len; > if (nvram->drive) { > alen = bdrv_pwrite(nvram->drive, offset, membuf, len); > - } else { > - assert(nvram->buf); > - > - memcpy(nvram->buf + offset, membuf, len); > - alen = len; > } > + > + assert(nvram->buf); > + memcpy(nvram->buf + offset, membuf, len); > + > cpu_physical_memory_unmap(membuf, len, 0, len); > > rtas_st(rets, 0, (alen < len) ? RTAS_OUT_HW_ERROR : RTAS_OUT_SUCCESS); > @@ -144,9 +150,10 @@ static int spapr_nvram_init(VIOsPAPRDevice *dev) > nvram->size = bdrv_getlength(nvram->drive); > } else { > nvram->size = DEFAULT_NVRAM_SIZE; > - nvram->buf = g_malloc0(nvram->size); > } > > + nvram->buf = g_malloc0(nvram->size); > + > if ((nvram->size < MIN_NVRAM_SIZE) || (nvram->size > MAX_NVRAM_SIZE)) { > fprintf(stderr, "spapr-nvram must be between %d and %d bytes in size\n", > MIN_NVRAM_SIZE, MAX_NVRAM_SIZE); > @@ -166,6 +173,48 @@ static int spapr_nvram_devnode(VIOsPAPRDevice *dev, void *fdt, int node_off) > return fdt_setprop_cell(fdt, node_off, "#bytes", nvram->size); > } > > +static int spapr_nvram_pre_load(void *opaque) > +{ > + sPAPRNVRAM *nvram = VIO_SPAPR_NVRAM(opaque); > + > + g_free(nvram->buf); > + nvram->buf = NULL; > + nvram->size = 0; > + > + return 0; > +} > + > +static int spapr_nvram_post_load(void *opaque, int version_id) > +{ > + sPAPRNVRAM *nvram = VIO_SPAPR_NVRAM(opaque); > + > + if (nvram->drive) { > + int alen = bdrv_pwrite(nvram->drive, 0, nvram->buf, nvram->size); In the file backed case you're already overwriting the disk backed version, no? Also, couldn't you just do the copy and provisioning of buf in a pre_save hook? Alex > + > + if (alen < 0) { > + return alen; > + } > + if (alen != nvram->size) { > + return -1; > + } > + } > + > + return 0; > +} > + > +static const VMStateDescription vmstate_spapr_nvram = { > + .name = "spapr_nvram", > + .version_id = 1, > + .minimum_version_id = 1, > + .pre_load = spapr_nvram_pre_load, > + .post_load = spapr_nvram_post_load, > + .fields = (VMStateField[]) { > + VMSTATE_UINT32(size, sPAPRNVRAM), > + VMSTATE_VBUFFER_ALLOC_UINT32(buf, sPAPRNVRAM, 1, NULL, 0, size), > + VMSTATE_END_OF_LIST() > + }, > +}; > + > static Property spapr_nvram_properties[] = { > DEFINE_SPAPR_PROPERTIES(sPAPRNVRAM, sdev), > DEFINE_PROP_DRIVE("drive", sPAPRNVRAM, drive), > @@ -184,6 +233,7 @@ static void spapr_nvram_class_init(ObjectClass *klass, void *data) > k->dt_compatible = "qemu,spapr-nvram"; > set_bit(DEVICE_CATEGORY_MISC, dc->categories); > dc->props = spapr_nvram_properties; > + dc->vmsd = &vmstate_spapr_nvram; > } > > static const TypeInfo spapr_nvram_type_info = { >