[libvirt] [PATCH 14/27] Add API for issuing a 'migrate' monitor command for TCP

[Mark McLoughlin]

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 API
>   qemuMonitorMigrateToHost() for doing TCP migration
> * src/qemu/qemu_driver.c: Convert to use qemuMonitorMigrateToHost().
>   Also handle proper URIs (tcp:// as well as tcp:)
> ---
>  src/qemu/qemu_driver.c       |   40 ++++++++++++-----------------
>  src/qemu/qemu_monitor_text.c |   56 ++++++++++++++++++++++++++++++++++++++++++
>  src/qemu/qemu_monitor_text.h |    4 +++
>  3 files changed, 77 insertions(+), 23 deletions(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index a6300c9..f234639 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -6494,12 +6494,10 @@ qemudDomainMigratePerform (virDomainPtr dom,
>      struct qemud_driver *driver = dom->conn->privateData;
>      virDomainObjPtr vm;
>      virDomainEventPtr event = NULL;
> -    char *safe_uri;
> -    char cmd[HOST_NAME_MAX+50];
> -    char *info = NULL;
>      int ret = -1;
>      int paused = 0;
>      int status;
> +    xmlURIPtr uribits = NULL;
>      unsigned long long transferred, remaining, total;
>  
>      qemuDriverLock(driver);
> @@ -6537,34 +6535,29 @@ qemudDomainMigratePerform (virDomainPtr dom,
>          goto cleanup;
>  
>      /* Issue the migrate command. */
> -    safe_uri = qemudEscapeMonitorArg (uri);
> -    if (!safe_uri) {
> -        virReportOOMError (dom->conn);
> -        goto cleanup;
> +    if (STRPREFIX(uri, "tcp:") && !STRPREFIX(uri, "tcp://")) {
> +        char *tmpuri;
> +        if (virAsprintf(&tmpuri, "tcp://%s", uri + strlen("tcp:")) < 0) {
> +            virReportOOMError(dom->conn);
> +            goto cleanup;
> +        }
> +        uribits = xmlParseURI(tmpuri);
> +        VIR_FREE(tmpuri);
> +    } else {
> +        uribits = xmlParseURI(uri);

This is all new stuff and there's no explanation in the ChangeLog; not
sure why you're so keen to split up the URI only to re-construct it
again

It looks okay, though, just made it harder to review the patch

>      }
> -    snprintf (cmd, sizeof cmd, "migrate \"%s\"", safe_uri);
> -    VIR_FREE (safe_uri);
> -
> -    if (qemudMonitorCommand (vm, cmd, &info) < 0) {
> -        qemudReportError (dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
> -                          "%s", _("migrate operation failed"));
> +    if (!uribits) {
> +        qemudReportError(dom->conn, dom, NULL, VIR_ERR_INTERNAL_ERROR,
> +                         _("cannot parse URI %s"), uri);
>          goto cleanup;
>      }
>  
> -    DEBUG ("%s: migrate reply: %s", vm->def->name, info);
> -
> -    /* Now check for "fail" in the output string */
> -    if (strstr(info, "fail") != NULL) {
> -        qemudReportError (dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
> -                          _("migrate failed: %s"), info);
> +    if (qemuMonitorMigrateToHost(vm, uribits->server, uribits->port) < 0)
>          goto cleanup;
> -    }
>  
>      /* it is also possible that the migrate didn't fail initially, but
>       * rather failed later on.  Check the output of "info migrate"
>       */
> -    VIR_FREE(info);
> -
>      if (qemuMonitorGetMigrationStatus(vm, &status,
>                                        &transferred,
>                                        &remaining,
> @@ -6608,7 +6601,8 @@ cleanup:
>                                           VIR_DOMAIN_EVENT_RESUMED_MIGRATED);
>      }
>  
> -    VIR_FREE(info);
> +    if (uribits)
> +        xmlFreeURI(uribits);
>      if (vm)
>          virDomainObjUnlock(vm);
>      if (event)
> diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c
> index 0b746b9..4f8d72e 100644
> --- a/src/qemu/qemu_monitor_text.c
> +++ b/src/qemu/qemu_monitor_text.c
> @@ -1088,3 +1088,59 @@ cleanup:
>      VIR_FREE(reply);
>      return ret;
>  }
> +
> +
> +static int qemuMonitorMigrate(const virDomainObjPtr vm,
> +                              const char *dest)
> +{
> +    char *cmd = NULL;
> +    char *info = NULL;
> +    int ret = -1;
> +
> +    if (virAsprintf(&cmd, "migrate %s", cmd) < 0) {

Should be passing dest here

Fixed in the next patch, but would be good to get it right for
bisectability

Otherwise, ACK

Cheers,
Mark.