[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Xen-devel] [PATCH] tools/many: Fix Generation ID restore interface.



The original Generation ID code was submitted before the specification had
been finalised, and changed completely between the final draft and formal
release.

Most notably, the size of the Generation ID has doubled to 128 bits, and
changing it now involves writing a new cryptographically random number in the
appropriate location, rather than simply incrementing it.

The xc_domain_save() side of the code is fine, but the xc_domain_restore()
needs substantial changes to be usable.

This patch replaces the old xc_domain_restore() parameters with a new optional
restore_callback.  If the callback is provided, and an appropriate hunk is
found in the migration stream, the appropriate piece of guest memory is mapped
and provided to the callback.

This patch also fixes all the in-tree callers of xc_domain_restore().  All
callers had the functionality disabled, and never exposed in a public way.

Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
CC: Ian Campbell <Ian.Campbell@xxxxxxxxxx>
CC: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
CC: George Dunlap <george.dunlap@xxxxxxxxxxxxx>

---

George:

  Despite this being a feature, I am requesting a freeze exemption.  It is
  functionality which was not used (or indeed useful) before, and remains that
  way as far as in-tree consumers are concerned.

  However, as there has been once recent xc_domain_restore() API change, it is
  less disruptive to other users to do another API change before the 4.4
  release rather than afterwards.
---
 tools/libxc/xc_domain_restore.c  |   68 +++++++++++++++++++-------------------
 tools/libxc/xc_nomigrate.c       |    3 +-
 tools/libxc/xenguest.h           |   24 +++++++++++---
 tools/libxl/libxl_create.c       |    2 +-
 tools/libxl/libxl_internal.h     |    3 +-
 tools/libxl/libxl_save_callout.c |    5 ++-
 tools/libxl/libxl_save_helper.c  |    4 +--
 tools/xcutils/xc_restore.c       |    2 +-
 8 files changed, 61 insertions(+), 50 deletions(-)

diff --git a/tools/libxc/xc_domain_restore.c b/tools/libxc/xc_domain_restore.c
index 80769a7..bb1f7fd 100644
--- a/tools/libxc/xc_domain_restore.c
+++ b/tools/libxc/xc_domain_restore.c
@@ -1414,8 +1414,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;
@@ -1641,43 +1640,44 @@ 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;
+            if ( pagebuf.vm_generationid_addr && callbacks->generation_id ) {
+                unsigned int offset;
+                unsigned char *buf;
 
-                    /*
-                     * 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;
-                    }
+                pfn = pagebuf.vm_generationid_addr >> PAGE_SHIFT;
+                offset = pagebuf.vm_generationid_addr & (PAGE_SIZE - 1);
 
-                    generationid = *(unsigned long long *)(buf + offset);
-                    *(unsigned long long *)(buf + offset) = generationid + 1;
+                if ( pfn >= dinfo->p2m_size )
+                {
+                    ERROR("generation id frame (pfn %#lx) outside p2m (size 
%#lx)\n",
+                          pfn, dinfo->p2m_size);
+                    rc = -1;
+                    goto out;
+                }
+                else if ( pfn_type[pfn] != XEN_DOMCTL_PFINFO_NOTAB )
+                {
+                    ERROR("generation id frame (pfn %#lx) bad type %#x",
+                          pfn, pfn_type[pfn]);
+                    rc = -1;
+                    goto out;
+                }
 
-                    munmap(buf, PAGE_SIZE);
+                mfn = ctx->p2m[pfn];
+                buf = xc_map_foreign_range(xch, dom, PAGE_SIZE,
+                                           PROT_READ | PROT_WRITE, mfn);
+                if ( !buf )
+                {
+                    PERROR("Failed to map generation id frame: (mfn %#lx)", 
mfn);
+                    rc = -1;
+                    goto out;
                 }
 
-                *vm_generationid_addr = pagebuf.vm_generationid_addr;
+                rc = callbacks->generation_id(dom, 
pagebuf.vm_generationid_addr,
+                                              buf + offset, callbacks->data);
+                munmap(buf, PAGE_SIZE);
+
+                if ( rc )
+                    goto out;
             }
 
             break;  /* our work here is done */
diff --git a/tools/libxc/xc_nomigrate.c b/tools/libxc/xc_nomigrate.c
index fb6d53e..8b4c5c8 100644
--- a/tools/libxc/xc_nomigrate.c
+++ b/tools/libxc/xc_nomigrate.c
@@ -35,8 +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 checkpointed_stream,
-                      unsigned long *vm_generationid_addr,
+                      int checkpointed_stream,
                       struct restore_callbacks *callbacks)
 {
     errno = ENOSYS;
diff --git a/tools/libxc/xenguest.h b/tools/libxc/xenguest.h
index a0e30e1..507bf65 100644
--- a/tools/libxc/xenguest.h
+++ b/tools/libxc/xenguest.h
@@ -96,6 +96,25 @@ struct restore_callbacks {
     int (*toolstack_restore)(uint32_t domid, const uint8_t *buf,
             uint32_t size, void* data);
 
+    /**
+     * Callback to allow the toolstack to manage a domain's generation ID.
+     * This function is called if a generation ID chunk is found in the
+     * migration stream, and the function pointer is provided.
+     *
+     * The generation ID location is a special chhunk in the migration stream.
+     * The toolstack must take a record of the generation ID address, to
+     * provide it back to xc_domain_save() in the future.  It must also update
+     * the value of the generation id when appropriate.
+     *
+     * @param domid The domain id.
+     * @param genid_gpa Guest physical address of the generation id.
+     * @param mapped_genid Pointer to the generation id, mapped from the 
domain.
+     * @param data General restore_callbacks data pointer.
+     * @returns 0 for success, -1 for error.
+     */
+    int (*generation_id)(uint32_t domid, uint64_t genid_gpa,
+                         void *mapped_genid, void *data);
+
     /* to be provided as the last argument to each callback function */
     void* data;
 };
@@ -113,9 +132,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 +142,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 fe7ba0d..4cd4205 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -838,7 +838,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:
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index a2d8247..d84890b 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -2614,8 +2614,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 6e45b2f..0121cc6 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,
     };
 
diff --git a/tools/libxl/libxl_save_helper.c b/tools/libxl/libxl_save_helper.c
index 880565e..0df97dc 100644
--- a/tools/libxl/libxl_save_helper.c
+++ b/tools/libxl/libxl_save_helper.c
@@ -250,7 +250,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);
@@ -265,8 +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, checkpointed, &genidad,
-                              &helper_restore_callbacks);
+                              checkpointed, &helper_restore_callbacks);
         helper_stub_restore_results(store_mfn,console_mfn,genidad,0);
         complete(r);
 
diff --git a/tools/xcutils/xc_restore.c b/tools/xcutils/xc_restore.c
index 4ea261b..39d9efb 100644
--- a/tools/xcutils/xc_restore.c
+++ b/tools/xcutils/xc_restore.c
@@ -56,7 +56,7 @@ main(int argc, char **argv)
 
     ret = xc_domain_restore(xch, io_fd, domid, store_evtchn, &store_mfn, 0,
                             console_evtchn, &console_mfn, 0, hvm, pae, 
superpages,
-                            0, checkpointed, NULL, NULL);
+                            checkpointed, NULL);
 
     if ( ret == 0 )
     {
-- 
1.7.10.4


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.