xen discuss - Apr 2008 - webrev: 6664938 xend needs to run qemu instances in a new contract

Can I please get reviews for:

6664938 xend needs to run qemu instances in a new contract
http://cr.opensolaris.org/~johnlev/qemu-contract/

thanks
john

John,

I am sorry I took so long to get back to you.
I only looked at the contract operations in contract.c and assumed 
everything that used Python was correct since I am not familiar with the 
Python-C interface. Please find my comments below.

contract.c: 157
   The call to ct_pr_tmpl_set_svc_aux() can fail. If it happens, the 
auxiliary field will revert to the default value, empty string, in the 
current code. If xend can cope with a contract that has 
svc:/system/xvm/xend as service FMRI and "" as auxiliary field, there 
will be no compelling reason to handle the error, but you should not 
ignore the value. Use (void).

close(2) and ct_ctl_abandon(3contract) return int but is ignored.

One way to avoid the abandon_contract() code, it to set the inherit 
parameter for the process contracts holding the qemu process. Just call
ct_pr_tmpl_set_param(cfd, CT_PR_INHERIT)
before you ct_tmpl_activate()

This way, if xend is restarted, svc.startd will transfer the qemu 
contracts to the contract holding xend. If desired, after xend is 
restarted, it can adopt the qemu contracts from the process contract 
xend is member of. I attached an example based on your code exemplifying 
the behavior described above. Please let me know if you have questions.


Antonello


John Levon wrote:
> Can I please get reviews for:
> 
> 6664938 xend needs to run qemu instances in a new contract
> http://cr.opensolaris.org/~johnlev/qemu-contract/
> 
> thanks
> john

On Sun, Apr 27, 2008 at 10:31:54PM -0700, Antonello Cruz wrote:
> contract.c: 157
>   The call to ct_pr_tmpl_set_svc_aux() can fail. If it happens, the 
> auxiliary field will revert to the default value, empty string, in the 
> current code. If xend can cope with a contract that has 
> svc:/system/xvm/xend as service FMRI and "" as auxiliary field,
there
> will be no compelling reason to handle the error, but you should not 
> ignore the value. Use (void).
> close(2) and ct_ctl_abandon(3contract) return int but is ignored.
We don''t want to handle the error indeed. I''m not using return
value
casts here because this code is not (and will never be) linted. The
style of upstream (both Python and Xen) is to use GCC extensions for
functions where ignoring the result is always a bad idea, and not use
this lint cast hack.
> One way to avoid the abandon_contract() code, it to set the inherit 
> parameter for the process contracts holding the qemu process. Just call
> ct_pr_tmpl_set_param(cfd, CT_PR_INHERIT)
> before you ct_tmpl_activate()
> 
> This way, if xend is restarted, svc.startd will transfer the qemu 
> contracts to the contract holding xend. If desired, after xend is 
> restarted, it can adopt the qemu contracts from the process contract 
> xend is member of. I attached an example based on your code exemplifying 
> the behavior described above. Please let me know if you have questions.
If it inherits, won''t that mean that restarting xend''s owner
(svc.startd) will then kill the qemu processes? We basically want them
to keep running at all times.

regards
john

John Levon wrote:
> On Sun, Apr 27, 2008 at 10:31:54PM -0700, Antonello Cruz wrote:
>> One way to avoid the abandon_contract() code, it to set the inherit 
>> parameter for the process contracts holding the qemu process. Just call
>> ct_pr_tmpl_set_param(cfd, CT_PR_INHERIT)
>> before you ct_tmpl_activate()
>>
>> This way, if xend is restarted, svc.startd will transfer the qemu 
>> contracts to the contract holding xend. If desired, after xend is 
>> restarted, it can adopt the qemu contracts from the process contract 
>> xend is member of. I attached an example based on your code
exemplifying
>> the behavior described above. Please let me know if you have questions.
> 
> If it inherits, won''t that mean that restarting xend''s
owner
> (svc.startd) will then kill the qemu processes? We basically want them
> to keep running at all times.
If xend is restarted and the qemu processes are in their own contract 
with the inherit bit set, they will not be killed by svc.startd. You can 
run the test code I sent you and check for yourself. Even better, just 
set the inherit bit in your create_contract() code and don''t call your 
abandon_contract() function to test.

Antonello

On Thu, Apr 24, 2008 at 02:49:37AM +0100, John Levon wrote:
> Can I please get reviews for:
> 
> 6664938 xend needs to run qemu instances in a new contract
> http://cr.opensolaris.org/~johnlev/qemu-contract/
I''ve updated this to use contract inheritance as Antonello suggested.

thanks
john

On Mon, Apr 28, 2008 at 07:47:42PM +0100, John Levon wrote:
> > Can I please get reviews for:
> > 
> > 6664938 xend needs to run qemu instances in a new contract
> > http://cr.opensolaris.org/~johnlev/qemu-contract/
> 
> I''ve updated this to use contract inheritance as Antonello
suggested.
Ping, Antonello, are you OK with the new version?

thanks
john

John Levon wrote:
> On Mon, Apr 28, 2008 at 07:47:42PM +0100, John Levon wrote:
> 
>>> Can I please get reviews for:
>>>
>>> 6664938 xend needs to run qemu instances in a new contract
>>> http://cr.opensolaris.org/~johnlev/qemu-contract/
>> I''ve updated this to use contract inheritance as Antonello
suggested.
> 
> Ping, Antonello, are you OK with the new version?
Yes I am. I also looked at the Python glue and it seems ok, but I had no 
time to test. I assume you''ve done the proper testing, right?

Antonello