Andrew Cooper
2013-Oct-10 13:56 UTC
[Xen-4.3 backport] [Patch] tools/migrate: Fix regression when migrating from older version of Xen
Commit 00a4b65f8534c9e6521eab2e6ce796ae36037774 Sep 7 2010 "libxc: provide notification of final checkpoint to restore end" broke migration from any version of Xen using tools from prior to that commit Older tools have no idea about an XC_SAVE_ID_LAST_CHECKPOINT, causing newer tools xc_domain_restore() to start reading the qemu save record, as ctx->last_checkpoint is 0. The failure looks like: xc: error: Max batch size exceeded (1970103633). Giving up. where 1970103633 = 0x756d6551 = *(uint32_t*)"Qemu" With this fix in place, the behaviour for normal migrations is reverted to how it was before the regression; the migration is considered non-checkpointed right from the start. A XC_SAVE_ID_LAST_CHECKPOINT chunk seen in the migration stream is a nop. For checkpointed migrations the behaviour is unchanged. Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: Ian Campbell <Ian.Campbell@citrix.com> CC: Ian Jackson <Ian.Jackson@eu.citrix.com> Acked-by: Shriram Rajagopalan <rshriram@cs.ubc.ca> (Remus bits) master commit: 7051d5c872e3e708b2d4b2088215d6ab1b33de1b master date: Thu, 10 Oct 2013 11:28:21 +0000 Backport to Xen-4.3: As remus was not a supported feature in libxl in 4.3 and earlier, this backport hardcodes a non-checkpointed stream from libxl, removing the need to any playing around with the IDL and API. Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> CC: Ian Jackson <Ian.Jackson@eu.citrix.com> --- tools/libxc/xc_domain_restore.c | 3 ++- tools/libxc/xc_nomigrate.c | 2 +- tools/libxc/xenguest.h | 3 ++- tools/libxl/libxl_save_helper.c | 2 +- tools/python/xen/xend/XendCheckpoint.py | 2 +- tools/xcutils/xc_restore.c | 14 +++++++++----- 6 files changed, 16 insertions(+), 10 deletions(-) diff --git a/tools/libxc/xc_domain_restore.c b/tools/libxc/xc_domain_restore.c index 63d36cd..297546c 100644 --- a/tools/libxc/xc_domain_restore.c +++ b/tools/libxc/xc_domain_restore.c @@ -1401,7 +1401,7 @@ int xc_domain_restore(xc_interface *xch, int io_fd, uint32_t dom, domid_t store_domid, unsigned int console_evtchn, unsigned long *console_mfn, domid_t console_domid, unsigned int hvm, unsigned int pae, int superpages, - int no_incr_generationid, + int no_incr_generationid, int checkpointed_stream, unsigned long *vm_generationid_addr, struct restore_callbacks *callbacks) { @@ -1472,6 +1472,7 @@ int xc_domain_restore(xc_interface *xch, int io_fd, uint32_t dom, ctx->superpages = superpages; ctx->hvm = hvm; + ctx->last_checkpoint = !checkpointed_stream; ctxt = xc_hypercall_buffer_alloc(xch, ctxt, sizeof(*ctxt)); diff --git a/tools/libxc/xc_nomigrate.c b/tools/libxc/xc_nomigrate.c index 73e7566..fb6d53e 100644 --- a/tools/libxc/xc_nomigrate.c +++ b/tools/libxc/xc_nomigrate.c @@ -35,7 +35,7 @@ int xc_domain_restore(xc_interface *xch, int io_fd, uint32_t dom, domid_t store_domid, unsigned int console_evtchn, unsigned long *console_mfn, domid_t console_domid, unsigned int hvm, unsigned int pae, int superpages, - int no_incr_generationid, + int no_incr_generationid, int checkpointed_stream, unsigned long *vm_generationid_addr, struct restore_callbacks *callbacks) { diff --git a/tools/libxc/xenguest.h b/tools/libxc/xenguest.h index 4714bd2..2101810 100644 --- a/tools/libxc/xenguest.h +++ b/tools/libxc/xenguest.h @@ -114,6 +114,7 @@ struct restore_callbacks { * @parm pae non-zero if this HVM domain has PAE support enabled * @parm superpages non-zero to allocate guest memory with superpages * @parm no_incr_generationid non-zero if generation id is NOT to be incremented + * @parm checkpointed_stream non-zero if the far end of the stream is using checkpointing * @parm vm_generationid_addr returned with the address of the generation id buffer * @parm callbacks non-NULL to receive a callback to restore toolstack * specific data @@ -124,7 +125,7 @@ int xc_domain_restore(xc_interface *xch, int io_fd, uint32_t dom, domid_t store_domid, unsigned int console_evtchn, unsigned long *console_mfn, domid_t console_domid, unsigned int hvm, unsigned int pae, int superpages, - int no_incr_generationid, + int no_incr_generationid, int checkpointed_stream, unsigned long *vm_generationid_addr, struct restore_callbacks *callbacks); /** diff --git a/tools/libxl/libxl_save_helper.c b/tools/libxl/libxl_save_helper.c index 772251a..8f14b8b 100644 --- a/tools/libxl/libxl_save_helper.c +++ b/tools/libxl/libxl_save_helper.c @@ -264,7 +264,7 @@ int main(int argc, char **argv) r = xc_domain_restore(xch, io_fd, dom, store_evtchn, &store_mfn, store_domid, console_evtchn, &console_mfn, console_domid, hvm, pae, superpages, - no_incr_genidad, &genidad, + no_incr_genidad, 0, &genidad, &helper_restore_callbacks); helper_stub_restore_results(store_mfn,console_mfn,genidad,0); complete(r); diff --git a/tools/python/xen/xend/XendCheckpoint.py b/tools/python/xen/xend/XendCheckpoint.py index fa09757..a433ffa 100644 --- a/tools/python/xen/xend/XendCheckpoint.py +++ b/tools/python/xen/xend/XendCheckpoint.py @@ -301,7 +301,7 @@ def restore(xd, fd, dominfo = None, paused = False, relocating = False): cmd = map(str, [xen.util.auxbin.pathTo(XC_RESTORE), fd, dominfo.getDomid(), - store_port, console_port, int(is_hvm), pae, apic, superpages]) + store_port, console_port, int(is_hvm), pae, apic, superpages, 1]) log.debug("[xc_restore]: %s", string.join(cmd)) handler = RestoreInputHandler() diff --git a/tools/xcutils/xc_restore.c b/tools/xcutils/xc_restore.c index 35d725c..5ec90ac 100644 --- a/tools/xcutils/xc_restore.c +++ b/tools/xcutils/xc_restore.c @@ -19,7 +19,7 @@ int main(int argc, char **argv) { unsigned int domid, store_evtchn, console_evtchn; - unsigned int hvm, pae, apic, lflags; + unsigned int hvm, pae, apic, lflags, checkpointed; xc_interface *xch; int io_fd, ret; int superpages; @@ -27,9 +27,9 @@ main(int argc, char **argv) xentoollog_level lvl; xentoollog_logger *l; - if ( (argc != 8) && (argc != 9) ) + if ( !( argc >= 8 && argc <= 10) ) errx(1, "usage: %s iofd domid store_evtchn " - "console_evtchn hvm pae apic [superpages]", argv[0]); + "console_evtchn hvm pae apic [superpages [checkpointed]]", argv[0]); lvl = XTL_DETAIL; lflags = XTL_STDIOSTREAM_SHOW_PID | XTL_STDIOSTREAM_HIDE_PROGRESS; @@ -45,14 +45,18 @@ main(int argc, char **argv) hvm = atoi(argv[5]); pae = atoi(argv[6]); apic = atoi(argv[7]); - if ( argc == 9 ) + if ( argc >= 9 ) superpages = atoi(argv[8]); else superpages = !!hvm; + if ( argc >= 10 ) + checkpointed = atoi(argv[9]); + else + checkpointed = 0; ret = xc_domain_restore(xch, io_fd, domid, store_evtchn, &store_mfn, 0, console_evtchn, &console_mfn, 0, hvm, pae, superpages, - 0, NULL, NULL); + 0, checkpointed, NULL, NULL); if ( ret == 0 ) { -- 1.7.10.4
Ian Jackson
2013-Nov-12 15:21 UTC
Re: [Xen-4.3 backport] [Patch] tools/migrate: Fix regression when migrating from older version of Xen
Andrew Cooper writes ("[Xen-devel] [Xen-4.3 backport] [Patch] tools/migrate: Fix regression when migrating from older version of Xen"): ...> With this fix in place, the behaviour for normal migrations is > reverted to how it was before the regression; the migration is > considered non-checkpointed right from the start. A > XC_SAVE_ID_LAST_CHECKPOINT chunk seen in the migration stream is a > nop. For checkpointed migrations the behaviour is unchanged.This patch has been sitting in unstable master for a while now without apparent problems, and the backport looks plausible. So I have applied your backport to staging-4.3. I had a quick look at applying this to 4.2, too, but it didn''t apply cleanly. If you have a version for 4.2, I''d be happy to commit it. Thanks, Ian. (CC Jan with his stable maintainer hat on)
Ian Campbell
2013-Nov-13 09:29 UTC
Re: [Xen-4.3 backport] [Patch] tools/migrate: Fix regression when migrating from older version of Xen
On Tue, 2013-11-12 at 15:21 +0000, Ian Jackson wrote:> Andrew Cooper writes ("[Xen-devel] [Xen-4.3 backport] [Patch] tools/migrate: Fix regression when migrating from older version of Xen"): > ... > > With this fix in place, the behaviour for normal migrations is > > reverted to how it was before the regression; the migration is > > considered non-checkpointed right from the start. A > > XC_SAVE_ID_LAST_CHECKPOINT chunk seen in the migration stream is a > > nop. For checkpointed migrations the behaviour is unchanged. > > This patch has been sitting in unstable master for a while now without > apparent problems, and the backport looks plausible. So I have > applied your backport to staging-4.3. > > I had a quick look at applying this to 4.2, too, but it didn''t apply > cleanly. If you have a version for 4.2, I''d be happy to commit it.Does the issue existing between 4.2->4.3 migrations?
Andrew Cooper
2013-Nov-13 12:45 UTC
Re: [Xen-4.3 backport] [Patch] tools/migrate: Fix regression when migrating from older version of Xen
On 13/11/13 09:29, Ian Campbell wrote:> On Tue, 2013-11-12 at 15:21 +0000, Ian Jackson wrote: >> Andrew Cooper writes ("[Xen-devel] [Xen-4.3 backport] [Patch] tools/migrate: Fix regression when migrating from older version of Xen"): >> ... >>> With this fix in place, the behaviour for normal migrations is >>> reverted to how it was before the regression; the migration is >>> considered non-checkpointed right from the start. A >>> XC_SAVE_ID_LAST_CHECKPOINT chunk seen in the migration stream is a >>> nop. For checkpointed migrations the behaviour is unchanged. >> This patch has been sitting in unstable master for a while now without >> apparent problems, and the backport looks plausible. So I have >> applied your backport to staging-4.3. >> >> I had a quick look at applying this to 4.2, too, but it didn''t apply >> cleanly. If you have a version for 4.2, I''d be happy to commit it. > Does the issue existing between 4.2->4.3 migrations? > >The issue exists migrating across the changeset 00a4b65f8534c9e6521eab2e6ce796ae36037774 This is a supported upgrade configuration for XenServer. I will endeavour to provide a 4.2 backport when I have enough time to do so. ~Andrew
Ian Jackson
2013-Nov-14 11:47 UTC
Re: [Xen-4.3 backport] [Patch] tools/migrate: Fix regression when migrating from older version of Xen
Andrew Cooper writes ("Re: [Xen-devel] [Xen-4.3 backport] [Patch] tools/migrate: Fix regression when migrating from older version of Xen"):> On 13/11/13 09:29, Ian Campbell wrote: > > Does the issue existing between 4.2->4.3 migrations? > > The issue exists migrating across the changeset > 00a4b65f8534c9e6521eab2e6ce796ae36037774 > > This is a supported upgrade configuration for XenServer. > > I will endeavour to provide a 4.2 backport when I have enough time to do so.Thanks, Ian.
Andrew Cooper
2013-Nov-15 13:23 UTC
[Xen 4.2 backport] tools/migrate: Fix regression when migrating from older version of Xen
Commit 00a4b65f8534c9e6521eab2e6ce796ae36037774 Sep 7 2010 "libxc: provide notification of final checkpoint to restore end" broke migration from any version of Xen using tools from prior to that commit Older tools have no idea about an XC_SAVE_ID_LAST_CHECKPOINT, causing newer tools xc_domain_restore() to start reading the qemu save record, as ctx->last_checkpoint is 0. The failure looks like: xc: error: Max batch size exceeded (1970103633). Giving up. where 1970103633 = 0x756d6551 = *(uint32_t*)"Qemu" With this fix in place, the behaviour for normal migrations is reverted to how it was before the regression; the migration is considered non-checkpointed right from the start. A XC_SAVE_ID_LAST_CHECKPOINT chunk seen in the migration stream is a nop. For checkpointed migrations the behaviour is unchanged. Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: Ian Campbell <Ian.Campbell@citrix.com> CC: Ian Jackson <Ian.Jackson@eu.citrix.com> Acked-by: Shriram Rajagopalan <rshriram@cs.ubc.ca> (Remus bits) master commit: 7051d5c872e3e708b2d4b2088215d6ab1b33de1b master date: Thu, 10 Oct 2013 11:28:21 +0000 Backport to Xen-4.2: As remus was not a supported feature in libxl in 4.3 and earlier, this backport hardcodes a non-checkpointed stream from libxl, removing the need to any playing around with the IDL and API. Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> CC: Ian Jackson <Ian.Jackson@eu.citrix.com> --- tools/libxc/xc_domain_restore.c | 4 +++- tools/libxc/xc_nomigrate.c | 2 +- tools/libxc/xenguest.h | 3 ++- tools/libxl/libxl_save_helper.c | 2 +- tools/python/xen/xend/XendCheckpoint.py | 2 +- tools/xcutils/xc_restore.c | 14 +++++++++----- 6 files changed, 17 insertions(+), 10 deletions(-) diff --git a/tools/libxc/xc_domain_restore.c b/tools/libxc/xc_domain_restore.c index f9ed6b2..d59b043 100644 --- a/tools/libxc/xc_domain_restore.c +++ b/tools/libxc/xc_domain_restore.c @@ -1327,7 +1327,7 @@ int xc_domain_restore(xc_interface *xch, int io_fd, uint32_t dom, domid_t store_domid, unsigned int console_evtchn, unsigned long *console_mfn, domid_t console_domid, unsigned int hvm, unsigned int pae, int superpages, - int no_incr_generationid, + int no_incr_generationid, int checkpointed_stream, unsigned long *vm_generationid_addr, struct restore_callbacks *callbacks) { @@ -1394,6 +1394,8 @@ int xc_domain_restore(xc_interface *xch, int io_fd, uint32_t dom, memset(ctx, 0, sizeof(*ctx)); + ctx->last_checkpoint = !checkpointed_stream; + ctxt = xc_hypercall_buffer_alloc(xch, ctxt, sizeof(*ctxt)); if ( ctxt == NULL ) diff --git a/tools/libxc/xc_nomigrate.c b/tools/libxc/xc_nomigrate.c index 27680a4..c8df0b5 100644 --- a/tools/libxc/xc_nomigrate.c +++ b/tools/libxc/xc_nomigrate.c @@ -35,7 +35,7 @@ int xc_domain_restore(xc_interface *xch, int io_fd, uint32_t dom, domid_t store_domid, unsigned int console_evtchn, unsigned long *console_mfn, domid_t console_domid, unsigned int hvm, unsigned int pae, int superpages, - int no_incr_generationid, + int no_incr_generationid, int checkpointed_stream, unsigned long *vm_generationid_addr, struct restore_callbacks *callbacks) { diff --git a/tools/libxc/xenguest.h b/tools/libxc/xenguest.h index 707e31c..49cb228 100644 --- a/tools/libxc/xenguest.h +++ b/tools/libxc/xenguest.h @@ -111,6 +111,7 @@ struct restore_callbacks { * @parm pae non-zero if this HVM domain has PAE support enabled * @parm superpages non-zero to allocate guest memory with superpages * @parm no_incr_generationid non-zero if generation id is NOT to be incremented + * @parm checkpointed_stream non-zero if the far end of the stream is using checkpointing * @parm vm_generationid_addr returned with the address of the generation id buffer * @parm callbacks non-NULL to receive a callback to restore toolstack * specific data @@ -121,7 +122,7 @@ int xc_domain_restore(xc_interface *xch, int io_fd, uint32_t dom, domid_t store_domid, unsigned int console_evtchn, unsigned long *console_mfn, domid_t console_domid, unsigned int hvm, unsigned int pae, int superpages, - int no_incr_generationid, + int no_incr_generationid, int checkpointed_stream, unsigned long *vm_generationid_addr, struct restore_callbacks *callbacks); /** diff --git a/tools/libxl/libxl_save_helper.c b/tools/libxl/libxl_save_helper.c index 772251a..8f14b8b 100644 --- a/tools/libxl/libxl_save_helper.c +++ b/tools/libxl/libxl_save_helper.c @@ -264,7 +264,7 @@ int main(int argc, char **argv) r = xc_domain_restore(xch, io_fd, dom, store_evtchn, &store_mfn, store_domid, console_evtchn, &console_mfn, console_domid, hvm, pae, superpages, - no_incr_genidad, &genidad, + no_incr_genidad, 0, &genidad, &helper_restore_callbacks); helper_stub_restore_results(store_mfn,console_mfn,genidad,0); complete(r); diff --git a/tools/python/xen/xend/XendCheckpoint.py b/tools/python/xen/xend/XendCheckpoint.py index fa09757..a433ffa 100644 --- a/tools/python/xen/xend/XendCheckpoint.py +++ b/tools/python/xen/xend/XendCheckpoint.py @@ -301,7 +301,7 @@ def restore(xd, fd, dominfo = None, paused = False, relocating = False): cmd = map(str, [xen.util.auxbin.pathTo(XC_RESTORE), fd, dominfo.getDomid(), - store_port, console_port, int(is_hvm), pae, apic, superpages]) + store_port, console_port, int(is_hvm), pae, apic, superpages, 1]) log.debug("[xc_restore]: %s", string.join(cmd)) handler = RestoreInputHandler() diff --git a/tools/xcutils/xc_restore.c b/tools/xcutils/xc_restore.c index 0235579..6b10685 100644 --- a/tools/xcutils/xc_restore.c +++ b/tools/xcutils/xc_restore.c @@ -19,15 +19,15 @@ int main(int argc, char **argv) { unsigned int domid, store_evtchn, console_evtchn; - unsigned int hvm, pae, apic; + unsigned int hvm, pae, apic, checkpointed; xc_interface *xch; int io_fd, ret; int superpages; unsigned long store_mfn, console_mfn; - if ( (argc != 8) && (argc != 9) ) + if ( !( argc >= 8 && argc <= 10) ) errx(1, "usage: %s iofd domid store_evtchn " - "console_evtchn hvm pae apic [superpages]", argv[0]); + "console_evtchn hvm pae apic [superpages [checkpointed]]", argv[0]); xch = xc_interface_open(0,0,0); if ( !xch ) @@ -40,14 +40,18 @@ main(int argc, char **argv) hvm = atoi(argv[5]); pae = atoi(argv[6]); apic = atoi(argv[7]); - if ( argc == 9 ) + if ( argc >= 9 ) superpages = atoi(argv[8]); else superpages = !!hvm; + if ( argc >= 10 ) + checkpointed = atoi(argv[9]); + else + checkpointed = 0; ret = xc_domain_restore(xch, io_fd, domid, store_evtchn, &store_mfn, 0, console_evtchn, &console_mfn, 0, hvm, pae, superpages, - 0, NULL, NULL); + 0, checkpointed, NULL, NULL); if ( ret == 0 ) { -- 1.7.10.4