12:53 p.m.
This series fixes a few issues found while testing migration with
latest Xen and libvirt. See the individual patches for details.
V2: rebased to latest git master and address Olaf's comment
from V1.
Jim Fehlig (3):
libxl: fix ref counting of libxlMigrationDstArgs
libxl: don't attempt to resume domain when suspend fails
libxl: acquire a job when receiving a migrating domain
src/libxl/libxl_migration.c | 54 +++++++++++++++++++--------------------------
1 file changed, 23 insertions(+), 31 deletions(-)
--
2.3.7
12:53 p.m.
New subject: [libvirt] [PATCH V2 1/3] libxl: fix ref counting of libxlMigrationDstArgs
This patch fixes some flawed logic around ref counting the
libxlMigrationDstArgs object.
First, when adding sockets to the event loop with
virNetSocketAddIOCallback(), the generic virObjectFreeCallback()
was registered as a free function, with libxlMigrationDstArgs as
its parameter. A reference was also taken on
libxlMigrationDstArgs for each successful call to
virNetSocketAddIOCallback(). The rational behind this logic was
that the libxlMigrationDstArgs object had to out-live the socket
objects. But virNetSocketAddIOCallback() already takes a
reference on socket objects, ensuring their life until removed
from the event loop and unref'ed in virNetSocketEventFree(). We
only need to ensure libxlMigrationDstArgs lives until
libxlDoMigrateReceive() finishes, which can be done by simply
unref'ing libxlMigrationDstArgs at the end of
libxlDoMigrateReceive().
The second flaw was unref'ing the sockets in the failure path of
libxlMigrateReceive() and at the end of libxlDoMigrateReceive().
As mentioned above, the sockets are already unref'ed by
virNetSocketEventFree() when removed from the event loop.
Attempting to unref the socket a second time resulted in a
libvirtd crash since the socket was previously unref'ed and
disposed.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
V2: Initialize args in libxlDomainMigrationPrepare
src/libxl/libxl_migration.c | 20 ++++++--------------
1 file changed, 6 insertions(+), 14 deletions(-)
diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c
index aa9547b..f9673c8 100644
--- a/src/libxl/libxl_migration.c
+++ b/src/libxl/libxl_migration.c
@@ -112,11 +112,11 @@ libxlDoMigrateReceive(void *opaque)
virNetSocketUpdateIOCallback(socks[i], 0);
virNetSocketRemoveIOCallback(socks[i]);
virNetSocketClose(socks[i]);
- virObjectUnref(socks[i]);
socks[i] = NULL;
}
args->nsocks = 0;
VIR_FORCE_CLOSE(recvfd);
+ virObjectUnref(args);
}
@@ -164,11 +164,11 @@ libxlMigrateReceive(virNetSocketPtr sock,
virNetSocketUpdateIOCallback(socks[i], 0);
virNetSocketRemoveIOCallback(socks[i]);
virNetSocketClose(socks[i]);
- virObjectUnref(socks[i]);
socks[i] = NULL;
}
args->nsocks = 0;
VIR_FORCE_CLOSE(recvfd);
+ virObjectUnref(args);
}
static int
@@ -318,7 +318,7 @@ libxlDomainMigrationPrepare(virConnectPtr dconn,
virNetSocketPtr *socks = NULL;
size_t nsocks = 0;
int nsocks_listen = 0;
- libxlMigrationDstArgs *args;
+ libxlMigrationDstArgs *args = NULL;
size_t i;
int ret = -1;
@@ -420,22 +420,12 @@ libxlDomainMigrationPrepare(virConnectPtr dconn,
VIR_EVENT_HANDLE_READABLE,
libxlMigrateReceive,
args,
- virObjectFreeCallback) < 0)
+ NULL) < 0)
continue;
- /*
- * Successfully added sock to event loop. Take a ref on args to
- * ensure it is not freed until sock is removed from the event loop.
- * Ref is dropped in virObjectFreeCallback after being removed
- * from the event loop.
- */
- virObjectRef(args);
nsocks_listen++;
}
- /* Done with args in this function, drop reference */
- virObjectUnref(args);
-
if (!nsocks_listen)
goto error;
@@ -448,6 +438,8 @@ libxlDomainMigrationPrepare(virConnectPtr dconn,
virObjectUnref(socks[i]);
}
VIR_FREE(socks);
+ virObjectUnref(args);
+
/* Remove virDomainObj from domain list */
if (vm) {
virDomainObjListRemove(driver->domains, vm);
--
2.3.7
7:16 p.m.
New subject: [libvirt] [PATCH V2 1/3] libxl: fix ref counting of libxlMigrationDstArgs
On 07.08.2015 19:53, Jim Fehlig wrote:
This patch fixes some flawed logic around ref counting the
libxlMigrationDstArgs object.
First, when adding sockets to the event loop with
virNetSocketAddIOCallback(), the generic virObjectFreeCallback()
was registered as a free function, with libxlMigrationDstArgs as
its parameter. A reference was also taken on
libxlMigrationDstArgs for each successful call to
virNetSocketAddIOCallback(). The rational behind this logic was
that the libxlMigrationDstArgs object had to out-live the socket
objects. But virNetSocketAddIOCallback() already takes a
reference on socket objects, ensuring their life until removed
from the event loop and unref'ed in virNetSocketEventFree(). We
only need to ensure libxlMigrationDstArgs lives until
libxlDoMigrateReceive() finishes, which can be done by simply
unref'ing libxlMigrationDstArgs at the end of
libxlDoMigrateReceive().
The second flaw was unref'ing the sockets in the failure path of
libxlMigrateReceive() and at the end of libxlDoMigrateReceive().
As mentioned above, the sockets are already unref'ed by
virNetSocketEventFree() when removed from the event loop.
Attempting to unref the socket a second time resulted in a
libvirtd crash since the socket was previously unref'ed and
disposed.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
V2: Initialize args in libxlDomainMigrationPrepare
src/libxl/libxl_migration.c | 20 ++++++--------------
1 file changed, 6 insertions(+), 14 deletions(-)
diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c
index aa9547b..f9673c8 100644
--- a/src/libxl/libxl_migration.c
+++ b/src/libxl/libxl_migration.c
@@ -112,11 +112,11 @@ libxlDoMigrateReceive(void *opaque)
virNetSocketUpdateIOCallback(socks[i], 0);
This is pre-existing, but since the socket callback is removed right
after, it does not make much sense to update its events to listen for.
virNetSocketRemoveIOCallback(socks[i]);
virNetSocketClose(socks[i]);
- virObjectUnref(socks[i]);
This will leak the socks[i] object, wouldn't it? I mean, in
libxlDomainMigrationPrepare() on line 392 virNetSocketNewListenTCP() is
called. This initialize the array with object pointers. Then
virNetSocketAddIOCallback() + virNetSocketRemoveIOCallback() pair keep
the ref counter consistent. This makes me think you should not remove
this line.
> socks[i] = NULL;
> }
> args->nsocks = 0;
> VIR_FORCE_CLOSE(recvfd);
> + virObjectUnref(args);
> }
>
>
> @@ -164,11 +164,11 @@ libxlMigrateReceive(virNetSocketPtr sock,
> virNetSocketUpdateIOCallback(socks[i], 0);
virNetSocketRemoveIOCallback(socks[i]);
virNetSocketClose(socks[i]);
- virObjectUnref(socks[i]);
> socks[i] = NULL;
> }
> args->nsocks = 0;
> VIR_FORCE_CLOSE(recvfd);
> + virObjectUnref(args);
> }
>
> static int
> @@ -318,7 +318,7 @@ libxlDomainMigrationPrepare(virConnectPtr dconn,
> virNetSocketPtr *socks = NULL;
> size_t nsocks = 0;
> int nsocks_listen = 0;
> - libxlMigrationDstArgs *args;
> + libxlMigrationDstArgs *args = NULL;
> size_t i;
> int ret = -1;
>
> @@ -420,22 +420,12 @@ libxlDomainMigrationPrepare(virConnectPtr dconn,
> VIR_EVENT_HANDLE_READABLE,
> libxlMigrateReceive,
> args,
> - virObjectFreeCallback) < 0)
> + NULL) < 0)
> continue;
>
> - /*
> - * Successfully added sock to event loop. Take a ref on args to
> - * ensure it is not freed until sock is removed from the event loop.
> - * Ref is dropped in virObjectFreeCallback after being removed
> - * from the event loop.
> - */
> - virObjectRef(args);
> nsocks_listen++;
> }
>
> - /* Done with args in this function, drop reference */
> - virObjectUnref(args);
> -
> if (!nsocks_listen)
> goto error;
>
> @@ -448,6 +438,8 @@ libxlDomainMigrationPrepare(virConnectPtr dconn,
> virObjectUnref(socks[i]);
> }
> VIR_FREE(socks);
> + virObjectUnref(args);
> +
> /* Remove virDomainObj from domain list */
> if (vm) {
> virDomainObjListRemove(driver->domains, vm);
>
Otherwise looking good.
Michal
4:38 p.m.
New subject: [libvirt] [PATCH V2 1/3] libxl: fix ref counting of libxlMigrationDstArgs
Michal Privoznik wrote:
On 07.08.2015 19:53, Jim Fehlig wrote:
> This patch fixes some flawed logic around ref counting the
> libxlMigrationDstArgs object.
>
> First, when adding sockets to the event loop with
> virNetSocketAddIOCallback(), the generic virObjectFreeCallback()
> was registered as a free function, with libxlMigrationDstArgs as
> its parameter. A reference was also taken on
> libxlMigrationDstArgs for each successful call to
> virNetSocketAddIOCallback(). The rational behind this logic was
> that the libxlMigrationDstArgs object had to out-live the socket
> objects. But virNetSocketAddIOCallback() already takes a
> reference on socket objects, ensuring their life until removed
> from the event loop and unref'ed in virNetSocketEventFree(). We
> only need to ensure libxlMigrationDstArgs lives until
> libxlDoMigrateReceive() finishes, which can be done by simply
> unref'ing libxlMigrationDstArgs at the end of
> libxlDoMigrateReceive().
>
> The second flaw was unref'ing the sockets in the failure path of
> libxlMigrateReceive() and at the end of libxlDoMigrateReceive().
> As mentioned above, the sockets are already unref'ed by
> virNetSocketEventFree() when removed from the event loop.
> Attempting to unref the socket a second time resulted in a
> libvirtd crash since the socket was previously unref'ed and
> disposed.
>
> Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
> ---
>
> V2: Initialize args in libxlDomainMigrationPrepare
>
> src/libxl/libxl_migration.c | 20 ++++++--------------
> 1 file changed, 6 insertions(+), 14 deletions(-)
>
> diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c
> index aa9547b..f9673c8 100644
> --- a/src/libxl/libxl_migration.c
> +++ b/src/libxl/libxl_migration.c
> @@ -112,11 +112,11 @@ libxlDoMigrateReceive(void *opaque)
> virNetSocketUpdateIOCallback(socks[i], 0);
>
This is pre-existing, but since the socket callback is removed right
after, it does not make much sense to update its events to listen for.
Right. Should be removed since I'm touching this code.
> virNetSocketRemoveIOCallback(socks[i]);
> virNetSocketClose(socks[i]);
> - virObjectUnref(socks[i]);
>
This will leak the socks[i] object, wouldn't it?
Yes, you are right. I verified the virNetSocket was not disposed with
the missing unref.
I mean, in
libxlDomainMigrationPrepare() on line 392 virNetSocketNewListenTCP() is
called. This initialize the array with object pointers. Then
virNetSocketAddIOCallback() + virNetSocketRemoveIOCallback() pair keep
the ref counter consistent. This makes me think you should not remove
this line.
> socks[i] = NULL;
> }
> args->nsocks = 0;
> VIR_FORCE_CLOSE(recvfd);
> + virObjectUnref(args);
> }
>
>
> @@ -164,11 +164,11 @@ libxlMigrateReceive(virNetSocketPtr sock,
> virNetSocketUpdateIOCallback(socks[i], 0);
> virNetSocketRemoveIOCallback(socks[i]);
> virNetSocketClose(socks[i]);
> - virObjectUnref(socks[i]);
> socks[i] = NULL;
> }
> args->nsocks = 0;
> VIR_FORCE_CLOSE(recvfd);
> + virObjectUnref(args);
> }
>
> static int
> @@ -318,7 +318,7 @@ libxlDomainMigrationPrepare(virConnectPtr dconn,
> virNetSocketPtr *socks = NULL;
> size_t nsocks = 0;
> int nsocks_listen = 0;
> - libxlMigrationDstArgs *args;
> + libxlMigrationDstArgs *args = NULL;
> size_t i;
> int ret = -1;
>
> @@ -420,22 +420,12 @@ libxlDomainMigrationPrepare(virConnectPtr dconn,
> VIR_EVENT_HANDLE_READABLE,
> libxlMigrateReceive,
> args,
> - virObjectFreeCallback) < 0)
> + NULL) < 0)
> continue;
>
> - /*
> - * Successfully added sock to event loop. Take a ref on args to
> - * ensure it is not freed until sock is removed from the event loop.
> - * Ref is dropped in virObjectFreeCallback after being removed
> - * from the event loop.
> - */
> - virObjectRef(args);
> nsocks_listen++;
> }
>
> - /* Done with args in this function, drop reference */
> - virObjectUnref(args);
> -
> if (!nsocks_listen)
> goto error;
>
> @@ -448,6 +438,8 @@ libxlDomainMigrationPrepare(virConnectPtr dconn,
> virObjectUnref(socks[i]);
> }
> VIR_FREE(socks);
> + virObjectUnref(args);
> +
> /* Remove virDomainObj from domain list */
> if (vm) {
> virDomainObjListRemove(driver->domains, vm);
>
>
Otherwise looking good.
How does it look with the following squashed in?
diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c
index 8db3aea..9609e06 100644
--- a/src/libxl/libxl_migration.c
+++ b/src/libxl/libxl_migration.c
@@ -112,9 +112,9 @@ libxlDoMigrateReceive(void *opaque)
/* Remove all listen socks from event handler, and close them. */
for (i = 0; i < nsocks; i++) {
- virNetSocketUpdateIOCallback(socks[i], 0);
virNetSocketRemoveIOCallback(socks[i]);
virNetSocketClose(socks[i]);
+ virObjectUnref(socks[i]);
socks[i] = NULL;
}
args->nsocks = 0;
Thanks for taking time to review this series!
Regards,
Jim
7:23 p.m.
New subject: [libvirt] [PATCH V2 1/3] libxl: fix ref counting of libxlMigrationDstArgs
On 27.08.2015 23:38, Jim Fehlig wrote:
Michal Privoznik wrote:
> On 07.08.2015 19:53, Jim Fehlig wrote:
>
>> This patch fixes some flawed logic around ref counting the
>> libxlMigrationDstArgs object.
>>
>> First, when adding sockets to the event loop with
>> virNetSocketAddIOCallback(), the generic virObjectFreeCallback()
>> was registered as a free function, with libxlMigrationDstArgs as
>> its parameter. A reference was also taken on
>> libxlMigrationDstArgs for each successful call to
>> virNetSocketAddIOCallback(). The rational behind this logic was
>> that the libxlMigrationDstArgs object had to out-live the socket
>> objects. But virNetSocketAddIOCallback() already takes a
>> reference on socket objects, ensuring their life until removed
>> from the event loop and unref'ed in virNetSocketEventFree(). We
>> only need to ensure libxlMigrationDstArgs lives until
>> libxlDoMigrateReceive() finishes, which can be done by simply
>> unref'ing libxlMigrationDstArgs at the end of
>> libxlDoMigrateReceive().
>>
>> The second flaw was unref'ing the sockets in the failure path of
>> libxlMigrateReceive() and at the end of libxlDoMigrateReceive().
>> As mentioned above, the sockets are already unref'ed by
>> virNetSocketEventFree() when removed from the event loop.
>> Attempting to unref the socket a second time resulted in a
>> libvirtd crash since the socket was previously unref'ed and
>> disposed.
>>
>> Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
>> ---
>>
>> V2: Initialize args in libxlDomainMigrationPrepare
>>
>> src/libxl/libxl_migration.c | 20 ++++++--------------
>> 1 file changed, 6 insertions(+), 14 deletions(-)
>>
>> diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c
>> index aa9547b..f9673c8 100644
>> --- a/src/libxl/libxl_migration.c
>> +++ b/src/libxl/libxl_migration.c
>> @@ -112,11 +112,11 @@ libxlDoMigrateReceive(void *opaque)
>> virNetSocketUpdateIOCallback(socks[i], 0);
>>
>
> This is pre-existing, but since the socket callback is removed right
> after, it does not make much sense to update its events to listen for.
>
Right. Should be removed since I'm touching this code.
>
>> virNetSocketRemoveIOCallback(socks[i]);
>> virNetSocketClose(socks[i]);
>> - virObjectUnref(socks[i]);
>>
>
> This will leak the socks[i] object, wouldn't it?
Yes, you are right. I verified the virNetSocket was not disposed with
the missing unref.
> I mean, in
> libxlDomainMigrationPrepare() on line 392 virNetSocketNewListenTCP() is
> called. This initialize the array with object pointers. Then
> virNetSocketAddIOCallback() + virNetSocketRemoveIOCallback() pair keep
> the ref counter consistent. This makes me think you should not remove
> this line.
>
>
>> socks[i] = NULL;
>> }
>> args->nsocks = 0;
>> VIR_FORCE_CLOSE(recvfd);
>> + virObjectUnref(args);
>> }
>>
>>
>> @@ -164,11 +164,11 @@ libxlMigrateReceive(virNetSocketPtr sock,
>> virNetSocketUpdateIOCallback(socks[i], 0);
>> virNetSocketRemoveIOCallback(socks[i]);
>> virNetSocketClose(socks[i]);
>> - virObjectUnref(socks[i]);
>> socks[i] = NULL;
>> }
>> args->nsocks = 0;
>> VIR_FORCE_CLOSE(recvfd);
>> + virObjectUnref(args);
>> }
>>
>> static int
>> @@ -318,7 +318,7 @@ libxlDomainMigrationPrepare(virConnectPtr dconn,
>> virNetSocketPtr *socks = NULL;
>> size_t nsocks = 0;
>> int nsocks_listen = 0;
>> - libxlMigrationDstArgs *args;
>> + libxlMigrationDstArgs *args = NULL;
>> size_t i;
>> int ret = -1;
>>
>> @@ -420,22 +420,12 @@ libxlDomainMigrationPrepare(virConnectPtr dconn,
>> VIR_EVENT_HANDLE_READABLE,
>> libxlMigrateReceive,
>> args,
>> - virObjectFreeCallback) < 0)
>> + NULL) < 0)
>> continue;
>>
>> - /*
>> - * Successfully added sock to event loop. Take a ref on args to
>> - * ensure it is not freed until sock is removed from the event loop.
>> - * Ref is dropped in virObjectFreeCallback after being removed
>> - * from the event loop.
>> - */
>> - virObjectRef(args);
>> nsocks_listen++;
>> }
>>
>> - /* Done with args in this function, drop reference */
>> - virObjectUnref(args);
>> -
>> if (!nsocks_listen)
>> goto error;
>>
>> @@ -448,6 +438,8 @@ libxlDomainMigrationPrepare(virConnectPtr dconn,
>> virObjectUnref(socks[i]);
>> }
>> VIR_FREE(socks);
>> + virObjectUnref(args);
>> +
>> /* Remove virDomainObj from domain list */
>> if (vm) {
>> virDomainObjListRemove(driver->domains, vm);
>>
>>
>
> Otherwise looking good.
>
How does it look with the following squashed in?
diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c
index 8db3aea..9609e06 100644
--- a/src/libxl/libxl_migration.c
+++ b/src/libxl/libxl_migration.c
@@ -112,9 +112,9 @@ libxlDoMigrateReceive(void *opaque)
/* Remove all listen socks from event handler, and close them. */
for (i = 0; i < nsocks; i++) {
- virNetSocketUpdateIOCallback(socks[i], 0);
virNetSocketRemoveIOCallback(socks[i]);
virNetSocketClose(socks[i]);
+ virObjectUnref(socks[i]);
socks[i] = NULL;
}
args->nsocks = 0;
Yup, with this piece it looks good. ACK
Er, we are currently in a freeze, but after all: a) this is a bugfix, b)
this has been sitting on the list quite a while. So I'd say push it.
Michal
9:55 a.m.
New subject: [libvirt] [PATCH V2 1/3] libxl: fix ref counting of libxlMigrationDstArgs
Michal Privoznik wrote:
On 27.08.2015 23:38, Jim Fehlig wrote:
> How does it look with the following squashed in?
>
> diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c
> index 8db3aea..9609e06 100644
> --- a/src/libxl/libxl_migration.c
> +++ b/src/libxl/libxl_migration.c
> @@ -112,9 +112,9 @@ libxlDoMigrateReceive(void *opaque)
>
> /* Remove all listen socks from event handler, and close them. */
> for (i = 0; i < nsocks; i++) {
> - virNetSocketUpdateIOCallback(socks[i], 0);
> virNetSocketRemoveIOCallback(socks[i]);
> virNetSocketClose(socks[i]);
> + virObjectUnref(socks[i]);
> socks[i] = NULL;
> }
> args->nsocks = 0;
>
>
Yup, with this piece it looks good. ACK
Er, we are currently in a freeze, but after all: a) this is a bugfix, b)
this has been sitting on the list quite a while. So I'd say push it.
Thanks. Series pushed.
Regards,
Jim
12:53 p.m.
New subject: [libvirt] [PATCH V2 2/3] libxl: don't attempt to resume domain when suspend fails
Failure of libxl_domain_suspend() does not leave the domain in
a suspended state, so no need to call libxl_domain_resume(),
which btw will fail with "domain not suspended".
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/libxl/libxl_migration.c | 14 --------------
1 file changed, 14 deletions(-)
diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c
index f9673c8..a407ad9 100644
--- a/src/libxl/libxl_migration.c
+++ b/src/libxl/libxl_migration.c
@@ -178,7 +178,6 @@ libxlDoMigrateSend(libxlDriverPrivatePtr driver,
int sockfd)
{
libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
- virObjectEventPtr event = NULL;
int xl_flags = 0;
int ret;
@@ -188,24 +187,11 @@ libxlDoMigrateSend(libxlDriverPrivatePtr driver,
ret = libxl_domain_suspend(cfg->ctx, vm->def->id, sockfd,
xl_flags, NULL);
if (ret != 0) {
- /* attempt to resume the domain on failure */
- if (libxl_domain_resume(cfg->ctx, vm->def->id, 1, 0) != 0) {
- VIR_DEBUG("Failed to resume domain following failed migration");
- virDomainObjSetState(vm, VIR_DOMAIN_PAUSED,
- VIR_DOMAIN_PAUSED_MIGRATION);
- event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_SUSPENDED,
- VIR_DOMAIN_EVENT_SUSPENDED_MIGRATED);
- ignore_value(virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm));
- }
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Failed to send migration data to destination
host"));
ret = -1;
- goto cleanup;
}
- cleanup:
- if (event)
- libxlDomainEventQueue(driver, event);
virObjectUnref(cfg);
return ret;
}
--
2.3.7
7:16 p.m.
New subject: [libvirt] [PATCH V2 2/3] libxl: don't attempt to resume domain when suspend fails
On 07.08.2015 19:53, Jim Fehlig wrote:
Failure of libxl_domain_suspend() does not leave the domain in
a suspended state, so no need to call libxl_domain_resume(),
which btw will fail with "domain not suspended".
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/libxl/libxl_migration.c | 14 --------------
1 file changed, 14 deletions(-)
diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c
index f9673c8..a407ad9 100644
--- a/src/libxl/libxl_migration.c
+++ b/src/libxl/libxl_migration.c
@@ -178,7 +178,6 @@ libxlDoMigrateSend(libxlDriverPrivatePtr driver,
int sockfd)
{
libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
- virObjectEventPtr event = NULL;
int xl_flags = 0;
int ret;
@@ -188,24 +187,11 @@ libxlDoMigrateSend(libxlDriverPrivatePtr driver,
ret = libxl_domain_suspend(cfg->ctx, vm->def->id, sockfd,
xl_flags, NULL);
if (ret != 0) {
- /* attempt to resume the domain on failure */
- if (libxl_domain_resume(cfg->ctx, vm->def->id, 1, 0) != 0) {
- VIR_DEBUG("Failed to resume domain following failed migration");
- virDomainObjSetState(vm, VIR_DOMAIN_PAUSED,
- VIR_DOMAIN_PAUSED_MIGRATION);
- event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_SUSPENDED,
- VIR_DOMAIN_EVENT_SUSPENDED_MIGRATED);
- ignore_value(virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm));
- }
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Failed to send migration data to destination
host"));
ret = -1;
- goto cleanup;
}
- cleanup:
- if (event)
- libxlDomainEventQueue(driver, event);
virObjectUnref(cfg);
return ret;
}
ACK
Michal
12:53 p.m.
New subject: [libvirt] [PATCH V2 3/3] libxl: acquire a job when receiving a migrating domain
Commit f86ae403 moved acquiring a job from libxlDomainStart()
to its callers. One spot missed was in libxlDoMigrateReceive().
Acquire a job in libxlDoMigrateReceive() before calling
libxlDomainStart().
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/libxl/libxl_migration.c | 20 +++++++++++++++++---
1 file changed, 17 insertions(+), 3 deletions(-)
diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c
index a407ad9..8db3aea 100644
--- a/src/libxl/libxl_migration.c
+++ b/src/libxl/libxl_migration.c
@@ -95,17 +95,20 @@ libxlDoMigrateReceive(void *opaque)
int recvfd = args->recvfd;
size_t i;
int ret;
+ bool remove_dom = 0;
+
+ virObjectLock(vm);
+ if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0)
+ goto cleanup;
/*
* Always start the domain paused. If needed, unpause in the
* finish phase, after transfer of the domain is complete.
*/
- virObjectLock(vm);
ret = libxlDomainStart(driver, vm, true, recvfd);
- virObjectUnlock(vm);
if (ret < 0 && !vm->persistent)
- virDomainObjListRemove(driver->domains, vm);
+ remove_dom = true;
/* Remove all listen socks from event handler, and close them. */
for (i = 0; i < nsocks; i++) {
@@ -117,6 +120,17 @@ libxlDoMigrateReceive(void *opaque)
args->nsocks = 0;
VIR_FORCE_CLOSE(recvfd);
virObjectUnref(args);
+
+ if (!libxlDomainObjEndJob(driver, vm))
+ vm = NULL;
+
+ cleanup:
+ if (remove_dom && vm) {
+ virDomainObjListRemove(driver->domains, vm);
+ vm = NULL;
+ }
+ if (vm)
+ virObjectUnlock(vm);
}
--
2.3.7
7:15 p.m.
New subject: [libvirt] [PATCH V2 3/3] libxl: acquire a job when receiving a migrating domain
On 07.08.2015 19:53, Jim Fehlig wrote:
Commit f86ae403 moved acquiring a job from libxlDomainStart()
to its callers. One spot missed was in libxlDoMigrateReceive().
Acquire a job in libxlDoMigrateReceive() before calling
libxlDomainStart().
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/libxl/libxl_migration.c | 20 +++++++++++++++++---
1 file changed, 17 insertions(+), 3 deletions(-)
diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c
index a407ad9..8db3aea 100644
--- a/src/libxl/libxl_migration.c
+++ b/src/libxl/libxl_migration.c
@@ -95,17 +95,20 @@ libxlDoMigrateReceive(void *opaque)
int recvfd = args->recvfd;
size_t i;
int ret;
+ bool remove_dom = 0;
+
+ virObjectLock(vm);
+ if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0)
+ goto cleanup;
/*
* Always start the domain paused. If needed, unpause in the
* finish phase, after transfer of the domain is complete.
*/
- virObjectLock(vm);
ret = libxlDomainStart(driver, vm, true, recvfd);
- virObjectUnlock(vm);
if (ret < 0 && !vm->persistent)
- virDomainObjListRemove(driver->domains, vm);
+ remove_dom = true;
/* Remove all listen socks from event handler, and close them. */
for (i = 0; i < nsocks; i++) {
@@ -117,6 +120,17 @@ libxlDoMigrateReceive(void *opaque)
args->nsocks = 0;
VIR_FORCE_CLOSE(recvfd);
virObjectUnref(args);
+
+ if (!libxlDomainObjEndJob(driver, vm))
+ vm = NULL;
+
+ cleanup:
+ if (remove_dom && vm) {
+ virDomainObjListRemove(driver->domains, vm);
+ vm = NULL;
+ }
+ if (vm)
+ virObjectUnlock(vm);
}
ACK
Michal
3377
days inactive
3398
days old
9 comments
2 participants
participants (2)
-
Jim Fehlig -
Michal Privoznik