Re: [Qemu-devel] [RFC PATCH 07/17] COLO buffer: implement colo buffer as well as QEMUFileOps based on it

[Eric Blake <eblake@redhat.com>]

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

On 07/23/2014 08:25 AM, Yang Hongyang wrote:
> We need a buffer to store migration data.
> 
> On save side:
>   all saved data was write into colo buffer first, so that we can know

s/was write/is written/

> the total size of the migration data. this can also separate the data
> transmission from colo control data, we use colo control data over
> socket fd to synchronous both side's stat.
> 
> On restore side:
>   all migration data was read into colo buffer first, then load data
> from the buffer: If network error happens while data transmission,

s/while/during/

> the slaver can still functinal because the migration data are not yet

s/slaver/slave/
s/functinal/function/
s/are/is/

> loaded.
> 
> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
> ---
>  migration-colo.c | 112 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 112 insertions(+)
> 

> +/* colo buffer */
> +
> +#define COLO_BUFFER_BASE_SIZE (1000*1000*4ULL)
> +#define COLO_BUFFER_MAX_SIZE (1000*1000*1000*10ULL)

Spaces around binary operators.

> +
> +typedef struct colo_buffer {

For consistency with the rest of the code base, name this ColoBuffer,
not colo_buffer.

> +    uint8_t *data;
> +    uint64_t used;
> +    uint64_t freed;
> +    uint64_t size;
> +} colo_buffer_t;

HACKING says to NOT name types with a trailing _t.  Just name the
typedef ColoBuffer.


> +static void colo_buffer_destroy(void)
> +{
> +    if (colo_buffer.data) {
> +        g_free(colo_buffer.data);
> +        colo_buffer.data = NULL;

g_free(NULL) behaves sanely, just make these two lines unconditional.


> +static void colo_buffer_extend(uint64_t len)
> +{
> +    if (len > colo_buffer.size - colo_buffer.used) {
> +        len = len + colo_buffer.used - colo_buffer.size;
> +        len = ROUND_UP(len, COLO_BUFFER_BASE_SIZE) + COLO_BUFFER_BASE_SIZE;
> +
> +        colo_buffer.size += len;
> +        if (colo_buffer.size > COLO_BUFFER_MAX_SIZE) {
> +            error_report("colo_buffer overflow!\n");

No trailing \n in error_report().

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]