[Qemu-devel] [PATCH 0/2] Here are 2 patches to enable sPAPR NVRAM migration.

[Qemu-devel] [PATCH 0/2] Here are 2 patches to enable sPAPR NVRAM migration.

[Alexey Kardashevskiy]

Please comment. Thanks!

Changes:
v2:
* reworked to always have a copy of NVRAM in RAM and use file only for writes




Alexey Kardashevskiy (2):
  vmstate: Allow dynamic allocation for VBUFFER during migration
  spapr_nvram: Enable migration

 hw/nvram/spapr_nvram.c      | 81 ++++++++++++++++++++++++++++++++++++---------
 include/migration/vmstate.h | 11 ++++++
 vmstate.c                   | 13 ++++++--
 3 files changed, 86 insertions(+), 19 deletions(-)

-- 
2.0.0

[Qemu-devel] [PATCH 1/2] vmstate: Allow dynamic allocation for VBUFFER during migration

[Alexey Kardashevskiy]

This extends use of VMS_ALLOC flag from arrays to VBUFFER as well.

This defines VMSTATE_VBUFFER_ALLOC_UINT32 which makes use of VMS_ALLOC
and uses uint32_t type for a size.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
 include/migration/vmstate.h | 11 +++++++++++
 vmstate.c                   | 13 ++++++++++---
 2 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
index 9a001bd..e45fc49 100644
--- a/include/migration/vmstate.h
+++ b/include/migration/vmstate.h
@@ -484,6 +484,17 @@ extern const VMStateInfo vmstate_info_bitmap;
     .start        = (_start),                                        \
 }
 
+#define VMSTATE_VBUFFER_ALLOC_UINT32(_field, _state, _version, _test, _start, _field_size) { \
+    .name         = (stringify(_field)),                             \
+    .version_id   = (_version),                                      \
+    .field_exists = (_test),                                         \
+    .size_offset  = vmstate_offset_value(_state, _field_size, uint32_t),\
+    .info         = &vmstate_info_buffer,                            \
+    .flags        = VMS_VBUFFER|VMS_POINTER|VMS_ALLOC,               \
+    .offset       = offsetof(_state, _field),                        \
+    .start        = (_start),                                        \
+}
+
 #define VMSTATE_BUFFER_UNSAFE_INFO(_field, _state, _version, _info, _size) { \
     .name       = (stringify(_field)),                               \
     .version_id = (_version),                                        \
diff --git a/vmstate.c b/vmstate.c
index ef2f87b..3dde574 100644
--- a/vmstate.c
+++ b/vmstate.c
@@ -49,9 +49,16 @@ static void *vmstate_base_addr(void *opaque, VMStateField *field, bool alloc)
 
     if (field->flags & VMS_POINTER) {
         if (alloc && (field->flags & VMS_ALLOC)) {
-            int n_elems = vmstate_n_elems(opaque, field);
-            if (n_elems) {
-                gsize size = n_elems * field->size;
+            gsize size = 0;
+            if (field->flags & VMS_VBUFFER) {
+                size = vmstate_size(opaque, field);
+            } else {
+                int n_elems = vmstate_n_elems(opaque, field);
+                if (n_elems) {
+                    size = n_elems * field->size;
+                }
+            }
+            if (size) {
                 *((void **)base_addr + field->start) = g_malloc(size);
             }
         }
-- 
2.0.0

Re: [Qemu-devel] [PATCH 1/2] vmstate: Allow dynamic allocation for VBUFFER during migration

[Alexey Kardashevskiy]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 10/02/2014 07:56 PM, Alexey Kardashevskiy wrote:
> This extends use of VMS_ALLOC flag from arrays to VBUFFER as well.
> 
> This defines VMSTATE_VBUFFER_ALLOC_UINT32 which makes use of
> VMS_ALLOC and uses uint32_t type for a size.


Adding Juan to cc:. Please, ack :)


> 
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> --- 
> include/migration/vmstate.h | 11 +++++++++++ vmstate.c
> | 13 ++++++++++--- 2 files changed, 21 insertions(+), 3 deletions(-)
> 
> diff --git a/include/migration/vmstate.h
> b/include/migration/vmstate.h index 9a001bd..e45fc49 100644 ---
> a/include/migration/vmstate.h +++ b/include/migration/vmstate.h @@
> -484,6 +484,17 @@ extern const VMStateInfo vmstate_info_bitmap; .start
> = (_start),                                        \ }
> 
> +#define VMSTATE_VBUFFER_ALLOC_UINT32(_field, _state, _version, _test,
> _start, _field_size) { \ +    .name         = (stringify(_field)),
> \ +    .version_id   = (_version),
> \ +    .field_exists = (_test),
> \ +    .size_offset  = vmstate_offset_value(_state, _field_size,
> uint32_t),\ +    .info         = &vmstate_info_buffer,
> \ +    .flags        = VMS_VBUFFER|VMS_POINTER|VMS_ALLOC,
> \ +    .offset       = offsetof(_state, _field),
> \ +    .start        = (_start),
> \ +} + #define VMSTATE_BUFFER_UNSAFE_INFO(_field, _state, _version,
> _info, _size) { \ .name       = (stringify(_field)),
> \ .version_id = (_version),                                        \ 
> diff --git a/vmstate.c b/vmstate.c index ef2f87b..3dde574 100644 ---
> a/vmstate.c +++ b/vmstate.c @@ -49,9 +49,16 @@ static void
> *vmstate_base_addr(void *opaque, VMStateField *field, bool alloc)
> 
> if (field->flags & VMS_POINTER) { if (alloc && (field->flags &
> VMS_ALLOC)) { -            int n_elems = vmstate_n_elems(opaque,
> field); -            if (n_elems) { -                gsize size =
> n_elems * field->size; +            gsize size = 0; +            if
> (field->flags & VMS_VBUFFER) { +                size =
> vmstate_size(opaque, field); +            } else { +
> int n_elems = vmstate_n_elems(opaque, field); +                if
> (n_elems) { +                    size = n_elems * field->size; +
> } +            } +            if (size) { *((void **)base_addr +
> field->start) = g_malloc(size); } }
> 


- -- 
Alexey
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1

iQIcBAEBAgAGBQJULeLkAAoJEIYTPdgrwSC5r2UP/RSiuKEanpXfhwyhudAB1nGM
9t3NJxrLO5PhkhfIveGgcf/t3hWpGOGu1JiiwpD8//ipjX0nBGt0O8p6Tp6xATtm
2NOid+9F6yUxfhLVB3vGQZ4KocguPt789t37HRgn9oT9YVxsJdeb1ftcDXw+opB7
qM113ZdrOVBPVeH1+vSC0vUwNYokjRQf/kX8IwwFXyJirt9/Rf9FaXf+KuturpJo
lp2yYVhuWmBy+MID5HZNlTfqVU5iMmX6ngqVm7rkaFxRA/3Ywlj1OtYGyf9htgya
OauFrvwUlM4yh3CiqLqu12Jabadu1Oa8yIeY9r2O5A4JLZt9JS2n3vxM70H8Z67Z
XJpQDF2Il26w9jIlmJ10GvVFt7sBaV0Yzu9SfgzNdSO/zteaODulOLj3eciUwnDk
DxbFvf8zn1vCuFjIwq3lqPaH1mNHEnLnV+6XlJLYNrD9Vngcc2UN/hXRMKwWovdL
rJAtOCObDB8VlilqS5/sFCekZsQasAnY7NckQvrx+0V/I7BJ99I7kKGcNIZF8pAU
84XHrw8j80+jh5W+6byn8M0lKgd7dn11KXcXzE1nuIVAP9KxG7Br5od3zePqhiIl
mdJ9meR3aOfNAr6N9iW/El+2g3FvhH3TQaZ+R5ZJMGCTVt9tqIVf1XLuXVbjdV3X
/xd/ACb10y32PH2sIEhl
=tSNm
-----END PGP SIGNATURE-----

Re: [Qemu-devel] [PATCH 1/2] vmstate: Allow dynamic allocation for VBUFFER during migration

[Alexey Kardashevskiy]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 10/03/2014 09:42 AM, Alexey Kardashevskiy wrote:
> On 10/02/2014 07:56 PM, Alexey Kardashevskiy wrote:
>> This extends use of VMS_ALLOC flag from arrays to VBUFFER as well.
> 
>> This defines VMSTATE_VBUFFER_ALLOC_UINT32 which makes use of 
>> VMS_ALLOC and uses uint32_t type for a size.
> 
> 
> Adding Juan to cc:. Please, ack :)


Juan, ping?



> 
> 
> 
>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> --- 
>> include/migration/vmstate.h | 11 +++++++++++ vmstate.c | 13
>> ++++++++++--- 2 files changed, 21 insertions(+), 3 deletions(-)
> 
>> diff --git a/include/migration/vmstate.h 
>> b/include/migration/vmstate.h index 9a001bd..e45fc49 100644 --- 
>> a/include/migration/vmstate.h +++ b/include/migration/vmstate.h @@ 
>> -484,6 +484,17 @@ extern const VMStateInfo vmstate_info_bitmap;
>> .start = (_start),                                        \ }
> 
>> +#define VMSTATE_VBUFFER_ALLOC_UINT32(_field, _state, _version,
>> _test, _start, _field_size) { \ +    .name         =
>> (stringify(_field)), \ +    .version_id   = (_version), \ +
>> .field_exists = (_test), \ +    .size_offset  =
>> vmstate_offset_value(_state, _field_size, uint32_t),\ +    .info
>> = &vmstate_info_buffer, \ +    .flags        =
>> VMS_VBUFFER|VMS_POINTER|VMS_ALLOC, \ +    .offset       =
>> offsetof(_state, _field), \ +    .start        = (_start), \ +} +
>> #define VMSTATE_BUFFER_UNSAFE_INFO(_field, _state, _version, _info,
>> _size) { \ .name       = (stringify(_field)), \ .version_id =
>> (_version),                                        \ diff --git
>> a/vmstate.c b/vmstate.c index ef2f87b..3dde574 100644 --- 
>> a/vmstate.c +++ b/vmstate.c @@ -49,9 +49,16 @@ static void 
>> *vmstate_base_addr(void *opaque, VMStateField *field, bool alloc)
> 
>> if (field->flags & VMS_POINTER) { if (alloc && (field->flags & 
>> VMS_ALLOC)) { -            int n_elems = vmstate_n_elems(opaque, 
>> field); -            if (n_elems) { -                gsize size = 
>> n_elems * field->size; +            gsize size = 0; +            if 
>> (field->flags & VMS_VBUFFER) { +                size = 
>> vmstate_size(opaque, field); +            } else { + int n_elems =
>> vmstate_n_elems(opaque, field); +                if (n_elems) { +
>> size = n_elems * field->size; + } +            } +            if
>> (size) { *((void **)base_addr + field->start) = g_malloc(size); } }
> 
> 
> 
> 

- -- 
Alexey
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1

iQIcBAEBAgAGBQJUNhsTAAoJEIYTPdgrwSC5CzgQAJbQ7C259BGZqPics7P1FhIm
wzHhwfUwfEML6nRtaVMeime8qqfbD048VNH43/nCiCoy2MjkvXV/uT+j2f+hLjcG
46Ys004S7SDn+bl+7Z24sFGZ5iqBO3is+kLj79w878IwC5SPM0jH7dfnU+ZSimVE
rCug/D95IvU4gOVfxtDbFg5oq6sgGHC+SpLsg9CAo2lBcOje5ylSAwyd5/s3pmnC
FMXf3nJI/aAQTcbrVeuB4+khGP4hncqzh2w07IP8olszZcyTzRIzQnyU3eqIbPrM
8nX6HdrauVj0VxtSZJzticaJnzO7qqOl+1TiCZWCV5m7jchbPbKnuvovoRQ1v5cw
DSmZ8BULGK6XnuOmqhEY8AMDAYcHP8DvnJ1N+dmAcsfCr6jFIb0HAW6VWsqMJxDG
J7G3jIKooOSXCmciFkS+2QMJ0TrAgavF/K2njdcA/7up+nVBDnPsEX+bFnox9N+w
KnyBYS8tWqP4NuK+q53BtkZllMXQYSLFQUoi5XgKoCL1dk1xoxlbZg/3rbXzMB3a
sWt3W9zxx5PzsNGEUfQyl8mgmR02y+N1cqJRu/6HJfxfXJ2m+LvTPhlp1oWJVuHT
YKam971lTrPja0Z7OfNgWjPgv/OOW7NjzMGbq51SV20TDKgn1sJMSGqJ/cdqBkeS
HZBj6AVhEzX4V+NDAkrH
=JyEK
-----END PGP SIGNATURE-----

Re: [Qemu-devel] [PATCH 1/2] vmstate: Allow dynamic allocation for VBUFFER during migration

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 571 bytes --]

On Thu, Oct 02, 2014 at 07:56:02PM +1000, Alexey Kardashevskiy wrote:
> This extends use of VMS_ALLOC flag from arrays to VBUFFER as well.
> 
> This defines VMSTATE_VBUFFER_ALLOC_UINT32 which makes use of VMS_ALLOC
> and uses uint32_t type for a size.
> 
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

Re: [Qemu-devel] [PATCH 1/2] vmstate: Allow dynamic allocation for VBUFFER during migration

[Juan Quintela]

Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
> This extends use of VMS_ALLOC flag from arrays to VBUFFER as well.
>
> This defines VMSTATE_VBUFFER_ALLOC_UINT32 which makes use of VMS_ALLOC
> and uses uint32_t type for a size.
>
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>

Reviewed-by: Juan Quintela <quintela@redhat.com>

[Qemu-devel] [PATCH 2/2] spapr_nvram: Enable migration

[Alexey Kardashevskiy]

The only case when sPAPR NVRAM migrates now is if is backed by a file and
copy-storage migration is performed. In other cases NVRAM does not
migrate regardless whether it is backed by a file or not.

This enables shadow copy of NVRAM in RAM which is read from a file
(if used) and used for reads. Writes to NVRAM are mirrored to the file.

This defines a VMSTATE descriptor for NVRAM device so the memory copy
of NVRAM can migrate and be flushed to a backing file on the destination
if one is specified.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
Changes:
v2:
* implemented full-time shadow copy of NVRAM which is read from a file
at start and stays in RAM during live of the guest
---
 hw/nvram/spapr_nvram.c | 81 ++++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 65 insertions(+), 16 deletions(-)

diff --git a/hw/nvram/spapr_nvram.c b/hw/nvram/spapr_nvram.c
index 6a72ef4..d34edfb 100644
--- a/hw/nvram/spapr_nvram.c
+++ b/hw/nvram/spapr_nvram.c
@@ -51,7 +51,6 @@ static void rtas_nvram_fetch(PowerPCCPU *cpu, sPAPREnvironment *spapr,
 {
     sPAPRNVRAM *nvram = spapr->nvram;
     hwaddr offset, buffer, len;
-    int alen;
     void *membuf;
 
     if ((nargs != 3) || (nret != 2)) {
@@ -76,19 +75,16 @@ static void rtas_nvram_fetch(PowerPCCPU *cpu, sPAPREnvironment *spapr,
         return;
     }
 
+    assert(nvram->buf);
+
     membuf = cpu_physical_memory_map(buffer, &len, 1);
-    if (nvram->drive) {
-        alen = bdrv_pread(nvram->drive, offset, membuf, len);
-    } else {
-        assert(nvram->buf);
 
-        memcpy(membuf, nvram->buf + offset, len);
-        alen = len;
-    }
+    memcpy(membuf, nvram->buf + offset, len);
+
     cpu_physical_memory_unmap(membuf, len, 1, len);
 
-    rtas_st(rets, 0, (alen < len) ? RTAS_OUT_HW_ERROR : RTAS_OUT_SUCCESS);
-    rtas_st(rets, 1, (alen < 0) ? 0 : alen);
+    rtas_st(rets, 0, RTAS_OUT_SUCCESS);
+    rtas_st(rets, 1, len);
 }
 
 static void rtas_nvram_store(PowerPCCPU *cpu, sPAPREnvironment *spapr,
@@ -122,14 +118,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,15 +141,24 @@ 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);
         return -1;
     }
 
+    if (nvram->drive) {
+        int alen = bdrv_pread(nvram->drive, 0, nvram->buf, nvram->size);
+
+        if (alen != nvram->size) {
+            return -1;
+        }
+    }
+
     spapr_rtas_register(RTAS_NVRAM_FETCH, "nvram-fetch", rtas_nvram_fetch);
     spapr_rtas_register(RTAS_NVRAM_STORE, "nvram-store", rtas_nvram_store);
 
@@ -166,6 +172,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);
+
+        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 +232,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 = {
-- 
2.0.0

Re: [Qemu-devel] [PATCH 2/2] spapr_nvram: Enable migration

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 919 bytes --]

On Thu, Oct 02, 2014 at 07:56:03PM +1000, Alexey Kardashevskiy wrote:
> The only case when sPAPR NVRAM migrates now is if is backed by a file and
> copy-storage migration is performed. In other cases NVRAM does not
> migrate regardless whether it is backed by a file or not.
> 
> This enables shadow copy of NVRAM in RAM which is read from a file
> (if used) and used for reads. Writes to NVRAM are mirrored to the file.
> 
> This defines a VMSTATE descriptor for NVRAM device so the memory copy
> of NVRAM can migrate and be flushed to a backing file on the destination
> if one is specified.
> 
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

Re: [Qemu-devel] [PATCH 0/2] Here are 2 patches to enable sPAPR NVRAM migration.

[Alexander Graf]

On 02.10.14 11:56, Alexey Kardashevskiy wrote:
> Please comment. Thanks!
> 
> Changes:
> v2:
> * reworked to always have a copy of NVRAM in RAM and use file only for writes

I like it, please get an ack from Juan on patch 1/2, then I can apply it.


Alex

Re: [Qemu-devel] [PATCH 0/2] Here are 2 patches to enable sPAPR NVRAM migration.

[Alexander Graf]

On 02.10.14 11:56, Alexey Kardashevskiy wrote:
> Please comment. Thanks!
> 
> Changes:
> v2:
> * reworked to always have a copy of NVRAM in RAM and use file only for writes

Thanks, applied to ppc-next.


Alex