[libvirt] [PATCH] qemu: Introduce two new job types
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. * src/qemu/qemu_driver.c --- src/qemu/qemu_driver.c | 39 ++++++++++++++++++++++++++++++--------- 1 files changed, 30 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 19ce9a6..aed48f4 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -104,6 +104,8 @@ enum qemuDomainJob { QEMU_JOB_UNSPECIFIED, QEMU_JOB_MIGRATION_OUT, QEMU_JOB_MIGRATION_IN, + QEMU_JOB_SAVE, + QEMU_JOB_DUMP, }; enum qemuDomainJobSignals { @@ -5389,21 +5391,36 @@ qemuDomainWaitForMigrationComplete(struct qemud_driver *driver, virDomainObjPtr struct timespec ts = { .tv_sec = 0, .tv_nsec = 50 * 1000 * 1000ull }; struct timeval now; int rc; + const char *job; + + 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); goto cleanup; } 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); qemuDomainObjEnterMonitorWithDriver(driver, vm); rc = qemuMonitorMigrateCancel(priv->mon); qemuDomainObjExitMonitorWithDriver(driver, vm); if (rc < 0) { - VIR_WARN0("Unable to cancel migration"); + VIR_WARN("Unable to cancel %s", job); } } else if (priv->jobSignals & QEMU_JOB_SIGNAL_SUSPEND) { priv->jobSignals ^= QEMU_JOB_SIGNAL_SUSPEND; @@ -5427,8 +5444,8 @@ qemuDomainWaitForMigrationComplete(struct qemud_driver *driver, virDomainObjPtr * guest to die */ 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); goto cleanup; } @@ -5459,7 +5476,7 @@ qemuDomainWaitForMigrationComplete(struct qemud_driver *driver, virDomainObjPtr case QEMU_MONITOR_MIGRATION_STATUS_INACTIVE: priv->jobInfo.type = VIR_DOMAIN_JOB_NONE; qemuReportError(VIR_ERR_OPERATION_FAILED, - "%s", _("Migration is not active")); + _("%s is not active"), job); break; case QEMU_MONITOR_MIGRATION_STATUS_ACTIVE: @@ -5480,13 +5497,13 @@ qemuDomainWaitForMigrationComplete(struct qemud_driver *driver, virDomainObjPtr case QEMU_MONITOR_MIGRATION_STATUS_ERROR: priv->jobInfo.type = VIR_DOMAIN_JOB_FAILED; qemuReportError(VIR_ERR_OPERATION_FAILED, - "%s", _("Migration unexpectedly failed")); + _("%s unexpectedly failed"), job); break; case QEMU_MONITOR_MIGRATION_STATUS_CANCELLED: priv->jobInfo.type = VIR_DOMAIN_JOB_CANCELLED; qemuReportError(VIR_ERR_OPERATION_FAILED, - "%s", _("Migration was cancelled by client")); + _("%s was cancelled by client"), job); break; } @@ -5606,6 +5623,8 @@ static int qemudDomainSaveFlag(struct qemud_driver *driver, virDomainPtr dom, goto endjob; } + priv->jobActive = QEMU_JOB_SAVE; + memset(&priv->jobInfo, 0, sizeof(priv->jobInfo)); priv->jobInfo.type = VIR_DOMAIN_JOB_UNBOUNDED; @@ -6198,6 +6217,8 @@ static int qemudDomainCoreDump(virDomainPtr dom, goto endjob; } + priv->jobActive = QEMU_JOB_DUMP; + /* Migrate will always stop the VM, so the resume condition is independent of whether the stop command is issued. */ resume = (vm->state == VIR_DOMAIN_RUNNING); -- 1.7.3.2
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
于 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.
On 12/10/2010 09:40 AM, Osier Yang wrote:
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.
The only other idea I have is to rearrange the error messages to be a form of concatenated complete phrases (and there's precedence for this). First, let's figure out if these look like reasonable strings, where everything left of : can be translated independently, and everything right of : can be translated independently, and the results still make sense when the two pieces are strung together.
+ qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("guest unexpectedly quit during %s"), job); goto cleanup;
xyz job: guest unexpectedly quit
- "%s", _("Migration is not active")); + _("%s is not active"), job); break;
xyz job: job not active
- "%s", _("Migration was cancelled by client")); + _("%s was cancelled by client"), job); break;
xyz job: canceled by client Then this is easy to do: case QEMU_JOB_MIGRATION_OUT: job = _("migration job"); break; case QEMU_JOB_SAVE: job = _("domain save job"); break; qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s: %s", job, _("guest unexpectedly quit")); (as for canceled vs. cancelled, that's a US vs. UK spelling thing; we've made patches in the past to change to US spelling within translated strings to give translators a single dialect to start from). -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
The only other idea I have is to rearrange the error messages to be a form of concatenated complete phrases (and there's precedence for this). First, let's figure out if these look like reasonable strings, where everything left of : can be translated independently, and everything right of : can be translated independently, and the results still make sense when the two pieces are strung together.
+ qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("guest unexpectedly quit during %s"), job); goto cleanup;
xyz job: guest unexpectedly quit
- "%s", _("Migration is not active")); + _("%s is not active"), job); break;
xyz job: job not active
- "%s", _("Migration was cancelled by client")); + _("%s was cancelled by client"), job); break;
xyz job: canceled by client
Then this is easy to do: case QEMU_JOB_MIGRATION_OUT: job = _("migration job"); break; case QEMU_JOB_SAVE: job = _("domain save job"); break;
qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s: %s", job, _("guest unexpectedly quit"));
hmm, yeah, this is a good idea
(as for canceled vs. cancelled, that's a US vs. UK spelling thing; we've made patches in the past to change to US spelling within translated strings to give translators a single dialect to start from).
good to known, will send v2 patch with these changes, thanks. - Osier
participants (2)
-
Eric Blake -
Osier Yang