On 07/14/2015 12:42 PM, John Ferlan wrote:
On 07/14/2015 04:15 AM, Wido den Hollander wrote:
We used to look at the librbd code version and depending on that
we would invoke rbd_create3() or rbd_create().
Since librbd version 0.67.9 we can however tell RBD that it should
create rbd format 2 images even if we invoke rbd_create().
The>> less options we pass to librbd, the more we can lean on the sane
defaults it uses.
For rbd_create3() we had things like the stripe count and unit hardcoded
in libvirt and that might cause problems down the road.
Hardcoding the feature bits is even worse. I think this is the right
approach.
Signed-off-by: Wido den Hollander <wido@xxxxxxxxx>
---
src/storage/storage_backend_rbd.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c
index 8e8d7a7..936ad18 100644
--- a/src/storage/storage_backend_rbd.c
+++ b/src/storage/storage_backend_rbd.c
@@ -66,6 +66,7 @@ static int virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr,
const char *client_mount_timeout = "30";
const char *mon_op_timeout = "30";
const char *osd_op_timeout = "30";
+ const char *rbd_default_format = "2";
if (authdef) {
VIR_DEBUG("Using cephx authorization, username: %s", authdef->username);
@@ -211,6 +212,14 @@ static int virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr,
VIR_DEBUG("Setting RADOS option rados_osd_op_timeout to %s", osd_op_timeout);
rados_conf_set(ptr->cluster, "rados_osd_op_timeout", osd_op_timeout);
+ /*
+ * Librbd supports creating RBD format 2 images. We no longer have to invoke
+ * rbd_create3(), we can tell librbd to default to format 2.
+ * This leaves us to simply use rbd_create() and use the default behavior of librbd
+ */
+ VIR_DEBUG("Setting RADOS option rbd_default_format to %s", rbd_default_format);
+ rados_conf_set(ptr->cluster, "rbd_default_format", rbd_default_format);
+
I assume (from above) 0.67.9 is the first time this option is recognized?
It's recognized in the bobtail (since 0.56.7) and cuttlefish (since
0.61.3) series as well. It was actually in all the dumpling (0.67.x)
releases.
ptr->starttime = time(0);
r = rados_connect(ptr->cluster);
if (r < 0) {
@@ -475,16 +484,7 @@ static int virStorageBackendRBDCreateImage(rados_ioctx_t io,
char *name, long capacity)
{
int order = 0;
-#if LIBRBD_VERSION_CODE > 260
- uint64_t features = 3;
- uint64_t stripe_count = 1;
- uint64_t stripe_unit = 4194304;
-
- if (rbd_create3(io, name, capacity, features, &order,
- stripe_unit, stripe_count) < 0) {
-#else
if (rbd_create(io, name, capacity, &order) < 0) {
-#endif
Not quite my area of expertise, but since this was a build time
check/change wouldn't this then impose a certain minimum version of rbd
on the libvirt package build/install environment (eg librbd1-devel)?
That is - if this were applied and installed on some host that didn't
have at least 0.67.9, then what would happen? Or one with less than 0.56?
The build time check was for the rbd_create3() function. This patch
removes that usage, and relies on the "rbd_default*" ceph options
which have no build time requirement.
At runtime a non-existent option will return ENOENT, which isn't checked
in this patch. I think that's fine, especially since dumpling (0.67.x)
is no longer maintained [1].
Just trying to prevent some less than obvious issue because some build
environment doesn't have the "latest and greatest" librbd.h installed
I'm glad you're vigilant about these, they're important. In this case
the patch looks good to me:
Reviewed-by: Josh Durgin <jdurgin@xxxxxxxxxx>
[1] http://ceph.com/docs/master/releases/
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list