You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by Marcus Sorensen <sh...@gmail.com> on 2013/04/24 23:23:52 UTC

Review Request: Delete KVMHA dir contents if found when deleting templates, volumes from nfs secondary storage

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10766/
-----------------------------------------------------------

Review request for cloudstack and Chip Childers.


Description
-------

the KVM HA runner uses any NFS secondary storage resource available to a host to store it's HA data. This causes template deletes to fail because it cannot delete KVMHA, which is a directory that is not empty. So if KVMHA directory is found, delete it's contents before trying to delete it.


This addresses bug CLOUDSTACK-2173.


Diffs
-----

  core/src/com/cloud/storage/resource/NfsSecondaryStorageResource.java e65cbe1 

Diff: https://reviews.apache.org/r/10766/diff/


Testing
-------

Had a bunch of templates that the scavenger was failing to clean up. Applied this patch, installed the code, and the templates all cleaned up successfully.


Thanks,

Marcus Sorensen


Re: Review Request: Delete KVMHA dir contents if found when deleting templates, volumes from nfs secondary storage

Posted by Marcus Sorensen <sh...@gmail.com>.

> On April 24, 2013, 11:05 p.m., edison su wrote:
> > The HA will write data into secondary storage also? It should only write data into primary storage.
> 
> Marcus Sorensen wrote:
>     The KVMStoragePoolManager is currently adding any NFS type to the list of HA pools to write to. I can change that as well, I'm just not very familiar with why it was working that way in the first place. Will look at it now.

Ok, I think I know what the issue is. In 4.0, primary storage pools were created via KVMStoragePoolManager's createStoragePool, which calls LibvirtStorageAdaptor's createStoragePool, and then adds it to the HA list. Secondary storage shortcuts directly into LibvirtStorageAdaptor's createStoragePool, and never gets added to the HA list. In 4.1, everything goes through KVMStoragePool so we can identify the proper storage adaptor. I see no simple way to differentiate primary vs secondary NFS storage from within KVMStoragePoolManager, other than the 'template/tmpl', 'volumes', and 'snapshots' directories in their paths. We may have to pass that in.


- Marcus


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10766/#review19676
-----------------------------------------------------------


On April 24, 2013, 9:23 p.m., Marcus Sorensen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10766/
> -----------------------------------------------------------
> 
> (Updated April 24, 2013, 9:23 p.m.)
> 
> 
> Review request for cloudstack and Chip Childers.
> 
> 
> Description
> -------
> 
> the KVM HA runner uses any NFS secondary storage resource available to a host to store it's HA data. This causes template deletes to fail because it cannot delete KVMHA, which is a directory that is not empty. So if KVMHA directory is found, delete it's contents before trying to delete it.
> 
> 
> This addresses bug CLOUDSTACK-2173.
> 
> 
> Diffs
> -----
> 
>   core/src/com/cloud/storage/resource/NfsSecondaryStorageResource.java e65cbe1 
> 
> Diff: https://reviews.apache.org/r/10766/diff/
> 
> 
> Testing
> -------
> 
> Had a bunch of templates that the scavenger was failing to clean up. Applied this patch, installed the code, and the templates all cleaned up successfully.
> 
> 
> Thanks,
> 
> Marcus Sorensen
> 
>


Re: Review Request: Delete KVMHA dir contents if found when deleting templates, volumes from nfs secondary storage

Posted by Marcus Sorensen <sh...@gmail.com>.

> On April 24, 2013, 11:05 p.m., edison su wrote:
> > The HA will write data into secondary storage also? It should only write data into primary storage.

The KVMStoragePoolManager is currently adding any NFS type to the list of HA pools to write to. I can change that as well, I'm just not very familiar with why it was working that way in the first place. Will look at it now.


- Marcus


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10766/#review19676
-----------------------------------------------------------


On April 24, 2013, 9:23 p.m., Marcus Sorensen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10766/
> -----------------------------------------------------------
> 
> (Updated April 24, 2013, 9:23 p.m.)
> 
> 
> Review request for cloudstack and Chip Childers.
> 
> 
> Description
> -------
> 
> the KVM HA runner uses any NFS secondary storage resource available to a host to store it's HA data. This causes template deletes to fail because it cannot delete KVMHA, which is a directory that is not empty. So if KVMHA directory is found, delete it's contents before trying to delete it.
> 
> 
> This addresses bug CLOUDSTACK-2173.
> 
> 
> Diffs
> -----
> 
>   core/src/com/cloud/storage/resource/NfsSecondaryStorageResource.java e65cbe1 
> 
> Diff: https://reviews.apache.org/r/10766/diff/
> 
> 
> Testing
> -------
> 
> Had a bunch of templates that the scavenger was failing to clean up. Applied this patch, installed the code, and the templates all cleaned up successfully.
> 
> 
> Thanks,
> 
> Marcus Sorensen
> 
>


Re: Review Request: Delete KVMHA dir contents if found when deleting templates, volumes from nfs secondary storage

Posted by edison su <ed...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10766/#review19676
-----------------------------------------------------------


The HA will write data into secondary storage also? It should only write data into primary storage.

- edison su


On April 24, 2013, 9:23 p.m., Marcus Sorensen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10766/
> -----------------------------------------------------------
> 
> (Updated April 24, 2013, 9:23 p.m.)
> 
> 
> Review request for cloudstack and Chip Childers.
> 
> 
> Description
> -------
> 
> the KVM HA runner uses any NFS secondary storage resource available to a host to store it's HA data. This causes template deletes to fail because it cannot delete KVMHA, which is a directory that is not empty. So if KVMHA directory is found, delete it's contents before trying to delete it.
> 
> 
> This addresses bug CLOUDSTACK-2173.
> 
> 
> Diffs
> -----
> 
>   core/src/com/cloud/storage/resource/NfsSecondaryStorageResource.java e65cbe1 
> 
> Diff: https://reviews.apache.org/r/10766/diff/
> 
> 
> Testing
> -------
> 
> Had a bunch of templates that the scavenger was failing to clean up. Applied this patch, installed the code, and the templates all cleaned up successfully.
> 
> 
> Thanks,
> 
> Marcus Sorensen
> 
>


Re: Review Request: Don't do KVM heartbeat on secondary storage sources, primary only

Posted by Chip Childers <ch...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10766/#review19702
-----------------------------------------------------------

Ship it!


Ship It!

- Chip Childers


On April 25, 2013, 1:28 p.m., Marcus Sorensen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10766/
> -----------------------------------------------------------
> 
> (Updated April 25, 2013, 1:28 p.m.)
> 
> 
> Review request for cloudstack and Chip Childers.
> 
> 
> Description
> -------
> 
> the KVM HA runner uses any NFS secondary storage resource available to a host to store it's HA data. This causes template deletes to fail because it cannot delete KVMHA, which is a directory that is not empty. So if KVMHA directory is found, delete it's contents before trying to delete it.
> 
> 
> This addresses bug CLOUDSTACK-2173.
> 
> 
> Diffs
> -----
> 
>   plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/KVMHAMonitor.java d1470d6 
>   plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/KVMStoragePoolManager.java c2bfad9 
> 
> Diff: https://reviews.apache.org/r/10766/diff/
> 
> 
> Testing
> -------
> 
> diff revision 2 was tested with a new 4.1 zone deployment. Verified bug was reproducable with 4.1 HEAD, applied patch, ran through adding two NFS primary storages, verified KVM heartbeat was working on them, then ran various secondary storage operations (register template, download volume, take snapshot) and verified that they worked, and that KVM heartbeat operations were NOT acting on them.
> 
> 
> Thanks,
> 
> Marcus Sorensen
> 
>


Re: Review Request: Don't do KVM heartbeat on secondary storage sources, primary only

Posted by Marcus Sorensen <sh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10766/
-----------------------------------------------------------

(Updated April 25, 2013, 1:28 p.m.)


Review request for cloudstack and Chip Childers.


Changes
-------

removing attachment


Description
-------

the KVM HA runner uses any NFS secondary storage resource available to a host to store it's HA data. This causes template deletes to fail because it cannot delete KVMHA, which is a directory that is not empty. So if KVMHA directory is found, delete it's contents before trying to delete it.


This addresses bug CLOUDSTACK-2173.


Diffs
-----

  plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/KVMHAMonitor.java d1470d6 
  plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/KVMStoragePoolManager.java c2bfad9 

Diff: https://reviews.apache.org/r/10766/diff/


Testing
-------

diff revision 2 was tested with a new 4.1 zone deployment. Verified bug was reproducable with 4.1 HEAD, applied patch, ran through adding two NFS primary storages, verified KVM heartbeat was working on them, then ran various secondary storage operations (register template, download volume, take snapshot) and verified that they worked, and that KVM heartbeat operations were NOT acting on them.


Thanks,

Marcus Sorensen


Re: Review Request: Don't do KVM heartbeat on secondary storage sources, primary only

Posted by Marcus Sorensen <sh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10766/
-----------------------------------------------------------

(Updated April 25, 2013, 4:52 a.m.)


Review request for cloudstack and Chip Childers.


Changes
-------

updated summary to reflect adjustment in patch


Summary (updated)
-----------------

Don't do KVM heartbeat on secondary storage sources, primary only


Description
-------

the KVM HA runner uses any NFS secondary storage resource available to a host to store it's HA data. This causes template deletes to fail because it cannot delete KVMHA, which is a directory that is not empty. So if KVMHA directory is found, delete it's contents before trying to delete it.


This addresses bug CLOUDSTACK-2173.


Diffs
-----

  plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/KVMHAMonitor.java d1470d6 
  plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/KVMStoragePoolManager.java c2bfad9 

Diff: https://reviews.apache.org/r/10766/diff/


Testing
-------

diff revision 2 was tested with a new 4.1 zone deployment. Verified bug was reproducable with 4.1 HEAD, applied patch, ran through adding two NFS primary storages, verified KVM heartbeat was working on them, then ran various secondary storage operations (register template, download volume, take snapshot) and verified that they worked, and that KVM heartbeat operations were NOT acting on them.


Thanks,

Marcus Sorensen


Re: Review Request: Delete KVMHA dir contents if found when deleting templates, volumes from nfs secondary storage

Posted by Marcus Sorensen <sh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10766/
-----------------------------------------------------------

(Updated April 25, 2013, 4:51 a.m.)


Review request for cloudstack and Chip Childers.


Changes
-------

edited testing notes


Description
-------

the KVM HA runner uses any NFS secondary storage resource available to a host to store it's HA data. This causes template deletes to fail because it cannot delete KVMHA, which is a directory that is not empty. So if KVMHA directory is found, delete it's contents before trying to delete it.


This addresses bug CLOUDSTACK-2173.


Diffs
-----

  plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/KVMHAMonitor.java d1470d6 
  plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/KVMStoragePoolManager.java c2bfad9 

Diff: https://reviews.apache.org/r/10766/diff/


Testing (updated)
-------

diff revision 2 was tested with a new 4.1 zone deployment. Verified bug was reproducable with 4.1 HEAD, applied patch, ran through adding two NFS primary storages, verified KVM heartbeat was working on them, then ran various secondary storage operations (register template, download volume, take snapshot) and verified that they worked, and that KVM heartbeat operations were NOT acting on them.


Thanks,

Marcus Sorensen


Re: Review Request: Delete KVMHA dir contents if found when deleting templates, volumes from nfs secondary storage

Posted by Marcus Sorensen <sh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10766/
-----------------------------------------------------------

(Updated April 25, 2013, 4:48 a.m.)


Review request for cloudstack and Chip Childers.


Changes
-------

In both 4.0 and 4.1, all secondary storage is added via KVMStoragePoolManager.getStoragePoolByURI, but in 4.0 this calls LibvirtComputingResource.createStoragePool, and in 4.1 it calls KVMStoragePoolManager.createStoragePool, the primary difference being that the KVMStoragePoolManager version adds the pool to the list of KVMHAMonitor pools to run heartbeat against, then also calls <appropriateStorageAdaptor>.createStoragePool.

This patch allows us to continue using everything through KVMStoragePoolManager so that we can support multiple storage adaptors, but any secondary storage registration (via getStoragePoolByURI) skips the adding to the KVMHA heartbeat list, and so functions like it did in 4.0

In the process a null pointer was discovered in KVMHAMonitor.java, this includes a fix for that as well.


Description
-------

the KVM HA runner uses any NFS secondary storage resource available to a host to store it's HA data. This causes template deletes to fail because it cannot delete KVMHA, which is a directory that is not empty. So if KVMHA directory is found, delete it's contents before trying to delete it.


This addresses bug CLOUDSTACK-2173.


Diffs (updated)
-----

  plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/KVMHAMonitor.java d1470d6 
  plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/KVMStoragePoolManager.java c2bfad9 

Diff: https://reviews.apache.org/r/10766/diff/


Testing
-------

Had a bunch of templates that the scavenger was failing to clean up. Applied this patch, installed the code, and the templates all cleaned up successfully.


Thanks,

Marcus Sorensen


Re: Review Request: Delete KVMHA dir contents if found when deleting templates, volumes from nfs secondary storage

Posted by Marcus Sorensen <sh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10766/
-----------------------------------------------------------

(Updated April 25, 2013, 12:39 a.m.)


Review request for cloudstack and Chip Childers.


Changes
-------

does this look better? if storage is NFS and we're fetching pool by URI, call LibvirtStorageAdaptor's createStoragePool instead of KVMStoragePoolManager's createStoragePool.

Testing now... will update status when I can.


Description
-------

the KVM HA runner uses any NFS secondary storage resource available to a host to store it's HA data. This causes template deletes to fail because it cannot delete KVMHA, which is a directory that is not empty. So if KVMHA directory is found, delete it's contents before trying to delete it.


This addresses bug CLOUDSTACK-2173.


Diffs
-----

  core/src/com/cloud/storage/resource/NfsSecondaryStorageResource.java e65cbe1 

Diff: https://reviews.apache.org/r/10766/diff/


Testing
-------

Had a bunch of templates that the scavenger was failing to clean up. Applied this patch, installed the code, and the templates all cleaned up successfully.


Thanks,

Marcus Sorensen