Re: [PATCH v10 11/11] (lib)xl: soft reset support

[Vitaly Kuznetsov <vkuznets@redhat.com>]

Wei Liu <wei.liu2@citrix.com> writes:

[skip]

>>  
>> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
>> index ff0d616..5f5559b 100644
>> --- a/tools/libxl/libxl.c
>> +++ b/tools/libxl/libxl.c
>> @@ -1478,6 +1478,7 @@ void libxl__domain_destroy(libxl__egc *egc, libxl__domain_destroy_state *dds)
>>          dds->stubdom.ao = ao;
>>          dds->stubdom.domid = stubdomid;
>>          dds->stubdom.callback = stubdom_destroy_callback;
>> +        dds->stubdom.soft_reset = false;
>>          libxl__destroy_domid(egc, &dds->stubdom);
>>      } else {
>>          dds->stubdom_finished = 1;
>> @@ -1486,6 +1487,7 @@ void libxl__domain_destroy(libxl__egc *egc, libxl__domain_destroy_state *dds)
>>      dds->domain.ao = ao;
>>      dds->domain.domid = dds->domid;
>>      dds->domain.callback = domain_destroy_callback;
>> +    dds->domain.soft_reset = dds->soft_reset;
>>      libxl__destroy_domid(egc, &dds->domain);
>>  }
>
> OOI have you tested stubdom case?
>

No, but ...

> Of course stubdom.soft_reset should be set to false, but do you really
> want to destroy stubdom?
>
> Sorry if my question is dumb or the answer is obvious. I expect stubdom
> QEMU would be rebuilt just like normal QEMU running in Dom0? From
> reading the code I think this is the case, but I prefer confirmation
> from you.

Yes, we need to restart domain's device model in all cases: 
1) We need it to release all memory mappings it has, and
2) XEN_DOMCTL_soft_reset will close all the existent event channels and
there is no way to ask QEMU to re-bind them.
Domain create path will re-launch QEMU and re-create stubdom if
required.

This actually resembles the migration case: we don't migrate stubdomains,
we re-create them.

>
> This is the only concern I have with this patch.
>

Thanks for the review! I'll fix all the issues you spotted and try to
test stubdomain case before sending out v11.

[skip]

-- 
  Vitaly