[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] Is seems necessary to pass "migratable=no/yes" to qemu.



On 2014/9/24 19:49, Ján Tomko wrote:

> On 09/24/2014 05:28 AM, zhang bo wrote:
>> The patch
>> http://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=de0aeafe9ce3eb414c8b5d3aa8995d776a2952de
>> removes invtsc flag in the host-model CPU.
>>
>> I'm wondering, will it be better to pass args "migratable=no/yes" to qemu, and let qemu complete the
>> remaining work? As that qemu has checked whether it's necessary to use invtsc or not.
> 
> The 'migratable' property is only for -cpu host (<cpu mode='host-passthrough'>
> in libvirt).
> 
> For mode='host-model', libvirt detects the model and features of the host CPU
> and passes it as -cpu <model>,+feat,+feat2,...
> so we can't leave that to QEMU.
> 
>> ----------
>> "invtsc is available only if using: -cpu host,migratable=no,+invtsc."
>> http://git.qemu.org/?p=qemu.git;a=commit;h=120eee7d1fdb2eba15766cfff7b9bcdc902690b4
>> ----------
>>
>> There's another problem, if we do not pass "migratable=no" to qemu.
>> Consider if we set host mode to pass-through
>>    ---------
>>       <cpu mode='host-passthrough'>
>>       </cpu>
>>    ---------
>> then the vm->def->cpu->features contains invtsc. however, qemu will automatically remove this cpu flag
>> as that "migration=no" is not passed to it. thus, the guest will not start up. This problem is in fact
>> caused by the patch:
>> http://libvirt.org/git/?p=libvirt.git;a=commit;h=fba6bc47cbcabbe08d42279691efb0dff3b9c997,
>> it forbids guest domain to start up if the host has INVTSC while the guest(qemu) does not.
> 
> Regardless of QEMU support for invtsc, I'm only able to start the domain,
> restore or migration fails.
> 
> As far as I know, only 'invtsc' is the problematic feature, because it both
> a) can appear in the host CPU (so libvirt assumes -cpu host will add it)
> b) is checked by qemuProcessVerifyGuestCPU (and libvirt complains when it's
> not there)
> 
> For other features, we only add them to qemu command line and let qemu filter
> out the unsupported ones.
> 
>>    -------------
>>         for (i = 0; def->cpu && i < def->cpu->nfeatures; i++) {
>>             virCPUFeatureDefPtr feature = &def->cpu->features[i];
>>
>>             if (feature->policy != VIR_CPU_FEATURE_REQUIRE)
>>                 continue;
>>
>>             if (STREQ(feature->name, "invtsc") &&
>>                 !cpuHasFeature(guestcpu, feature->name)) {
>>                 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>>                                _("host doesn't support invariant TSC"));
>>                 goto cleanup;
>>             }
>>         }
>>         break;
>>    --------------
>>
>>
>> In conclusion:
>> 1 Will it better to pass args "migratable=yes/no" to qemu rather than doing the mask-invtsc job in libvirt?
>> 2 If the guest has "pass-through" cpu mode, then it's unable to start up, because qemu removes invtsc, and
>> vm->def->cpu->features has it. It seems a BUG.
>>
> 
> I think the simplest fix for host-passthrough would be to apply the same
> filter host-model has.
> 
> But since using invtsc with host-passthrough requires both +invtsc and
> migratable=no, so we'd need to either add a 'migratable' option to
> host-passthrough (this would skip the filter and add migratable=on), or allow
> fine-tuning the features for host-passthrough too.
> 
> Jan
> 

   Additional to the 2 suggestions, will that be OK to remove the codes in qemuProcessVerifyGuestCPU that checks whether the vm->def has
   invtsc flag while qemu doesn't?

-             if (STREQ(feature->name, "invtsc") &&
-                 !cpuHasFeature(guestcpu, feature->name)) {
-                 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-                                _("host doesn't support invariant TSC"));
-                 goto cleanup;
-             }

   Removing these codes, plus with the solution that "add 'migratable' option to host-passthrough", it seems the problem would
   be gone, and invtsc would not be so 'distinctive' in libvirt any more.


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]