Re: [libvirt] [PATCH 13/27] Add API for running 'info migration' monitor command

On Thu, 2009-09-24 at 16:00 +0100, Daniel P. Berrange wrote: > * src/qemu/qemu_monitor.c, src/qemu/qemu_monitor.h: Add new > qemuMonitorGetMigrationStatus() command. > * src/qemu/qemu_driver.c: Use new qemuMonitorGetMigrationStatus() > command to check completion status. > --- > src/qemu/qemu_driver.c | 15 +++++-- > src/qemu/qemu_monitor_text.c | 91 ++++++++++++++++++++++++++++++++++++++++++ > src/qemu/qemu_monitor_text.h | 16 +++++++ > 3 files changed, 117 insertions(+), 5 deletions(-) > > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c > index ccc13c4..a6300c9 100644 > --- a/src/qemu/qemu_driver.c > +++ b/src/qemu/qemu_driver.c > @@ -6499,6 +6499,8 @@ qemudDomainMigratePerform (virDomainPtr dom, > char *info = NULL; > int ret = -1; > int paused = 0; > + int status; > + unsigned long long transferred, remaining, total; > > qemuDriverLock(driver); > vm = virDomainFindByUUID(&driver->domains, dom->uuid); > @@ -6562,14 +6564,17 @@ qemudDomainMigratePerform (virDomainPtr dom, > * rather failed later on. Check the output of "info migrate" > */ > VIR_FREE(info); > - if (qemudMonitorCommand(vm, "info migrate", &info) < 0) { > - qemudReportError (dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, > - "%s", _("could not get info about migration")); > + > + if (qemuMonitorGetMigrationStatus(vm, &status, > + &transferred, > + &remaining, > + &total) < 0) { > goto cleanup; > } > - if (strstr(info, "fail") != NULL) { > + > + if (status != QEMU_MONITOR_MIGRATION_STATUS_COMPLETED) { > qemudReportError (dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, > - _("migrate failed: %s"), info); > + "%s", _("migrate did not successfully complete")); > goto cleanup; > } > > diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c > index d9227a2..0b746b9 100644 > --- a/src/qemu/qemu_monitor_text.c > +++ b/src/qemu/qemu_monitor_text.c > @@ -28,6 +28,7 @@ > #include <sys/un.h> > #include <poll.h> > #include <unistd.h> > +#include <string.h> > > #include "qemu_monitor_text.h" > #include "qemu_conf.h" > @@ -997,3 +998,93 @@ cleanup: > VIR_FREE(cmd); > return ret; > } > + > + > +#define MIGRATION_PREFIX "Migration status: " > +#define MIGRATION_TRANSFER_PREFIX "transferred ram: " > +#define MIGRATION_REMAINING_PREFIX "remaining ram: " > +#define MIGRATION_TOTAL_PREFIX "total ram: " > + > +VIR_ENUM_DECL(qemuMonitorMigrationStatus) > +VIR_ENUM_IMPL(qemuMonitorMigrationStatus, > + QEMU_MONITOR_MIGRATION_STATUS_LAST, > + "inactive", "active", "completed", "failed", "cancelled") > + > +int qemuMonitorGetMigrationStatus(const virDomainObjPtr vm, > + int *status, > + unsigned long long *transferred, > + unsigned long long *remaining, > + unsigned long long *total) { You went a bit crazy here! New code to parse a bunch of stuff that is then ignored ...

Looks fine, but have you checked this format has always been the same? A comment showing the format being parsed would be good too