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

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



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.

> 
> +
> +        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;

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.

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

-- 
Eric Blake   eblake redhat com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


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