Re: [PATCH v4 8/9] libxl: soft reset support

[Vitaly Kuznetsov <vkuznets@redhat.com>]

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

> (I've skipped the internal implementation since I don't know what's
>  required to fulfil soft reset.)
>
> On Wed, Dec 03, 2014 at 06:16:20PM +0100, Vitaly Kuznetsov wrote:
> [...]
>> +                                         libxl__domain_create_state *dcs);
>>  
>>  /* Each time the dm needs to be saved, we must call suspend and then save */
>>  _hidden int libxl__domain_suspend_device_model(libxl__gc *gc,
>> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
>> index 53611dc..eb833f0 100644
>> --- a/tools/libxl/xl_cmdimpl.c
>> +++ b/tools/libxl/xl_cmdimpl.c
>> @@ -2043,7 +2043,8 @@ static void reload_domain_config(uint32_t domid,
>>  }
>>  
>>  /* Returns 1 if domain should be restarted,
>> - * 2 if domain should be renamed then restarted, or 0
>> + * 2 if domain should be renamed then restarted,
>> + * 3 if domain performed soft reset, or 0
>>   * Can update r_domid if domain is destroyed etc */
>>  static int handle_domain_death(uint32_t *r_domid,
>>                                 libxl_event *event,
>> @@ -2069,6 +2070,9 @@ static int handle_domain_death(uint32_t *r_domid,
>>      case LIBXL_SHUTDOWN_REASON_WATCHDOG:
>>          action = d_config->on_watchdog;
>>          break;
>> +    case LIBXL_SHUTDOWN_REASON_SOFT_RESET:
>> +        LOG("Domain performed soft reset.");
>> +        return 3;
>
> Would it be useful to provide "on_soft_reset" option in xl? Will the
> admin be interested in performing some other action when domain does
> soft reset? Say, for security reason admin want to prohibit domain from
> soft resetting itself.
>

Makes sense, let's add it.

>>      default:
>>          LOG("Unknown shutdown reason code %d. Destroying domain.",
>>              event->u.domain_shutdown.shutdown_reason);
>> @@ -2285,6 +2289,7 @@ static void evdisable_disk_ejects(libxl_evgen_disk_eject **diskws,
>>  static uint32_t create_domain(struct domain_create *dom_info)
>>  {
>>      uint32_t domid = INVALID_DOMID;
>> +    uint32_t domid_old = INVALID_DOMID;
>>  
>>      libxl_domain_config d_config;
>>  
>> @@ -2510,7 +2515,18 @@ start:
>>           * restore/migrate-receive it again.
>>           */
>>          restoring = 0;
>> -    }else{
>> +    } else if (domid_old != INVALID_DOMID) {
>> +        /* Do soft reset */
>> +        d_config.b_info.nodemap.size = 0;
>
> What's the reason for doing this?
>
> If you encounter problem with this it should probably be fixed in
> libxl.

Ah, sorry, I forgot about this hackaround (which was required since
194e7183 if I'm not mistaken). The root cause is that
reload_domain_config() was missing on soft_reset path and we were
hitting "Can run NUMA placement only if the domain does not have any
NUMA node affinity set already" clause.

I will fix this along with "on_soft_reset" implementation.

>
> Wei.
>
>> +        ret = libxl_domain_soft_reset(ctx, &d_config,
>> +                                      &domid, domid_old,
>> +                                      0, 0);
>> +
>> +        if ( ret ) {
>> +            goto error_out;
>> +        }
>> +        domid_old = INVALID_DOMID;
>> +    } else {
>>          ret = libxl_domain_create_new(ctx, &d_config, &domid,
>>                                        0, autoconnect_console_how);
>>      }
>> @@ -2574,6 +2590,8 @@ start:
>>                  event->u.domain_shutdown.shutdown_reason,
>>                  event->u.domain_shutdown.shutdown_reason);
>>              switch (handle_domain_death(&domid, event, &d_config)) {
>> +            case 3:
>> +                domid_old = domid;
>>              case 2:
>>                  if (!preserve_domain(&domid, event, &d_config)) {
>>                      /* If we fail then exit leaving the old domain in place. */
>> -- 
>> 1.9.3
>> 
>> 
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xen.org
>> http://lists.xen.org/xen-devel

-- 
  Vitaly