[Libvirt-cim] [PATCH 3 of 4] Add MigrationType support to VirtualSystemMigrationService

[Kaitlin Rupert]

Heidi Eckhart wrote:
> Kaitlin Rupert wrote:
>> @@ -424,25 +448,39 @@ static CMPIStatus migrate_vs(struct migr
>>                  goto out;
>>          }
>>
>> -        CU_DEBUG("Migrating %s -> %s", job->domain, uri);
>> -
>> -        ddom = virDomainMigrate(dom, dconn, VIR_MIGRATE_LIVE, NULL, 
>> NULL, 0);
>> -        if (ddom == NULL) {
>> -                CU_DEBUG("Migration failed");
>> -                cu_statusf(_BROKER, &s,
>> -                           CMPI_RC_ERR_FAILED,
>> -                           "Migration Failed");
>> -                goto out;
>> -        }
>> +        switch(job->type) {
>> +        case CIM_MIGRATE_OTHER:
>> +                /* FIXME - Handle offline migration here */
>> +                CU_DEBUG("Preparing for offline migration");
>> +                break;
>> +        case CIM_MIGRATE_LIVE:
>> +                CU_DEBUG("Preparing for live migration");
>> +                s = handle_migrate(dconn, dom, uri, VIR_MIGRATE_LIVE, 
>> job);
>>   
> Is the type CIM_MIGRATE_LIVE or VIR_MIGRATE_LIVE ?

The CIM_MIGRATE_LIVE corresponds to the value defined in the mof, which 
is 2.  However, the value of VIR_MIGRATE_LIVE is defined by libvirt.

So when someone specifies a migration type of 2 (CIM_MIGRATE_LIVE) in 
the MigrationSettingData, we'll need to use VIR_MIGRATE_LIVE when we 
call virDomainMigrate().

>> +                break;
>> +        case CIM_MIGRATE_RESUME:
>> +        case CIM_MIGRATE_RESTART:
>> +                CU_DEBUG("Preparing for static migration");
>> +                s = handle_migrate(dconn, dom, uri, 0, job);
>>   
> The type should be CIM_MIGRATE_RESTART


For static migration, libvirt uses a value of 0. There's two type values 
here.. the value set for the MigrationType attribute of 
MigrationSettingData and the value libvirt uses to differentiate between 
static and live migration.

But it looks like the way I have it coded is confusing.  Would the 
following make more sense:

static CMPIStatus handle_migrate(virConnectPtr dconn,
                                  virDomainPtr dom,
                                  char *uri,
                                  int type,
                                  struct migration_job *job)
{
         CMPIStatus s = {CMPI_RC_OK, NULL};
         virDomainPtr ddom = NULL;

         CU_DEBUG("Migrating %s -> %s", job->domain, uri);

         if (type == CIM_MIGRATE_LIVE)
                 ddom = virDomainMigrate(dom, dconn, VIR_MIGRATE_LIVE, 
NULL, NULL, 0);
         else if (type == CIM_MIGRATE_RESUME) || (type == 
CIM_MIGRATE_RESTART)
                 ddom = virDomainMigrate(dom, dconn, 0, NULL, NULL, 0);
	else
		//Print error here and return

         if (ddom == NULL) {
                 CU_DEBUG("Migration failed");
                 cu_statusf(_BROKER, &s,
                            CMPI_RC_ERR_FAILED,
                            "Migration Failed");
         }

         virDomainFree(ddom);

         return s;
}

And then in migrate_vs() have:

         switch(job->type) {
         case CIM_MIGRATE_OTHER:
                 /* FIXME - Handle offline migration here */
                 CU_DEBUG("Preparing for offline migration");
                 break;
         case CIM_MIGRATE_LIVE:
         case CIM_MIGRATE_RESUME:
         case CIM_MIGRATE_RESTART:
                 s = handle_migrate(dconn, dom, uri, job->type, job);
                 break;
         default:
                 CU_DEBUG("Unsupported migration type (%d)", job->type);
                 cu_statusf(_BROKER, &s,
                            CMPI_RC_ERR_FAILED,
                            "Unsupported migration type (%d)", job->type);
                 goto out;
         }

It is a little more redundant this way, but it might help with readability.

>> +                break;
>> +        default: +                CU_DEBUG("Unsupported migration 
>> type (%d)", job->type);
>> +                cu_statusf(_BROKER, &s,
>> +                           CMPI_RC_ERR_FAILED,
>> +                           "Unsupported migration type (%d)", 
>> job->type);
>> +                goto out;
>> +        }
>> +
>> +        if (s.rc != CMPI_RC_OK)
>> +                goto out;
>>
>>  
> 
> 


-- 
Kaitlin Rupert
IBM Linux Technology Center
kaitlin at linux.vnet.ibm.com