Re: [PATCH 7/7] tools/libxl: Allow dom0 to be destroyed

[Daniel De Graaf <dgdegra@tycho.nsa.gov>]

On 03/19/2014 07:02 AM, Ian Campbell wrote:
> On Tue, 2014-03-18 at 17:34 -0400, Daniel De Graaf wrote:
>> @@ -3076,12 +3076,13 @@ static void unpause_domain(uint32_t domid)
>>       libxl_domain_unpause(ctx, domid);
>>   }
>>
>> -static void destroy_domain(uint32_t domid)
>> +static void destroy_domain(uint32_t domid, int force)
>>   {
>>       int rc;
>>
>> -    if (domid == 0) {
>> -        fprintf(stderr, "Cannot destroy privileged domain 0.\n\n");
>> +    if (domid == 0 && !force) {
>> +        fprintf(stderr, "Destroying domain 0 is only supported in a"
>> +                " disaggregated environment and requires the -f flag.\n\n");
>
>>From a user point of view perhaps this would be clearer if worded
> something like:
>
>          Not destroying domain 0, use -f to force.

OK, I like that error message better.

> Is the error message on attempted self destruction informative? If not
> then append "This can only be done from another domain, e.g. a
> disaggregated toolstack domain".

libxl: error: libxl.c:1411:libxl__destroy_domid: xc_domain_pause failed for 0
libxl: error: libxl.c:1471:devices_destroy_cb: xc_domain_destroy failed for 0
libxl: error: libxl.c:1342:domain_destroy_callback: unable to destroy guest with domid 0
libxl: error: libxl.c:1269:domain_destroy_cb: destruction of domain 0 failed
destroy failed (rc=-3)

It seems that libxl should include the errno from the hypervisor (EINVAL
in this case) somewhere in its output.

> Last random thought: Does this code know or have the potential to know
> which domain it is running in?

Not easily; I discussed this with Ian Jackson for the v1 patch, and we
decided that determining your own domain ID here was too cumbersome.

> All of this info should also be added to the man page and the help text
> in xl_cmdtable.c needs to have the -f added to it.

Ah, yes, documentation... will do.

>>           exit(-1);
>>       }
>>       rc = libxl_domain_destroy(ctx, domid, 0);
>> @@ -4166,12 +4167,15 @@ int main_unpause(int argc, char **argv)
>>   int main_destroy(int argc, char **argv)
>>   {
>>       int opt;
>> +    int force = 0;
>>
>> -    SWITCH_FOREACH_OPT(opt, "", NULL, "destroy", 1) {
>> -        /* No options */
>> +    SWITCH_FOREACH_OPT(opt, "f", NULL, "destroy", 1) {
>> +    case 'f':
>> +        force = 1;
>> +        break;
>>       }
>>
>> -    destroy_domain(find_domain(argv[optind]));
>> +    destroy_domain(find_domain(argv[optind]), force);
>>       return 0;
>>   }


-- 
Daniel De Graaf
National Security Agency