Xen project Mailing List

On Tue, Jan 31, 2012 at 10:58 AM, Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx> wrote:

On Tue, 31 Jan 2012, Shriram Rajagopalan wrote:
> Or if you have to receive them in the pagebuf, then have two pieces of the same state,
> consistent_state, curr_state, such that you do
>
> pagebuf_get(curr_state)
> tailbuf_get(..)..
> if (no error so far), then
> copy curr_state to consistent_state.
> goto copypages;
>
> finish:
> ..
> finish_hvm:
> apply consistent_state.

I think I am starting to understand: see appended patch if it makes
things better.

Instead of adding yet another parameter to pagebuf_get_one, you could
simplify the patch by adding a toolstack_data_t field to the pagebuf struct.
See comments below.

---

diff --git a/tools/libxc/xc_domain_restore.c b/tools/libxc/xc_domain_restore.c
index 3fda6f8..02b523d 100644
--- a/tools/libxc/xc_domain_restore.c
+++ b/tools/libxc/xc_domain_restore.c
@@ -684,6 +684,11 @@ typedef struct {
uint64_t vm_generationid_addr;
} pagebuf_t;

+struct toolstack_data_t {
+ uint8_t *data;

+ uint32_t len;
+};
+

Add an instance of this struct to the pagebuf_t structure
(gets initialized to NULL, by pagebuf_init)

and dont forget to add the appropriate free calls to pagebuf_free.

static int pagebuf_init(pagebuf_t* buf)
{
memset(buf, 0, sizeof(*buf));
@@ -703,7 +708,8 @@ static void pagebuf_free(pagebuf_t* buf)

}

static int pagebuf_get_one(xc_interface *xch, struct restore_ctx *ctx,

- pagebuf_t* buf, int fd, uint32_t dom)
+ pagebuf_t* buf, int fd, uint32_t dom,

+ struct toolstack_data_t *tdata)

Eliminate the extra arg. and all the fixes to pagebuf_get_one() calls.

+ {

+ case XC_SAVE_ID_TOOLSTACK:
+ RDEXACT(fd, &tdata->len, sizeof(tdata->len));
+ tdata->data = "" realloc(tdata->data, tdata->len);
+ if ( tdata->data == NULL )
+ {

+ PERROR("error memory allocation");

+ return -1;
+ }
+ RDEXACT(fd, tdata->data, tdata->len);
+ return pagebuf_get_one(xch, ctx, buf, fd, dom, tdata);

+ }

RDEXACT(fd, &buf->tdata->len, sizeof())
...

@@ -1262,7 +1282,8 @@ int xc_domain_restore(xc_interface *xch, int io_fd, uint32_t dom,

unsigned int console_evtchn, unsigned long *console_mfn,
unsigned int hvm, unsigned int pae, int superpages,
int no_incr_generationid,
- unsigned long *vm_generationid_addr)
+ unsigned long *vm_generationid_addr,
+ struct restore_callbacks *callbacks)
{
DECLARE_DOMCTL;
int rc = 1, frc, i, j, n, m, pae_extended_cr3 = 0, ext_vcpucontext = 0;

@@ -1310,6 +1331,7 @@ int xc_domain_restore(xc_interface *xch, int io_fd, uint32_t dom,

pagebuf_t pagebuf;
tailbuf_t tailbuf, tmptail;
+ struct toolstack_data_t tdata, tdata2, tdatatmp;

struct toolstack_data_t tdata, tdatatmp;

and the memsets ofcourse.

@@ -1441,7 +1465,7 @@ int xc_domain_restore(xc_interface *xch, int io_fd, uint32_t dom,

if ( !ctx->completed ) {
pagebuf.nr_physpages = pagebuf.nr_pages = 0;
pagebuf.compbuf_pos = pagebuf.compbuf_size = 0;
- if ( pagebuf_get_one(xch, ctx, &pagebuf, io_fd, dom) < 0 ) {

+ if ( pagebuf_get_one(xch, ctx, &pagebuf, io_fd, dom, &tdata) < 0 ) {

PERROR("Error when reading batch");
goto out;
}

Both live migration and Remus reach this piece of code, while applying
the 1st or Nth checkpoint (respectively)

if (ctx->last_checkpoint)
{
// DPRINTF("Last checkpoint, finishing\n");
goto finish;
}

// DPRINTF("Buffered checkpoint\n");

if ( pagebuf_get(xch, ctx, &pagebuf, io_fd, dom) ) {

So, your code could be plugged right on top of the first "if".

+ tdatatmp = tdata;
+ tdata = pagebuf.tdata;
+ pagebuf.tdata = tdatatmp;

if (ctx->last_checkpoint)
...

@@ -2023,6 +2050,20 @@ int xc_domain_restore(xc_interface *xch, int io_fd, uint32_t dom,
goto out;

finish_hvm:

+ if ( callbacks != NULL && callbacks->toolstack_restore != NULL )

how about another (tdata.data != NULL) ? If older versions of xen (with lets say older libxl toolstack)
were to migrate to xen 4.2, they wouldnt be sending the TOOLSTACK_ID would they?

+ {
+ if ( callbacks->toolstack_restore(dom, tdata.data, tdata.len,

+ callbacks->data) < 0 )
+ {

+ PERROR("error calling toolstack_restore");

+ free(tdata.data);
+ free(tdata2.data);

+ goto out;
+ }
+ }

+ free(tdata.data);
+ free(tdata2.data);

+

Add free(buf->tdata.data) to pagebuf_free and
just do free(tdata.data) here.

Re: [Xen-devel] [PATCH v3 1/2] libxc: introduce XC_SAVE_ID_TOOLSTACK