[libvirt] Draft of pre migration checks framework
Hi guys, I'm working on a pre migration checks implementation. The patch files attached provide a framework to execute pre-migration-checks before the domain migration. ================================================================ 1 - FRAMEWORK OVERVIEW ================================================================ Pre-Migration-Checks are enabled by adding a "--pmc" option in "virsh migrate" command $ virsh migrate --pmc ... In this way, before the execution of function virDomainMigrate(), an array of "checks" are performed. Return value of every check set if the migration will continue or not. ================================================================ 2 - FRAMEWORK STRUCTURE ================================================================ Every check is defined in the file libvirt/src/util/pmc.c Check list is defined by object "chklist". This is a struct defined as static virPMCCheckList chklist = { .count = 2, .check = {&chk1,&chk2,NULL} }; where "count" define the current array size, and "check" is an array of pointer to virPMCCheck object. virPMCCheck is a stuct defined as struct _virPMCCheck { const char *name; virPMCCheckRun run; }; where "name" is the name of the check needed for debugging messages, and "run" is a pointer to check function. ================================================================ 3 - HOW TO ADD NEW CHECK ================================================================ Adding new check is very simple! You have to define a check funcion as static int pmcCheckFooCheck(virDomainPtr domain, virConnectPtr dconn) { /* do something */ return CHECK_SUCCESS } then you have to define a "check-hook" as static virPMCCheck chk3 = { .name = "this is fooCheck", .run = pmcCheckFooCheck }; and update the object "chklist" static virPMCCheckList chklist = { .count = 3, /* previous value was 2 */ .check = {&chk1,&chk2,&chk3,NULL} /* previous content not * include a pointer to * chk3 object */ }; ================================================================ 4 - POSSIBLE ENHANCHEMENT ================================================================ 1. Adding infos in _virPMCCheck struct about check security level. Example: if a check with level CRITICAL return CHECK_FAIL, migration process is stopped, 2. Implementing framework not as util, but as driver. For example this can permit to define new checks by xml file. 3. Define external checks loadable by shared library. ================================================================ 5 - PATCH FILE DESCRIPTION ================================================================ file: datatypes.h.patch desc: define new type virPMCCheckRun define struct _virPMCCheck define struct _virPMCCheckList file: libvirt.c.patch desc: impement public API virDomainMigratePMC() file: libvirt.h.in.patch desc: impement public API virDomainMigratePMC() prototype define type virPMCCheck define type virPMCCheckPtr define type virPMCCheckList define type virPMCCheckListPtr define macro PMC_LIST_SIZE file: libvirt_public.syms desc: export public API virDomainMigratePMC() file: pmc.c.patch, pmc.h.patch desc: define and implements pre migration checks and some auxiliary functions. file: virsh.c.patch desc: add option "pmc" to virsh migrate command ================================================================ 6 - NOTES ================================================================ I start to implement this framework on git sources, but the latest sources produce a regress on my code. For this reason I applied my code on latest stable release of libvirt (0.8.1). What do you think about that? Is this a good approach to implement pre migration checks? Have you a suggestions? There is a possibility to include this patch in future libvirt distributions? Waiting for feedback... Paolo Smiraglia -- PAOLO SMIRAGLIA http://portale.isf.polito.it/paolo-smiraglia
On 05/21/2010 07:16 AM, Paolo Smiraglia wrote:
Hi guys, I'm working on a pre migration checks implementation.
The patch files attached provide a framework to execute pre-migration-checks before the domain migration.
First, thanks for the efforts (sometimes, as open source developers, we get too caught up in our own work, and forget to acknowledge the work of others; at which point our reviews come across as overly-critical, even though that is not the intent). Rather than sending lots of attachments, one per file, it would be nicer to send one commit (if this is all atomic) or a series of commit files (incremental atomic changes), and preferably inline to make it easier to respond to files in email. 'git send-email' can be quite handy in this regards. The file HACKING has some more tips for submitting patches that are easier to review.
================================================================ 1 - FRAMEWORK OVERVIEW ================================================================ Pre-Migration-Checks are enabled by adding a "--pmc" option in "virsh migrate" command
$ virsh migrate --pmc ...
In this way, before the execution of function virDomainMigrate(), an array of "checks" are performed. Return value of every check set if the migration will continue or not.
How does this fit in with the recent addition of hooks? While you've done a good job explaining _what_ your patch does, I'm still not sure you've done a good job of saying _why_ we need it, and how it solves problems not already solvable with the hooks mechanism.
Adding new check is very simple! You have to define a check funcion as
static int pmcCheckFooCheck(virDomainPtr domain, virConnectPtr dconn) { /* do something */ return CHECK_SUCCESS }
then you have to define a "check-hook" as
static virPMCCheck chk3 = { .name = "this is fooCheck", .run = pmcCheckFooCheck };
This requires recompilation, whereas the hooks mechanism allows addition of a new hook via editing an external file with no recompilation. I'm still wondering why a builtin mechanism is more extensible than something that uses an external file. Caveat - I have not glanced at any of your patch files, neither for style reviews nor for algorithmic review. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
First, thanks for the efforts (sometimes, as open source developers, we get too caught up in our own work, and forget to acknowledge the work of others; at which point our reviews come across as overly-critical, even though that is not the intent).
Thank you for the fast answer! I preferred to learn the hook mechanism before to send you my reply.
Rather than sending lots of attachments, one per file, it would be nicer to send one commit (if this is all atomic) or a series of commit files (incremental atomic changes), and preferably inline to make it easier to respond to files in email. 'git send-email' can be quite handy in this regards.
The file HACKING has some more tips for submitting patches that are easier to review.
Ok! I will bear in mind for the next patch.... ;-)
... I'm still not sure you've done a good job of saying _why_ we need it, and how it solves problems not already solvable with the hooks mechanism.
This requires recompilation, whereas the hooks mechanism allows addition of a new hook via editing an external file with no recompilation. I'm still wondering why a builtin mechanism is more extensible than something that uses an external file.
I started my work before the introducion of hooks, and for this reason I didn't consider them as a possible developing way. Besides the core of my work is the "security" of migration process, and for this reason I think that a built-in solution application is better then one external. I've a second version of my code wich uses shared objects and the function ldopen(). In this way only one libvirt code recompilation is required: just to add a ldopen() support. All checks are implemented as shared objects in external file. So the code previously posted can be used as hook-like mechanism. To respect the libvirt-guidelines I've also implemented a third version of my code. It's a draft implementation of "qemu driver hook". Before sending the patch I would prefer to discuss with you my hook design. I supposed that my hook recives from libvirt the parameters listed below: object: name of domain to migrate operation: migrate suboperation: pmc (this enable pre migration checks) extra: destination uri The hook is called during migration process in particular in function qemudDomainMigratePerform() - /src/qemu/qemu_driver.c Hook performs some checks listed in libvirt site http://wiki.libvirt.org/page/TodoPreMigrationChecks moreover, I defiend a simple check (not listed in libvirt site) that uses Trusted Platform Module (TPM) measurations (quote). This is the real core of my work! Now I'm try to convince you that my work is a "what you need"! :-D Immagine the scenario: we have to migrate a VM to another remote host. This VM hosts a database whose data are privates (like bank account data). As well as to ckeck the libvirt compatibility (capabilities, network configuration, ...) we need to check also the "security" of the remote host. We can perform this check by using Trusted Platform Module (TPM) measurations. For example, we can compare a "good quote" of remote host stored on local host, with quote recived by remote host during the check. Here a simple graph that represent tpm check phases: http://pastebin.com/qG6GUXBa In this way we are sure that the remote and local host are equals both libirt-compatibility-side and security-side! Did I convince you? I hope your answer is "Yes!" ;-) What about my hook design? Any suggetstions | criticisms |questions | swearwords are appreciated?
Caveat - I have not glanced at any of your patch files, neither for style reviews nor for algorithmic review.
Mmmm....for this reason I don't send you invitation for my saturday party! ;-) -- PAOLO SMIRAGLIA http://portale.isf.polito.it/paolo-smiraglia
participants (2)
-
Eric Blake -
Paolo Smiraglia