[libvirt] [PATCH] cmdMigrate: move vshConnect before vshWatchJob

[Shivaprasad bhat]

Hi Jan,

Is the auto login a pre-requisite for p2p & direct migration ? The
current patch mostly focus on avoiding the virsh migrate hang.

Thanks,
Shiva

On Thu, Aug 14, 2014 at 9:40 PM, Ján Tomko <jtomko at redhat.com> wrote:
> On 08/08/2014 10:44 AM, Chunyan Liu wrote:
>> A possible fix to issue:
>> http://www.redhat.com/archives/libvir-list/2014-August/thread.html#00227
>>
>> While doing migration on KVM host, found problem sometimes:
>> VM is already running on the target host and disappears from source
>> host, but 'virsh migrate' command line hangs, cannot exit normally.
>> If pressing "ENTER" key, it will exit.
>>
>> The code hangs at tools/virsh-domain.c: cmdMigrate
>> ->vshWatchJob->poll():
>> poll() is trying to select pipe_fd, which is used to receive message
>> from doMigrate thread. In debugging, found that doMigrate finishes
>> and at the end it does call safewrite() to write the retval ('0' or
>> '1') to pipe_fd, and the write is completed. But cmdMigrate poll()
>> cannot get the event. If pressing "ENTER" key, poll() can get the
>> event and select pipe_fd, then command line can exit.
>>
>> In current code, authentication thread which is called by vshConnect
>> will use stdin, and at the same time, in cmdMigrate main process,
>> poll() is listening to stdin, that probably affect poll() to get
>> pipe_fd event. Better to move authentication before vshWatchJob. With
>> this change, above problem does not exist.
>>
>> Signed-off-by: Chunyan Liu <cyliu at suse.com>
>> ---
>>  tools/virsh-domain.c | 26 ++++++++++++++++++++------
>>  tools/virsh.h        |  1 +
>>  2 files changed, 21 insertions(+), 6 deletions(-)
>
> ACK
>
>> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
>> index f7193cb..2562326 100644
>> --- a/tools/virsh-domain.c
>> +++ b/tools/virsh-domain.c
>> @@ -8965,6 +8965,7 @@ doMigrate(void *opaque)
>>      virTypedParameterPtr params = NULL;
>>      int nparams = 0;
>>      int maxparams = 0;
>> +    virConnectPtr dconn = data->dconn;
>>
>>      sigemptyset(&sigmask);
>>      sigaddset(&sigmask, SIGINT);
>> @@ -9079,18 +9080,12 @@ doMigrate(void *opaque)
>>              ret = '0';
>>      } else {
>
> I'd rather rewrite this condition as:
> if (dconn) {
>   /* Traditional live migration */
> } else {
>   /* peer2peer or direct */
> }
>
>
>