8:11 a.m.
This series of patches changes a couple of things in the RBD storage pool
driver.
The first change is that it includes the return status of librados and librbd
in log and debug messages making it easier for users to track down the real
cause of the problem.
The second patch simplifies some code.
The third patch leverages the new timeout options from librados. Should the
backing Ceph cluster not respond for any reason librados will timeout.
This prevents libvirt from handing on a rados_* or rbd_* call and locking up
libvirt.
If the librados version on the system does not support these timeout options
nothing happens. libvirt compiles and runs just fine, but the timeout simply
won't work.
Wido den Hollander (3):
rbd: Include return statusses from librados/librbd in logging
rbd: Simplify opening RADOS IoCTX
rbd: Set timeout options for librados
src/storage/storage_backend_rbd.c | 137 ++++++++++++++++++++++---------------
1 file changed, 80 insertions(+), 57 deletions(-)
--
1.7.9.5
8:11 a.m.
New subject: [libvirt] [PATCH 1/3] rbd: Include return statusses from librados/librbd in logging
With this information it's easier for the user to debug what is
going wrong.
---
src/storage/storage_backend_rbd.c | 119 +++++++++++++++++++++----------------
1 file changed, 68 insertions(+), 51 deletions(-)
diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c
index 1b35cc4..bd21873 100644
--- a/src/storage/storage_backend_rbd.c
+++ b/src/storage/storage_backend_rbd.c
@@ -51,6 +51,7 @@ static int
virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr,
virStoragePoolObjPtr pool)
{
int ret = -1;
+ int r = 0;
unsigned char *secret_value = NULL;
size_t secret_value_size;
char *rados_key = NULL;
@@ -65,10 +66,10 @@ static int
virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr,
if (pool->def->source.auth.cephx.username != NULL) {
VIR_DEBUG("Using cephx authorization");
- if (rados_create(&ptr->cluster,
- pool->def->source.auth.cephx.username) < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("failed to initialize RADOS"));
+ r = rados_create(&ptr->cluster,
pool->def->source.auth.cephx.username);
+ if (r < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("failed to initialize RADOS: %d"), r);
goto cleanup;
}
@@ -198,10 +199,11 @@ static int
virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr,
}
ptr->starttime = time(0);
- if (rados_connect(ptr->cluster) < 0) {
+ r = rados_connect(ptr->cluster);
+ if (r < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
- _("failed to connect to the RADOS monitor on: %s"),
- mon_buff);
+ _("failed to connect to the RADOS monitor on: %s: %d"),
+ mon_buff, r);
goto cleanup;
}
@@ -248,18 +250,22 @@ static int volStorageBackendRBDRefreshVolInfo(virStorageVolDefPtr
vol,
virStorageBackendRBDStatePtr ptr)
{
int ret = -1;
+ int r = 0;
rbd_image_t image;
- if (rbd_open(ptr->ioctx, vol->name, &image, NULL) < 0) {
+
+ r = rbd_open(ptr->ioctx, vol->name, &image, NULL);
+ if (r < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
- _("failed to open the RBD image '%s'"),
- vol->name);
+ _("failed to open the RBD image '%s': %d"),
+ vol->name, r);
return ret;
}
rbd_image_info_t info;
- if (rbd_stat(image, &info, sizeof(info)) < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("failed to stat the RBD image"));
+ r = rbd_stat(image, &info, sizeof(info));
+ if (r < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("failed to stat the RBD image: %d"), r);
goto cleanup;
}
@@ -297,6 +303,7 @@ static int virStorageBackendRBDRefreshPool(virConnectPtr conn,
size_t max_size = 1024;
int ret = -1;
int len = -1;
+ int r = 0;
char *name, *names = NULL;
virStorageBackendRBDState ptr;
ptr.cluster = NULL;
@@ -306,26 +313,28 @@ static int virStorageBackendRBDRefreshPool(virConnectPtr conn,
goto cleanup;
}
- if (rados_ioctx_create(ptr.cluster,
- pool->def->source.name, &ptr.ioctx) < 0) {
+ r = rados_ioctx_create(ptr.cluster, pool->def->source.name, &ptr.ioctx);
+ if (r < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
- _("failed to create the RBD IoCTX. Does the pool '%s'
exist?"),
- pool->def->source.name);
+ _("failed to create the RBD IoCTX. Does the pool '%s'
exist?: %d"),
+ pool->def->source.name, r);
goto cleanup;
}
struct rados_cluster_stat_t clusterstat;
- if (rados_cluster_stat(ptr.cluster, &clusterstat) < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("failed to stat the RADOS cluster"));
+ r = rados_cluster_stat(ptr.cluster, &clusterstat);
+ if (r < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("failed to stat the RADOS cluster: %d"), r);
goto cleanup;
}
struct rados_pool_stat_t poolstat;
- if (rados_ioctx_pool_stat(ptr.ioctx, &poolstat) < 0) {
+ r = rados_ioctx_pool_stat(ptr.ioctx, &poolstat);
+ if (r < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
- _("failed to stat the RADOS pool '%s'"),
- pool->def->source.name);
+ _("failed to stat the RADOS pool '%s': %d"),
+ pool->def->source.name, r);
goto cleanup;
}
@@ -398,6 +407,7 @@ static int virStorageBackendRBDDeleteVol(virConnectPtr conn,
unsigned int flags)
{
int ret = -1;
+ int r = 0;
virStorageBackendRBDState ptr;
ptr.cluster = NULL;
ptr.ioctx = NULL;
@@ -412,19 +422,20 @@ static int virStorageBackendRBDDeleteVol(virConnectPtr conn,
goto cleanup;
}
- if (rados_ioctx_create(ptr.cluster,
- pool->def->source.name, &ptr.ioctx) < 0) {
+ r = rados_ioctx_create(ptr.cluster, pool->def->source.name, &ptr.ioctx);
+ if (r < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
- _("failed to create the RBD IoCTX. Does the pool '%s'
exist?"),
- pool->def->source.name);
+ _("failed to create the RBD IoCTX. Does the pool '%s'
exist?: %d"),
+ pool->def->source.name, r);
goto cleanup;
}
- if (rbd_remove(ptr.ioctx, vol->name) < 0) {
+ r = rbd_remove(ptr.ioctx, vol->name);
+ if (r < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
- _("failed to remove volume '%s/%s'"),
+ _("failed to remove volume '%s/%s': %d"),
pool->def->source.name,
- vol->name);
+ vol->name, r);
goto cleanup;
}
@@ -488,6 +499,7 @@ virStorageBackendRBDBuildVol(virConnectPtr conn,
ptr.cluster = NULL;
ptr.ioctx = NULL;
int ret = -1;
+ int r = 0;
VIR_DEBUG("Creating RBD image %s/%s with size %llu",
pool->def->source.name,
@@ -498,11 +510,11 @@ virStorageBackendRBDBuildVol(virConnectPtr conn,
if (virStorageBackendRBDOpenRADOSConn(&ptr, conn, pool) < 0)
goto cleanup;
- if (rados_ioctx_create(ptr.cluster,
- pool->def->source.name, &ptr.ioctx) < 0) {
+ r = rados_ioctx_create(ptr.cluster, pool->def->source.name, &ptr.ioctx);
+ if (r < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
- _("failed to create the RBD IoCTX. Does the pool '%s'
exist?"),
- pool->def->source.name);
+ _("failed to create the RBD IoCTX. Does the pool '%s'
exist?: %d"),
+ pool->def->source.name, r);
goto cleanup;
}
@@ -512,11 +524,12 @@ virStorageBackendRBDBuildVol(virConnectPtr conn,
goto cleanup;
}
- if (virStorageBackendRBDCreateImage(ptr.ioctx, vol->name, vol->capacity) <
0) {
+ r = virStorageBackendRBDCreateImage(ptr.ioctx, vol->name, vol->capacity);
+ if (r < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
- _("failed to create volume '%s/%s'"),
+ _("failed to create volume '%s/%s': %d"),
pool->def->source.name,
- vol->name);
+ vol->name, r);
goto cleanup;
}
@@ -538,16 +551,17 @@ static int virStorageBackendRBDRefreshVol(virConnectPtr conn,
ptr.cluster = NULL;
ptr.ioctx = NULL;
int ret = -1;
+ int r = 0;
if (virStorageBackendRBDOpenRADOSConn(&ptr, conn, pool) < 0) {
goto cleanup;
}
- if (rados_ioctx_create(ptr.cluster,
- pool->def->source.name, &ptr.ioctx) < 0) {
+ r = rados_ioctx_create(ptr.cluster, pool->def->source.name, &ptr.ioctx);
+ if (r < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
- _("failed to create the RBD IoCTX. Does the pool '%s'
exist?"),
- pool->def->source.name);
+ _("failed to create the RBD IoCTX. Does the pool '%s'
exist?: %d"),
+ pool->def->source.name, r);
goto cleanup;
}
@@ -573,6 +587,7 @@ static int virStorageBackendRBDResizeVol(virConnectPtr conn
ATTRIBUTE_UNUSED,
ptr.ioctx = NULL;
rbd_image_t image = NULL;
int ret = -1;
+ int r = 0;
virCheckFlags(0, -1);
@@ -580,25 +595,27 @@ static int virStorageBackendRBDResizeVol(virConnectPtr conn
ATTRIBUTE_UNUSED,
goto cleanup;
}
- if (rados_ioctx_create(ptr.cluster,
- pool->def->source.name, &ptr.ioctx) < 0) {
+ r = rados_ioctx_create(ptr.cluster, pool->def->source.name, &ptr.ioctx);
+ if (r < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
- _("failed to create the RBD IoCTX. Does the pool '%s'
exist?"),
- pool->def->source.name);
+ _("failed to create the RBD IoCTX. Does the pool '%s'
exist?: %d"),
+ pool->def->source.name, r);
goto cleanup;
}
- if (rbd_open(ptr.ioctx, vol->name, &image, NULL) < 0) {
+ r = rbd_open(ptr.ioctx, vol->name, &image, NULL);
+ if (r < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
- _("failed to open the RBD image '%s'"),
- vol->name);
+ _("failed to open the RBD image '%s': %d"),
+ vol->name, r);
goto cleanup;
}
- if (rbd_resize(image, capacity) < 0) {
+ r = rbd_resize(image, capacity);
+ if (r < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
- _("failed to resize the RBD image '%s'"),
- vol->name);
+ _("failed to resize the RBD image '%s': %d"),
+ vol->name, r);
goto cleanup;
}
--
1.7.9.5
8:11 a.m.
New subject: [libvirt] [PATCH 2/3] rbd: Simplify opening RADOS IoCTX
Reduces code and brings logging back to one function.
---
src/storage/storage_backend_rbd.c | 43 ++++++++++++++-----------------------
1 file changed, 16 insertions(+), 27 deletions(-)
diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c
index bd21873..8d1e320 100644
--- a/src/storage/storage_backend_rbd.c
+++ b/src/storage/storage_backend_rbd.c
@@ -221,6 +221,17 @@ cleanup:
return ret;
}
+static int virStorageBackendRBDOpenIoCTX(virStorageBackendRBDStatePtr ptr,
virStoragePoolObjPtr pool)
+{
+ int r = rados_ioctx_create(ptr->cluster, pool->def->source.name,
&ptr->ioctx);
+ if (r < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("failed to create the RBD IoCTX. Does the pool '%s'
exist?: %d"),
+ pool->def->source.name, r);
+ }
+ return r;
+}
+
static int virStorageBackendRBDCloseRADOSConn(virStorageBackendRBDStatePtr ptr)
{
int ret = 0;
@@ -313,11 +324,7 @@ static int virStorageBackendRBDRefreshPool(virConnectPtr conn,
goto cleanup;
}
- r = rados_ioctx_create(ptr.cluster, pool->def->source.name, &ptr.ioctx);
- if (r < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("failed to create the RBD IoCTX. Does the pool '%s'
exist?: %d"),
- pool->def->source.name, r);
+ if (virStorageBackendRBDOpenIoCTX(&ptr, pool) < 0) {
goto cleanup;
}
@@ -422,11 +429,7 @@ static int virStorageBackendRBDDeleteVol(virConnectPtr conn,
goto cleanup;
}
- r = rados_ioctx_create(ptr.cluster, pool->def->source.name, &ptr.ioctx);
- if (r < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("failed to create the RBD IoCTX. Does the pool '%s'
exist?: %d"),
- pool->def->source.name, r);
+ if (virStorageBackendRBDOpenIoCTX(&ptr, pool) < 0) {
goto cleanup;
}
@@ -510,13 +513,8 @@ virStorageBackendRBDBuildVol(virConnectPtr conn,
if (virStorageBackendRBDOpenRADOSConn(&ptr, conn, pool) < 0)
goto cleanup;
- r = rados_ioctx_create(ptr.cluster, pool->def->source.name, &ptr.ioctx);
- if (r < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("failed to create the RBD IoCTX. Does the pool '%s'
exist?: %d"),
- pool->def->source.name, r);
+ if (virStorageBackendRBDOpenIoCTX(&ptr, pool) < 0)
goto cleanup;
- }
if (vol->target.encryption != NULL) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
@@ -551,17 +549,12 @@ static int virStorageBackendRBDRefreshVol(virConnectPtr conn,
ptr.cluster = NULL;
ptr.ioctx = NULL;
int ret = -1;
- int r = 0;
if (virStorageBackendRBDOpenRADOSConn(&ptr, conn, pool) < 0) {
goto cleanup;
}
- r = rados_ioctx_create(ptr.cluster, pool->def->source.name, &ptr.ioctx);
- if (r < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("failed to create the RBD IoCTX. Does the pool '%s'
exist?: %d"),
- pool->def->source.name, r);
+ if (virStorageBackendRBDOpenIoCTX(&ptr, pool) < 0) {
goto cleanup;
}
@@ -595,11 +588,7 @@ static int virStorageBackendRBDResizeVol(virConnectPtr conn
ATTRIBUTE_UNUSED,
goto cleanup;
}
- r = rados_ioctx_create(ptr.cluster, pool->def->source.name, &ptr.ioctx);
- if (r < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("failed to create the RBD IoCTX. Does the pool '%s'
exist?: %d"),
- pool->def->source.name, r);
+ if (virStorageBackendRBDOpenIoCTX(&ptr, pool) < 0) {
goto cleanup;
}
--
1.7.9.5
8:04 a.m.
New subject: [libvirt] [PATCH 2/3] rbd: Simplify opening RADOS IoCTX
On 02/12/2014 03:11 PM, Wido den Hollander wrote:
Reduces code and brings logging back to one function.
---
src/storage/storage_backend_rbd.c | 43 ++++++++++++++-----------------------
1 file changed, 16 insertions(+), 27 deletions(-)
diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c
index bd21873..8d1e320 100644
--- a/src/storage/storage_backend_rbd.c
+++ b/src/storage/storage_backend_rbd.c
@@ -221,6 +221,17 @@ cleanup:
return ret;
}
+static int virStorageBackendRBDOpenIoCTX(virStorageBackendRBDStatePtr ptr,
virStoragePoolObjPtr pool)
+{
+ int r = rados_ioctx_create(ptr->cluster, pool->def->source.name,
&ptr->ioctx);
+ if (r < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("failed to create the RBD IoCTX. Does the pool
'%s' exist?: %d"),
+ pool->def->source.name, r);
Looking at the documentation [1], librados returns -errno.
It would be more user-friendly to report it as a string instead of a number.
I'd suggest using:
virReportSystemError(-r,
_("failed to create the RBD IoCTX. "
"Does the pool '%s' exist?'),
pool->def->source.name);
Also, putting this patch first would reduce the number of error messages that
need to be changed.
Jan
[1] http://ceph.com/docs/master/rados/api/librados/
3:50 a.m.
New subject: [libvirt] [PATCH 0/2] Improve RBD storage pool backend
The first patch improves the logging for the user. Currently the return
codes from librados are not written to any logfile, which might leave a
user clueless to why the storage pool is not working.
The second patch sets three timeout options in librados. These are useful for
when the Ceph cluster is down. Librados will then not block for ever, but
return a -ETIMEDOUT so libvirt does not block for ever.
Wido den Hollander (2):
rbd: Include return statusses from librados/librbd in logging
rbd: Set timeout options for librados
src/storage/storage_backend_rbd.c | 144 ++++++++++++++++++++-----------------
1 file changed, 78 insertions(+), 66 deletions(-)
--
1.7.9.5
3:50 a.m.
New subject: [libvirt] [PATCH 1/2] rbd: Include return statusses from librados/librbd in logging
With this information it's easier for the user to debug what is
going wrong.
---
src/storage/storage_backend_rbd.c | 127 ++++++++++++++++++-------------------
1 file changed, 61 insertions(+), 66 deletions(-)
diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c
index 1b35cc4..abb110e 100644
--- a/src/storage/storage_backend_rbd.c
+++ b/src/storage/storage_backend_rbd.c
@@ -51,6 +51,7 @@ static int
virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr,
virStoragePoolObjPtr pool)
{
int ret = -1;
+ int r = 0;
unsigned char *secret_value = NULL;
size_t secret_value_size;
char *rados_key = NULL;
@@ -65,10 +66,9 @@ static int
virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr,
if (pool->def->source.auth.cephx.username != NULL) {
VIR_DEBUG("Using cephx authorization");
- if (rados_create(&ptr->cluster,
- pool->def->source.auth.cephx.username) < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("failed to initialize RADOS"));
+ r = rados_create(&ptr->cluster,
pool->def->source.auth.cephx.username);
+ if (r < 0) {
+ virReportSystemError(-r, "%s", _("failed to initialize
RADOS"));
goto cleanup;
}
@@ -198,10 +198,10 @@ static int
virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr,
}
ptr->starttime = time(0);
- if (rados_connect(ptr->cluster) < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("failed to connect to the RADOS monitor on: %s"),
- mon_buff);
+ r = rados_connect(ptr->cluster);
+ if (r < 0) {
+ virReportSystemError(-r, _("failed to connect to the RADOS monitor on:
%s"),
+ mon_buff);
goto cleanup;
}
@@ -219,6 +219,16 @@ cleanup:
return ret;
}
+static int virStorageBackendRBDOpenIoCTX(virStorageBackendRBDStatePtr ptr,
virStoragePoolObjPtr pool)
+{
+ int r = rados_ioctx_create(ptr->cluster, pool->def->source.name,
&ptr->ioctx);
+ if (r < 0) {
+ virReportSystemError(-r, _("failed to create the RBD IoCTX. Does the pool
'%s' exist?"),
+ pool->def->source.name);
+ }
+ return r;
+}
+
static int virStorageBackendRBDCloseRADOSConn(virStorageBackendRBDStatePtr ptr)
{
int ret = 0;
@@ -248,18 +258,21 @@ static int volStorageBackendRBDRefreshVolInfo(virStorageVolDefPtr
vol,
virStorageBackendRBDStatePtr ptr)
{
int ret = -1;
+ int r = 0;
rbd_image_t image;
- if (rbd_open(ptr->ioctx, vol->name, &image, NULL) < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("failed to open the RBD image '%s'"),
- vol->name);
+
+ r = rbd_open(ptr->ioctx, vol->name, &image, NULL);
+ if (r < 0) {
+ virReportSystemError(-r, _("failed to open the RBD image
'%s'"),
+ vol->name);
return ret;
}
rbd_image_info_t info;
- if (rbd_stat(image, &info, sizeof(info)) < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("failed to stat the RBD image"));
+ r = rbd_stat(image, &info, sizeof(info));
+ if (r < 0) {
+ virReportSystemError(-r, _("failed to stat the RBD image
'%s'"),
+ vol->name);
goto cleanup;
}
@@ -297,6 +310,7 @@ static int virStorageBackendRBDRefreshPool(virConnectPtr conn,
size_t max_size = 1024;
int ret = -1;
int len = -1;
+ int r = 0;
char *name, *names = NULL;
virStorageBackendRBDState ptr;
ptr.cluster = NULL;
@@ -306,26 +320,22 @@ static int virStorageBackendRBDRefreshPool(virConnectPtr conn,
goto cleanup;
}
- if (rados_ioctx_create(ptr.cluster,
- pool->def->source.name, &ptr.ioctx) < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("failed to create the RBD IoCTX. Does the pool '%s'
exist?"),
- pool->def->source.name);
+ if (virStorageBackendRBDOpenIoCTX(&ptr, pool) < 0) {
goto cleanup;
}
struct rados_cluster_stat_t clusterstat;
- if (rados_cluster_stat(ptr.cluster, &clusterstat) < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("failed to stat the RADOS cluster"));
+ r = rados_cluster_stat(ptr.cluster, &clusterstat);
+ if (r < 0) {
+ virReportSystemError(-r, "%s", _("failed to stat the RADOS
cluster"));
goto cleanup;
}
struct rados_pool_stat_t poolstat;
- if (rados_ioctx_pool_stat(ptr.ioctx, &poolstat) < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("failed to stat the RADOS pool '%s'"),
- pool->def->source.name);
+ r = rados_ioctx_pool_stat(ptr.ioctx, &poolstat);
+ if (r < 0) {
+ virReportSystemError(-r, _("failed to stat the RADOS pool
'%s'"),
+ pool->def->source.name);
goto cleanup;
}
@@ -398,6 +408,7 @@ static int virStorageBackendRBDDeleteVol(virConnectPtr conn,
unsigned int flags)
{
int ret = -1;
+ int r = 0;
virStorageBackendRBDState ptr;
ptr.cluster = NULL;
ptr.ioctx = NULL;
@@ -412,19 +423,14 @@ static int virStorageBackendRBDDeleteVol(virConnectPtr conn,
goto cleanup;
}
- if (rados_ioctx_create(ptr.cluster,
- pool->def->source.name, &ptr.ioctx) < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("failed to create the RBD IoCTX. Does the pool '%s'
exist?"),
- pool->def->source.name);
+ if (virStorageBackendRBDOpenIoCTX(&ptr, pool) < 0) {
goto cleanup;
}
- if (rbd_remove(ptr.ioctx, vol->name) < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("failed to remove volume '%s/%s'"),
- pool->def->source.name,
- vol->name);
+ r = rbd_remove(ptr.ioctx, vol->name);
+ if (r < 0) {
+ virReportSystemError(-r, _("failed to remove volume '%s/%s'"),
+ pool->def->source.name, vol->name);
goto cleanup;
}
@@ -488,6 +494,7 @@ virStorageBackendRBDBuildVol(virConnectPtr conn,
ptr.cluster = NULL;
ptr.ioctx = NULL;
int ret = -1;
+ int r = 0;
VIR_DEBUG("Creating RBD image %s/%s with size %llu",
pool->def->source.name,
@@ -498,13 +505,8 @@ virStorageBackendRBDBuildVol(virConnectPtr conn,
if (virStorageBackendRBDOpenRADOSConn(&ptr, conn, pool) < 0)
goto cleanup;
- if (rados_ioctx_create(ptr.cluster,
- pool->def->source.name, &ptr.ioctx) < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("failed to create the RBD IoCTX. Does the pool '%s'
exist?"),
- pool->def->source.name);
+ if (virStorageBackendRBDOpenIoCTX(&ptr, pool) < 0)
goto cleanup;
- }
if (vol->target.encryption != NULL) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
@@ -512,11 +514,11 @@ virStorageBackendRBDBuildVol(virConnectPtr conn,
goto cleanup;
}
- if (virStorageBackendRBDCreateImage(ptr.ioctx, vol->name, vol->capacity) <
0) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("failed to create volume '%s/%s'"),
- pool->def->source.name,
- vol->name);
+ r = virStorageBackendRBDCreateImage(ptr.ioctx, vol->name, vol->capacity);
+ if (r < 0) {
+ virReportSystemError(-r, _("failed to create volume '%s/%s'"),
+ pool->def->source.name,
+ vol->name);
goto cleanup;
}
@@ -543,11 +545,7 @@ static int virStorageBackendRBDRefreshVol(virConnectPtr conn,
goto cleanup;
}
- if (rados_ioctx_create(ptr.cluster,
- pool->def->source.name, &ptr.ioctx) < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("failed to create the RBD IoCTX. Does the pool '%s'
exist?"),
- pool->def->source.name);
+ if (virStorageBackendRBDOpenIoCTX(&ptr, pool) < 0) {
goto cleanup;
}
@@ -573,6 +571,7 @@ static int virStorageBackendRBDResizeVol(virConnectPtr conn
ATTRIBUTE_UNUSED,
ptr.ioctx = NULL;
rbd_image_t image = NULL;
int ret = -1;
+ int r = 0;
virCheckFlags(0, -1);
@@ -580,25 +579,21 @@ static int virStorageBackendRBDResizeVol(virConnectPtr conn
ATTRIBUTE_UNUSED,
goto cleanup;
}
- if (rados_ioctx_create(ptr.cluster,
- pool->def->source.name, &ptr.ioctx) < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("failed to create the RBD IoCTX. Does the pool '%s'
exist?"),
- pool->def->source.name);
+ if (virStorageBackendRBDOpenIoCTX(&ptr, pool) < 0) {
goto cleanup;
}
- if (rbd_open(ptr.ioctx, vol->name, &image, NULL) < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("failed to open the RBD image '%s'"),
- vol->name);
+ r = rbd_open(ptr.ioctx, vol->name, &image, NULL);
+ if (r < 0) {
+ virReportSystemError(-r, _("failed to open the RBD image
'%s'"),
+ vol->name);
goto cleanup;
}
- if (rbd_resize(image, capacity) < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("failed to resize the RBD image '%s'"),
- vol->name);
+ r = rbd_resize(image, capacity);
+ if (r < 0) {
+ virReportSystemError(-r, _("failed to resize the RBD image
'%s'"),
+ vol->name);
goto cleanup;
}
--
1.7.9.5
3:50 a.m.
New subject: [libvirt] [PATCH 2/2] rbd: Set timeout options for librados
These timeout values make librados/librbd return -ETIMEDOUT when a
operation is blocking due to a failing/unreachable Ceph cluster.
By having the operations time out libvirt will not block.
---
src/storage/storage_backend_rbd.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c
index abb110e..22ed7de 100644
--- a/src/storage/storage_backend_rbd.c
+++ b/src/storage/storage_backend_rbd.c
@@ -60,6 +60,9 @@ static int
virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr,
char secretUuid[VIR_UUID_STRING_BUFLEN];
size_t i;
char *mon_buff = NULL;
+ const char *client_mount_timeout = "30";
+ const char *mon_op_timeout = "30";
+ const char *osd_op_timeout = "30";
VIR_DEBUG("Found Cephx username: %s",
pool->def->source.auth.cephx.username);
@@ -197,6 +200,20 @@ static int
virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr,
goto cleanup;
}
+ /*
+ * Set timeout options for librados.
+ * In case the Ceph cluster is down libvirt won't block forever.
+ * Operations in librados will return -ETIMEDOUT when the timeout is reached.
+ */
+ VIR_DEBUG("Setting RADOS option client_mount_timeout to %s",
client_mount_timeout);
+ rados_conf_set(ptr->cluster, "client_mount_timeout",
client_mount_timeout);
+
+ VIR_DEBUG("Setting RADOS option rados_mon_op_timeout to %s",
mon_op_timeout);
+ rados_conf_set(ptr->cluster, "rados_mon_op_timeout", mon_op_timeout);
+
+ 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);
+
ptr->starttime = time(0);
r = rados_connect(ptr->cluster);
if (r < 0) {
--
1.7.9.5
4:23 a.m.
New subject: [libvirt] [PATCH 0/2] Improve RBD storage pool backend
On 02/25/2014 10:50 AM, Wido den Hollander wrote:
The first patch improves the logging for the user. Currently the
return
codes from librados are not written to any logfile, which might leave a
user clueless to why the storage pool is not working.
The second patch sets three timeout options in librados. These are useful for
when the Ceph cluster is down. Librados will then not block for ever, but
return a -ETIMEDOUT so libvirt does not block for ever.
Wido den Hollander (2):
rbd: Include return statusses from librados/librbd in logging
s/statusses/statuses/
rbd: Set timeout options for librados
src/storage/storage_backend_rbd.c | 144 ++++++++++++++++++++-----------------
1 file changed, 78 insertions(+), 66 deletions(-)
ACK and pushed.
Jan
8:11 a.m.
New subject: [libvirt] [PATCH 3/3] rbd: Set timeout options for librados
These timeout values make librados/librbd return -ETIMEDOUT when a
operation is blocking due to a failing/unreachable Ceph cluster.
By having the operations time out libvirt will not block.
---
src/storage/storage_backend_rbd.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c
index 8d1e320..2c97bf4 100644
--- a/src/storage/storage_backend_rbd.c
+++ b/src/storage/storage_backend_rbd.c
@@ -60,6 +60,9 @@ static int
virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr,
char secretUuid[VIR_UUID_STRING_BUFLEN];
size_t i;
char *mon_buff = NULL;
+ const char *client_mount_timeout = "30";
+ const char *mon_op_timeout = "30";
+ const char *osd_op_timeout = "30";
VIR_DEBUG("Found Cephx username: %s",
pool->def->source.auth.cephx.username);
@@ -198,6 +201,20 @@ static int
virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr,
goto cleanup;
}
+ /*
+ * Set timeout options for librados.
+ * In case the Ceph cluster is down libvirt won't block forever.
+ * Operations in librados will return -ETIMEDOUT when the timeout is reached.
+ */
+ VIR_DEBUG("Setting RADOS option client_mount_timeout to %s",
client_mount_timeout);
+ rados_conf_set(ptr->cluster, "client_mount_timeout",
client_mount_timeout);
+
+ VIR_DEBUG("Setting RADOS option rados_mon_op_timeout to %s",
mon_op_timeout);
+ rados_conf_set(ptr->cluster, "rados_mon_op_timeout", mon_op_timeout);
+
+ 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);
+
ptr->starttime = time(0);
r = rados_connect(ptr->cluster);
if (r < 0) {
--
1.7.9.5
3923
days inactive
3936
days old
8 comments
2 participants
participants (2)
-
Ján Tomko -
Wido den Hollander