CloudStack
  1. CloudStack
  2. CLOUDSTACK-1638

Network plugins won't be notified VM migration.

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 4.0.0, 4.0.1, 4.0.2, 4.1.0, 4.1.1, 4.2.0
    • Fix Version/s: 4.2.0
    • Component/s: Management Server
    • Security Level: Public (Anyone can view this level - this is the default.)
    • Labels:

      Description

      The location of the virtual machine is provided by DeployDestination, which will be passed in NetworkGuru#reserve and NetworkElement#prepare.

      During the virtual machine migration, it actually changes DeployDestination and it looks like that it will tell that event to network components as it has NetworkManager#prepareNicForMigration. The problem is that althogh the interface has that method, NetworkManagerImpl does not tell the DeployDestination changes to network components.

      So IMHO, we need to add calls of NetworkGuru#reserve and NetworkElement#prepare in NetworkManagerImpl#prepareNicForMigration . And then, we also need to add calls NetworkGuru#release and NetworkElement#release after the migration, otherwise the network resources that plugin reserved will be kept even when the vm leaves off.

        Activity

        Hide
        ASF subversion and git services added a comment -

        Commit 7260e8d83f07d90b48c34adaeb227de265019487 in branch refs/heads/pvlan from Hiroaki Kawai
        [ https://git-wip-us.apache.org/repos/asf?p=cloudstack.git;h=7260e8d ]

        CLOUDSTACK-1638: Introduce NetworkMigrationResponder

        The location of the virtual machine is provided by DeployDestination, which will
        be passed in NetworkGuru#reserve and NetworkElement#prepare.

        During the virtual machine migration, it actually changes DeployDestination and
        it looks like that it will tell that event to network components as it has
        NetworkManager#prepareNicForMigration. The problem is that althogh the interface
        has that method, NetworkManagerImpl does not tell the DeployDestination changes
        to network components.

        So IMHO, we need to add calls of NetworkGuru#reserve and NetworkElement#prepare
        in NetworkManagerImpl#prepareNicForMigration . And then, we also need to add
        calls NetworkGuru#release and NetworkElement#release after the migration,
        otherwise the network resources that plugin reserved will be kept even when the
        vm leaves off.

        (Sheng Yang: rebase code, add license header)

        Signed-off-by: Sheng Yang <sheng.yang@citrix.com>

        Show
        ASF subversion and git services added a comment - Commit 7260e8d83f07d90b48c34adaeb227de265019487 in branch refs/heads/pvlan from Hiroaki Kawai [ https://git-wip-us.apache.org/repos/asf?p=cloudstack.git;h=7260e8d ] CLOUDSTACK-1638 : Introduce NetworkMigrationResponder The location of the virtual machine is provided by DeployDestination, which will be passed in NetworkGuru#reserve and NetworkElement#prepare. During the virtual machine migration, it actually changes DeployDestination and it looks like that it will tell that event to network components as it has NetworkManager#prepareNicForMigration. The problem is that althogh the interface has that method, NetworkManagerImpl does not tell the DeployDestination changes to network components. So IMHO, we need to add calls of NetworkGuru#reserve and NetworkElement#prepare in NetworkManagerImpl#prepareNicForMigration . And then, we also need to add calls NetworkGuru#release and NetworkElement#release after the migration, otherwise the network resources that plugin reserved will be kept even when the vm leaves off. (Sheng Yang: rebase code, add license header) Signed-off-by: Sheng Yang <sheng.yang@citrix.com>
        Hide
        ASF subversion and git services added a comment -

        Commit 7260e8d83f07d90b48c34adaeb227de265019487 in branch refs/heads/master from Sheng Yang <sheng.yang@citrix.com>
        [ https://git-wip-us.apache.org/repos/asf?p=cloudstack.git;h=7260e8d ]

        CLOUDSTACK-1638: Introduce NetworkMigrationResponder

        The location of the virtual machine is provided by DeployDestination, which will
        be passed in NetworkGuru#reserve and NetworkElement#prepare.

        During the virtual machine migration, it actually changes DeployDestination and
        it looks like that it will tell that event to network components as it has
        NetworkManager#prepareNicForMigration. The problem is that althogh the interface
        has that method, NetworkManagerImpl does not tell the DeployDestination changes
        to network components.

        So IMHO, we need to add calls of NetworkGuru#reserve and NetworkElement#prepare
        in NetworkManagerImpl#prepareNicForMigration . And then, we also need to add
        calls NetworkGuru#release and NetworkElement#release after the migration,
        otherwise the network resources that plugin reserved will be kept even when the
        vm leaves off.

        (Sheng Yang: rebase code, add license header)

        Signed-off-by: Sheng Yang <sheng.yang@citrix.com>

        Show
        ASF subversion and git services added a comment - Commit 7260e8d83f07d90b48c34adaeb227de265019487 in branch refs/heads/master from Sheng Yang <sheng.yang@citrix.com> [ https://git-wip-us.apache.org/repos/asf?p=cloudstack.git;h=7260e8d ] CLOUDSTACK-1638 : Introduce NetworkMigrationResponder The location of the virtual machine is provided by DeployDestination, which will be passed in NetworkGuru#reserve and NetworkElement#prepare. During the virtual machine migration, it actually changes DeployDestination and it looks like that it will tell that event to network components as it has NetworkManager#prepareNicForMigration. The problem is that althogh the interface has that method, NetworkManagerImpl does not tell the DeployDestination changes to network components. So IMHO, we need to add calls of NetworkGuru#reserve and NetworkElement#prepare in NetworkManagerImpl#prepareNicForMigration . And then, we also need to add calls NetworkGuru#release and NetworkElement#release after the migration, otherwise the network resources that plugin reserved will be kept even when the vm leaves off. (Sheng Yang: rebase code, add license header) Signed-off-by: Sheng Yang <sheng.yang@citrix.com>
        Hide
        Hiroaki Kawai added a comment -

        Can I ask you which part is unsuitable for 4.0.x? I could not get it.

        Show
        Hiroaki Kawai added a comment - Can I ask you which part is unsuitable for 4.0.x? I could not get it.
        Hide
        Joe Brockmeier added a comment -

        Assigning to Chiradeep so he can take a look at the new patch here: https://reviews.apache.org/r/9871/

        Show
        Joe Brockmeier added a comment - Assigning to Chiradeep so he can take a look at the new patch here: https://reviews.apache.org/r/9871/
        Hide
        Joe Brockmeier added a comment -

        It looks like Hiroaki has addressed the comments and submitted a new patch, though I'm wondering if the changes being made due to Chiradeep's suggestions might make this unsuitable for 4.0.x.

        Show
        Joe Brockmeier added a comment - It looks like Hiroaki has addressed the comments and submitted a new patch, though I'm wondering if the changes being made due to Chiradeep's suggestions might make this unsuitable for 4.0.x.
        Hide
        Prasanna Santhanam added a comment -

        There are comments in the patch from Chiradeep on reviewboard. Please take a look

        Show
        Prasanna Santhanam added a comment - There are comments in the patch from Chiradeep on reviewboard. Please take a look
        Hide
        Hiroaki Kawai added a comment -

        Yes. As far as a plugin that handles migration wants to be run on, all the versions of cloudstack will be affected. I shall update affected versions.

        Show
        Hiroaki Kawai added a comment - Yes. As far as a plugin that handles migration wants to be run on, all the versions of cloudstack will be affected. I shall update affected versions.
        Hide
        Chip Childers added a comment -

        Does this affect the 4.1 branch as well? Looking at your patch, I think it does.

        Show
        Chip Childers added a comment - Does this affect the 4.1 branch as well? Looking at your patch, I think it does.
        Hide
        Hiroaki Kawai added a comment -
        Show
        Hiroaki Kawai added a comment - created a patch as https://reviews.apache.org/r/9871/

          People

          • Assignee:
            Hiroaki Kawai
            Reporter:
            Hiroaki Kawai
          • Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development