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

Re: [Xen-devel] [PATCH v1 1/5] xsplice: Design document.



On 10/27/2015 08:08 AM, Martin Pohlack wrote:
On 06.10.2015 14:57, Ross Lagerwall wrote:
On 09/16/2015 10:01 PM, Konrad Rzeszutek Wilk wrote:
+### xSplice interdependencies
+
+xSplice patches interdependencies are tricky.
+
+There are the ways this can be addressed:
+ * A single large patch that subsumes and replaces all previous ones.
+   Over the life-time of patching the hypervisor this large patch
+   grows to accumulate all the code changes.
+ * Hotpatch stack - where an mechanism exists that loads the hotpatches
+   in the same order they were built in. We would need an build-id
+   of the hypevisor to make sure the hot-patches are build against the
+   correct build.
+ * Payload containing the old code to check against that. That allows
+   the hotpatches to be loaded indepedently (if they don't overlap) - or
+   if the old code also containst previously patched code - even if they
+   overlap.
+
+The disadvantage of the first large patch is that it can grow over
+time and not provide an bisection mechanism to identify faulty patches.
+
+The hot-patch stack puts stricts requirements on the order of the patches
+being loaded and requires an hypervisor build-id to match against.
+
+The old code allows much more flexibility and an additional guard,
+but is more complex to implement.
+

If the single large patch mechanism is used, a new REPLACE action is
needed to atomically replace one patch with another to prevent a window
where the hypervisor is unpatched. kpatch has a "replace" command for
this purpose. This may be useful even for the other mechanisms listed above.

  From what I can tell:
* kSplice uses old code checking (method [3] above), although in
practice the userspace tools implement dependency logic to enforce a
linear stack of patches.
* kPatch and kGraft recommend using the single large patch mechanism
although there's nothing preventing two independent patches from being
loaded.

To make sure this argument is not lost: I wrote in an earlier email

* There is a general (and mostly obscure) limitation on unloading
   hotpatches:

   In contrast to normal kernel modules where the module code adheres
   to specific conventions around resource allocation and locking,
   hotpatches typically contain code from any context.  That code is
   usually not aware that it can be unloaded.

   That code could leave behind in Xen references to itself, e.g., by
   installing a function pointer in a global data structure, without
   incrementing something like a usage count.  While most hotpatch code
   will probably be very simple and small, a similar effect could even
   be achieved by code called from the hotpatch in Xen, e.g., some code
   patch could dynamically generate a backtrace and later decide to
   inspect individual elements from the collected trace, later being a
   time, where the hotpatch has been unloaded again.

   One could approach that proplem from multiple angles: code
   inspection of generated hotpatches, testing, and by making unloading
   a very special and exceptional operation.

If we follow that argument, we should consider patch unloading to be a
potentially unsafe operation which should not be part of standard
workflows.  Consequently, a similar argument holds for REPLACE, because
removing / unloading older code is part of replacement.

One could now either aim to have special inspection and testing
regarding unloading to lower the risk there, or:

Structure hotpatches as an ever-increasing stack of modules that are
loaded on top of each other.  This approach works around the unloading
problems as well as the temporary-unsafe problem outlined above.  Memory
usage would be similar to an ever-increasing monolithic hotpatch (+ some
fragmentation, - the temporary additional memory requirement for
replacing the monolithic hotpatch).


I agree that this could be an issue with the replace operation; in practice I don't believe it would be an issue (and it is the choice that both kGraft and kpatch use). Since it is trivial to implement the replace operation (it's already done in my branch), I think we should have both and let vendors choose.

I think we should also aim for a slightly more general concept than a stack, and let vendors implement the stack approach if they want. Here is what I wrote in an earlier mail:
* Each hypervisor has an embedded build id.
* Each binary patch has an embedded build id.
* The hypervisor should expose its build id and the build id of every loaded binary patch. * Each binary patch specifies one or more build ids on which it depends. These build ids can be either a hypervisor build id or another patch build id. The dependencies could either identified automatically or by a developer. * The userspace tool enforces dependency management (user can optionally force patch apply). I don't see any reason to involve the
hypervisor for dependency management.

The vendor could very easily implement a stack of patches with this approach (afaik this is sort of what ksplice does).

The one caveat with having multiple patches is that the patches need to be dynamically linked (e.g. consider a patch on top of a patch which introduces a new function). I've got a basically working version of dynamic linking of modules, but it is certainly more complex than static linking and we probably shouldn't aim for it in V1.

--
Ross Lagerwall

_______________________________________________
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®.