[libvirt] [PATCH] qemu: Introduce two new job types

[Osier Yang]

于 2010年12月11日 00:12, Eric Blake 写道:
> On 12/10/2010 01:43 AM, Osier Yang wrote:
>> Currently, all of domain "save/dump/managed save/migration"
>> use the same function "qemudDomainWaitForMigrationComplete"
>> to wait the job finished, but the error messages are all
>> about "migration", e.g. when a domain saving job is canceled
>> by user, "migration was cancled by client" will be throwed as
>> an error message, which will be confused for user.
>>
>> As a solution, intoduce two new job types(QEMU_JOB_SAVE,
>> QEMU_JOB_DUMP), and set "priv->jobActive" to "QEMU_JOB_SAVE"
>> before saving, to "QEMU_JOB_DUMP" before dumping, so that we
>> could get the real job type in
>> "qemudDomainWaitForMigrationComplete", and give more clear
>> message further.
>
> NACK - needs a v2, or the translators will come after you.
>

Guess it is.. :-)

>>
>> +
>> +        switch (priv->jobActive) {
>> +            case QEMU_JOB_MIGRATION_OUT:
>> +                job = "migration";
>> +                break;
>> +            case QEMU_JOB_SAVE:
>> +                job = "domain saving";
>> +                break;
>> +            case QEMU_JOB_DUMP:
>> +                job = "domain core dump";
>> +                break;
>> +            default:
>> +                job = "job";
>> +        }
>>
>>           if (!virDomainObjIsActive(vm)) {
>> -            qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> -                            _("guest unexpectedly quit during migration"));
>> +            qemuReportError(VIR_ERR_INTERNAL_ERROR,
>> +                            _("guest unexpectedly quit during %s"), job);
>
> You failed to translate the string in job.  Also, it's bad form to
> piece-meal sentences together, since some languages change the
> translation of the rest of the sentence depending on the gender of the
> phrase you placed in job.  Rather, you should set job to the entire
> translated string:
>
> case QEMU_JOB_MIGRATION_OUT:
>      job = _("guest unexpectedly quit during migration");
>      break;
> case QEMU_JOB_SAVE:
>      job = _("guest unexpectedly quit during domain saving");
>      break;
>

I realized this problem, but "job" will be used in many places,
it's hard to make a uniform translated string for all of them,
unless we try to define many diffrent variables, e.g. job0, job1,
etc. but that's really ugly.

Do you have any further idea? Thanks a lot.

> qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", job);
>
>>
>>           if (priv->jobSignals&  QEMU_JOB_SIGNAL_CANCEL) {
>>               priv->jobSignals ^= QEMU_JOB_SIGNAL_CANCEL;
>> -            VIR_DEBUG0("Cancelling migration at client request");
>> +            VIR_DEBUG("Cancelling %s at client request", job);
>
> Then again, you're also using job in the (intentionally untranslated)
> debug string.  So it almost looks like you need two strings; the short
> untranslated English string for VIR_DEBUG and the full translated
> sentence for qemuReportError.
>

Yeah, I guess for debug, it's no need to make translated string.

> But I like the idea, so looking forward to v2.
>