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