Re: [PATCH 2/9] Add a second URI parameter to virDomainMigratePerform3 method

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

 



2011/5/24 Daniel P. Berrange <berrange@xxxxxxxxxx>:
> The virDomainMigratePerform3 currently has a single URI parameter
> whose meaning varies. It is either
>
> Â- A QEMU migration URI (normal migration)
> Â- A libvirtd connection URI (peer2peer migration)
>
> Unfortunately when using peer2peer migration, without also
> using tunnelled migration, it is possible that both URIs are
> required.
>
> This adds a second URI parameter to the virDomainMigratePerform3
> method, to cope with this scenario. Each parameter how has a fixed
> meaning.
>
> NB, there is no way to actually take advantage of this yet,
> since virDomainMigrate/virDomainMigrateToURI do not have any
> way to provide the 2 separate URIs
>
> * daemon/remote.c, src/remote/remote_driver.c,
> Âsrc/remote/remote_protocol.x, src/remote_protocol-structs: Add
> Âthe second URI parameter to perform3 message
> * src/driver.h, src/libvirt.c, src/libvirt_internal.h: Add
> Âthe second URI parameter to Perform3 method
> * src/libvirt_internal.h, src/qemu/qemu_migration.c,
> Âsrc/qemu/qemu_migration.h: Update to handle URIs correctly
> ---
> Âdaemon/remote.c       Â|  13 +++++++++++--
> Âsrc/driver.h         |  Â2 ++
> Âsrc/libvirt.c        Â|  37 +++++++++++++++++++++++++------------
> Âsrc/libvirt_internal.h    |  Â6 ++++--
> Âsrc/qemu/qemu_driver.c    |  12 ++++++++++--
> Âsrc/qemu/qemu_migration.c  Â|  26 +++++++++++++++++---------
> Âsrc/qemu/qemu_migration.h  Â|  Â1 +
> Âsrc/remote/remote_driver.c  |  Â8 ++++++--
> Âsrc/remote/remote_protocol.x | Â Â6 ++++--
> Âsrc/remote_protocol-structs Â| Â Â1 +
> Â10 files changed, 81 insertions(+), 31 deletions(-)
>

> diff --git a/src/libvirt.c b/src/libvirt.c
> index 7b7323e..2b2c1fd 100644
> --- a/src/libvirt.c
> +++ b/src/libvirt.c

> @@ -4766,10 +4778,10 @@ virDomainMigratePerform3(virDomainPtr domain,
> Â Â virConnectPtr conn;
>
> Â Â VIR_DOMAIN_DEBUG(domain, "xmlin=%s cookiein=%p, cookieinlen=%d, "
> - Â Â Â Â Â Â Â Â Â Â "cookieout=%p, cookieoutlen=%p, "
> + Â Â Â Â Â Â Â Â Â Â "cookieout=%p, cookieoutlen=%p, dconnuri=%s, "
> Â Â Â Â Â Â Â Â Â Â Â"uri=%s, flags=%lu, dname=%s, bandwidth=%lu",
> Â Â Â Â Â Â Â Â Â Â ÂNULLSTR(xmlin), cookiein, cookieinlen,
> - Â Â Â Â Â Â Â Â Â Â cookieout, cookieoutlen,
> + Â Â Â Â Â Â Â Â Â Â cookieout, cookieoutlen, NULLSTR(dconnuri),
> Â Â Â Â Â Â Â Â Â Â Âuri, flags, NULLSTR(dname), bandwidth);

uri can be NULL here, can't it? So this should use NULLSTR(uri)
instead of plain uri.

> @@ -4817,15 +4829,16 @@ virDomainMigrateFinish3(virConnectPtr dconn,
> Â Â Â Â Â Â Â Â Â Â Â Â int cookieinlen,
> Â Â Â Â Â Â Â Â Â Â Â Â char **cookieout,
> Â Â Â Â Â Â Â Â Â Â Â Â int *cookieoutlen,
> + Â Â Â Â Â Â Â Â Â Â Â Âconst char *dconnuri,
> Â Â Â Â Â Â Â Â Â Â Â Â const char *uri,
> Â Â Â Â Â Â Â Â Â Â Â Â unsigned long flags,
> Â Â Â Â Â Â Â Â Â Â Â Â int cancelled,
> Â Â Â Â Â Â Â Â Â Â Â Â virDomainPtr *newdom)
> Â{
> Â Â VIR_DEBUG("dconn=%p, dname=%s, cookiein=%p, cookieinlen=%d, cookieout=%p,"
> - Â Â Â Â Â Â Â"cookieoutlen=%p, uri=%s, flags=%lu, retcode=%d newdom=%p",
> + Â Â Â Â Â Â Â"cookieoutlen=%p, dconnuri=%s, uri=%s, flags=%lu, retcode=%d newdom=%p",
> Â Â Â Â Â Â Â dconn, NULLSTR(dname), cookiein, cookieinlen, cookieout,
> - Â Â Â Â Â Â Âcookieoutlen, uri, flags, cancelled, newdom);
> + Â Â Â Â Â Â Âcookieoutlen, NULLSTR(dconnuri), NULLSTR(uri), flags, cancelled, newdom);
>
> Â Â virResetLastError();

Here it's correct.

> diff --git a/src/libvirt_internal.h b/src/libvirt_internal.h
> index c7c1932..3144271 100644
> --- a/src/libvirt_internal.h
> +++ b/src/libvirt_internal.h
> @@ -161,7 +161,8 @@ int virDomainMigratePerform3(virDomainPtr dom,
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âint cookieinlen,
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âchar **cookieout,
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âint *cookieoutlen,
> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â const char *uri,
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â const char *dconnuri, /* libvirtd URI if Peer2Peer, NULL otherwise */
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â const char *uri, /* VM Migration URI */
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âunsigned long flags,
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âconst char *dname,
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âunsigned long resource);
> @@ -172,7 +173,8 @@ int virDomainMigrateFinish3(virConnectPtr dconn,
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â int cookieinlen,
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â char **cookieout,
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â int *cookieoutlen,
> - Â Â Â Â Â Â Â Â Â Â Â Â Â Âconst char *uri,
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Âconst char *dconnuri, /* libvirtd URI if Peer2Peer, NULL otherwise */
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Âconst char *uri, /* VM Migration URI */
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â unsigned long flags,
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â int cancelled, /* Kill the dst VM */
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â virDomainPtr *newdom);

Maybe state in this comments that uri is NULL in the Peer2Peer case,
just for clarity.

> diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs
> index 971f53a..014e5ac 100644
> --- a/src/remote_protocol-structs
> +++ b/src/remote_protocol-structs
> @@ -1510,6 +1510,7 @@ struct remote_domain_migrate_perform3_args {
>                 u_int              cookie_in_len;
>                 char *             cookie_in_val;
>         } cookie_in;
> +        remote_string              dconnuri;
>         remote_nonnull_string      uri;
>         uint64_t                   flags;
>         remote_string              dname;

As uri is now optional you need to change its type from
remote_nonnull_string to remote_string here and in
remote_domain_migrate_finish3_args. And you need to add remote_string
dconnuri to remote_domain_migrate_finish3_args to satisfy make
syntax-check.

ACK, with this nits fixed.

Matthias

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]