You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by Niels de Vos <nd...@redhat.com> on 2013/12/01 15:07:34 UTC
Review Request 15932: Add support for Primary Storage on Gluster using the
libvirt backend
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15932/
-----------------------------------------------------------
Review request for cloudstack.
Repository: cloudstack-git
Description
-------
The support for Gluster as Primary Storage is mostly based on the
implementation for NFS. Like NFS, libvirt can address a Gluster environment
through the 'netfs' pool-type.
Diffs
-----
api/src/com/cloud/storage/Storage.java 07b6667
plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java 182cb22
plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtStoragePoolDef.java e181cea
plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java 0760e51
plugins/storage/volume/default/src/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackPrimaryDataStoreLifeCycleImpl.java 7555c1e
Diff: https://reviews.apache.org/r/15932/diff/
Testing
-------
See http://blog.nixpanic.net/2013/12/using-gluster-as-primary-storage-in.html
Thanks,
Niels de Vos
Re: Review Request 15932: Add support for Primary Storage on Gluster
using the libvirt backend
Posted by John Mark Walker <jo...@johnmark.org>.
Awesome. Thanks, guys!
On Feb 7, 2014 5:15 AM, "Niels de Vos" <nd...@redhat.com> wrote:
>
>
> > On Feb. 7, 2014, 10:33 a.m., Wido den Hollander wrote:
> > > My sincere apologies Niels! I completely missed the second version of
> your patch!
> > >
> > > I just took a look at it again and it seems pretty straight forward.
> Most of the RBD code did a lot of work for you, so it's fairly easy to have
> GlusterFS in CS.
> > >
> > > I tried applying the patch to the master branch and that failed. It
> seems that you wrote the patch against the 4.2 branch, correct?
> > >
> > > Could you try to rebase it again master? If it then applies we might
> be able to get GlusterFS into 4.4! :)
>
> Sure, no problem!
> I've done a rebase against 4.3 already, moving to master or 4.4 should not
> take too much time.
>
>
> - Niels
>
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15932/#review33922
> -----------------------------------------------------------
>
>
> On Jan. 14, 2014, 4:54 p.m., Niels de Vos wrote:
> >
> > -----------------------------------------------------------
> > This is an automatically generated e-mail. To reply, visit:
> > https://reviews.apache.org/r/15932/
> > -----------------------------------------------------------
> >
> > (Updated Jan. 14, 2014, 4:54 p.m.)
> >
> >
> > Review request for cloudstack.
> >
> >
> > Repository: cloudstack-git
> >
> >
> > Description
> > -------
> >
> > The support for Gluster as Primary Storage is mostly based on the
> > implementation for NFS. Like NFS, libvirt can address a Gluster
> environment
> > through the 'netfs' pool-type.
> >
> >
> > Diffs
> > -----
> >
> > api/src/com/cloud/storage/Storage.java 07b6667
> >
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java
> 182cb22
> >
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtStoragePoolDef.java
> e181cea
> >
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtStoragePoolXMLParser.java
> a707a0b
> >
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtVMDef.java
> 6aaabc5
> >
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java
> aaefc16
> >
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java
> 0760e51
> >
> plugins/storage/volume/default/src/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackPrimaryDataStoreLifeCycleImpl.java
> 7555c1e
> >
> > Diff: https://reviews.apache.org/r/15932/diff/
> >
> >
> > Testing
> > -------
> >
> > See
> http://blog.nixpanic.net/2013/12/using-gluster-as-primary-storage-in.html
> >
> >
> > Thanks,
> >
> > Niels de Vos
> >
> >
>
>
Re: Review Request 15932: Add support for Primary Storage on Gluster using
the libvirt backend
Posted by Niels de Vos <nd...@redhat.com>.
> On Feb. 7, 2014, 10:33 a.m., Wido den Hollander wrote:
> > My sincere apologies Niels! I completely missed the second version of your patch!
> >
> > I just took a look at it again and it seems pretty straight forward. Most of the RBD code did a lot of work for you, so it's fairly easy to have GlusterFS in CS.
> >
> > I tried applying the patch to the master branch and that failed. It seems that you wrote the patch against the 4.2 branch, correct?
> >
> > Could you try to rebase it again master? If it then applies we might be able to get GlusterFS into 4.4! :)
Sure, no problem!
I've done a rebase against 4.3 already, moving to master or 4.4 should not take too much time.
- Niels
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15932/#review33922
-----------------------------------------------------------
On Jan. 14, 2014, 4:54 p.m., Niels de Vos wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15932/
> -----------------------------------------------------------
>
> (Updated Jan. 14, 2014, 4:54 p.m.)
>
>
> Review request for cloudstack.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> The support for Gluster as Primary Storage is mostly based on the
> implementation for NFS. Like NFS, libvirt can address a Gluster environment
> through the 'netfs' pool-type.
>
>
> Diffs
> -----
>
> api/src/com/cloud/storage/Storage.java 07b6667
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java 182cb22
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtStoragePoolDef.java e181cea
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtStoragePoolXMLParser.java a707a0b
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtVMDef.java 6aaabc5
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java aaefc16
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java 0760e51
> plugins/storage/volume/default/src/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackPrimaryDataStoreLifeCycleImpl.java 7555c1e
>
> Diff: https://reviews.apache.org/r/15932/diff/
>
>
> Testing
> -------
>
> See http://blog.nixpanic.net/2013/12/using-gluster-as-primary-storage-in.html
>
>
> Thanks,
>
> Niels de Vos
>
>
Re: Review Request 15932: Add support for Primary Storage on Gluster using
the libvirt backend
Posted by Wido den Hollander <wi...@widodh.nl>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15932/#review33922
-----------------------------------------------------------
My sincere apologies Niels! I completely missed the second version of your patch!
I just took a look at it again and it seems pretty straight forward. Most of the RBD code did a lot of work for you, so it's fairly easy to have GlusterFS in CS.
I tried applying the patch to the master branch and that failed. It seems that you wrote the patch against the 4.2 branch, correct?
Could you try to rebase it again master? If it then applies we might be able to get GlusterFS into 4.4! :)
- Wido den Hollander
On Jan. 14, 2014, 3:54 p.m., Niels de Vos wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15932/
> -----------------------------------------------------------
>
> (Updated Jan. 14, 2014, 3:54 p.m.)
>
>
> Review request for cloudstack.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> The support for Gluster as Primary Storage is mostly based on the
> implementation for NFS. Like NFS, libvirt can address a Gluster environment
> through the 'netfs' pool-type.
>
>
> Diffs
> -----
>
> api/src/com/cloud/storage/Storage.java 07b6667
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java 182cb22
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtStoragePoolDef.java e181cea
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtStoragePoolXMLParser.java a707a0b
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtVMDef.java 6aaabc5
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java aaefc16
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java 0760e51
> plugins/storage/volume/default/src/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackPrimaryDataStoreLifeCycleImpl.java 7555c1e
>
> Diff: https://reviews.apache.org/r/15932/diff/
>
>
> Testing
> -------
>
> See http://blog.nixpanic.net/2013/12/using-gluster-as-primary-storage-in.html
>
>
> Thanks,
>
> Niels de Vos
>
>
Re: Review Request 15932: Add support for Primary Storage on Gluster
using the libvirt backend
Posted by Sebastien Goasguen <ru...@gmail.com>.
> On Feb. 19, 2014, 1:35 p.m., Wido den Hollander wrote:
> > It seems good to me. Applies cleanly to master and builds just fine.
> >
> > Code-wise it's simple but effective, should allow us to support Gluster.
>
> Wido den Hollander wrote:
> I just merged it into master and pushed.
>
> So gluster is in master right now! Niels, can I ask you to test it all again? Just to make sure the code all works like you intended.
>
> Niels de Vos wrote:
> Thanks Wido!
> This seems to be working OK for me. Note that the UI modification (https://reviews.apache.org/r/15933/) have not been reviewed/merged yet. Without these, it's rather difficult for users to configure Primary Storage on Gluster.
>
> Also, I've got asked about the dependencies and configuration. I'll add that here for now, and I'll try figure out how to get it added to the documentation:
>
> In /etc/glusterfs/glusterd.vol, allow unprivileged ports to contact the 'management' volume to get the volume configuration:
>
> option rpc-auth-allow-insecure on
>
> After changing the glusterd.vol file, restart the glusterd service to apply the changes.
>
> Per volume, allow unprivileged ports to access the brick processes (glusterfsd):
>
> # gluster volume set <volname> server.allow-insecure on
> # gluster volume stop <volume>
> # gluster volume start <volume>
>
> Per volume make sure that the kvm user (uid=36) and kvm group (gid=36) can access the images on the volume:
>
> # gluster volume set <volname> storage.owner-uid 36
> # gluster volume set <volname> storage.owner-gid 36
>
> Other dependencies:
> * libvirt version 1.0.1 (gluster protocol/network disk support)
> * qemu version 1.3 (gluster block backend support)
>
> Note that RHEL-6.5 and derived distributions contain backports that add sufficient functionality too.
>
> Niels de Vos wrote:
> Some further testing showed that there can be some issues with starting virtual machines which have disks on Gluster. https://reviews.apache.org/r/18412/ contains a solution for that.
Wido, could you check that this is in 4.4, 4.5 and master and if not commit it ?
trying to clean review board a bit.
thanks
- Sebastien
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15932/#review34859
-----------------------------------------------------------
On Feb. 19, 2014, 8:24 a.m., Niels de Vos wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15932/
> -----------------------------------------------------------
>
> (Updated Feb. 19, 2014, 8:24 a.m.)
>
>
> Review request for cloudstack.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> The support for Gluster as Primary Storage is mostly based on the
> implementation for NFS. Like NFS, libvirt can address a Gluster environment
> through the 'netfs' pool-type.
>
>
> Diffs
> -----
>
> api/src/com/cloud/storage/Storage.java ff83dfc
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java d63b643
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtStoragePoolDef.java dbe5d4b
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtStoragePoolXMLParser.java a6186f6
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtVMDef.java ff75d61
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java 8cdecd8
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java a5f33eb
> plugins/storage/volume/default/src/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackPrimaryDataStoreLifeCycleImpl.java b90d5fc
>
> Diff: https://reviews.apache.org/r/15932/diff/
>
>
> Testing
> -------
>
> See http://blog.nixpanic.net/2013/12/using-gluster-as-primary-storage-in.html
>
>
> Thanks,
>
> Niels de Vos
>
>
Re: Review Request 15932: Add support for Primary Storage on Gluster
using the libvirt backend
Posted by Marcus <sh...@gmail.com>.
If you get a moment, please look at resize-root branch and see if it
works for Gluster. I didn't see any code specific to creating volumes
in your patch, mostly just pool stuff, so hopefully it's covered.
Also, Wido, as mentioned on the bug for resize-root I'd prefer it if
you could review the changes made for RBD support. We basically just
override the size for new root volumes with the size of the volume
object passed rather than assuming template size, if it's greater than
the template size.
On Sun, Feb 23, 2014 at 8:23 AM, Niels de Vos <nd...@redhat.com> wrote:
>
>
>> On Feb. 19, 2014, 2:35 p.m., Wido den Hollander wrote:
>> > It seems good to me. Applies cleanly to master and builds just fine.
>> >
>> > Code-wise it's simple but effective, should allow us to support Gluster.
>>
>> Wido den Hollander wrote:
>> I just merged it into master and pushed.
>>
>> So gluster is in master right now! Niels, can I ask you to test it all again? Just to make sure the code all works like you intended.
>>
>> Niels de Vos wrote:
>> Thanks Wido!
>> This seems to be working OK for me. Note that the UI modification (https://reviews.apache.org/r/15933/) have not been reviewed/merged yet. Without these, it's rather difficult for users to configure Primary Storage on Gluster.
>>
>> Also, I've got asked about the dependencies and configuration. I'll add that here for now, and I'll try figure out how to get it added to the documentation:
>>
>> In /etc/glusterfs/glusterd.vol, allow unprivileged ports to contact the 'management' volume to get the volume configuration:
>>
>> option rpc-auth-allow-insecure on
>>
>> After changing the glusterd.vol file, restart the glusterd service to apply the changes.
>>
>> Per volume, allow unprivileged ports to access the brick processes (glusterfsd):
>>
>> # gluster volume set <volname> server.allow-insecure on
>> # gluster volume stop <volume>
>> # gluster volume start <volume>
>>
>> Per volume make sure that the kvm user (uid=36) and kvm group (gid=36) can access the images on the volume:
>>
>> # gluster volume set <volname> storage.owner-uid 36
>> # gluster volume set <volname> storage.owner-gid 36
>>
>> Other dependencies:
>> * libvirt version 1.0.1 (gluster protocol/network disk support)
>> * qemu version 1.3 (gluster block backend support)
>>
>> Note that RHEL-6.5 and derived distributions contain backports that add sufficient functionality too.
>
> Some further testing showed that there can be some issues with starting virtual machines which have disks on Gluster. https://reviews.apache.org/r/18412/ contains a solution for that.
>
>
> - Niels
>
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15932/#review34859
> -----------------------------------------------------------
>
>
> On Feb. 19, 2014, 9:24 a.m., Niels de Vos wrote:
>>
>> -----------------------------------------------------------
>> This is an automatically generated e-mail. To reply, visit:
>> https://reviews.apache.org/r/15932/
>> -----------------------------------------------------------
>>
>> (Updated Feb. 19, 2014, 9:24 a.m.)
>>
>>
>> Review request for cloudstack.
>>
>>
>> Repository: cloudstack-git
>>
>>
>> Description
>> -------
>>
>> The support for Gluster as Primary Storage is mostly based on the
>> implementation for NFS. Like NFS, libvirt can address a Gluster environment
>> through the 'netfs' pool-type.
>>
>>
>> Diffs
>> -----
>>
>> api/src/com/cloud/storage/Storage.java ff83dfc
>> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java d63b643
>> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtStoragePoolDef.java dbe5d4b
>> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtStoragePoolXMLParser.java a6186f6
>> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtVMDef.java ff75d61
>> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java 8cdecd8
>> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java a5f33eb
>> plugins/storage/volume/default/src/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackPrimaryDataStoreLifeCycleImpl.java b90d5fc
>>
>> Diff: https://reviews.apache.org/r/15932/diff/
>>
>>
>> Testing
>> -------
>>
>> See http://blog.nixpanic.net/2013/12/using-gluster-as-primary-storage-in.html
>>
>>
>> Thanks,
>>
>> Niels de Vos
>>
>>
>
Re: Review Request 15932: Add support for Primary Storage on Gluster using
the libvirt backend
Posted by Niels de Vos <nd...@redhat.com>.
> On Feb. 19, 2014, 2:35 p.m., Wido den Hollander wrote:
> > It seems good to me. Applies cleanly to master and builds just fine.
> >
> > Code-wise it's simple but effective, should allow us to support Gluster.
>
> Wido den Hollander wrote:
> I just merged it into master and pushed.
>
> So gluster is in master right now! Niels, can I ask you to test it all again? Just to make sure the code all works like you intended.
>
> Niels de Vos wrote:
> Thanks Wido!
> This seems to be working OK for me. Note that the UI modification (https://reviews.apache.org/r/15933/) have not been reviewed/merged yet. Without these, it's rather difficult for users to configure Primary Storage on Gluster.
>
> Also, I've got asked about the dependencies and configuration. I'll add that here for now, and I'll try figure out how to get it added to the documentation:
>
> In /etc/glusterfs/glusterd.vol, allow unprivileged ports to contact the 'management' volume to get the volume configuration:
>
> option rpc-auth-allow-insecure on
>
> After changing the glusterd.vol file, restart the glusterd service to apply the changes.
>
> Per volume, allow unprivileged ports to access the brick processes (glusterfsd):
>
> # gluster volume set <volname> server.allow-insecure on
> # gluster volume stop <volume>
> # gluster volume start <volume>
>
> Per volume make sure that the kvm user (uid=36) and kvm group (gid=36) can access the images on the volume:
>
> # gluster volume set <volname> storage.owner-uid 36
> # gluster volume set <volname> storage.owner-gid 36
>
> Other dependencies:
> * libvirt version 1.0.1 (gluster protocol/network disk support)
> * qemu version 1.3 (gluster block backend support)
>
> Note that RHEL-6.5 and derived distributions contain backports that add sufficient functionality too.
Some further testing showed that there can be some issues with starting virtual machines which have disks on Gluster. https://reviews.apache.org/r/18412/ contains a solution for that.
- Niels
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15932/#review34859
-----------------------------------------------------------
On Feb. 19, 2014, 9:24 a.m., Niels de Vos wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15932/
> -----------------------------------------------------------
>
> (Updated Feb. 19, 2014, 9:24 a.m.)
>
>
> Review request for cloudstack.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> The support for Gluster as Primary Storage is mostly based on the
> implementation for NFS. Like NFS, libvirt can address a Gluster environment
> through the 'netfs' pool-type.
>
>
> Diffs
> -----
>
> api/src/com/cloud/storage/Storage.java ff83dfc
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java d63b643
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtStoragePoolDef.java dbe5d4b
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtStoragePoolXMLParser.java a6186f6
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtVMDef.java ff75d61
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java 8cdecd8
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java a5f33eb
> plugins/storage/volume/default/src/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackPrimaryDataStoreLifeCycleImpl.java b90d5fc
>
> Diff: https://reviews.apache.org/r/15932/diff/
>
>
> Testing
> -------
>
> See http://blog.nixpanic.net/2013/12/using-gluster-as-primary-storage-in.html
>
>
> Thanks,
>
> Niels de Vos
>
>
Re: Review Request 15932: Add support for Primary Storage on Gluster using
the libvirt backend
Posted by Niels de Vos <nd...@redhat.com>.
> On Feb. 19, 2014, 2:35 p.m., Wido den Hollander wrote:
> > It seems good to me. Applies cleanly to master and builds just fine.
> >
> > Code-wise it's simple but effective, should allow us to support Gluster.
>
> Wido den Hollander wrote:
> I just merged it into master and pushed.
>
> So gluster is in master right now! Niels, can I ask you to test it all again? Just to make sure the code all works like you intended.
Thanks Wido!
This seems to be working OK for me. Note that the UI modification (https://reviews.apache.org/r/15933/) have not been reviewed/merged yet. Without these, it's rather difficult for users to configure Primary Storage on Gluster.
Also, I've got asked about the dependencies and configuration. I'll add that here for now, and I'll try figure out how to get it added to the documentation:
In /etc/glusterfs/glusterd.vol, allow unprivileged ports to contact the 'management' volume to get the volume configuration:
option rpc-auth-allow-insecure on
After changing the glusterd.vol file, restart the glusterd service to apply the changes.
Per volume, allow unprivileged ports to access the brick processes (glusterfsd):
# gluster volume set <volname> server.allow-insecure on
# gluster volume stop <volume>
# gluster volume start <volume>
Per volume make sure that the kvm user (uid=36) and kvm group (gid=36) can access the images on the volume:
# gluster volume set <volname> storage.owner-uid 36
# gluster volume set <volname> storage.owner-gid 36
Other dependencies:
* libvirt version 1.0.1 (gluster protocol/network disk support)
* qemu version 1.3 (gluster block backend support)
Note that RHEL-6.5 and derived distributions contain backports that add sufficient functionality too.
- Niels
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15932/#review34859
-----------------------------------------------------------
On Feb. 19, 2014, 9:24 a.m., Niels de Vos wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15932/
> -----------------------------------------------------------
>
> (Updated Feb. 19, 2014, 9:24 a.m.)
>
>
> Review request for cloudstack.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> The support for Gluster as Primary Storage is mostly based on the
> implementation for NFS. Like NFS, libvirt can address a Gluster environment
> through the 'netfs' pool-type.
>
>
> Diffs
> -----
>
> api/src/com/cloud/storage/Storage.java ff83dfc
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java d63b643
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtStoragePoolDef.java dbe5d4b
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtStoragePoolXMLParser.java a6186f6
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtVMDef.java ff75d61
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java 8cdecd8
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java a5f33eb
> plugins/storage/volume/default/src/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackPrimaryDataStoreLifeCycleImpl.java b90d5fc
>
> Diff: https://reviews.apache.org/r/15932/diff/
>
>
> Testing
> -------
>
> See http://blog.nixpanic.net/2013/12/using-gluster-as-primary-storage-in.html
>
>
> Thanks,
>
> Niels de Vos
>
>
Re: Review Request 15932: Add support for Primary Storage on Gluster using
the libvirt backend
Posted by Wido den Hollander <wi...@widodh.nl>.
> On Feb. 19, 2014, 1:35 p.m., Wido den Hollander wrote:
> > It seems good to me. Applies cleanly to master and builds just fine.
> >
> > Code-wise it's simple but effective, should allow us to support Gluster.
I just merged it into master and pushed.
So gluster is in master right now! Niels, can I ask you to test it all again? Just to make sure the code all works like you intended.
- Wido
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15932/#review34859
-----------------------------------------------------------
On Feb. 19, 2014, 8:24 a.m., Niels de Vos wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15932/
> -----------------------------------------------------------
>
> (Updated Feb. 19, 2014, 8:24 a.m.)
>
>
> Review request for cloudstack.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> The support for Gluster as Primary Storage is mostly based on the
> implementation for NFS. Like NFS, libvirt can address a Gluster environment
> through the 'netfs' pool-type.
>
>
> Diffs
> -----
>
> api/src/com/cloud/storage/Storage.java ff83dfc
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java d63b643
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtStoragePoolDef.java dbe5d4b
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtStoragePoolXMLParser.java a6186f6
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtVMDef.java ff75d61
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java 8cdecd8
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java a5f33eb
> plugins/storage/volume/default/src/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackPrimaryDataStoreLifeCycleImpl.java b90d5fc
>
> Diff: https://reviews.apache.org/r/15932/diff/
>
>
> Testing
> -------
>
> See http://blog.nixpanic.net/2013/12/using-gluster-as-primary-storage-in.html
>
>
> Thanks,
>
> Niels de Vos
>
>
Re: Review Request 15932: Add support for Primary Storage on Gluster using
the libvirt backend
Posted by Wido den Hollander <wi...@widodh.nl>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15932/#review34859
-----------------------------------------------------------
Ship it!
It seems good to me. Applies cleanly to master and builds just fine.
Code-wise it's simple but effective, should allow us to support Gluster.
- Wido den Hollander
On Feb. 19, 2014, 8:24 a.m., Niels de Vos wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15932/
> -----------------------------------------------------------
>
> (Updated Feb. 19, 2014, 8:24 a.m.)
>
>
> Review request for cloudstack.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> The support for Gluster as Primary Storage is mostly based on the
> implementation for NFS. Like NFS, libvirt can address a Gluster environment
> through the 'netfs' pool-type.
>
>
> Diffs
> -----
>
> api/src/com/cloud/storage/Storage.java ff83dfc
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java d63b643
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtStoragePoolDef.java dbe5d4b
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtStoragePoolXMLParser.java a6186f6
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtVMDef.java ff75d61
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java 8cdecd8
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java a5f33eb
> plugins/storage/volume/default/src/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackPrimaryDataStoreLifeCycleImpl.java b90d5fc
>
> Diff: https://reviews.apache.org/r/15932/diff/
>
>
> Testing
> -------
>
> See http://blog.nixpanic.net/2013/12/using-gluster-as-primary-storage-in.html
>
>
> Thanks,
>
> Niels de Vos
>
>
Re: Review Request 15932: Add support for Primary Storage on Gluster
using the libvirt backend
Posted by Sebastien Goasguen <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15932/#review79012
-----------------------------------------------------------
Thank you for submitting your CloudStack contribution through review board. After discussion on the dev@cloudstack.apache.org the community decided to close down review board and start accepting contributiong through GitHub pull requests. We have been using GH PR for several months now and the process is better than review board.
We will keep Review Board open for another week to give you time to migrate your patch to a github PR if you wish. After that time, your patch will no longer be viewable (even though it will not be deleted).
Please consider submitting a pull request.
Great instructions are available at:
https://github.com/apache/cloudstack/blob/master/CONTRIBUTING.md
Thank you very much for your time and your contribution to Apache CloudStack, we hope that using this new process will encourage you to do more.
- Sebastien Goasguen
On Feb. 19, 2014, 8:24 a.m., Niels de Vos wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15932/
> -----------------------------------------------------------
>
> (Updated Feb. 19, 2014, 8:24 a.m.)
>
>
> Review request for cloudstack.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> The support for Gluster as Primary Storage is mostly based on the
> implementation for NFS. Like NFS, libvirt can address a Gluster environment
> through the 'netfs' pool-type.
>
>
> Diffs
> -----
>
> api/src/com/cloud/storage/Storage.java ff83dfc
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java d63b643
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtStoragePoolDef.java dbe5d4b
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtStoragePoolXMLParser.java a6186f6
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtVMDef.java ff75d61
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java 8cdecd8
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java a5f33eb
> plugins/storage/volume/default/src/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackPrimaryDataStoreLifeCycleImpl.java b90d5fc
>
> Diff: https://reviews.apache.org/r/15932/diff/
>
>
> Testing
> -------
>
> See http://blog.nixpanic.net/2013/12/using-gluster-as-primary-storage-in.html
>
>
> Thanks,
>
> Niels de Vos
>
>
Re: Review Request 15932: Add support for Primary Storage on Gluster using
the libvirt backend
Posted by Niels de Vos <nd...@redhat.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15932/
-----------------------------------------------------------
(Updated Feb. 19, 2014, 9:24 a.m.)
Review request for cloudstack.
Changes
-------
Rebased to recent master (4.4) and addressed all (?) review comments.
Repository: cloudstack-git
Description
-------
The support for Gluster as Primary Storage is mostly based on the
implementation for NFS. Like NFS, libvirt can address a Gluster environment
through the 'netfs' pool-type.
Diffs (updated)
-----
api/src/com/cloud/storage/Storage.java ff83dfc
plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java d63b643
plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtStoragePoolDef.java dbe5d4b
plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtStoragePoolXMLParser.java a6186f6
plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtVMDef.java ff75d61
plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java 8cdecd8
plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java a5f33eb
plugins/storage/volume/default/src/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackPrimaryDataStoreLifeCycleImpl.java b90d5fc
Diff: https://reviews.apache.org/r/15932/diff/
Testing
-------
See http://blog.nixpanic.net/2013/12/using-gluster-as-primary-storage-in.html
Thanks,
Niels de Vos
Re: Review Request 15932: Add support for Primary Storage on Gluster using
the libvirt backend
Posted by Niels de Vos <nd...@redhat.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15932/
-----------------------------------------------------------
(Updated Jan. 14, 2014, 4:54 p.m.)
Review request for cloudstack.
Changes
-------
Addressed all/most of the previous comments.
This change also extend the previous version of the patch, and adds support for QEMU+libgfapi native communication. Mounting the the Gluster volume is only used to perform management tasks on the storage pool (create, delete disks, etc.).
Comments much appreciated, thanks,
Niels
Repository: cloudstack-git
Description
-------
The support for Gluster as Primary Storage is mostly based on the
implementation for NFS. Like NFS, libvirt can address a Gluster environment
through the 'netfs' pool-type.
Diffs (updated)
-----
api/src/com/cloud/storage/Storage.java 07b6667
plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java 182cb22
plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtStoragePoolDef.java e181cea
plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtStoragePoolXMLParser.java a707a0b
plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtVMDef.java 6aaabc5
plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java aaefc16
plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java 0760e51
plugins/storage/volume/default/src/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackPrimaryDataStoreLifeCycleImpl.java 7555c1e
Diff: https://reviews.apache.org/r/15932/diff/
Testing
-------
See http://blog.nixpanic.net/2013/12/using-gluster-as-primary-storage-in.html
Thanks,
Niels de Vos
Re: Review Request 15932: Add support for Primary Storage on Gluster using
the libvirt backend
Posted by Amogh Vasekar <am...@citrix.com>.
> On Dec. 4, 2013, 4:17 p.m., John Burwell wrote:
> >
>
> Niels de Vos wrote:
> Thanks John! I'll have a go at impoving the patch and will update it (hopefully) soon.
Reminder -
Hi,
This review has been pending for long. Please do update the patch so it may be shipped soon.
Thanks!
- Amogh
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15932/#review29591
-----------------------------------------------------------
On Dec. 1, 2013, 2:07 p.m., Niels de Vos wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15932/
> -----------------------------------------------------------
>
> (Updated Dec. 1, 2013, 2:07 p.m.)
>
>
> Review request for cloudstack.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> The support for Gluster as Primary Storage is mostly based on the
> implementation for NFS. Like NFS, libvirt can address a Gluster environment
> through the 'netfs' pool-type.
>
>
> Diffs
> -----
>
> api/src/com/cloud/storage/Storage.java 07b6667
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java 182cb22
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtStoragePoolDef.java e181cea
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java 0760e51
> plugins/storage/volume/default/src/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackPrimaryDataStoreLifeCycleImpl.java 7555c1e
>
> Diff: https://reviews.apache.org/r/15932/diff/
>
>
> Testing
> -------
>
> See http://blog.nixpanic.net/2013/12/using-gluster-as-primary-storage-in.html
>
>
> Thanks,
>
> Niels de Vos
>
>
Re: Review Request 15932: Add support for Primary Storage on Gluster
using the libvirt backend
Posted by Marcus Sorensen <sh...@gmail.com>.
What's the minimum libvirt version that supports glusterfs?
On Dec 5, 2013 6:54 AM, "Niels de Vos" <nd...@redhat.com> wrote:
>
>
> > On Dec. 4, 2013, 5:17 p.m., John Burwell wrote:
> > >
>
> Thanks John! I'll have a go at impoving the patch and will update it
> (hopefully) soon.
>
>
> - Niels
>
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15932/#review29591
> -----------------------------------------------------------
>
>
> On Dec. 1, 2013, 3:07 p.m., Niels de Vos wrote:
> >
> > -----------------------------------------------------------
> > This is an automatically generated e-mail. To reply, visit:
> > https://reviews.apache.org/r/15932/
> > -----------------------------------------------------------
> >
> > (Updated Dec. 1, 2013, 3:07 p.m.)
> >
> >
> > Review request for cloudstack.
> >
> >
> > Repository: cloudstack-git
> >
> >
> > Description
> > -------
> >
> > The support for Gluster as Primary Storage is mostly based on the
> > implementation for NFS. Like NFS, libvirt can address a Gluster
> environment
> > through the 'netfs' pool-type.
> >
> >
> > Diffs
> > -----
> >
> > api/src/com/cloud/storage/Storage.java 07b6667
> >
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java
> 182cb22
> >
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtStoragePoolDef.java
> e181cea
> >
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java
> 0760e51
> >
> plugins/storage/volume/default/src/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackPrimaryDataStoreLifeCycleImpl.java
> 7555c1e
> >
> > Diff: https://reviews.apache.org/r/15932/diff/
> >
> >
> > Testing
> > -------
> >
> > See
> http://blog.nixpanic.net/2013/12/using-gluster-as-primary-storage-in.html
> >
> >
> > Thanks,
> >
> > Niels de Vos
> >
> >
>
>
Re: Review Request 15932: Add support for Primary Storage on Gluster using
the libvirt backend
Posted by Niels de Vos <nd...@redhat.com>.
> On Dec. 4, 2013, 5:17 p.m., John Burwell wrote:
> >
Thanks John! I'll have a go at impoving the patch and will update it (hopefully) soon.
- Niels
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15932/#review29591
-----------------------------------------------------------
On Dec. 1, 2013, 3:07 p.m., Niels de Vos wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15932/
> -----------------------------------------------------------
>
> (Updated Dec. 1, 2013, 3:07 p.m.)
>
>
> Review request for cloudstack.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> The support for Gluster as Primary Storage is mostly based on the
> implementation for NFS. Like NFS, libvirt can address a Gluster environment
> through the 'netfs' pool-type.
>
>
> Diffs
> -----
>
> api/src/com/cloud/storage/Storage.java 07b6667
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java 182cb22
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtStoragePoolDef.java e181cea
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java 0760e51
> plugins/storage/volume/default/src/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackPrimaryDataStoreLifeCycleImpl.java 7555c1e
>
> Diff: https://reviews.apache.org/r/15932/diff/
>
>
> Testing
> -------
>
> See http://blog.nixpanic.net/2013/12/using-gluster-as-primary-storage-in.html
>
>
> Thanks,
>
> Niels de Vos
>
>
Re: Review Request 15932: Add support for Primary Storage on Gluster using
the libvirt backend
Posted by Niels de Vos <nd...@redhat.com>.
> On Dec. 4, 2013, 5:17 p.m., John Burwell wrote:
> >
>
> Niels de Vos wrote:
> Thanks John! I'll have a go at impoving the patch and will update it (hopefully) soon.
>
> Amogh Vasekar wrote:
> Reminder -
> Hi,
> This review has been pending for long. Please do update the patch so it may be shipped soon.
> Thanks!
Thanks for the reminder. I think all comments have been addressed in the next version of the patch. I have not rewritten existing parts that could get improved based on the comments (like StringBuilder). My preference is to keep this patch Gluster specific and not modify unrelated bits.
Cheers,
Niels
> On Dec. 4, 2013, 5:17 p.m., John Burwell wrote:
> > plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtStoragePoolDef.java, line 133
> > <https://reviews.apache.org/r/15932/diff/1/?file=392660#file392660line133>
> >
> > If the Gluster poolType is netfs, why isn't this rule captured in the enumeration? This feels like a leaky abstraction that will be difficult to maintain.
> >
> > Also, concatenating strings as part of StringBuilder append cancels out the performance benefits of StringBuilder. This code should be converted to the following:
> >
> > storagePoolBuilder.append("<pool type='");
> > storagePoolBuilder.append(_poolType);
> > storagePoolBuilder.append("'>");
> > storagePoolBuilder.append(System.lineSeparator());
Currently libvirt can only mount glusterfs with help from the glusterfs-fuse client. In libvirt, this is configured as a "netfs"-pool with type=glusterfs. In future, libvirt will be able to use the native GlusterFS protocol (through libgfapi.so), which will result in a different type of pool.
I do not see how to write the code for a netfs+glusterfs much cleaner.
- StringBuilder changes have been applied.
> On Dec. 4, 2013, 5:17 p.m., John Burwell wrote:
> > plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java, line 362
> > <https://reviews.apache.org/r/15932/diff/1/?file=392661#file392661line362>
> >
> > Extract this if block to factory method on the poolType enumeration (e.g. toStoragePoolType). It should also include a catch all that throws a CloudRuntimeException for an unsupported type mapping.
I'm not sure how you intend this. It seems very common to use if-else-if statements for this...
> On Dec. 4, 2013, 5:17 p.m., John Burwell wrote:
> > plugins/storage/volume/default/src/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackPrimaryDataStoreLifeCycleImpl.java, line 286
> > <https://reviews.apache.org/r/15932/diff/1/?file=392662#file392662line286>
> >
> > Magic value. This default port value should be captured somewhere in the Gluster driver as a constant.
It follows the same concept as the existing other drivers... I'm not sure where it would be more suitable to place this.
- Niels
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15932/#review29591
-----------------------------------------------------------
On Dec. 1, 2013, 3:07 p.m., Niels de Vos wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15932/
> -----------------------------------------------------------
>
> (Updated Dec. 1, 2013, 3:07 p.m.)
>
>
> Review request for cloudstack.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> The support for Gluster as Primary Storage is mostly based on the
> implementation for NFS. Like NFS, libvirt can address a Gluster environment
> through the 'netfs' pool-type.
>
>
> Diffs
> -----
>
> api/src/com/cloud/storage/Storage.java 07b6667
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java 182cb22
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtStoragePoolDef.java e181cea
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java 0760e51
> plugins/storage/volume/default/src/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackPrimaryDataStoreLifeCycleImpl.java 7555c1e
>
> Diff: https://reviews.apache.org/r/15932/diff/
>
>
> Testing
> -------
>
> See http://blog.nixpanic.net/2013/12/using-gluster-as-primary-storage-in.html
>
>
> Thanks,
>
> Niels de Vos
>
>
Re: Review Request 15932: Add support for Primary Storage on Gluster using
the libvirt backend
Posted by John Burwell <jb...@basho.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15932/#review29591
-----------------------------------------------------------
plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtStoragePoolDef.java
<https://reviews.apache.org/r/15932/#comment57191>
If the Gluster poolType is netfs, why isn't this rule captured in the enumeration? This feels like a leaky abstraction that will be difficult to maintain.
Also, concatenating strings as part of StringBuilder append cancels out the performance benefits of StringBuilder. This code should be converted to the following:
storagePoolBuilder.append("<pool type='");
storagePoolBuilder.append(_poolType);
storagePoolBuilder.append("'>");
storagePoolBuilder.append(System.lineSeparator());
plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtStoragePoolDef.java
<https://reviews.apache.org/r/15932/#comment56987>
By coding standards, please remove the tab characters.
plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtStoragePoolDef.java
<https://reviews.apache.org/r/15932/#comment57192>
Remove the string concatenations from the StringBuilder appends to avoid needless string allocation that StringBuilder is intended to avoid.
plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java
<https://reviews.apache.org/r/15932/#comment57193>
The log level of this message should be a WARN to let operators know that the state of the underlying storage may be out of sync with the CloudStack configuration.
plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java
<https://reviews.apache.org/r/15932/#comment57194>
Extract this if block to factory method on the poolType enumeration (e.g. toStoragePoolType). It should also include a catch all that throws a CloudRuntimeException for an unsupported type mapping.
plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java
<https://reviews.apache.org/r/15932/#comment57195>
Lines 518 and 519 can be collapsed to the following:
s_logger.error("Failed to create mount.", e).
Also, please include some state information (e.g. mount name, point, etc) in the error message so that an operator can correlate the error with the CloudStack configuration and/or the underlying storage device.
plugins/storage/volume/default/src/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackPrimaryDataStoreLifeCycleImpl.java
<https://reviews.apache.org/r/15932/#comment57196>
Magic value. This default port value should be captured somewhere in the Gluster driver as a constant.
- John Burwell
On Dec. 1, 2013, 9:07 a.m., Niels de Vos wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15932/
> -----------------------------------------------------------
>
> (Updated Dec. 1, 2013, 9:07 a.m.)
>
>
> Review request for cloudstack.
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> The support for Gluster as Primary Storage is mostly based on the
> implementation for NFS. Like NFS, libvirt can address a Gluster environment
> through the 'netfs' pool-type.
>
>
> Diffs
> -----
>
> api/src/com/cloud/storage/Storage.java 07b6667
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java 182cb22
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtStoragePoolDef.java e181cea
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java 0760e51
> plugins/storage/volume/default/src/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackPrimaryDataStoreLifeCycleImpl.java 7555c1e
>
> Diff: https://reviews.apache.org/r/15932/diff/
>
>
> Testing
> -------
>
> See http://blog.nixpanic.net/2013/12/using-gluster-as-primary-storage-in.html
>
>
> Thanks,
>
> Niels de Vos
>
>