[libvirt] [PATCHv7 1/2] qemu: conf Implement domain RBD storage pool support

[Adam Walters]

On Thu, Jan 30, 2014 at 9:46 AM, Ján Tomko <jtomko at redhat.com> wrote:

> On 01/23/2014 08:45 PM, Adam Walters wrote:
> > This patch adds a helper function, qemuAddRBDPoolSourceHost, and
> > implements the usage of this function to allow RBD storage pools in QEMU
> > domain XML.
> >
> > The new function grabs RBD monitor hosts from the storage pool
> > definition and applies them to the domain's disk definition at runtime.
> > This function is used by my modifications to qemuTranslateDiskSourcePool
> > similar to the function used by the iSCSI code.
> >
> > My modifications to qemuTranslateDiskSourcePool is based heavily on the
> > existing iSCSI code, but modified to support RBD install. It will place
> > all relevant information into the domain's disk definition at runtime to
> > allow access to a RBD storage pool.
> >
> > Signed-off-by: Adam Walters <adam at pandorasboxen.com>
> > ---
> >  src/qemu/qemu_conf.c | 62
> +++++++++++++++++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 61 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
> > index ac53f6d..b1a6bfe 100644
> > --- a/src/qemu/qemu_conf.c
> > +++ b/src/qemu/qemu_conf.c
> > @@ -1439,8 +1478,29 @@ qemuTranslateDiskSourcePool(virConnectPtr conn,
> >         }
> >         break;
> >
> > -    case VIR_STORAGE_POOL_MPATH:
> >      case VIR_STORAGE_POOL_RBD:
> > +        if (def->startupPolicy) {
> > +            virReportError(VIR_ERR_XML_ERROR, "%s",
> > +                           _("'startupPolicy' is only valid for "
> > +                             "'file' file volume"));
> > +            goto cleanup;
> > +        }
>
> You should also check if srcpool->mode is DIRECT and document that it's
> valid
> for rbd pools in docs/formatdomain.html.in in the description of the mode
> attribute.
>

One of my early versions of this patch actually did check srcpool->mode
(and set it
to DIRECT if it wasn't set yet), but it was requested that I remove that
check since
there was only one mode for RBD pools. The removal of that code is what
prompted
my modification to the secret type checking in another patch in this
series. So which
is the correct method of implementation? Should pools that only have one
access
mode check for srcpool->mode to be set (and set a default if there isn't a
setting)?
My original thoughts were that the check would allow for a HOST access mode
in the
future, which might allow use of the kernel rbd module (which creates block
devices)
while also knowing the volume was served by Ceph.


>
> You could also extend the qemuxml2argvtest to check the generated QEMU
> command
> line (as we do for iscsi in qemuxml2argv-disk-source-pool-mode test).
>

I will certainly look into doing this.


>
> > +
> > +        def->srcpool->actualtype = VIR_DOMAIN_DISK_TYPE_NETWORK;
> > +        def->protocol = VIR_DOMAIN_DISK_PROTOCOL_RBD;
> > +
> > +        if (!(def->src = virStorageVolGetPath(vol)))
> > +            goto cleanup;
> > +
> > +        if (qemuTranslateDiskSourcePoolAuth(def, pooldef) < 0)
> > +            goto cleanup;
> > +
> > +        if (qemuAddRBDPoolSourceHost(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:
> >
>
> Jan
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140130/f7a81530/attachment-0001.htm>