[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH v2] storage backend: Add sheepdog support



On 06/14/2012 05:14 AM, Sebastian Wiedenroth wrote:
> 
> This patch brings support to manage sheepdog pools and volumes to libvirt.
> It uses the "collie" command-line utility that comes with sheepdog for that.
> 
> A sheepdog pool in libvirt maps to a sheepdog cluster.
> It needs a host and port to connect to, which in most cases
> is just going to be the default of localhost on port 7000.
> 
> A sheepdog volume in libvirt maps to a sheepdog vdi.
> To create one specify the pool, a name and the capacity.
> Volumes can also be resized later.
> 
> In the volume XML the vdi name is prefixed with "sheepdog:"
> and put into the <target><path>. To use the volume as a disk source for
> virtual machines specify the vdi name as "name" attribute of the <source>.
> The host and port information from the pool are specified inside the host tag.
> 
>   <disk type='network'>
>     ...
>     <source protocol="sheepdog" name="vdi_name">
>       <host name="localhost" port="7000"/>
>     </source>
>   </disk>
> 
> To work right this patch parses the output of collie,
> so it relies on the raw output option. There recently was a bug which caused
> size information to be reported wrong. This is fixed upstream already and
> will be in the next release.

Should we be checking at configure and/or runtime whether we are talking
to a fixed collie?

> 
> Signed-off-by: Sebastian Wiedenroth <wiedi frubar net>

I've gone ahead and added you to AUTHORS in order to get 'make
syntax-check' to pass; let me know if you prefer an alternate alias.

> @@ -565,5 +568,64 @@
>        The RBD pool does not use the volume format type element.
>      </p>
>  
> +    <h2><a name="StorageBackendSheepdog">Sheepdog pools</a></h2>
> +    <p>
> +      This provides a pool based on a Sheepdog Cluster.
> +      Sheepdog is a distributed storage system for QEMU/KVM.
> +      It provides highly available block level storage volumes that
> +      can be attached to QEMU/KVM virtual machines.
> +
> +      The cluster must already be formated.

s/formated/formatted/


> +
> +    <h3>Example disk attachement</h3>

s/attachement/attachment/

> +++ b/src/storage/storage_backend_sheepdog.c
> @@ -0,0 +1,311 @@
> +/*
> + * storage_backend_sheepdog.c: storage backend for Sheepdog handling
> + *
> + * Copyright (C) 2012 Wido den Hollander
> + * Copyright (C) 2012 Frank Spijkerman
> + * Copyright (C) 2012 Sebastian Wiedenroth
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307  USA

/note to self - I really ought to finish the work I started on scrubbing
copyright notices for consistency, but that doesn't affect the
applicability of this patch


> +
> +void virStorageBackendSheepdogAddHostArg(virCommandPtr cmd, virStoragePoolObjPtr pool)

Long line; I would have formatted it:

void
virStorageBackendSheepdogAddHostArg(virCommandPtr cmd,
                                    virStoragePoolObjPtr pool)


> +static int
> +virStorageBackendSheepdogRefreshPool(virConnectPtr conn ATTRIBUTE_UNUSED,
> +                                     virStoragePoolObjPtr pool)
> +{
> +    int ret;
> +    char *output = NULL;
> +    virCommandPtr cmd;
> +
> +    cmd = virCommandNew(COLLIE);
> +    virCommandAddArgList(cmd, "node", "info", "-r", NULL);

Minor optimization - you could do this in one step:

cmd = virCommandNewArgList(COLLIE, "node", "info", "-r", NULL);

> +
> +static int
> +virStorageBackendSheepdogDeleteVol(virConnectPtr conn ATTRIBUTE_UNUSED,
> +                                   virStoragePoolObjPtr pool,
> +                                   virStorageVolDefPtr vol,
> +                                   unsigned int flags)

And unfortunately this is as far as I got today; I'll have to resume my
review later.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org



Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]