[libvirt] [PATCH] storage: add RBD support to disk source pool translation

[John Ferlan]

On 06/02/2015 09:56 AM, Thibault VINCENT wrote:
> Domains can now reference disks of type 'volume' with an underlying RBD pool.
> It won't allow mapping snapshots, pools don't list them yet, only COW clones.
> 
>  - virStorageTranslateDiskSourcePool: add case to copy RBD attributes
>  - virStorageAddRBDPoolSourceHost: new helper to copy monitor hosts
> ---
>  src/storage/storage_driver.c | 49 +++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 48 insertions(+), 1 deletion(-)
> 

Started writing this before your most recent response - figured I'd
finish so you'd get my current thoughts...

Your cover letter indicates you didn't find any bit of documentation,
but I'll point out that the formatdomain.html.in describes the "<disk
type='volume'.../>" and how the "<source..." are described in order to
use a disk type volume..

There's no tests in this patch to "show" or "prove" that by simply
adding this code that libvirt will generate the correct qemu command in
order to find the disk and it's auth information.

Using gitk to find the commits that added the code it seems you copied
from the ISCSI version into virStorageTranslateDiskSourcePool, you'll
find a series of 3 patches - commit id's c00b2f0dd, 1f49b05a8, and
1b4eaa619 which allowed the direct access.  Patches since then have
moved the sources around a bit and changed the logic, but the intentions
are the same. That might be a good place to start to ensure you have a
way to have the domain XML recognize what it is you want and the qemu
command to include/find the disk for the domain.

Additionally, I just posted a patch from a recent bz regarding the auth
and 'secrettype' setting (or lack thereof), see:

http://www.redhat.com/archives/libvir-list/2015-June/msg00329.html

and it's follow-up.  Curiously that bz was generated because the domain
XML didn't have the 'secrettype' defined so when formatting for a
snapshot, there was an error. (OK - so you found this already...)

John
> diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
> index 394e4d4..8c27f4b 100644
> --- a/src/storage/storage_driver.c
> +++ b/src/storage/storage_driver.c
> @@ -3173,6 +3173,39 @@ virStorageAddISCSIPoolSourceHost(virDomainDiskDefPtr def,
>  
>  
>  static int
> +virStorageAddRBDPoolSourceHost(virDomainDiskDefPtr def,
> +                                virStoragePoolDefPtr pooldef)
> +{
> +    int ret = -1;
> +    size_t i;
> +
> +    if (pooldef->source.nhost > 0) {
> +        def->src->nhosts = pooldef->source.nhost;
> +
> +        if (VIR_ALLOC_N(def->src->hosts, def->src->nhosts) < 0)
> +            goto cleanup;
> +
> +        for (i = 0; i < def->src->nhosts; i++) {
> +            if (VIR_STRDUP(def->src->hosts[i].name,
> +                                        pooldef->source.hosts[i].name) < 0)
> +                goto cleanup;
> +
> +            if (pooldef->source.hosts[i].port) {
> +                if (virAsprintf(&def->src->hosts[i].port, "%d",
> +                                        pooldef->source.hosts[i].port) < 0)
> +                    goto cleanup;
> +            }
> +        }
> +    }
> +
> +    ret = 0;
> +
> + cleanup:
> +    return ret;
> +}
> +
> +
> +static int
>  virStorageTranslateDiskSourcePoolAuth(virDomainDiskDefPtr def,
>                                        virStoragePoolSourcePtr source)
>  {
> @@ -3324,8 +3357,22 @@ virStorageTranslateDiskSourcePool(virConnectPtr conn,
>         }
>         break;
>  
> -    case VIR_STORAGE_POOL_MPATH:
>      case VIR_STORAGE_POOL_RBD:
> +        if (!(def->src->path = virStorageVolGetPath(vol)))
> +            goto cleanup;
> +
> +        def->src->srcpool->actualtype = VIR_STORAGE_TYPE_NETWORK;
> +        def->src->protocol = VIR_STORAGE_NET_PROTOCOL_RBD;
> +
> +        if (virStorageTranslateDiskSourcePoolAuth(def, &pooldef->source) < 0)
> +            goto cleanup;
> +
> +        if (virStorageAddRBDPoolSourceHost(def, pooldef) < 0)
> +            goto cleanup;
> +
> +        break;
> +
> +    case VIR_STORAGE_POOL_MPATH:
>      case VIR_STORAGE_POOL_SHEEPDOG:
>      case VIR_STORAGE_POOL_GLUSTER:
>      case VIR_STORAGE_POOL_LAST:
>