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

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

 



On 01/30/2014 06:06 PM, Adam Walters wrote:
> On Thu, Jan 30, 2014 at 9:46 AM, Ján Tomko <jtomko@xxxxxxxxxx
> <mailto:jtomko@xxxxxxxxxx>> 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@xxxxxxxxxxxxxxxxx
>     <mailto:adam@xxxxxxxxxxxxxxxxx>>
>     > ---
>     >  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 <http://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.

Right, I didn't notice it was already checked above the switch:
    if (def->srcpool->mode && pooldef->type != VIR_STORAGE_POOL_ISCSI) {
        virReportError(VIR_ERR_XML_ERROR, "%s",
                       _("disk source mode is only valid when "
                         "storage pool is of iscsi type"));
        goto cleanup;
    }


> 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)?

If there is only one access mode, there is no need to specify it and no need
for a check.

> 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.
>  

If that happens, we can allow MODE_DIRECT and MODE_HOST too, along with
MODE_DEFAULT.

Jan

Attachment: signature.asc
Description: OpenPGP digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]