Maximilian Wilhelm wrote:
> Anno domini 2009 Daniel Veillard scripsit:
>
>> On Mon, Oct 12, 2009 at 01:32:26PM +0200, Chris Lalancette wrote:
>>> Normally, when you migrate a domain from host A to host B,
>>> the domain on host A remains defined but shutoff and the domain
>>> on host B remains running but is a "transient". Add a new
>>> flag to virDomainMigrate() to allow the original domain to be
>>> undefined on source host A, and a new flag to virDomainMigrate() to
>>> allow the new domain to be persisted on the destination host B.
>
>>> Signed-off-by: Chris Lalancette <clalance@xxxxxxxxxx>
>
> Thanks for the ground work!
>
> Attached you can find a patch implementing the same flags for Xen:
>
> * src/xen/xen_driver.c: Add support for VIR_MIGRATE_PERSIST_DEST flag
> * src/xen/xend_internal.c: Add support for VIR_MIGRATE_UNDEFINE_SOURCE flag
>
> I'm not totaly sure if there are better ways to handle all the error
> cases. The current solution seemed to be a same one for me.
Well, I do think that we need to return a proper error in all cases except for
the one with the long comment. I've pointed out where I think we need to add
error messages below. Otherwise, I think it looks good.
>
> diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c
> index 5273a11..18882c0 100644
> --- a/src/xen/xen_driver.c
> +++ b/src/xen/xen_driver.c
> @@ -1204,9 +1204,53 @@ xenUnifiedDomainMigrateFinish (virConnectPtr dconn,
> const char *cookie ATTRIBUTE_UNUSED,
> int cookielen ATTRIBUTE_UNUSED,
> const char *uri ATTRIBUTE_UNUSED,
> - unsigned long flags ATTRIBUTE_UNUSED)
> + unsigned long flags)
> {
> - return xenUnifiedDomainLookupByName (dconn, dname);
> + virDomainPtr dom = NULL;
> + char *domain_xml = NULL;
> + virDomainPtr dom_new = NULL;
> +
> + dom = xenUnifiedDomainLookupByName (dconn, dname);
> + if (! dom) {
You are probably going to want to raise some error here, otherwise the user will
get back "unknown error", which isn't very helpful.
> + return NULL;
> + }
> +
> + if (flags & VIR_MIGRATE_PERSIST_DEST) {
> + domain_xml = xenDaemonDomainDumpXML (dom, 0, NULL);
> + if (! domain_xml) {
> + goto failure;
> + }
This seems whitespace damaged (using tabs instead of spaces), and also needs to
raise a proper error.
> +
> + dom_new = xenDaemonDomainDefineXML (dconn, domain_xml);
> + if (! dom_new) {
> + /* Hmpf. Migration was successful, but making it persistent
> + * was not. If we report successful, then when this domain
> + * shuts down, management tools are in for a surprise. On the
> + * other hand, if we report failure, then the management tools
> + * might try to restart the domain on the source side, even
> + * though the domain is actually running on the destination.
> + * Return a NULL dom pointer, and hope that this is a rare
> + * situation and management tools are smart.
> + */
> + goto failure;
> + }
> +
> + /* Free additional reference added by Define */
> + virDomainFree (dom_new);
> + }
> +
> + VIR_FREE (domain_xml);
> +
> + return dom;
> +
> +
> +failure:
> + virDomainFree (dom);
> +
> + VIR_FREE (domain_xml);
> +
> + return NULL;
> +
> }
>
> static int
> diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
> index 27d215e..9fbb616 100644
> --- a/src/xen/xend_internal.c
> +++ b/src/xen/xend_internal.c
> @@ -4386,6 +4386,8 @@ xenDaemonDomainMigratePerform (virDomainPtr domain,
> int ret;
> char *p, *hostname = NULL;
>
> + int undefined_source = 0;
> +
> /* Xen doesn't support renaming domains during migration. */
> if (dname) {
> virXendError (conn, VIR_ERR_NO_SUPPORT,
> @@ -4404,11 +4406,25 @@ xenDaemonDomainMigratePerform (virDomainPtr domain,
> return -1;
> }
>
> - /* Check the flags. */
> + /*
> + * Check the flags.
> + */
> if ((flags & VIR_MIGRATE_LIVE)) {
> strcpy (live, "1");
> flags &= ~VIR_MIGRATE_LIVE;
> }
> +
> + /* Undefine the VM on the source host after migration ? */
> + if (flags & VIR_MIGRATE_UNDEFINE_SOURCE) {
> + undefined_source = 1;
> + flags &= ~VIR_MIGRATE_UNDEFINE_SOURCE;
> + }
> +
> + /* Ignore the persist_dest flag here */
> + if (flags & VIR_MIGRATE_PERSIST_DEST) {
> + flags &= ~VIR_MIGRATE_PERSIST_DEST;
> + }
Again a nit, but I think the libvirt style currently is to leave braces off for
single line statements. That is, this should be:
if (flags & VIR_MIGRATE_PERSIST_DEST)
flags &= ~VIR_MIGRATE_PERSIST_DEST;
> +
> /* XXX we could easily do tunnelled & peer2peer migration too
> if we want to. support these... */
> if (flags != 0) {
> @@ -4494,6 +4510,10 @@ xenDaemonDomainMigratePerform (virDomainPtr domain,
> NULL);
> VIR_FREE (hostname);
>
> + if (ret == 0 && undefined_source) {
> + xenDaemonDomainUndefine (domain);
> + }
> +
Same here.
> DEBUG0("migration done");
>
> return ret;
>
>
--
Chris Lalancette
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list