* [PATCHv2 0/6] tools: rework VM Generation ID
@ 2014-05-27 17:31 David Vrabel
2014-05-27 17:31 ` [PATCH 1/6] docs: update docs for the ~/platform/generation-id key David Vrabel
` (5 more replies)
0 siblings, 6 replies; 16+ messages in thread
From: David Vrabel @ 2014-05-27 17:31 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson, Ian Campbell, Stefano Stabellini
This series reworks the VM Generation ID to a) conform to the
published spec from Microsoft; b) simplify the save/restore code; and
c) extend the libxl API to allow toolstacks to use this feature.
The VM Generation ID must be regenerated with a new random ID after
certain VM operations. Since xl lacks infrastructure for tracking the
life-cycle of snapshots and clones (etc), the safe option of always
using a new generation ID is used.
You can download the spec from:
http://www.microsoft.com/en-us/download/details.aspx?id=30707
Changes in v2:
- Use libxl_uuid for the generation ID.
- Add "generation_id" option to xl domain configuration file and use
this to set a random generation ID every time.
David
^ permalink raw reply [flat|nested] 16+ messages in thread* [PATCH 1/6] docs: update docs for the ~/platform/generation-id key 2014-05-27 17:31 [PATCHv2 0/6] tools: rework VM Generation ID David Vrabel @ 2014-05-27 17:31 ` David Vrabel 2014-05-28 14:44 ` Ian Campbell 2014-05-27 17:31 ` [PATCH 2/6] hvm: add HVM_PARAM_VM_GENERATION_ID_ADDR David Vrabel ` (4 subsequent siblings) 5 siblings, 1 reply; 16+ messages in thread From: David Vrabel @ 2014-05-27 17:31 UTC (permalink / raw) To: xen-devel; +Cc: David Vrabel, Ian Jackson, Ian Campbell, Stefano Stabellini Signed-off-by: David Vrabel <david.vrabel@citrix.com> --- docs/misc/xenstore-paths.markdown | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/docs/misc/xenstore-paths.markdown b/docs/misc/xenstore-paths.markdown index 70ab7f4..41d7d6d 100644 --- a/docs/misc/xenstore-paths.markdown +++ b/docs/misc/xenstore-paths.markdown @@ -166,11 +166,6 @@ use the xenstore-based protocol instead (see ~/control/shutdown, below) even if the guest has advertised support for the event channel protocol. -#### ~/hvmloader/generation-id-address = ADDRESS [r,HVM,INTERNAL] - -The hexadecimal representation of the address of the domain's -"generation id". - #### ~/hvmloader/allow-memory-relocate = ("1"|"0") [HVM,INTERNAL] If the default low MMIO hole (below 4GiB) is not big enough for all @@ -193,9 +188,22 @@ Various platform properties. #### ~/platform/generation-id = INTEGER ":" INTEGER [HVM,INTERNAL] -Two 64 bit values that represent the Windows Generation ID. -Is used by the BIOS initializer to get this value. -If not present or "0:0" (all zeroes) device will not be present to the machine. +The upper and lower 64-bit words of the 128-bit VM Generation ID. + +This key is used by hvmloader to create the ACPI VM Generation ID +device. It initialises a 16 octet region of guest memory with this +value. The guest physical address of this region is saved in the +HVM_PARAM_VM_GENERATION_ID_ADDR HVM parameter. + +If this key is not present, is empty, or is all-zeros ("0:0") then the +ACPI device is not created. + +The toolstack should, before unpausing a created or restored HVM +domain, set this key and write the same ID to the guest memory +location in HVM_PARAM_VM_GENERATION_ID_ADDR (if this address is +non-zero). + +See Microsoft's "Virtual Machine Generation ID" specification. ### Frontend device paths -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 1/6] docs: update docs for the ~/platform/generation-id key 2014-05-27 17:31 ` [PATCH 1/6] docs: update docs for the ~/platform/generation-id key David Vrabel @ 2014-05-28 14:44 ` Ian Campbell 2014-05-28 14:51 ` Andrew Cooper 0 siblings, 1 reply; 16+ messages in thread From: Ian Campbell @ 2014-05-28 14:44 UTC (permalink / raw) To: David Vrabel; +Cc: xen-devel, Ian Jackson, Stefano Stabellini On Tue, 2014-05-27 at 18:31 +0100, David Vrabel wrote: > Signed-off-by: David Vrabel <david.vrabel@citrix.com> > --- > docs/misc/xenstore-paths.markdown | 24 ++++++++++++++++-------- > 1 file changed, 16 insertions(+), 8 deletions(-) > > diff --git a/docs/misc/xenstore-paths.markdown b/docs/misc/xenstore-paths.markdown > index 70ab7f4..41d7d6d 100644 > --- a/docs/misc/xenstore-paths.markdown > +++ b/docs/misc/xenstore-paths.markdown > @@ -166,11 +166,6 @@ use the xenstore-based protocol instead (see ~/control/shutdown, > below) even if the guest has advertised support for the event channel > protocol. > > -#### ~/hvmloader/generation-id-address = ADDRESS [r,HVM,INTERNAL] > - > -The hexadecimal representation of the address of the domain's > -"generation id". > - > #### ~/hvmloader/allow-memory-relocate = ("1"|"0") [HVM,INTERNAL] > > If the default low MMIO hole (below 4GiB) is not big enough for all > @@ -193,9 +188,22 @@ Various platform properties. > > #### ~/platform/generation-id = INTEGER ":" INTEGER [HVM,INTERNAL] > > -Two 64 bit values that represent the Windows Generation ID. > -Is used by the BIOS initializer to get this value. > -If not present or "0:0" (all zeroes) device will not be present to the machine. > +The upper and lower 64-bit words of the 128-bit VM Generation ID. > + > +This key is used by hvmloader to create the ACPI VM Generation ID > +device. It initialises a 16 octet region of guest memory with this > +value. The guest physical address of this region is saved in the > +HVM_PARAM_VM_GENERATION_ID_ADDR HVM parameter. > + > +If this key is not present, is empty, or is all-zeros ("0:0") then the > +ACPI device is not created. > + > +The toolstack should, before unpausing a created or restored HVM > +domain, set this key and write the same ID to the guest memory > +location in HVM_PARAM_VM_GENERATION_ID_ADDR (if this address is > +non-zero). Is the toolstack or hvmloader now responsible for writing this value to guest memory? It seems like it might be a shared responsibility via some mode of cooperation that I'm not following? Ian. > + > +See Microsoft's "Virtual Machine Generation ID" specification. > > ### Frontend device paths > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/6] docs: update docs for the ~/platform/generation-id key 2014-05-28 14:44 ` Ian Campbell @ 2014-05-28 14:51 ` Andrew Cooper 2014-05-28 14:58 ` Ian Campbell 0 siblings, 1 reply; 16+ messages in thread From: Andrew Cooper @ 2014-05-28 14:51 UTC (permalink / raw) To: Ian Campbell; +Cc: xen-devel, Ian Jackson, David Vrabel, Stefano Stabellini On 28/05/14 15:44, Ian Campbell wrote: > On Tue, 2014-05-27 at 18:31 +0100, David Vrabel wrote: >> Signed-off-by: David Vrabel <david.vrabel@citrix.com> >> --- >> docs/misc/xenstore-paths.markdown | 24 ++++++++++++++++-------- >> 1 file changed, 16 insertions(+), 8 deletions(-) >> >> diff --git a/docs/misc/xenstore-paths.markdown b/docs/misc/xenstore-paths.markdown >> index 70ab7f4..41d7d6d 100644 >> --- a/docs/misc/xenstore-paths.markdown >> +++ b/docs/misc/xenstore-paths.markdown >> @@ -166,11 +166,6 @@ use the xenstore-based protocol instead (see ~/control/shutdown, >> below) even if the guest has advertised support for the event channel >> protocol. >> >> -#### ~/hvmloader/generation-id-address = ADDRESS [r,HVM,INTERNAL] >> - >> -The hexadecimal representation of the address of the domain's >> -"generation id". >> - >> #### ~/hvmloader/allow-memory-relocate = ("1"|"0") [HVM,INTERNAL] >> >> If the default low MMIO hole (below 4GiB) is not big enough for all >> @@ -193,9 +188,22 @@ Various platform properties. >> >> #### ~/platform/generation-id = INTEGER ":" INTEGER [HVM,INTERNAL] >> >> -Two 64 bit values that represent the Windows Generation ID. >> -Is used by the BIOS initializer to get this value. >> -If not present or "0:0" (all zeroes) device will not be present to the machine. >> +The upper and lower 64-bit words of the 128-bit VM Generation ID. >> + >> +This key is used by hvmloader to create the ACPI VM Generation ID >> +device. It initialises a 16 octet region of guest memory with this >> +value. The guest physical address of this region is saved in the >> +HVM_PARAM_VM_GENERATION_ID_ADDR HVM parameter. >> + >> +If this key is not present, is empty, or is all-zeros ("0:0") then the >> +ACPI device is not created. >> + >> +The toolstack should, before unpausing a created or restored HVM >> +domain, set this key and write the same ID to the guest memory >> +location in HVM_PARAM_VM_GENERATION_ID_ADDR (if this address is >> +non-zero). > Is the toolstack or hvmloader now responsible for writing this value to > guest memory? It seems like it might be a shared responsibility via some > mode of cooperation that I'm not following? > > Ian. HVMLoader is responsible for allocating the space inside the guest to start with, and for populating it with the initial value. However, HVMLoader is *not* rerun on suspend/resume, so something external in the toolstack needs to be able to update the value when needed. ~Andrew ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/6] docs: update docs for the ~/platform/generation-id key 2014-05-28 14:51 ` Andrew Cooper @ 2014-05-28 14:58 ` Ian Campbell 0 siblings, 0 replies; 16+ messages in thread From: Ian Campbell @ 2014-05-28 14:58 UTC (permalink / raw) To: Andrew Cooper; +Cc: xen-devel, Ian Jackson, David Vrabel, Stefano Stabellini On Wed, 2014-05-28 at 15:51 +0100, Andrew Cooper wrote: > On 28/05/14 15:44, Ian Campbell wrote: > > On Tue, 2014-05-27 at 18:31 +0100, David Vrabel wrote: > >> Signed-off-by: David Vrabel <david.vrabel@citrix.com> > >> --- > >> docs/misc/xenstore-paths.markdown | 24 ++++++++++++++++-------- > >> 1 file changed, 16 insertions(+), 8 deletions(-) > >> > >> diff --git a/docs/misc/xenstore-paths.markdown b/docs/misc/xenstore-paths.markdown > >> index 70ab7f4..41d7d6d 100644 > >> --- a/docs/misc/xenstore-paths.markdown > >> +++ b/docs/misc/xenstore-paths.markdown > >> @@ -166,11 +166,6 @@ use the xenstore-based protocol instead (see ~/control/shutdown, > >> below) even if the guest has advertised support for the event channel > >> protocol. > >> > >> -#### ~/hvmloader/generation-id-address = ADDRESS [r,HVM,INTERNAL] > >> - > >> -The hexadecimal representation of the address of the domain's > >> -"generation id". > >> - > >> #### ~/hvmloader/allow-memory-relocate = ("1"|"0") [HVM,INTERNAL] > >> > >> If the default low MMIO hole (below 4GiB) is not big enough for all > >> @@ -193,9 +188,22 @@ Various platform properties. > >> > >> #### ~/platform/generation-id = INTEGER ":" INTEGER [HVM,INTERNAL] > >> > >> -Two 64 bit values that represent the Windows Generation ID. > >> -Is used by the BIOS initializer to get this value. > >> -If not present or "0:0" (all zeroes) device will not be present to the machine. > >> +The upper and lower 64-bit words of the 128-bit VM Generation ID. > >> + > >> +This key is used by hvmloader to create the ACPI VM Generation ID > >> +device. It initialises a 16 octet region of guest memory with this > >> +value. The guest physical address of this region is saved in the > >> +HVM_PARAM_VM_GENERATION_ID_ADDR HVM parameter. > >> + > >> +If this key is not present, is empty, or is all-zeros ("0:0") then the > >> +ACPI device is not created. > >> + > >> +The toolstack should, before unpausing a created or restored HVM > >> +domain, set this key and write the same ID to the guest memory > >> +location in HVM_PARAM_VM_GENERATION_ID_ADDR (if this address is > >> +non-zero). > > Is the toolstack or hvmloader now responsible for writing this value to > > guest memory? It seems like it might be a shared responsibility via some > > mode of cooperation that I'm not following? > > > > Ian. > > HVMLoader is responsible for allocating the space inside the guest to > start with, and for populating it with the initial value. > > However, HVMLoader is *not* rerun on suspend/resume, so something > external in the toolstack needs to be able to update the value when needed. The new docs say "before unpausing a created", which is misleading then. Ian. ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 2/6] hvm: add HVM_PARAM_VM_GENERATION_ID_ADDR 2014-05-27 17:31 [PATCHv2 0/6] tools: rework VM Generation ID David Vrabel 2014-05-27 17:31 ` [PATCH 1/6] docs: update docs for the ~/platform/generation-id key David Vrabel @ 2014-05-27 17:31 ` David Vrabel 2014-05-27 17:31 ` [PATCH 3/6] tools/hvmloader: add helper functions to get/set HVM params David Vrabel ` (3 subsequent siblings) 5 siblings, 0 replies; 16+ messages in thread From: David Vrabel @ 2014-05-27 17:31 UTC (permalink / raw) To: xen-devel; +Cc: David Vrabel, Ian Jackson, Ian Campbell, Stefano Stabellini HVM_PARAM_VM_GENERATION_ID_ADDR is the guest physical address of the VM Generation ID. This parameter will be written by hvmloader and read by the toolstack when updating the gen. ID (e.g., after restoring from a snapshot). A HVM parameter is easier for the save/restore code to work with (than a XenStore key). Signed-off-by: David Vrabel <david.vrabel@citrix.com> --- xen/include/public/hvm/params.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/xen/include/public/hvm/params.h b/xen/include/public/hvm/params.h index 517a184..4d7c692 100644 --- a/xen/include/public/hvm/params.h +++ b/xen/include/public/hvm/params.h @@ -145,6 +145,9 @@ /* SHUTDOWN_* action in case of a triple fault */ #define HVM_PARAM_TRIPLE_FAULT_REASON 31 -#define HVM_NR_PARAMS 32 +/* Location of the VM Generation ID in guest physical address space. */ +#define HVM_PARAM_VM_GENERATION_ID_ADDR 32 + +#define HVM_NR_PARAMS 33 #endif /* __XEN_PUBLIC_HVM_PARAMS_H__ */ -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 3/6] tools/hvmloader: add helper functions to get/set HVM params 2014-05-27 17:31 [PATCHv2 0/6] tools: rework VM Generation ID David Vrabel 2014-05-27 17:31 ` [PATCH 1/6] docs: update docs for the ~/platform/generation-id key David Vrabel 2014-05-27 17:31 ` [PATCH 2/6] hvm: add HVM_PARAM_VM_GENERATION_ID_ADDR David Vrabel @ 2014-05-27 17:31 ` David Vrabel 2014-05-27 17:31 ` [PATCH 4/6] libxc, libxl, hvmloader: strip out outdated VM generation ID implementation David Vrabel ` (2 subsequent siblings) 5 siblings, 0 replies; 16+ messages in thread From: David Vrabel @ 2014-05-27 17:31 UTC (permalink / raw) To: xen-devel; +Cc: David Vrabel, Ian Jackson, Ian Campbell, Stefano Stabellini Signed-off-by: David Vrabel <david.vrabel@citrix.com> --- tools/firmware/hvmloader/Makefile | 1 + tools/firmware/hvmloader/hvm_param.c | 36 ++++++++++++++++++++++++++++++++++ tools/firmware/hvmloader/hvmloader.c | 14 ++----------- tools/firmware/hvmloader/util.h | 9 +++++++++ tools/firmware/hvmloader/xenbus.c | 14 +++++-------- 5 files changed, 53 insertions(+), 21 deletions(-) create mode 100644 tools/firmware/hvmloader/hvm_param.c diff --git a/tools/firmware/hvmloader/Makefile b/tools/firmware/hvmloader/Makefile index 00ee952..46a79c5 100644 --- a/tools/firmware/hvmloader/Makefile +++ b/tools/firmware/hvmloader/Makefile @@ -31,6 +31,7 @@ CFLAGS += $(CFLAGS_xeninclude) OBJS = hvmloader.o mp_tables.o util.o smbios.o OBJS += smp.o cacheattr.o xenbus.o OBJS += e820.o pci.o pir.o ctype.o +OBJS += hvm_param.o ifeq ($(debug),y) OBJS += tests.o endif diff --git a/tools/firmware/hvmloader/hvm_param.c b/tools/firmware/hvmloader/hvm_param.c new file mode 100644 index 0000000..f7d8720 --- /dev/null +++ b/tools/firmware/hvmloader/hvm_param.c @@ -0,0 +1,36 @@ +/* + * hvm_param.c: get/set HVM params. + * + * Copyright (C) 2014 Citrix Systems R&D Ltd. + */ +#include "util.h" +#include "config.h" +#include "hypercall.h" + +#include <xen/hvm/params.h> + +int hvm_param_get(uint32_t index, uint64_t *value) +{ + struct xen_hvm_param p; + int ret; + + p.domid = DOMID_SELF; + p.index = index; + + ret = hypercall_hvm_op(HVMOP_get_param, &p); + if (ret == 0) + *value = p.value; + + return ret; +} + +int hvm_param_set(uint32_t index, uint64_t value) +{ + struct xen_hvm_param p; + + p.domid = DOMID_SELF; + p.index = index; + p.value = value; + + return hypercall_hvm_op(HVMOP_set_param, &p); +} diff --git a/tools/firmware/hvmloader/hvmloader.c b/tools/firmware/hvmloader/hvmloader.c index 1cc8cf2..7b0da38 100644 --- a/tools/firmware/hvmloader/hvmloader.c +++ b/tools/firmware/hvmloader/hvmloader.c @@ -176,14 +176,10 @@ static void cmos_write_memory_size(void) static void init_vm86_tss(void) { void *tss; - struct xen_hvm_param p; tss = mem_alloc(128, 128); memset(tss, 0, 128); - p.domid = DOMID_SELF; - p.index = HVM_PARAM_VM86_TSS; - p.value = virt_to_phys(tss); - hypercall_hvm_op(HVMOP_set_param, &p); + hvm_param_set(HVM_PARAM_VM86_TSS, virt_to_phys(tss)); printf("vm86 TSS at %08lx\n", virt_to_phys(tss)); } @@ -314,12 +310,6 @@ int main(void) if ( acpi_enabled ) { - struct xen_hvm_param p = { - .domid = DOMID_SELF, - .index = HVM_PARAM_ACPI_IOPORTS_LOCATION, - .value = 1, - }; - if ( bios->acpi_build_tables ) { printf("Loading ACPI ...\n"); @@ -328,7 +318,7 @@ int main(void) acpi_enable_sci(); - hypercall_hvm_op(HVMOP_set_param, &p); + hvm_param_set(HVM_PARAM_ACPI_IOPORTS_LOCATION, 1); } init_vm86_tss(); diff --git a/tools/firmware/hvmloader/util.h b/tools/firmware/hvmloader/util.h index 9ccb905..e4b6be4 100644 --- a/tools/firmware/hvmloader/util.h +++ b/tools/firmware/hvmloader/util.h @@ -210,6 +210,15 @@ const char *xenstore_read(const char *path, const char *default_resp); */ int xenstore_write(const char *path, const char *value); + +/* Get a HVM param. + */ +int hvm_param_get(uint32_t index, uint64_t *value); + +/* Set a HVM param. + */ +int hvm_param_set(uint32_t index, uint64_t value); + /* Setup PCI bus */ void pci_setup(void); diff --git a/tools/firmware/hvmloader/xenbus.c b/tools/firmware/hvmloader/xenbus.c index fe72e97..64c2176 100644 --- a/tools/firmware/hvmloader/xenbus.c +++ b/tools/firmware/hvmloader/xenbus.c @@ -41,21 +41,17 @@ static char payload[XENSTORE_PAYLOAD_MAX + 1]; /* Unmarshalling area */ * Call once, before any other xenbus actions. */ void xenbus_setup(void) { - xen_hvm_param_t param; + uint64_t val; /* Ask Xen where the xenbus shared page is. */ - param.domid = DOMID_SELF; - param.index = HVM_PARAM_STORE_PFN; - if ( hypercall_hvm_op(HVMOP_get_param, ¶m) ) + if ( hvm_param_get(HVM_PARAM_STORE_PFN, &val) ) BUG(); - rings = (void *) (unsigned long) (param.value << PAGE_SHIFT); + rings = (void *) (unsigned long) (val << PAGE_SHIFT); /* Ask Xen where the xenbus event channel is. */ - param.domid = DOMID_SELF; - param.index = HVM_PARAM_STORE_EVTCHN; - if ( hypercall_hvm_op(HVMOP_get_param, ¶m) ) + if ( hvm_param_get(HVM_PARAM_STORE_EVTCHN, &val) ) BUG(); - event = param.value; + event = val; printf("Xenbus rings @0x%lx, event channel %lu\n", (unsigned long) rings, (unsigned long) event); -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 4/6] libxc, libxl, hvmloader: strip out outdated VM generation ID implementation 2014-05-27 17:31 [PATCHv2 0/6] tools: rework VM Generation ID David Vrabel ` (2 preceding siblings ...) 2014-05-27 17:31 ` [PATCH 3/6] tools/hvmloader: add helper functions to get/set HVM params David Vrabel @ 2014-05-27 17:31 ` David Vrabel 2014-05-28 14:50 ` Ian Campbell 2014-05-27 17:31 ` [PATCH 5/6] libxl: add libxl_vm_generation_id_set() David Vrabel 2014-05-27 17:31 ` [PATCH 6/6] xl: generate a new random VM generation ID if requested David Vrabel 5 siblings, 1 reply; 16+ messages in thread From: David Vrabel @ 2014-05-27 17:31 UTC (permalink / raw) To: xen-devel; +Cc: David Vrabel, Ian Jackson, Ian Campbell, Stefano Stabellini The VM generation ID support in libxc/libxl was based on a draft specification which subsequently changed considerably. Remove much of the code in anticipation of introducing something simpler that conforms to the current specification from Microsoft. Switch to using a HVM param (HVM_PARAM_VM_GENERATION_ID_ADDR) instead of the hvmloader/generation-id-address XenStore key. This simplifies save/restore since it only needs to transfer the value of this param. Signed-off-by: David Vrabel <david.vrabel@citrix.com> --- tools/firmware/hvmloader/acpi/build.c | 9 +++---- tools/libxc/xc_domain_restore.c | 44 +++------------------------------ tools/libxc/xc_domain_save.c | 5 ++-- tools/libxc/xenguest.h | 8 ++---- tools/libxl/libxl_create.c | 12 +++------ tools/libxl/libxl_dom.c | 25 ++----------------- tools/libxl/libxl_internal.h | 8 ++---- tools/libxl/libxl_save_callout.c | 10 +++----- tools/libxl/libxl_save_helper.c | 9 +++---- tools/libxl/libxl_save_msgs_gen.pl | 3 +-- 10 files changed, 26 insertions(+), 107 deletions(-) diff --git a/tools/firmware/hvmloader/acpi/build.c b/tools/firmware/hvmloader/acpi/build.c index f1dd3f0..edaafd7 100644 --- a/tools/firmware/hvmloader/acpi/build.c +++ b/tools/firmware/hvmloader/acpi/build.c @@ -24,6 +24,7 @@ #include "../config.h" #include "../util.h" #include <xen/hvm/hvm_xs_strings.h> +#include <xen/hvm/params.h> #define ACPI_MAX_SECONDARY_TABLES 16 @@ -361,7 +362,6 @@ static int construct_secondary_tables(unsigned long *table_ptrs, static int new_vm_gid(struct acpi_info *acpi_info) { uint64_t vm_gid[2], *buf; - char addr[12]; const char * s; char *end; @@ -382,12 +382,9 @@ static int new_vm_gid(struct acpi_info *acpi_info) return 0; memcpy(buf, vm_gid, sizeof(vm_gid)); - /* set into ACPI table and XenStore the address */ + /* set into ACPI table and HVM param the address */ acpi_info->vm_gid_addr = virt_to_phys(buf); - if ( snprintf(addr, sizeof(addr), "0x%lx", virt_to_phys(buf)) - >= sizeof(addr) ) - return 0; - xenstore_write("hvmloader/generation-id-address", addr); + hvm_param_set(HVM_PARAM_VM_GENERATION_ID_ADDR, acpi_info->vm_gid_addr); return 1; } diff --git a/tools/libxc/xc_domain_restore.c b/tools/libxc/xc_domain_restore.c index bcb0ae0..cf1c489 100644 --- a/tools/libxc/xc_domain_restore.c +++ b/tools/libxc/xc_domain_restore.c @@ -1408,8 +1408,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 checkpointed_stream, - unsigned long *vm_generationid_addr, + int checkpointed_stream, struct restore_callbacks *callbacks) { DECLARE_DOMCTL; @@ -1634,44 +1633,9 @@ int xc_domain_restore(xc_interface *xch, int io_fd, uint32_t dom, xc_set_hvm_param(xch, dom, HVM_PARAM_VM86_TSS, pagebuf.vm86_tss); if ( pagebuf.console_pfn ) console_pfn = pagebuf.console_pfn; - if ( pagebuf.vm_generationid_addr ) { - if ( !no_incr_generationid ) { - unsigned int offset; - unsigned char *buf; - unsigned long long generationid; - - /* - * Map the VM generation id buffer and inject the new value. - */ - - pfn = pagebuf.vm_generationid_addr >> PAGE_SHIFT; - offset = pagebuf.vm_generationid_addr & (PAGE_SIZE - 1); - - if ( (pfn >= dinfo->p2m_size) || - (pfn_type[pfn] != XEN_DOMCTL_PFINFO_NOTAB) ) - { - ERROR("generation id buffer frame is bad"); - goto out; - } - - mfn = ctx->p2m[pfn]; - buf = xc_map_foreign_range(xch, dom, PAGE_SIZE, - PROT_READ | PROT_WRITE, mfn); - if ( buf == NULL ) - { - ERROR("xc_map_foreign_range for generation id" - " buffer failed"); - goto out; - } - - generationid = *(unsigned long long *)(buf + offset); - *(unsigned long long *)(buf + offset) = generationid + 1; - - munmap(buf, PAGE_SIZE); - } - - *vm_generationid_addr = pagebuf.vm_generationid_addr; - } + if ( pagebuf.vm_generationid_addr ) + xc_set_hvm_param(xch, dom, HVM_PARAM_VM_GENERATION_ID_ADDR, + pagebuf.vm_generationid_addr); break; /* our work here is done */ } diff --git a/tools/libxc/xc_domain_save.c b/tools/libxc/xc_domain_save.c index 71f9b59..42094e8 100644 --- a/tools/libxc/xc_domain_save.c +++ b/tools/libxc/xc_domain_save.c @@ -802,8 +802,7 @@ static int save_tsc_info(xc_interface *xch, uint32_t dom, int io_fd) 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, - unsigned long vm_generationid_addr) + struct save_callbacks* callbacks, int hvm) { xc_dominfo_t info; DECLARE_DOMCTL; @@ -1634,7 +1633,7 @@ int xc_domain_save(xc_interface *xch, int io_fd, uint32_t dom, uint32_t max_iter } chunk = { 0, }; chunk.id = XC_SAVE_ID_HVM_GENERATION_ID_ADDR; - chunk.data = vm_generationid_addr; + xc_get_hvm_param(xch, dom, HVM_PARAM_VM_GENERATION_ID_ADDR, &chunk.data); if ( (chunk.data != 0) && wrexact(io_fd, &chunk, sizeof(chunk)) ) diff --git a/tools/libxc/xenguest.h b/tools/libxc/xenguest.h index 1f216cd..40bbac8 100644 --- a/tools/libxc/xenguest.h +++ b/tools/libxc/xenguest.h @@ -86,8 +86,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, - unsigned long vm_generationid_addr); + struct save_callbacks* callbacks, int hvm); /* callbacks provided by xc_domain_restore */ @@ -113,9 +112,7 @@ struct restore_callbacks { * @parm hvm non-zero if this is a HVM restore * @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 * @return 0 on success, -1 on failure @@ -125,8 +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 checkpointed_stream, - unsigned long *vm_generationid_addr, + int checkpointed_stream, struct restore_callbacks *callbacks); /** * xc_domain_restore writes a file to disk that contains the device diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c index d015cf4..cae6f53 100644 --- a/tools/libxl/libxl_create.c +++ b/tools/libxl/libxl_create.c @@ -584,12 +584,7 @@ retry_transaction: ARRAY_SIZE(rwperm)); } - if (info->type == LIBXL_DOMAIN_TYPE_HVM) - libxl__xs_mkdir(gc, t, - libxl__sprintf(gc, "%s/hvmloader/generation-id-address", dom_path), - rwperm, ARRAY_SIZE(rwperm)); - - vm_list = libxl_list_vm(ctx, &nb_vm); + vm_list = libxl_list_vm(ctx, &nb_vm); if (!vm_list) { LOG(ERROR, "cannot get number of running guests"); rc = ERROR_FAIL; @@ -903,7 +898,7 @@ static void domcreate_bootloader_done(libxl__egc *egc, goto out; } libxl__xc_domain_restore(egc, dcs, - hvm, pae, superpages, 1); + hvm, pae, superpages); return; out: @@ -911,7 +906,7 @@ static void domcreate_bootloader_done(libxl__egc *egc, } void libxl__srm_callout_callback_restore_results(unsigned long store_mfn, - unsigned long console_mfn, unsigned long genidad, void *user) + unsigned long console_mfn, void *user) { libxl__save_helper_state *shs = user; libxl__domain_create_state *dcs = CONTAINER_OF(shs, *dcs, shs); @@ -920,7 +915,6 @@ void libxl__srm_callout_callback_restore_results(unsigned long store_mfn, state->store_mfn = store_mfn; state->console_mfn = console_mfn; - state->vm_generationid_addr = genidad; shs->need_results = 0; } diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c index 661999c..4804fdf 100644 --- a/tools/libxl/libxl_dom.c +++ b/tools/libxl/libxl_dom.c @@ -279,7 +279,6 @@ int libxl__build_pre(libxl__gc *gc, uint32_t domid, state->store_port = xc_evtchn_alloc_unbound(ctx->xch, domid, state->store_domid); state->console_port = xc_evtchn_alloc_unbound(ctx->xch, domid, state->console_domid); - state->vm_generationid_addr = 0; if (info->type == LIBXL_DOMAIN_TYPE_HVM) hvm_set_conf_params(ctx->xch, domid, info); @@ -297,7 +296,7 @@ int libxl__build_post(libxl__gc *gc, uint32_t domid, libxl_ctx *ctx = libxl__gc_owner(gc); char *dom_path, *vm_path; xs_transaction_t t; - char **ents, **hvm_ents; + char **ents; int i, rc; rc = libxl_domain_sched_params_set(CTX, domid, &info->sched_params); @@ -334,13 +333,6 @@ int libxl__build_post(libxl__gc *gc, uint32_t domid, ? "online" : "offline"; } - hvm_ents = NULL; - if (info->type == LIBXL_DOMAIN_TYPE_HVM) { - hvm_ents = libxl__calloc(gc, 3, sizeof(char *)); - hvm_ents[0] = "hvmloader/generation-id-address"; - hvm_ents[1] = GCSPRINTF("0x%lx", state->vm_generationid_addr); - } - dom_path = libxl__xs_get_dompath(gc, domid); if (!dom_path) { return ERROR_FAIL; @@ -351,9 +343,6 @@ retry_transaction: t = xs_transaction_start(ctx->xsh); libxl__xs_writev(gc, t, dom_path, ents); - if (info->type == LIBXL_DOMAIN_TYPE_HVM) - libxl__xs_writev(gc, t, dom_path, hvm_ents); - libxl__xs_writev(gc, t, dom_path, local_ents); libxl__xs_writev(gc, t, vm_path, vms_ents); @@ -1502,7 +1491,6 @@ void libxl__domain_suspend(libxl__egc *egc, libxl__domain_suspend_state *dss) STATE_AO_GC(dss->ao); int port; int rc = ERROR_FAIL; - unsigned long vm_generationid_addr; /* Convenience aliases */ const uint32_t domid = dss->domid; @@ -1521,19 +1509,10 @@ void libxl__domain_suspend(libxl__egc *egc, libxl__domain_suspend_state *dss) switch (type) { case LIBXL_DOMAIN_TYPE_HVM: { - char *path; - char *addr; - - path = GCSPRINTF("%s/hvmloader/generation-id-address", - libxl__xs_get_dompath(gc, domid)); - addr = libxl__xs_read(gc, XBT_NULL, path); - - vm_generationid_addr = (addr) ? strtoul(addr, NULL, 0) : 0; dss->hvm = 1; break; } case LIBXL_DOMAIN_TYPE_PV: - vm_generationid_addr = 0; dss->hvm = 0; break; default: @@ -1580,7 +1559,7 @@ void libxl__domain_suspend(libxl__egc *egc, libxl__domain_suspend_state *dss) callbacks->switch_qemu_logdirty = libxl__domain_suspend_common_switch_qemu_logdirty; dss->shs.callbacks.save.toolstack_save = libxl__toolstack_save; - libxl__xc_domain_save(egc, dss, vm_generationid_addr); + libxl__xc_domain_save(egc, dss); return; out: diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h index c2b73c4..7ac38fe 100644 --- a/tools/libxl/libxl_internal.h +++ b/tools/libxl/libxl_internal.h @@ -960,8 +960,6 @@ typedef struct { uint32_t console_domid; unsigned long console_mfn; - unsigned long vm_generationid_addr; - char *saved_state; libxl__file_reference pv_kernel; @@ -2737,8 +2735,7 @@ _hidden void libxl__domain_suspend(libxl__egc *egc, /* calls libxl__xc_domain_suspend_done when done */ -_hidden void libxl__xc_domain_save(libxl__egc*, libxl__domain_suspend_state*, - unsigned long vm_generationid_addr); +_hidden void libxl__xc_domain_save(libxl__egc*, libxl__domain_suspend_state*); /* If rc==0 then retval is the return value from xc_domain_save * and errnoval is the errno value it provided. * If rc!=0, retval and errnoval are undefined. */ @@ -2762,8 +2759,7 @@ _hidden int libxl__toolstack_save(uint32_t domid, uint8_t **buf, /* calls libxl__xc_domain_restore_done when done */ _hidden void libxl__xc_domain_restore(libxl__egc *egc, libxl__domain_create_state *dcs, - int hvm, int pae, int superpages, - int no_incr_generationid); + int hvm, int pae, int superpages); /* If rc==0 then retval is the return value from xc_domain_save * and errnoval is the errno value it provided. * If rc!=0, retval and errnoval are undefined. */ diff --git a/tools/libxl/libxl_save_callout.c b/tools/libxl/libxl_save_callout.c index e3bda8f..b37ab3b 100644 --- a/tools/libxl/libxl_save_callout.c +++ b/tools/libxl/libxl_save_callout.c @@ -41,8 +41,7 @@ static void helper_done(libxl__egc *egc, libxl__save_helper_state *shs); /*----- entrypoints -----*/ void libxl__xc_domain_restore(libxl__egc *egc, libxl__domain_create_state *dcs, - int hvm, int pae, int superpages, - int no_incr_generationid) + int hvm, int pae, int superpages) { STATE_AO_GC(dcs->ao); @@ -59,7 +58,7 @@ void libxl__xc_domain_restore(libxl__egc *egc, libxl__domain_create_state *dcs, state->store_port, state->store_domid, state->console_port, state->console_domid, - hvm, pae, superpages, no_incr_generationid, + hvm, pae, superpages, cbflags, dcs->checkpointed_stream, }; @@ -75,8 +74,7 @@ void libxl__xc_domain_restore(libxl__egc *egc, libxl__domain_create_state *dcs, argnums, ARRAY_SIZE(argnums)); } -void libxl__xc_domain_save(libxl__egc *egc, libxl__domain_suspend_state *dss, - unsigned long vm_generationid_addr) +void libxl__xc_domain_save(libxl__egc *egc, libxl__domain_suspend_state *dss) { STATE_AO_GC(dss->ao); int r, rc, toolstack_data_fd = -1; @@ -108,7 +106,7 @@ void libxl__xc_domain_save(libxl__egc *egc, libxl__domain_suspend_state *dss, } const unsigned long argnums[] = { - dss->domid, 0, 0, dss->xcflags, dss->hvm, vm_generationid_addr, + dss->domid, 0, 0, dss->xcflags, dss->hvm, toolstack_data_fd, toolstack_data_len, cbflags, }; diff --git a/tools/libxl/libxl_save_helper.c b/tools/libxl/libxl_save_helper.c index c36314c..a94a8e9 100644 --- a/tools/libxl/libxl_save_helper.c +++ b/tools/libxl/libxl_save_helper.c @@ -212,7 +212,6 @@ int main(int argc, char **argv) uint32_t max_factor = strtoul(NEXTARG,0,10); uint32_t flags = strtoul(NEXTARG,0,10); int hvm = atoi(NEXTARG); - unsigned long genidad = strtoul(NEXTARG,0,10); toolstack_save_fd = atoi(NEXTARG); toolstack_save_len = strtoul(NEXTARG,0,10); unsigned cbflags = strtoul(NEXTARG,0,10); @@ -225,7 +224,7 @@ int main(int argc, char **argv) startup("save"); r = xc_domain_save(xch, io_fd, dom, max_iters, max_factor, flags, - &helper_save_callbacks, hvm, genidad); + &helper_save_callbacks, hvm); complete(r); } else if (!strcmp(mode,"--restore-domain")) { @@ -239,7 +238,6 @@ int main(int argc, char **argv) unsigned int hvm = strtoul(NEXTARG,0,10); unsigned int pae = strtoul(NEXTARG,0,10); int superpages = strtoul(NEXTARG,0,10); - int no_incr_genidad = strtoul(NEXTARG,0,10); unsigned cbflags = strtoul(NEXTARG,0,10); int checkpointed = strtoul(NEXTARG,0,10); assert(!*++argv); @@ -248,15 +246,14 @@ int main(int argc, char **argv) unsigned long store_mfn = 0; unsigned long console_mfn = 0; - unsigned long genidad = 0; startup("restore"); 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, checkpointed, &genidad, + checkpointed, &helper_restore_callbacks); - helper_stub_restore_results(store_mfn,console_mfn,genidad,0); + helper_stub_restore_results(store_mfn,console_mfn,0); complete(r); } else { diff --git a/tools/libxl/libxl_save_msgs_gen.pl b/tools/libxl/libxl_save_msgs_gen.pl index 745e2ac..88f4921 100755 --- a/tools/libxl/libxl_save_msgs_gen.pl +++ b/tools/libxl/libxl_save_msgs_gen.pl @@ -32,8 +32,7 @@ our @msgs = ( [ 7, 'rcxW', "toolstack_restore", [qw(uint32_t domid BLOCK tsdata)] ], [ 8, 'r', "restore_results", ['unsigned long', 'store_mfn', - 'unsigned long', 'console_mfn', - 'unsigned long', 'genidad'] ], + 'unsigned long', 'console_mfn'] ], [ 9, 'srW', "complete", [qw(int retval int errnoval)] ], ); -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 4/6] libxc, libxl, hvmloader: strip out outdated VM generation ID implementation 2014-05-27 17:31 ` [PATCH 4/6] libxc, libxl, hvmloader: strip out outdated VM generation ID implementation David Vrabel @ 2014-05-28 14:50 ` Ian Campbell 2014-06-02 9:23 ` David Vrabel 0 siblings, 1 reply; 16+ messages in thread From: Ian Campbell @ 2014-05-28 14:50 UTC (permalink / raw) To: David Vrabel; +Cc: xen-devel, Ian Jackson, Stefano Stabellini On Tue, 2014-05-27 at 18:31 +0100, David Vrabel wrote: > The VM generation ID support in libxc/libxl was based on a draft > specification which subsequently changed considerably. Remove much of > the code in anticipation of introducing something simpler that > conforms to the current specification from Microsoft. > > Switch to using a HVM param (HVM_PARAM_VM_GENERATION_ID_ADDR) instead > of the hvmloader/generation-id-address XenStore key. This simplifies > save/restore since it only needs to transfer the value of this param. > > Signed-off-by: David Vrabel <david.vrabel@citrix.com> > --- > tools/firmware/hvmloader/acpi/build.c | 9 +++---- > tools/libxc/xc_domain_restore.c | 44 +++------------------------------ > tools/libxc/xc_domain_save.c | 5 ++-- > tools/libxc/xenguest.h | 8 ++---- > tools/libxl/libxl_create.c | 12 +++------ > tools/libxl/libxl_dom.c | 25 ++----------------- > tools/libxl/libxl_internal.h | 8 ++---- > tools/libxl/libxl_save_callout.c | 10 +++----- > tools/libxl/libxl_save_helper.c | 9 +++---- > tools/libxl/libxl_save_msgs_gen.pl | 3 +-- I've forgotten how this existing stuff works, but since there is no touching of libxl_types.idl or libxl.h here I think there was no application facing nob for the existing stuff, right? (the relevant libxc parameter is hardcoded in libxl). Hence nothing to remove and nothing to add a comment about. > diff --git a/tools/libxc/xc_domain_save.c b/tools/libxc/xc_domain_save.c > index 71f9b59..42094e8 100644 > --- a/tools/libxc/xc_domain_save.c > +++ b/tools/libxc/xc_domain_save.c > @@ -802,8 +802,7 @@ static int save_tsc_info(xc_interface *xch, uint32_t dom, int io_fd) > > 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, > - unsigned long vm_generationid_addr) > + struct save_callbacks* callbacks, int hvm) > { > xc_dominfo_t info; > DECLARE_DOMCTL; > @@ -1634,7 +1633,7 @@ int xc_domain_save(xc_interface *xch, int io_fd, uint32_t dom, uint32_t max_iter > } chunk = { 0, }; > > chunk.id = XC_SAVE_ID_HVM_GENERATION_ID_ADDR; > - chunk.data = vm_generationid_addr; > + xc_get_hvm_param(xch, dom, HVM_PARAM_VM_GENERATION_ID_ADDR, &chunk.data); Are there any N->N+1 migration concerns here? I don't think so, since the stream always contains the address and what happens to it is entirely internal to the given host. Is that correct? Ian. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 4/6] libxc, libxl, hvmloader: strip out outdated VM generation ID implementation 2014-05-28 14:50 ` Ian Campbell @ 2014-06-02 9:23 ` David Vrabel 0 siblings, 0 replies; 16+ messages in thread From: David Vrabel @ 2014-06-02 9:23 UTC (permalink / raw) To: Ian Campbell; +Cc: xen-devel, Ian Jackson, Stefano Stabellini On 28/05/14 15:50, Ian Campbell wrote: > On Tue, 2014-05-27 at 18:31 +0100, David Vrabel wrote: >> The VM generation ID support in libxc/libxl was based on a draft >> specification which subsequently changed considerably. Remove much of >> the code in anticipation of introducing something simpler that >> conforms to the current specification from Microsoft. >> >> Switch to using a HVM param (HVM_PARAM_VM_GENERATION_ID_ADDR) instead >> of the hvmloader/generation-id-address XenStore key. This simplifies >> save/restore since it only needs to transfer the value of this param. >> >> Signed-off-by: David Vrabel <david.vrabel@citrix.com> >> --- >> tools/firmware/hvmloader/acpi/build.c | 9 +++---- >> tools/libxc/xc_domain_restore.c | 44 +++------------------------------ >> tools/libxc/xc_domain_save.c | 5 ++-- >> tools/libxc/xenguest.h | 8 ++---- >> tools/libxl/libxl_create.c | 12 +++------ >> tools/libxl/libxl_dom.c | 25 ++----------------- >> tools/libxl/libxl_internal.h | 8 ++---- >> tools/libxl/libxl_save_callout.c | 10 +++----- >> tools/libxl/libxl_save_helper.c | 9 +++---- >> tools/libxl/libxl_save_msgs_gen.pl | 3 +-- > > I've forgotten how this existing stuff works, but since there is no > touching of libxl_types.idl or libxl.h here I think there was no > application facing nob for the existing stuff, right? (the relevant > libxc parameter is hardcoded in libxl). > > Hence nothing to remove and nothing to add a comment about. Yes. >> --- a/tools/libxc/xc_domain_save.c >> +++ b/tools/libxc/xc_domain_save.c >> @@ -1634,7 +1633,7 @@ int xc_domain_save(xc_interface *xch, int io_fd, uint32_t dom, uint32_t max_iter >> } chunk = { 0, }; >> >> chunk.id = XC_SAVE_ID_HVM_GENERATION_ID_ADDR; >> - chunk.data = vm_generationid_addr; >> + xc_get_hvm_param(xch, dom, HVM_PARAM_VM_GENERATION_ID_ADDR, &chunk.data); > > Are there any N->N+1 migration concerns here? > > I don't think so, since the stream always contains the address and what > happens to it is entirely internal to the given host. Is that correct? Yes, that is correct. David ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 5/6] libxl: add libxl_vm_generation_id_set() 2014-05-27 17:31 [PATCHv2 0/6] tools: rework VM Generation ID David Vrabel ` (3 preceding siblings ...) 2014-05-27 17:31 ` [PATCH 4/6] libxc, libxl, hvmloader: strip out outdated VM generation ID implementation David Vrabel @ 2014-05-27 17:31 ` David Vrabel 2014-05-28 14:56 ` Ian Campbell 2014-05-27 17:31 ` [PATCH 6/6] xl: generate a new random VM generation ID if requested David Vrabel 5 siblings, 1 reply; 16+ messages in thread From: David Vrabel @ 2014-05-27 17:31 UTC (permalink / raw) To: xen-devel; +Cc: David Vrabel, Ian Jackson, Ian Campbell, Stefano Stabellini Add a function to allow a toolstack to set a domain's VM generation ID. Although the specification requires that a ACPI Notify event is raised if the generation ID is changed, in practice this appears not to be necessary if the generation ID is changed while the Windows VM is suspended. Signed-off-by: David Vrabel <david.vrabel@citrix.com> --- tools/libxl/Makefile | 1 + tools/libxl/libxl.h | 3 ++ tools/libxl/libxl_vm_genid.c | 87 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 91 insertions(+) create mode 100644 tools/libxl/libxl_vm_genid.c diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile index 4cfa275..5d0e2ec 100644 --- a/tools/libxl/Makefile +++ b/tools/libxl/Makefile @@ -77,6 +77,7 @@ LIBXL_OBJS = flexarray.o libxl.o libxl_create.o libxl_dm.o libxl_pci.o \ libxl_json.o libxl_aoutils.o libxl_numa.o \ libxl_save_callout.o _libxl_save_msgs_callout.o \ libxl_qmp.o libxl_event.o libxl_fork.o $(LIBXL_OBJS-y) +LIBXL_OBJS += libxl_vm_genid.o LIBXL_OBJS += _libxl_types.o libxl_flask.o _libxl_types_internal.o LIBXL_TESTS += timedereg diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h index c7aa817..8dfd715 100644 --- a/tools/libxl/libxl.h +++ b/tools/libxl/libxl.h @@ -1163,6 +1163,9 @@ int libxl_flask_getenforce(libxl_ctx *ctx); int libxl_flask_setenforce(libxl_ctx *ctx, int mode); int libxl_flask_loadpolicy(libxl_ctx *ctx, void *policy, uint32_t size); +int libxl_vm_generation_id_set(libxl_ctx *ctx, uint32_t domid, + const libxl_uuid *uuid); + /* misc */ /* Each of these sets or clears the flag according to whether the diff --git a/tools/libxl/libxl_vm_genid.c b/tools/libxl/libxl_vm_genid.c new file mode 100644 index 0000000..1545a97 --- /dev/null +++ b/tools/libxl/libxl_vm_genid.c @@ -0,0 +1,87 @@ +/* + * Copyright (C) 2014 Citrix Systems R&D Ltd. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as published + * by the Free Software Foundation; version 2.1 only. with the special + * exception on linking described in file LICENSE. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Lesser General Public License for more details. + */ + +#include "libxl_osdeps.h" /* must come before any other headers */ + +#include "libxl_internal.h" + +#include <xenctrl.h> +#include <xen/hvm/params.h> + +/* + * Set a HVM domain's VM Generation ID. + * + * Toolstacks should call this after building or restoring the domain, + * but before unpausing it. + * + * For further details, refer to the "Virtual Machine Generation ID" + * document from Microsoft. + * + * http://www.microsoft.com/en-us/download/details.aspx?id=30707 + */ +int libxl_vm_generation_id_set(libxl_ctx *ctx, uint32_t domid, + const libxl_uuid *uuid) +{ + GC_INIT(ctx); + char *dom_path; + char *key; + uint64_t genid[2]; + uint64_t paddr = 0; + int rc; + + memcpy(genid, libxl_uuid_bytearray((libxl_uuid *)uuid), 16); + + /* + * Set the "platform/generation-id" XenStore key to pass the ID to + * hvmloader. + */ + dom_path = libxl__xs_get_dompath(gc, domid); + if (!dom_path) { + rc = ERROR_FAIL; + goto out; + } + key = libxl__sprintf(gc, "%s/platform/generation-id", dom_path); + rc = libxl__xs_write(gc, XBT_NULL, key, "%"PRIu64 ":%" PRIu64, + genid[0], genid[1]); + if (rc < 0) + goto out; + + /* + * Update the ID in guest memory (if available). + */ + xc_get_hvm_param(ctx->xch, domid, HVM_PARAM_VM_GENERATION_ID_ADDR, &paddr); + if (paddr) { + void *vaddr; + + vaddr = xc_map_foreign_range(ctx->xch, domid, XC_PAGE_SIZE, + PROT_READ | PROT_WRITE, + paddr >> XC_PAGE_SHIFT); + if (vaddr == NULL) { + rc = ERROR_FAIL; + goto out; + } + memcpy(vaddr + (paddr & ~XC_PAGE_MASK), genid, 2 * sizeof(*genid)); + munmap(vaddr, XC_PAGE_SIZE); + + /* + * FIXME: Inject ACPI Notify event. + */ + } + + rc = 0; + + out: + GC_FREE; + return rc; +} -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 5/6] libxl: add libxl_vm_generation_id_set() 2014-05-27 17:31 ` [PATCH 5/6] libxl: add libxl_vm_generation_id_set() David Vrabel @ 2014-05-28 14:56 ` Ian Campbell 2014-06-02 9:25 ` David Vrabel 0 siblings, 1 reply; 16+ messages in thread From: Ian Campbell @ 2014-05-28 14:56 UTC (permalink / raw) To: David Vrabel; +Cc: xen-devel, Ian Jackson, Stefano Stabellini On Tue, 2014-05-27 at 18:31 +0100, David Vrabel wrote: > Add a function to allow a toolstack to set a domain's VM generation > ID. > > Although the specification requires that a ACPI Notify event is raised > if the generation ID is changed, in practice this appears not to be > necessary if the generation ID is changed while the Windows VM is > suspended. Do we (or did Microsoft) envisage scenarios where the ID might change while the VM was not suspended? > diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h > index c7aa817..8dfd715 100644 > --- a/tools/libxl/libxl.h > +++ b/tools/libxl/libxl.h > @@ -1163,6 +1163,9 @@ int libxl_flask_getenforce(libxl_ctx *ctx); > int libxl_flask_setenforce(libxl_ctx *ctx, int mode); > int libxl_flask_loadpolicy(libxl_ctx *ctx, void *policy, uint32_t size); > > +int libxl_vm_generation_id_set(libxl_ctx *ctx, uint32_t domid, > + const libxl_uuid *uuid); This needs a #define LIBXL_HAVE_FOO thing as described in the comment about compatibility near the top of the file. Each toolstack needs to be modified to call this new function. Why is that preferred to a flag in libxl_domain_config/libxl_domain_restore_params which can do the right thing by default for all toolstacks? > + /* > + * Update the ID in guest memory (if available). Does "if available" here translate to "on restore"? (with on create being handled by hvmloader, IOW this is the cooperation I was wondering about on the docs patch) Ian. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 5/6] libxl: add libxl_vm_generation_id_set() 2014-05-28 14:56 ` Ian Campbell @ 2014-06-02 9:25 ` David Vrabel 0 siblings, 0 replies; 16+ messages in thread From: David Vrabel @ 2014-06-02 9:25 UTC (permalink / raw) To: Ian Campbell; +Cc: xen-devel, Ian Jackson, Stefano Stabellini On 28/05/14 15:56, Ian Campbell wrote: > On Tue, 2014-05-27 at 18:31 +0100, David Vrabel wrote: >> Add a function to allow a toolstack to set a domain's VM generation >> ID. >> >> Although the specification requires that a ACPI Notify event is raised >> if the generation ID is changed, in practice this appears not to be >> necessary if the generation ID is changed while the Windows VM is >> suspended. > > Do we (or did Microsoft) envisage scenarios where the ID might change > while the VM was not suspended? I think the specification has been written to allow for this. But when running on Xen I would not anticipate this being necessary. >> + /* >> + * Update the ID in guest memory (if available). > > Does "if available" here translate to "on restore"? (with on create > being handled by hvmloader, IOW this is the cooperation I was wondering > about on the docs patch) Yes. David ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 6/6] xl: generate a new random VM generation ID if requested 2014-05-27 17:31 [PATCHv2 0/6] tools: rework VM Generation ID David Vrabel ` (4 preceding siblings ...) 2014-05-27 17:31 ` [PATCH 5/6] libxl: add libxl_vm_generation_id_set() David Vrabel @ 2014-05-27 17:31 ` David Vrabel 5 siblings, 0 replies; 16+ messages in thread From: David Vrabel @ 2014-05-27 17:31 UTC (permalink / raw) To: xen-devel; +Cc: David Vrabel, Ian Jackson, Ian Campbell, Stefano Stabellini If the "generation_id" option is set in the domain configuration, generate and set a new random VM generation ID every time a domain is created or restored. xl lacks the infrastructure to fully track the lifecycle of VM images as they are snapshotted and cloned (etc) so always using a new ID is the safe option and ensures that a new one will be used where it matters. Signed-off-by: David Vrabel <david.vrabel@citrix.com> --- docs/man/xl.cfg.pod.5 | 14 ++++++++++++++ tools/libxl/xl_cmdimpl.c | 14 ++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5 index 0ca37bc..0ec0386 100644 --- a/docs/man/xl.cfg.pod.5 +++ b/docs/man/xl.cfg.pod.5 @@ -936,6 +936,20 @@ number of vendor defined SMBIOS structures (type 128 - 255). Since SMBIOS structures do not present their overall size, each entry in the file must be preceded by a 32b integer indicating the size of the next structure. +=item B<generation_id=BOOLEAN> + +Provide a VM generation ID to the guest. + +The VM generation ID as a 128-bit random number that a guest may use +to determine if the guest has been restored from an earlier snapshot, +or cloned. + +This is required for Microsoft Windows Server 2012 (and later) domain +controllers. + +See also "Virtual Machine Generation ID" by Microsoft +(http://www.microsoft.com/en-us/download/details.aspx?id=30707). + =back =head3 Guest Virtual Time Controls diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c index 5195914..843ef14 100644 --- a/tools/libxl/xl_cmdimpl.c +++ b/tools/libxl/xl_cmdimpl.c @@ -155,6 +155,7 @@ struct domain_create { const char *restore_file; int migrate_fd; /* -1 means none */ char **migration_domname_r; /* from malloc */ + bool set_genid; }; @@ -1058,6 +1059,8 @@ static void parse_config_data(const char *config_source, &b_info->u.hvm.smbios_firmware, 0); xlu_cfg_replace_string(config, "acpi_firmware", &b_info->u.hvm.acpi_firmware, 0); + if (dom_info && !xlu_cfg_get_long(config, "generation_id", &l, 0)) + dom_info->set_genid = !!l; break; case LIBXL_DOMAIN_TYPE_PV: { @@ -2254,6 +2257,17 @@ start: if ( ret ) goto error_out; + /* Generate and set a new random VM Generation ID? */ + if (dom_info->set_genid) { + libxl_uuid genid; + + libxl_uuid_generate(&genid); + + ret = libxl_vm_generation_id_set(ctx, domid, &genid); + if (ret) + goto error_out; + } + /* If single vcpu to pcpu mapping was requested, honour it */ if (vcpu_to_pcpu) { libxl_bitmap vcpu_cpumap; -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCHv3 0/6] tools: rework VM Generation ID @ 2014-06-03 13:15 David Vrabel 2014-06-03 13:15 ` [PATCH 4/6] libxc, libxl, hvmloader: strip out outdated VM generation ID implementation David Vrabel 0 siblings, 1 reply; 16+ messages in thread From: David Vrabel @ 2014-06-03 13:15 UTC (permalink / raw) To: xen-devel; +Cc: Ian Jackson, Ian Campbell, Stefano Stabellini This series reworks the VM Generation ID to a) conform to the published spec from Microsoft; b) simplify the save/restore code; and c) extend the libxl API to allow toolstacks to use this feature. The VM Generation ID must be regenerated with a new random ID after certain VM operations. Since xl lacks infrastructure for tracking the life-cycle of snapshots and clones (etc), the safe option of always using a new generation ID is used. You can download the spec from: http://www.microsoft.com/en-us/download/details.aspx?id=30707 Changes in v3: - Specify generation ID in libxl_domain_build_info instead of having to call a function to set it. - Improve docs. Changes in v2: - Use libxl_uuid for the generation ID. - Add "generation_id" option to xl domain configuration file and use this to set a random generation ID every time. David ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 4/6] libxc, libxl, hvmloader: strip out outdated VM generation ID implementation 2014-06-03 13:15 [PATCHv3 0/6] tools: rework VM Generation ID David Vrabel @ 2014-06-03 13:15 ` David Vrabel 2014-06-10 10:42 ` Ian Campbell 0 siblings, 1 reply; 16+ messages in thread From: David Vrabel @ 2014-06-03 13:15 UTC (permalink / raw) To: xen-devel; +Cc: David Vrabel, Ian Jackson, Ian Campbell, Stefano Stabellini The VM generation ID support in libxc/libxl was based on a draft specification which subsequently changed considerably. Remove much of the code in anticipation of introducing something simpler that conforms to the current specification from Microsoft. Switch to using a HVM param (HVM_PARAM_VM_GENERATION_ID_ADDR) instead of the hvmloader/generation-id-address XenStore key. This simplifies save/restore since it only needs to transfer the value of this param. There are no changes to the migration stream format or the public libxl API. Signed-off-by: David Vrabel <david.vrabel@citrix.com> --- tools/firmware/hvmloader/acpi/build.c | 9 +++---- tools/libxc/xc_domain_restore.c | 44 +++------------------------------ tools/libxc/xc_domain_save.c | 5 ++-- tools/libxc/xenguest.h | 8 ++---- tools/libxl/libxl_create.c | 12 +++------ tools/libxl/libxl_dom.c | 25 ++----------------- tools/libxl/libxl_internal.h | 8 ++---- tools/libxl/libxl_save_callout.c | 10 +++----- tools/libxl/libxl_save_helper.c | 9 +++---- tools/libxl/libxl_save_msgs_gen.pl | 3 +-- 10 files changed, 26 insertions(+), 107 deletions(-) diff --git a/tools/firmware/hvmloader/acpi/build.c b/tools/firmware/hvmloader/acpi/build.c index 5c42d89..1431296 100644 --- a/tools/firmware/hvmloader/acpi/build.c +++ b/tools/firmware/hvmloader/acpi/build.c @@ -24,6 +24,7 @@ #include "../config.h" #include "../util.h" #include <xen/hvm/hvm_xs_strings.h> +#include <xen/hvm/params.h> #define ACPI_MAX_SECONDARY_TABLES 16 @@ -362,7 +363,6 @@ static int construct_secondary_tables(unsigned long *table_ptrs, static int new_vm_gid(struct acpi_info *acpi_info) { uint64_t vm_gid[2], *buf; - char addr[12]; const char * s; char *end; @@ -383,12 +383,9 @@ static int new_vm_gid(struct acpi_info *acpi_info) return 0; memcpy(buf, vm_gid, sizeof(vm_gid)); - /* set into ACPI table and XenStore the address */ + /* set into ACPI table and HVM param the address */ acpi_info->vm_gid_addr = virt_to_phys(buf); - if ( snprintf(addr, sizeof(addr), "0x%lx", virt_to_phys(buf)) - >= sizeof(addr) ) - return 0; - xenstore_write("hvmloader/generation-id-address", addr); + hvm_param_set(HVM_PARAM_VM_GENERATION_ID_ADDR, acpi_info->vm_gid_addr); return 1; } diff --git a/tools/libxc/xc_domain_restore.c b/tools/libxc/xc_domain_restore.c index bcb0ae0..cf1c489 100644 --- a/tools/libxc/xc_domain_restore.c +++ b/tools/libxc/xc_domain_restore.c @@ -1408,8 +1408,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 checkpointed_stream, - unsigned long *vm_generationid_addr, + int checkpointed_stream, struct restore_callbacks *callbacks) { DECLARE_DOMCTL; @@ -1634,44 +1633,9 @@ int xc_domain_restore(xc_interface *xch, int io_fd, uint32_t dom, xc_set_hvm_param(xch, dom, HVM_PARAM_VM86_TSS, pagebuf.vm86_tss); if ( pagebuf.console_pfn ) console_pfn = pagebuf.console_pfn; - if ( pagebuf.vm_generationid_addr ) { - if ( !no_incr_generationid ) { - unsigned int offset; - unsigned char *buf; - unsigned long long generationid; - - /* - * Map the VM generation id buffer and inject the new value. - */ - - pfn = pagebuf.vm_generationid_addr >> PAGE_SHIFT; - offset = pagebuf.vm_generationid_addr & (PAGE_SIZE - 1); - - if ( (pfn >= dinfo->p2m_size) || - (pfn_type[pfn] != XEN_DOMCTL_PFINFO_NOTAB) ) - { - ERROR("generation id buffer frame is bad"); - goto out; - } - - mfn = ctx->p2m[pfn]; - buf = xc_map_foreign_range(xch, dom, PAGE_SIZE, - PROT_READ | PROT_WRITE, mfn); - if ( buf == NULL ) - { - ERROR("xc_map_foreign_range for generation id" - " buffer failed"); - goto out; - } - - generationid = *(unsigned long long *)(buf + offset); - *(unsigned long long *)(buf + offset) = generationid + 1; - - munmap(buf, PAGE_SIZE); - } - - *vm_generationid_addr = pagebuf.vm_generationid_addr; - } + if ( pagebuf.vm_generationid_addr ) + xc_set_hvm_param(xch, dom, HVM_PARAM_VM_GENERATION_ID_ADDR, + pagebuf.vm_generationid_addr); break; /* our work here is done */ } diff --git a/tools/libxc/xc_domain_save.c b/tools/libxc/xc_domain_save.c index 71f9b59..42094e8 100644 --- a/tools/libxc/xc_domain_save.c +++ b/tools/libxc/xc_domain_save.c @@ -802,8 +802,7 @@ static int save_tsc_info(xc_interface *xch, uint32_t dom, int io_fd) 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, - unsigned long vm_generationid_addr) + struct save_callbacks* callbacks, int hvm) { xc_dominfo_t info; DECLARE_DOMCTL; @@ -1634,7 +1633,7 @@ int xc_domain_save(xc_interface *xch, int io_fd, uint32_t dom, uint32_t max_iter } chunk = { 0, }; chunk.id = XC_SAVE_ID_HVM_GENERATION_ID_ADDR; - chunk.data = vm_generationid_addr; + xc_get_hvm_param(xch, dom, HVM_PARAM_VM_GENERATION_ID_ADDR, &chunk.data); if ( (chunk.data != 0) && wrexact(io_fd, &chunk, sizeof(chunk)) ) diff --git a/tools/libxc/xenguest.h b/tools/libxc/xenguest.h index 1f216cd..40bbac8 100644 --- a/tools/libxc/xenguest.h +++ b/tools/libxc/xenguest.h @@ -86,8 +86,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, - unsigned long vm_generationid_addr); + struct save_callbacks* callbacks, int hvm); /* callbacks provided by xc_domain_restore */ @@ -113,9 +112,7 @@ struct restore_callbacks { * @parm hvm non-zero if this is a HVM restore * @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 * @return 0 on success, -1 on failure @@ -125,8 +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 checkpointed_stream, - unsigned long *vm_generationid_addr, + int checkpointed_stream, struct restore_callbacks *callbacks); /** * xc_domain_restore writes a file to disk that contains the device diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c index d015cf4..cae6f53 100644 --- a/tools/libxl/libxl_create.c +++ b/tools/libxl/libxl_create.c @@ -584,12 +584,7 @@ retry_transaction: ARRAY_SIZE(rwperm)); } - if (info->type == LIBXL_DOMAIN_TYPE_HVM) - libxl__xs_mkdir(gc, t, - libxl__sprintf(gc, "%s/hvmloader/generation-id-address", dom_path), - rwperm, ARRAY_SIZE(rwperm)); - - vm_list = libxl_list_vm(ctx, &nb_vm); + vm_list = libxl_list_vm(ctx, &nb_vm); if (!vm_list) { LOG(ERROR, "cannot get number of running guests"); rc = ERROR_FAIL; @@ -903,7 +898,7 @@ static void domcreate_bootloader_done(libxl__egc *egc, goto out; } libxl__xc_domain_restore(egc, dcs, - hvm, pae, superpages, 1); + hvm, pae, superpages); return; out: @@ -911,7 +906,7 @@ static void domcreate_bootloader_done(libxl__egc *egc, } void libxl__srm_callout_callback_restore_results(unsigned long store_mfn, - unsigned long console_mfn, unsigned long genidad, void *user) + unsigned long console_mfn, void *user) { libxl__save_helper_state *shs = user; libxl__domain_create_state *dcs = CONTAINER_OF(shs, *dcs, shs); @@ -920,7 +915,6 @@ void libxl__srm_callout_callback_restore_results(unsigned long store_mfn, state->store_mfn = store_mfn; state->console_mfn = console_mfn; - state->vm_generationid_addr = genidad; shs->need_results = 0; } diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c index 661999c..4804fdf 100644 --- a/tools/libxl/libxl_dom.c +++ b/tools/libxl/libxl_dom.c @@ -279,7 +279,6 @@ int libxl__build_pre(libxl__gc *gc, uint32_t domid, state->store_port = xc_evtchn_alloc_unbound(ctx->xch, domid, state->store_domid); state->console_port = xc_evtchn_alloc_unbound(ctx->xch, domid, state->console_domid); - state->vm_generationid_addr = 0; if (info->type == LIBXL_DOMAIN_TYPE_HVM) hvm_set_conf_params(ctx->xch, domid, info); @@ -297,7 +296,7 @@ int libxl__build_post(libxl__gc *gc, uint32_t domid, libxl_ctx *ctx = libxl__gc_owner(gc); char *dom_path, *vm_path; xs_transaction_t t; - char **ents, **hvm_ents; + char **ents; int i, rc; rc = libxl_domain_sched_params_set(CTX, domid, &info->sched_params); @@ -334,13 +333,6 @@ int libxl__build_post(libxl__gc *gc, uint32_t domid, ? "online" : "offline"; } - hvm_ents = NULL; - if (info->type == LIBXL_DOMAIN_TYPE_HVM) { - hvm_ents = libxl__calloc(gc, 3, sizeof(char *)); - hvm_ents[0] = "hvmloader/generation-id-address"; - hvm_ents[1] = GCSPRINTF("0x%lx", state->vm_generationid_addr); - } - dom_path = libxl__xs_get_dompath(gc, domid); if (!dom_path) { return ERROR_FAIL; @@ -351,9 +343,6 @@ retry_transaction: t = xs_transaction_start(ctx->xsh); libxl__xs_writev(gc, t, dom_path, ents); - if (info->type == LIBXL_DOMAIN_TYPE_HVM) - libxl__xs_writev(gc, t, dom_path, hvm_ents); - libxl__xs_writev(gc, t, dom_path, local_ents); libxl__xs_writev(gc, t, vm_path, vms_ents); @@ -1502,7 +1491,6 @@ void libxl__domain_suspend(libxl__egc *egc, libxl__domain_suspend_state *dss) STATE_AO_GC(dss->ao); int port; int rc = ERROR_FAIL; - unsigned long vm_generationid_addr; /* Convenience aliases */ const uint32_t domid = dss->domid; @@ -1521,19 +1509,10 @@ void libxl__domain_suspend(libxl__egc *egc, libxl__domain_suspend_state *dss) switch (type) { case LIBXL_DOMAIN_TYPE_HVM: { - char *path; - char *addr; - - path = GCSPRINTF("%s/hvmloader/generation-id-address", - libxl__xs_get_dompath(gc, domid)); - addr = libxl__xs_read(gc, XBT_NULL, path); - - vm_generationid_addr = (addr) ? strtoul(addr, NULL, 0) : 0; dss->hvm = 1; break; } case LIBXL_DOMAIN_TYPE_PV: - vm_generationid_addr = 0; dss->hvm = 0; break; default: @@ -1580,7 +1559,7 @@ void libxl__domain_suspend(libxl__egc *egc, libxl__domain_suspend_state *dss) callbacks->switch_qemu_logdirty = libxl__domain_suspend_common_switch_qemu_logdirty; dss->shs.callbacks.save.toolstack_save = libxl__toolstack_save; - libxl__xc_domain_save(egc, dss, vm_generationid_addr); + libxl__xc_domain_save(egc, dss); return; out: diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h index 082749e..457424f 100644 --- a/tools/libxl/libxl_internal.h +++ b/tools/libxl/libxl_internal.h @@ -960,8 +960,6 @@ typedef struct { uint32_t console_domid; unsigned long console_mfn; - unsigned long vm_generationid_addr; - char *saved_state; libxl__file_reference pv_kernel; @@ -2757,8 +2755,7 @@ _hidden void libxl__domain_suspend(libxl__egc *egc, /* calls libxl__xc_domain_suspend_done when done */ -_hidden void libxl__xc_domain_save(libxl__egc*, libxl__domain_suspend_state*, - unsigned long vm_generationid_addr); +_hidden void libxl__xc_domain_save(libxl__egc*, libxl__domain_suspend_state*); /* If rc==0 then retval is the return value from xc_domain_save * and errnoval is the errno value it provided. * If rc!=0, retval and errnoval are undefined. */ @@ -2782,8 +2779,7 @@ _hidden int libxl__toolstack_save(uint32_t domid, uint8_t **buf, /* calls libxl__xc_domain_restore_done when done */ _hidden void libxl__xc_domain_restore(libxl__egc *egc, libxl__domain_create_state *dcs, - int hvm, int pae, int superpages, - int no_incr_generationid); + int hvm, int pae, int superpages); /* If rc==0 then retval is the return value from xc_domain_save * and errnoval is the errno value it provided. * If rc!=0, retval and errnoval are undefined. */ diff --git a/tools/libxl/libxl_save_callout.c b/tools/libxl/libxl_save_callout.c index c35da4d..1c9f806 100644 --- a/tools/libxl/libxl_save_callout.c +++ b/tools/libxl/libxl_save_callout.c @@ -41,8 +41,7 @@ static void helper_done(libxl__egc *egc, libxl__save_helper_state *shs); /*----- entrypoints -----*/ void libxl__xc_domain_restore(libxl__egc *egc, libxl__domain_create_state *dcs, - int hvm, int pae, int superpages, - int no_incr_generationid) + int hvm, int pae, int superpages) { STATE_AO_GC(dcs->ao); @@ -59,7 +58,7 @@ void libxl__xc_domain_restore(libxl__egc *egc, libxl__domain_create_state *dcs, state->store_port, state->store_domid, state->console_port, state->console_domid, - hvm, pae, superpages, no_incr_generationid, + hvm, pae, superpages, cbflags, dcs->checkpointed_stream, }; @@ -75,8 +74,7 @@ void libxl__xc_domain_restore(libxl__egc *egc, libxl__domain_create_state *dcs, argnums, ARRAY_SIZE(argnums)); } -void libxl__xc_domain_save(libxl__egc *egc, libxl__domain_suspend_state *dss, - unsigned long vm_generationid_addr) +void libxl__xc_domain_save(libxl__egc *egc, libxl__domain_suspend_state *dss) { STATE_AO_GC(dss->ao); int r, rc, toolstack_data_fd = -1; @@ -112,7 +110,7 @@ void libxl__xc_domain_save(libxl__egc *egc, libxl__domain_suspend_state *dss, } const unsigned long argnums[] = { - dss->domid, 0, 0, dss->xcflags, dss->hvm, vm_generationid_addr, + dss->domid, 0, 0, dss->xcflags, dss->hvm, toolstack_data_fd, toolstack_data_len, cbflags, }; diff --git a/tools/libxl/libxl_save_helper.c b/tools/libxl/libxl_save_helper.c index b259bd0..c760dd3 100644 --- a/tools/libxl/libxl_save_helper.c +++ b/tools/libxl/libxl_save_helper.c @@ -211,7 +211,6 @@ int main(int argc, char **argv) uint32_t max_factor = strtoul(NEXTARG,0,10); uint32_t flags = strtoul(NEXTARG,0,10); int hvm = atoi(NEXTARG); - unsigned long genidad = strtoul(NEXTARG,0,10); toolstack_save_fd = atoi(NEXTARG); toolstack_save_len = strtoul(NEXTARG,0,10); unsigned cbflags = strtoul(NEXTARG,0,10); @@ -224,7 +223,7 @@ int main(int argc, char **argv) startup("save"); r = xc_domain_save(xch, io_fd, dom, max_iters, max_factor, flags, - &helper_save_callbacks, hvm, genidad); + &helper_save_callbacks, hvm); complete(r); } else if (!strcmp(mode,"--restore-domain")) { @@ -238,7 +237,6 @@ int main(int argc, char **argv) unsigned int hvm = strtoul(NEXTARG,0,10); unsigned int pae = strtoul(NEXTARG,0,10); int superpages = strtoul(NEXTARG,0,10); - int no_incr_genidad = strtoul(NEXTARG,0,10); unsigned cbflags = strtoul(NEXTARG,0,10); int checkpointed = strtoul(NEXTARG,0,10); assert(!*++argv); @@ -247,15 +245,14 @@ int main(int argc, char **argv) unsigned long store_mfn = 0; unsigned long console_mfn = 0; - unsigned long genidad = 0; startup("restore"); 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, checkpointed, &genidad, + checkpointed, &helper_restore_callbacks); - helper_stub_restore_results(store_mfn,console_mfn,genidad,0); + helper_stub_restore_results(store_mfn,console_mfn,0); complete(r); } else { diff --git a/tools/libxl/libxl_save_msgs_gen.pl b/tools/libxl/libxl_save_msgs_gen.pl index 745e2ac..88f4921 100755 --- a/tools/libxl/libxl_save_msgs_gen.pl +++ b/tools/libxl/libxl_save_msgs_gen.pl @@ -32,8 +32,7 @@ our @msgs = ( [ 7, 'rcxW', "toolstack_restore", [qw(uint32_t domid BLOCK tsdata)] ], [ 8, 'r', "restore_results", ['unsigned long', 'store_mfn', - 'unsigned long', 'console_mfn', - 'unsigned long', 'genidad'] ], + 'unsigned long', 'console_mfn'] ], [ 9, 'srW', "complete", [qw(int retval int errnoval)] ], ); -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 4/6] libxc, libxl, hvmloader: strip out outdated VM generation ID implementation 2014-06-03 13:15 ` [PATCH 4/6] libxc, libxl, hvmloader: strip out outdated VM generation ID implementation David Vrabel @ 2014-06-10 10:42 ` Ian Campbell 0 siblings, 0 replies; 16+ messages in thread From: Ian Campbell @ 2014-06-10 10:42 UTC (permalink / raw) To: David Vrabel; +Cc: xen-devel, Ian Jackson, Stefano Stabellini On Tue, 2014-06-03 at 14:15 +0100, David Vrabel wrote: > The VM generation ID support in libxc/libxl was based on a draft > specification which subsequently changed considerably. Remove much of > the code in anticipation of introducing something simpler that > conforms to the current specification from Microsoft. > > Switch to using a HVM param (HVM_PARAM_VM_GENERATION_ID_ADDR) instead > of the hvmloader/generation-id-address XenStore key. This simplifies > save/restore since it only needs to transfer the value of this param. > > There are no changes to the migration stream format or the public > libxl API. > > Signed-off-by: David Vrabel <david.vrabel@citrix.com> Acked-by: Ian Campbell <ian.campbell@citrix.com> ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2014-06-10 10:42 UTC | newest] Thread overview: 16+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-05-27 17:31 [PATCHv2 0/6] tools: rework VM Generation ID David Vrabel 2014-05-27 17:31 ` [PATCH 1/6] docs: update docs for the ~/platform/generation-id key David Vrabel 2014-05-28 14:44 ` Ian Campbell 2014-05-28 14:51 ` Andrew Cooper 2014-05-28 14:58 ` Ian Campbell 2014-05-27 17:31 ` [PATCH 2/6] hvm: add HVM_PARAM_VM_GENERATION_ID_ADDR David Vrabel 2014-05-27 17:31 ` [PATCH 3/6] tools/hvmloader: add helper functions to get/set HVM params David Vrabel 2014-05-27 17:31 ` [PATCH 4/6] libxc, libxl, hvmloader: strip out outdated VM generation ID implementation David Vrabel 2014-05-28 14:50 ` Ian Campbell 2014-06-02 9:23 ` David Vrabel 2014-05-27 17:31 ` [PATCH 5/6] libxl: add libxl_vm_generation_id_set() David Vrabel 2014-05-28 14:56 ` Ian Campbell 2014-06-02 9:25 ` David Vrabel 2014-05-27 17:31 ` [PATCH 6/6] xl: generate a new random VM generation ID if requested David Vrabel -- strict thread matches above, loose matches on Subject: below -- 2014-06-03 13:15 [PATCHv3 0/6] tools: rework VM Generation ID David Vrabel 2014-06-03 13:15 ` [PATCH 4/6] libxc, libxl, hvmloader: strip out outdated VM generation ID implementation David Vrabel 2014-06-10 10:42 ` Ian Campbell
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).