| Message ID | 1454045254-3711-13-git-send-email-wency@cn.fujitsu.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Jan 29, 2016 at 01:27:28PM +0800, Wen Congyang wrote: > In COLO mode, both VMs are running, and are considered in sync if the > visible network traffic is identical. After some time, they fall out of > sync. > > At this point, the two VMs have definitely diverged. Lets call the > primary dirty bitmap set A, while the secondary dirty bitmap set B. > > Sets A and B are different. > > Under normal migration, the page data for set A will be sent from the > primary to the secondary. > > However, the set difference B - A (the one in B but not in A, lets > call this C) is out-of-date on the secondary (with respect to the > primary) and will not be sent by the primary (to secondary), as it > was not memory dirtied by the primary. The secondary needs C page data > to reconstruct an exact copy of the primary at the checkpoint. > > The secondary cannot calculate C as it doesn't know A. Instead, the > secondary must send B to the primary, at which point the primary > calculates the union of A and B (lets call this D) which is all the > pages dirtied by both the primary and the secondary, and sends all page > data covered by D. > > In the general case, D is a superset of both A and B. Without the > backchannel dirty bitmap, a COLO checkpoint can't reconstruct a valid > copy of the primary. > > We transfer the dirty bitmap on libxc side, so we need to introduce back > channel to libxc. > > Note: it is different from the paper. We change the original design to > the current one, according to our following concerns: > 1. The original design needs extra memory on Secondary host. When there's > multiple backups on one host, the memory cost is high. > 2. The memory cache code will be another 1k+, it will make the review > more time consuming. > > Note: the back channel will be used in the patch > libxc/restore: send dirty pfn list to primary when checkpoint under COLO > to send dirty pfn list from secondary to primary. The patch is posted in > another series. > > Signed-off-by: Yang Hongyang <hongyang.yang@easystack.cn> > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> > CC: Ian Campbell <Ian.Campbell@citrix.com> > CC: Ian Jackson <Ian.Jackson@eu.citrix.com> > CC: Wei Liu <wei.liu2@citrix.com> It is a bit confusing to have 'back_fd' and then 'send_fd'. Could you change the 'send_fd' (in this patch) to be called 'send_back_fd' so that the connection between: tools/libxl: Add back channel to allow migration target send data back and this patch is clear? Or perhaps also add it in the commit description that you are using the 'send_fd' provided by ' tools/libxl: Add back channel to allow migration target send data back' Otherwise: Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
On 01/30/2016 12:38 AM, Konrad Rzeszutek Wilk wrote: > On Fri, Jan 29, 2016 at 01:27:28PM +0800, Wen Congyang wrote: >> In COLO mode, both VMs are running, and are considered in sync if the >> visible network traffic is identical. After some time, they fall out of >> sync. >> >> At this point, the two VMs have definitely diverged. Lets call the >> primary dirty bitmap set A, while the secondary dirty bitmap set B. >> >> Sets A and B are different. >> >> Under normal migration, the page data for set A will be sent from the >> primary to the secondary. >> >> However, the set difference B - A (the one in B but not in A, lets >> call this C) is out-of-date on the secondary (with respect to the >> primary) and will not be sent by the primary (to secondary), as it >> was not memory dirtied by the primary. The secondary needs C page data >> to reconstruct an exact copy of the primary at the checkpoint. >> >> The secondary cannot calculate C as it doesn't know A. Instead, the >> secondary must send B to the primary, at which point the primary >> calculates the union of A and B (lets call this D) which is all the >> pages dirtied by both the primary and the secondary, and sends all page >> data covered by D. >> >> In the general case, D is a superset of both A and B. Without the >> backchannel dirty bitmap, a COLO checkpoint can't reconstruct a valid >> copy of the primary. >> >> We transfer the dirty bitmap on libxc side, so we need to introduce back >> channel to libxc. >> >> Note: it is different from the paper. We change the original design to >> the current one, according to our following concerns: >> 1. The original design needs extra memory on Secondary host. When there's >> multiple backups on one host, the memory cost is high. >> 2. The memory cache code will be another 1k+, it will make the review >> more time consuming. >> >> Note: the back channel will be used in the patch >> libxc/restore: send dirty pfn list to primary when checkpoint under COLO >> to send dirty pfn list from secondary to primary. The patch is posted in >> another series. >> >> Signed-off-by: Yang Hongyang <hongyang.yang@easystack.cn> >> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> >> CC: Ian Campbell <Ian.Campbell@citrix.com> >> CC: Ian Jackson <Ian.Jackson@eu.citrix.com> >> CC: Wei Liu <wei.liu2@citrix.com> > > It is a bit confusing to have 'back_fd' and then 'send_fd'. > > Could you change the 'send_fd' (in this patch) to be called > 'send_back_fd' so that the connection between: > tools/libxl: Add back channel to allow migration target send data back > and this patch is clear? > > Or perhaps also add it in the commit description that you are using > the 'send_fd' provided by ' tools/libxl: Add back channel to allow migration target send data back' Before this series: In libxl: we have send_fd/recv_fd(libxl_domain_remus_start()), and only have restore_fd(libxl_domain_create_restore()) In libxc: We have io_fd(xc_domain_save()/xc_domain_restore()) The fd in libxc is provided by libxl. I think after this series, we can add the following fd: 1. add a send_back_fd in libxl_domain_create_restore() 2. add a recv_fd in xc_domain_save() 3. add a send_back_fd in xc_domain_restore() What about this? Thanks Wen Congyang > > Otherwise: Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> > > > . >
On Fri, Jan 29, 2016 at 01:27:28PM +0800, Wen Congyang wrote: > In COLO mode, both VMs are running, and are considered in sync if the > visible network traffic is identical. After some time, they fall out of > sync. > > At this point, the two VMs have definitely diverged. Lets call the > primary dirty bitmap set A, while the secondary dirty bitmap set B. > > Sets A and B are different. > > Under normal migration, the page data for set A will be sent from the > primary to the secondary. > > However, the set difference B - A (the one in B but not in A, lets > call this C) is out-of-date on the secondary (with respect to the > primary) and will not be sent by the primary (to secondary), as it > was not memory dirtied by the primary. The secondary needs C page data > to reconstruct an exact copy of the primary at the checkpoint. > > The secondary cannot calculate C as it doesn't know A. Instead, the > secondary must send B to the primary, at which point the primary > calculates the union of A and B (lets call this D) which is all the > pages dirtied by both the primary and the secondary, and sends all page > data covered by D. > > In the general case, D is a superset of both A and B. Without the > backchannel dirty bitmap, a COLO checkpoint can't reconstruct a valid > copy of the primary. > > We transfer the dirty bitmap on libxc side, so we need to introduce back > channel to libxc. > > Note: it is different from the paper. We change the original design to > the current one, according to our following concerns: > 1. The original design needs extra memory on Secondary host. When there's > multiple backups on one host, the memory cost is high. > 2. The memory cache code will be another 1k+, it will make the review > more time consuming. > > Note: the back channel will be used in the patch "will not be used" ? I don't see any read / write to the newly introduced fd. > libxc/restore: send dirty pfn list to primary when checkpoint under COLO > to send dirty pfn list from secondary to primary. The patch is posted in > another series. > > Signed-off-by: Yang Hongyang <hongyang.yang@easystack.cn> > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> > CC: Ian Campbell <Ian.Campbell@citrix.com> > CC: Ian Jackson <Ian.Jackson@eu.citrix.com> > CC: Wei Liu <wei.liu2@citrix.com> > --- [...] > > /*----- helper execution -----*/ > +static int dup_fd_helper(libxl__gc *gc, int fd, const char *what) > +{ > + int dup_fd = fd; > + > + if (fd <= 2) { > + dup_fd = dup(fd); > + if (dup_fd < 0) { > + LOGE(ERROR,"dup %s", what); > + exit(-1); > + } > + } > + libxl_fd_set_cloexec(CTX, dup_fd, 0); > + > + return dup_fd; > +} > It would be better if introduction of this helper to be separated into a different patch.
On 02/04/2016 03:40 AM, Wei Liu wrote: > On Fri, Jan 29, 2016 at 01:27:28PM +0800, Wen Congyang wrote: >> In COLO mode, both VMs are running, and are considered in sync if the >> visible network traffic is identical. After some time, they fall out of >> sync. >> >> At this point, the two VMs have definitely diverged. Lets call the >> primary dirty bitmap set A, while the secondary dirty bitmap set B. >> >> Sets A and B are different. >> >> Under normal migration, the page data for set A will be sent from the >> primary to the secondary. >> >> However, the set difference B - A (the one in B but not in A, lets >> call this C) is out-of-date on the secondary (with respect to the >> primary) and will not be sent by the primary (to secondary), as it >> was not memory dirtied by the primary. The secondary needs C page data >> to reconstruct an exact copy of the primary at the checkpoint. >> >> The secondary cannot calculate C as it doesn't know A. Instead, the >> secondary must send B to the primary, at which point the primary >> calculates the union of A and B (lets call this D) which is all the >> pages dirtied by both the primary and the secondary, and sends all page >> data covered by D. >> >> In the general case, D is a superset of both A and B. Without the >> backchannel dirty bitmap, a COLO checkpoint can't reconstruct a valid >> copy of the primary. >> >> We transfer the dirty bitmap on libxc side, so we need to introduce back >> channel to libxc. >> >> Note: it is different from the paper. We change the original design to >> the current one, according to our following concerns: >> 1. The original design needs extra memory on Secondary host. When there's >> multiple backups on one host, the memory cost is high. >> 2. The memory cache code will be another 1k+, it will make the review >> more time consuming. >> >> Note: the back channel will be used in the patch > > "will not be used" ? > > I don't see any read / write to the newly introduced fd. It is used in COLO series. Some patches in this series just introduce an API. Thess APIs will be used in COLO series. Do you mean that these patches should be put in COLO series? If so, I will check all patches. > >> libxc/restore: send dirty pfn list to primary when checkpoint under COLO >> to send dirty pfn list from secondary to primary. The patch is posted in >> another series. >> >> Signed-off-by: Yang Hongyang <hongyang.yang@easystack.cn> >> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> >> CC: Ian Campbell <Ian.Campbell@citrix.com> >> CC: Ian Jackson <Ian.Jackson@eu.citrix.com> >> CC: Wei Liu <wei.liu2@citrix.com> >> --- > [...] >> >> /*----- helper execution -----*/ >> +static int dup_fd_helper(libxl__gc *gc, int fd, const char *what) >> +{ >> + int dup_fd = fd; >> + >> + if (fd <= 2) { >> + dup_fd = dup(fd); >> + if (dup_fd < 0) { >> + LOGE(ERROR,"dup %s", what); >> + exit(-1); >> + } >> + } >> + libxl_fd_set_cloexec(CTX, dup_fd, 0); >> + >> + return dup_fd; >> +} >> > > It would be better if introduction of this helper to be separated into a > different patch. OK, will fix it in the next version. Thanks Wen Congyang > > > . >
On Thu, Feb 04, 2016 at 01:28:14PM +0800, Wen Congyang wrote: > On 02/04/2016 03:40 AM, Wei Liu wrote: > > On Fri, Jan 29, 2016 at 01:27:28PM +0800, Wen Congyang wrote: > >> In COLO mode, both VMs are running, and are considered in sync if the > >> visible network traffic is identical. After some time, they fall out of > >> sync. > >> > >> At this point, the two VMs have definitely diverged. Lets call the > >> primary dirty bitmap set A, while the secondary dirty bitmap set B. > >> > >> Sets A and B are different. > >> > >> Under normal migration, the page data for set A will be sent from the > >> primary to the secondary. > >> > >> However, the set difference B - A (the one in B but not in A, lets > >> call this C) is out-of-date on the secondary (with respect to the > >> primary) and will not be sent by the primary (to secondary), as it > >> was not memory dirtied by the primary. The secondary needs C page data > >> to reconstruct an exact copy of the primary at the checkpoint. > >> > >> The secondary cannot calculate C as it doesn't know A. Instead, the > >> secondary must send B to the primary, at which point the primary > >> calculates the union of A and B (lets call this D) which is all the > >> pages dirtied by both the primary and the secondary, and sends all page > >> data covered by D. > >> > >> In the general case, D is a superset of both A and B. Without the > >> backchannel dirty bitmap, a COLO checkpoint can't reconstruct a valid > >> copy of the primary. > >> > >> We transfer the dirty bitmap on libxc side, so we need to introduce back > >> channel to libxc. > >> > >> Note: it is different from the paper. We change the original design to > >> the current one, according to our following concerns: > >> 1. The original design needs extra memory on Secondary host. When there's > >> multiple backups on one host, the memory cost is high. > >> 2. The memory cache code will be another 1k+, it will make the review > >> more time consuming. > >> > >> Note: the back channel will be used in the patch > > > > "will not be used" ? > > > > I don't see any read / write to the newly introduced fd. > > It is used in COLO series. > > Some patches in this series just introduce an API. Thess APIs will be used > in COLO series. Do you mean that these patches should be put in COLO series? > If so, I will check all patches. > Fine by me. Wei.
diff --git a/tools/libxc/include/xenguest.h b/tools/libxc/include/xenguest.h index affc42b..ff230a4 100644 --- a/tools/libxc/include/xenguest.h +++ b/tools/libxc/include/xenguest.h @@ -88,7 +88,7 @@ struct save_callbacks { int xc_domain_save(xc_interface *xch, int io_fd, uint32_t dom, uint32_t max_iters, uint32_t max_factor, uint32_t flags /* XCFLAGS_xxx */, struct save_callbacks* callbacks, int hvm, - int checkpointed_stream); + int checkpointed_stream, int back_fd); /* callbacks provided by xc_domain_restore */ struct restore_callbacks { @@ -127,7 +127,7 @@ int xc_domain_restore(xc_interface *xch, int io_fd, uint32_t dom, unsigned long *console_mfn, domid_t console_domid, unsigned int hvm, unsigned int pae, int superpages, int checkpointed_stream, - struct restore_callbacks *callbacks); + struct restore_callbacks *callbacks, int back_fd); /** * This function will create a domain for a paravirtualized Linux diff --git a/tools/libxc/xc_nomigrate.c b/tools/libxc/xc_nomigrate.c index c9124df..089f767 100644 --- a/tools/libxc/xc_nomigrate.c +++ b/tools/libxc/xc_nomigrate.c @@ -23,7 +23,7 @@ int xc_domain_save(xc_interface *xch, int io_fd, uint32_t dom, uint32_t max_iters, uint32_t max_factor, uint32_t flags, struct save_callbacks* callbacks, int hvm, - int checkpointed_stream) + int checkpointed_stream, int back_fd) { errno = ENOSYS; return -1; @@ -35,7 +35,7 @@ int xc_domain_restore(xc_interface *xch, int io_fd, uint32_t dom, unsigned long *console_mfn, domid_t console_domid, unsigned int hvm, unsigned int pae, int superpages, int checkpointed_stream, - struct restore_callbacks *callbacks) + struct restore_callbacks *callbacks, int back_fd) { errno = ENOSYS; return -1; diff --git a/tools/libxc/xc_sr_restore.c b/tools/libxc/xc_sr_restore.c index d4d33fd..b0f47b5 100644 --- a/tools/libxc/xc_sr_restore.c +++ b/tools/libxc/xc_sr_restore.c @@ -726,7 +726,7 @@ int xc_domain_restore(xc_interface *xch, int io_fd, uint32_t dom, unsigned long *console_gfn, domid_t console_domid, unsigned int hvm, unsigned int pae, int superpages, int checkpointed_stream, - struct restore_callbacks *callbacks) + struct restore_callbacks *callbacks, int back_fd) { struct xc_sr_context ctx = { diff --git a/tools/libxc/xc_sr_save.c b/tools/libxc/xc_sr_save.c index 0bea97e..2cc5b45 100644 --- a/tools/libxc/xc_sr_save.c +++ b/tools/libxc/xc_sr_save.c @@ -831,7 +831,7 @@ static int save(struct xc_sr_context *ctx, uint16_t guest_type) int xc_domain_save(xc_interface *xch, int io_fd, uint32_t dom, uint32_t max_iters, uint32_t max_factor, uint32_t flags, struct save_callbacks* callbacks, int hvm, - int checkpointed_stream) + int checkpointed_stream, int back_fd) { struct xc_sr_context ctx = { diff --git a/tools/libxl/libxl_save_callout.c b/tools/libxl/libxl_save_callout.c index 416b318..631e3e2 100644 --- a/tools/libxl/libxl_save_callout.c +++ b/tools/libxl/libxl_save_callout.c @@ -27,7 +27,7 @@ */ static void run_helper(libxl__egc *egc, libxl__save_helper_state *shs, const char *mode_arg, - int stream_fd, + int stream_fd, int back_fd, const int *preserve_fds, int num_preserve_fds, const unsigned long *argnums, int num_argnums); @@ -50,6 +50,7 @@ void libxl__xc_domain_restore(libxl__egc *egc, libxl__domain_create_state *dcs, /* Convenience aliases */ const uint32_t domid = dcs->guest_domid; const int restore_fd = dcs->libxc_fd; + const int send_fd = dcs->send_fd; libxl__domain_build_state *const state = &dcs->build_state; unsigned cbflags = @@ -71,7 +72,7 @@ void libxl__xc_domain_restore(libxl__egc *egc, libxl__domain_create_state *dcs, shs->caller_state = dcs; shs->need_results = 1; - run_helper(egc, shs, "--restore-domain", restore_fd, 0, 0, + run_helper(egc, shs, "--restore-domain", restore_fd, send_fd, 0, 0, argnums, ARRAY_SIZE(argnums)); } @@ -95,7 +96,7 @@ void libxl__xc_domain_save(libxl__egc *egc, libxl__domain_save_state *dss, shs->caller_state = dss; shs->need_results = 0; - run_helper(egc, shs, "--save-domain", dss->fd, + run_helper(egc, shs, "--save-domain", dss->fd, dss->recv_fd, NULL, 0, argnums, ARRAY_SIZE(argnums)); return; @@ -118,14 +119,29 @@ void libxl__save_helper_init(libxl__save_helper_state *shs) } /*----- helper execution -----*/ +static int dup_fd_helper(libxl__gc *gc, int fd, const char *what) +{ + int dup_fd = fd; + + if (fd <= 2) { + dup_fd = dup(fd); + if (dup_fd < 0) { + LOGE(ERROR,"dup %s", what); + exit(-1); + } + } + libxl_fd_set_cloexec(CTX, dup_fd, 0); + + return dup_fd; +} static void run_helper(libxl__egc *egc, libxl__save_helper_state *shs, - const char *mode_arg, int stream_fd, + const char *mode_arg, int stream_fd, int back_fd, const int *preserve_fds, int num_preserve_fds, const unsigned long *argnums, int num_argnums) { STATE_AO_GC(shs->ao); - const char *args[4 + num_argnums]; + const char *args[5 + num_argnums]; const char **arg = args; int i, rc; @@ -153,6 +169,7 @@ static void run_helper(libxl__egc *egc, libxl__save_helper_state *shs, *arg++ = getenv("LIBXL_SAVE_HELPER") ?: LIBEXEC_BIN "/" "libxl-save-helper"; *arg++ = mode_arg; const char **stream_fd_arg = arg++; + const char **back_fd_arg = arg++; for (i=0; i<num_argnums; i++) *arg++ = GCSPRINTF("%lu", argnums[i]); *arg++ = 0; @@ -177,16 +194,12 @@ static void run_helper(libxl__egc *egc, libxl__save_helper_state *shs, pid_t pid = libxl__ev_child_fork(gc, &shs->child, helper_exited); if (!pid) { - if (stream_fd <= 2) { - stream_fd = dup(stream_fd); - if (stream_fd < 0) { - LOGE(ERROR,"dup migration stream fd"); - exit(-1); - } - } - libxl_fd_set_cloexec(CTX, stream_fd, 0); + stream_fd = dup_fd_helper(gc, stream_fd, "migration stream fd"); *stream_fd_arg = GCSPRINTF("%d", stream_fd); + back_fd = dup_fd_helper(gc, back_fd, "migration back channel fd"); + *back_fd_arg = GCSPRINTF("%d", back_fd); + for (i=0; i<num_preserve_fds; i++) if (preserve_fds[i] >= 0) { assert(preserve_fds[i] > 2); diff --git a/tools/libxl/libxl_save_helper.c b/tools/libxl/libxl_save_helper.c index 6bdcf13..9bdcf41 100644 --- a/tools/libxl/libxl_save_helper.c +++ b/tools/libxl/libxl_save_helper.c @@ -238,6 +238,7 @@ static struct restore_callbacks helper_restore_callbacks; int main(int argc, char **argv) { int r; + int back_fd; #define NEXTARG (++argv, assert(*argv), *argv) @@ -247,6 +248,7 @@ int main(int argc, char **argv) if (!strcmp(mode,"--save-domain")) { io_fd = atoi(NEXTARG); + back_fd = atoi(NEXTARG); uint32_t dom = strtoul(NEXTARG,0,10); uint32_t max_iters = strtoul(NEXTARG,0,10); uint32_t max_factor = strtoul(NEXTARG,0,10); @@ -262,12 +264,14 @@ int main(int argc, char **argv) setup_signals(save_signal_handler); r = xc_domain_save(xch, io_fd, dom, max_iters, max_factor, flags, - &helper_save_callbacks, hvm, checkpointed_stream); + &helper_save_callbacks, hvm, checkpointed_stream, + back_fd); complete(r); } else if (!strcmp(mode,"--restore-domain")) { io_fd = atoi(NEXTARG); + back_fd = atoi(NEXTARG); uint32_t dom = strtoul(NEXTARG,0,10); unsigned store_evtchn = strtoul(NEXTARG,0,10); domid_t store_domid = strtoul(NEXTARG,0,10); @@ -292,7 +296,7 @@ int main(int argc, char **argv) store_domid, console_evtchn, &console_mfn, console_domid, hvm, pae, superpages, checkpointed, - &helper_restore_callbacks); + &helper_restore_callbacks, back_fd); helper_stub_restore_results(store_mfn,console_mfn,0); complete(r);