Mailing-List: contact dev-help@cloudstack.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@cloudstack.apache.org
Received-SPF: pass (nike.apache.org: domain of shadowsor@gmail.com designates
 209.85.220.176 as permitted sender)
MIME-Version: 1.0
In-Reply-To: 
 <CALFpzo5=_hX-nxHxODN2BRTAgoyX+bTF5pdgWj1Frw_ogRTS+g@mail.gmail.com>
References: 
 <CALFpzo4k=J7JzUr3cVrToM4SJTQ9J9f28nMYPBPoWtMJRHJajQ@mail.gmail.com>
	<20130425200640.GX64138@USLT-205755.sungardas.corp>
	<CALFpzo6CYcqjghHad1zf4gF__zWRnbeUYT+YQfD7GwKLaaO+8w@mail.gmail.com>
	<C66C814C1ABFA8449FF65CB44953A8B40143FD3CFF60@SJCPMAILBOX01.citrite.net>
	<CALFpzo64027oXqRxcrLu+yJcFtECkm5isnceS23mZAzS5Ke5=A@mail.gmail.com>
	<CALFpzo6_9oyWX_9fhcFEopa_xKOadj+0rHDhmTD1=o1Kcis4oA@mail.gmail.com>
	<CALFpzo6TwdkDakfwkTbFsiAOuUXD5Pa62ELLk08W7Vnr9-LK9g@mail.gmail.com>
	<CALFpzo5=_hX-nxHxODN2BRTAgoyX+bTF5pdgWj1Frw_ogRTS+g@mail.gmail.com>
Date: Thu, 25 Apr 2013 22:55:34 -0600
Message-ID: 
 <CALFpzo7JkPKQ6FQxZSa03avSamfyG=jADogpdSmuTEgFBBcobA@mail.gmail.com>
Subject: Re: [ACS41] new critical bug
From: Marcus Sorensen <shadowsor@gmail.com>
To: "dev@cloudstack.apache.org" <dev@cloudstack.apache.org>
Content-Type: multipart/alternative; boundary=f46d043c7ca661263204db3c582f

--f46d043c7ca661263204db3c582f
Content-Type: text/plain; charset=ISO-8859-1

Ok, review request in. I applied it to master, but I think there's a
whitespace issue or something that keeps it from applying to 4.1. It's
basically Edison's patch plus a logger message
in VirtualMachineManagerImpl.java that tells the admin why stopping the VM
failed (unable to connect to agent).


On Thu, Apr 25, 2013 at 10:13 PM, Marcus Sorensen <shadowsor@gmail.com>wrote:

> argh, that version of stopVirtualMachine (the one returning boolean) isn't
> even used!
>
>
> On Thu, Apr 25, 2013 at 10:07 PM, Marcus Sorensen <shadowsor@gmail.com>wrote:
>
>> Actually it's that your patch is for the 'forced' stop. Still doesn't
>> apply to 4.1, but the code I'm looking at for the moment is the non-forced
>> stop, which returns a boolean.
>>
>>
>> On Thu, Apr 25, 2013 at 10:05 PM, Marcus Sorensen <shadowsor@gmail.com>wrote:
>>
>>> Uh, nevermind, of course I can't return null there because it requires a
>>> boolean in 4.1. I'll play with this a bit and get back with you.
>>>
>>>
>>> On Thu, Apr 25, 2013 at 10:01 PM, Marcus Sorensen <shadowsor@gmail.com>wrote:
>>>
>>>> Thanks, Edison. That doesn't apply to 4.1, but it's basically the same
>>>> as the first hunk of my patch above, just that it returns null instead of
>>>> throwing an exception.  I'm going to test that now. I didn't see why that
>>>> CloudRuntimeException in the first hunk would be squelched, but the finally
>>>> clause makes sense in the second hunk.
>>>>
>>>>
>>>> On Thu, Apr 25, 2013 at 6:08 PM, Edison Su <Edison.su@citrix.com>wrote:
>>>>
>>>>> This code will do the magic:
>>>>> } finally {
>>>>> if (!stopped) {
>>>>>                 if (!forced) {
>>>>>                     s_logger.warn("Unable to stop vm " + vm);
>>>>>                     try {
>>>>>                         stateTransitTo(vm, Event.OperationFailed,
>>>>> vm.getHostId());
>>>>>                     } catch (NoTransitionException e) {
>>>>>                         s_logger.warn("Unable to transition the state
>>>>> " + vm);
>>>>>                     }
>>>>>                     return false;
>>>>>                 } else {
>>>>>                     s_logger.warn("Unable to actually stop " + vm + "
>>>>> but continue with release because it's a force stop");
>>>>>                     vmGuru.finalizeStop(profile, answer);
>>>>>                 }
>>>>>             }
>>>>> }
>>>>>
>>>>>
>>>>> Basically, it means, if stop failed and it's not force stop, then mark
>>>>> the vm as running.
>>>>> I think the logic here sounds correct, as cloudstack can't delete the
>>>>> vm(due to the connection between mgt server and kvm agent is broken), then
>>>>> all the operations on the VM should fail, and the VM status shouldn't be
>>>>> changed.
>>>>>
>>>>> But the problem is, the stop api call should fail, as stop vm actually
>>>>> failed.
>>>>> Could you try the following patch:
>>>>>
>>>>> diff --git a/server/src/com/cloud/vm/UserVmManagerImpl.java
>>>>> b/server/src/com/cloud/vm/UserVmManagerImpl.java
>>>>> index 7bf04ec..ff20c54 100755
>>>>> --- a/server/src/com/cloud/vm/UserVmManagerImpl.java
>>>>> +++ b/server/src/com/cloud/vm/UserVmManagerImpl.java
>>>>> @@ -3684,10 +3684,15 @@ public class UserVmManagerImpl extends
>>>>> ManagerBase implements UserVmManager, Use
>>>>>          }
>>>>>
>>>>>          UserVO user = _userDao.findById(userId);
>>>>> -
>>>>> +        boolean status = false;
>>>>>          try {
>>>>>              VirtualMachineEntity vmEntity =
>>>>> _orchSrvc.getVirtualMachine(vm.getUuid());
>>>>> -            vmEntity.stop(new Long(userId).toString());
>>>>> +            status = vmEntity.stop(new Long(userId).toString());
>>>>> +            if (status) {
>>>>> +               return _vmDao.findById(vmId);
>>>>> +            } else {
>>>>> +               return null;
>>>>> +            }
>>>>>          } catch (ResourceUnavailableException e) {
>>>>>              throw new CloudRuntimeException(
>>>>>                      "Unable to contact the agent to stop the virtual
>>>>> machine "
>>>>> @@ -3698,7 +3703,7 @@ public class UserVmManagerImpl extends
>>>>> ManagerBase implements UserVmManager, Use
>>>>>                              + vm, e);
>>>>>          }
>>>>>
>>>>> -        return _vmDao.findById(vmId);
>>>>> +
>>>>>      }
>>>>>
>>>>>      @Override
>>>>>
>>>>>
>>>>> > -----Original Message-----
>>>>> > From: Marcus Sorensen [mailto:shadowsor@gmail.com]
>>>>> > Sent: Thursday, April 25, 2013 4:46 PM
>>>>> > To: dev@cloudstack.apache.org
>>>>> > Subject: Re: [ACS41] new critical bug
>>>>> >
>>>>> > I tried a few things, including throwing a CloudRuntimeException in
>>>>> several
>>>>> > places where I thought it made sense, such as an empty
>>>>> > AgentUnavailableException catch block, but it doesn't seem to do
>>>>> anything at
>>>>> > all, I don't see it in the logs, even though I do see the debug code
>>>>> that I
>>>>> > placed next to it.  So I give up for now :-)
>>>>> >
>>>>> > diff --git a/server/src/com/cloud/vm/UserVmManagerImpl.java
>>>>> > b/server/src/com/cloud/vm/UserVmManagerImpl.java
>>>>> > index 7bf04ec..0373690 100755
>>>>> > --- a/server/src/com/cloud/vm/UserVmManagerImpl.java
>>>>> > +++ b/server/src/com/cloud/vm/UserVmManagerImpl.java
>>>>> > @@ -685,7 +685,8 @@ public class UserVmManagerImpl extends
>>>>> > ManagerBase implements UserVmManager, Use
>>>>> >          if (status) {
>>>>> >              return status;
>>>>> >          } else {
>>>>> > -            return status;
>>>>> > +            throw new CloudRuntimeException(
>>>>> > +                    "Unable to stop the virtual machine" + vm );
>>>>> >          }
>>>>> >      }
>>>>> >
>>>>> > diff --git a/server/src/com/cloud/vm/VirtualMachineManagerImpl.java
>>>>> > b/server/src/com/cloud/vm/VirtualMachineManagerImpl.
>>>>> > index 2c2986f..e320ff1 100755
>>>>> > --- a/server/src/com/cloud/vm/VirtualMachineManagerImpl.java
>>>>> > +++ b/server/src/com/cloud/vm/VirtualMachineManagerImpl.java
>>>>> > @@ -1083,7 +1083,11 @@ public class VirtualMachineManagerImpl extends
>>>>> > ManagerBase implements VirtualMac
>>>>> >              vmGuru.finalizeStop(profile, answer);
>>>>> >
>>>>> >          } catch (AgentUnavailableException e) {
>>>>> > +            s_logger.error("Unable to stop vm, agent unavailable: "
>>>>> +
>>>>> > e.toString());
>>>>> > +            throw new CloudRuntimeException("Unable to stop vm,
>>>>> agent
>>>>> > unavailable");
>>>>> >          } catch (OperationTimedoutException e) {
>>>>> > +            s_logger.error("Unable to stop vm, operation timed out:
>>>>> " +
>>>>> > e.toString());
>>>>> > +            throw new CloudRuntimeException("Unable to stop vm,
>>>>> > + operation
>>>>> > timed out");
>>>>> >          } finally {
>>>>> >              if (!stopped) {
>>>>> >                  if (!forced) {
>>>>> >
>>>>> >
>>>>> > On Thu, Apr 25, 2013 at 2:06 PM, Chip Childers
>>>>> > <chip.childers@sungard.com>wrote:
>>>>> >
>>>>> > > On Thu, Apr 25, 2013 at 02:01:56PM -0600, Marcus Sorensen wrote:
>>>>> > > > I didn't mark this one as a blocker, but it's still pretty bad
>>>>> for
>>>>> > > someone
>>>>> > > > managing VMs in an automated fashion. Trying to stop a VM when
>>>>> the
>>>>> > > > KVM agent is disconnected reports success.
>>>>> > > >
>>>>> > > > https://issues.apache.org/jira/browse/CLOUDSTACK-2195
>>>>> > >
>>>>> > > I'll pull a fix in, if we have one ready before the final blockers.
>>>>> > > Otherwise I'd pull it into a 4.1.1 release.
>>>>> > >
>>>>>
>>>>
>>>>
>>>
>>
>

--f46d043c7ca661263204db3c582f--