| Message ID | 20210519142359.23083-2-pl@kamp.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | block/rbd: migrate to coroutines and add write zeroes support | expand |
On Wed, May 19, 2021 at 4:26 PM Peter Lieven <pl@kamp.de> wrote: > > even luminous (version 12.2) is unmaintained for over 3 years now. > Bump the requirement to get rid of the ifdef'ry in the code. > Qemu 6.1 dropped the support for RHEL-7 which was the last supported > OS that required an older librbd. > > Signed-off-by: Peter Lieven <pl@kamp.de> > --- > block/rbd.c | 120 ++++------------------------------------------------ > meson.build | 7 ++- > 2 files changed, 13 insertions(+), 114 deletions(-) > > diff --git a/block/rbd.c b/block/rbd.c > index 26f64cce7c..6b1cbe1d75 100644 > --- a/block/rbd.c > +++ b/block/rbd.c > @@ -55,24 +55,10 @@ > * leading "\". > */ > > -/* rbd_aio_discard added in 0.1.2 */ > -#if LIBRBD_VERSION_CODE >= LIBRBD_VERSION(0, 1, 2) > -#define LIBRBD_SUPPORTS_DISCARD > -#else > -#undef LIBRBD_SUPPORTS_DISCARD > -#endif > - > #define OBJ_MAX_SIZE (1UL << OBJ_DEFAULT_OBJ_ORDER) > > #define RBD_MAX_SNAPS 100 > > -/* The LIBRBD_SUPPORTS_IOVEC is defined in librbd.h */ > -#ifdef LIBRBD_SUPPORTS_IOVEC > -#define LIBRBD_USE_IOVEC 1 > -#else > -#define LIBRBD_USE_IOVEC 0 > -#endif > - > typedef enum { > RBD_AIO_READ, > RBD_AIO_WRITE, > @@ -84,7 +70,6 @@ typedef struct RBDAIOCB { > BlockAIOCB common; > int64_t ret; > QEMUIOVector *qiov; > - char *bounce; > RBDAIOCmd cmd; > int error; > struct BDRVRBDState *s; > @@ -94,7 +79,6 @@ typedef struct RADOSCB { > RBDAIOCB *acb; > struct BDRVRBDState *s; > int64_t size; > - char *buf; > int64_t ret; > } RADOSCB; > > @@ -342,13 +326,9 @@ static int qemu_rbd_set_keypairs(rados_t cluster, const char *keypairs_json, > > static void qemu_rbd_memset(RADOSCB *rcb, int64_t offs) > { > - if (LIBRBD_USE_IOVEC) { > - RBDAIOCB *acb = rcb->acb; > - iov_memset(acb->qiov->iov, acb->qiov->niov, offs, 0, > - acb->qiov->size - offs); > - } else { > - memset(rcb->buf + offs, 0, rcb->size - offs); > - } > + RBDAIOCB *acb = rcb->acb; > + iov_memset(acb->qiov->iov, acb->qiov->niov, offs, 0, > + acb->qiov->size - offs); > } > > /* FIXME Deprecate and remove keypairs or make it available in QMP. */ > @@ -504,13 +484,6 @@ static void qemu_rbd_complete_aio(RADOSCB *rcb) > > g_free(rcb); > > - if (!LIBRBD_USE_IOVEC) { > - if (acb->cmd == RBD_AIO_READ) { > - qemu_iovec_from_buf(acb->qiov, 0, acb->bounce, acb->qiov->size); > - } > - qemu_vfree(acb->bounce); > - } > - > acb->common.cb(acb->common.opaque, (acb->ret > 0 ? 0 : acb->ret)); > > qemu_aio_unref(acb); > @@ -878,28 +851,6 @@ static void rbd_finish_aiocb(rbd_completion_t c, RADOSCB *rcb) > rbd_finish_bh, rcb); > } > > -static int rbd_aio_discard_wrapper(rbd_image_t image, > - uint64_t off, > - uint64_t len, > - rbd_completion_t comp) > -{ > -#ifdef LIBRBD_SUPPORTS_DISCARD > - return rbd_aio_discard(image, off, len, comp); > -#else > - return -ENOTSUP; > -#endif > -} > - > -static int rbd_aio_flush_wrapper(rbd_image_t image, > - rbd_completion_t comp) > -{ > -#ifdef LIBRBD_SUPPORTS_AIO_FLUSH > - return rbd_aio_flush(image, comp); > -#else > - return -ENOTSUP; > -#endif > -} > - > static BlockAIOCB *rbd_start_aio(BlockDriverState *bs, > int64_t off, > QEMUIOVector *qiov, > @@ -922,21 +873,6 @@ static BlockAIOCB *rbd_start_aio(BlockDriverState *bs, > > rcb = g_new(RADOSCB, 1); > > - if (!LIBRBD_USE_IOVEC) { > - if (cmd == RBD_AIO_DISCARD || cmd == RBD_AIO_FLUSH) { > - acb->bounce = NULL; > - } else { > - acb->bounce = qemu_try_blockalign(bs, qiov->size); > - if (acb->bounce == NULL) { > - goto failed; > - } > - } > - if (cmd == RBD_AIO_WRITE) { > - qemu_iovec_to_buf(acb->qiov, 0, acb->bounce, qiov->size); > - } > - rcb->buf = acb->bounce; > - } > - > acb->ret = 0; > acb->error = 0; > acb->s = s; > @@ -950,7 +886,7 @@ static BlockAIOCB *rbd_start_aio(BlockDriverState *bs, > } > > switch (cmd) { > - case RBD_AIO_WRITE: { > + case RBD_AIO_WRITE: > /* > * RBD APIs don't allow us to write more than actual size, so in order > * to support growing images, we resize the image before write > @@ -962,25 +898,16 @@ static BlockAIOCB *rbd_start_aio(BlockDriverState *bs, > goto failed_completion; > } > } > -#ifdef LIBRBD_SUPPORTS_IOVEC > - r = rbd_aio_writev(s->image, qiov->iov, qiov->niov, off, c); > -#else > - r = rbd_aio_write(s->image, off, size, rcb->buf, c); > -#endif > + r = rbd_aio_writev(s->image, qiov->iov, qiov->niov, off, c); > break; > - } > case RBD_AIO_READ: > -#ifdef LIBRBD_SUPPORTS_IOVEC > - r = rbd_aio_readv(s->image, qiov->iov, qiov->niov, off, c); > -#else > - r = rbd_aio_read(s->image, off, size, rcb->buf, c); > -#endif > + r = rbd_aio_readv(s->image, qiov->iov, qiov->niov, off, c); > break; > case RBD_AIO_DISCARD: > - r = rbd_aio_discard_wrapper(s->image, off, size, c); > + r = rbd_aio_discard(s->image, off, size, c); > break; > case RBD_AIO_FLUSH: > - r = rbd_aio_flush_wrapper(s->image, c); > + r = rbd_aio_flush(s->image, c); > break; > default: > r = -EINVAL; > @@ -995,9 +922,6 @@ failed_completion: > rbd_aio_release(c); > failed: > g_free(rcb); > - if (!LIBRBD_USE_IOVEC) { > - qemu_vfree(acb->bounce); > - } > > qemu_aio_unref(acb); > return NULL; > @@ -1023,7 +947,6 @@ static BlockAIOCB *qemu_rbd_aio_pwritev(BlockDriverState *bs, > RBD_AIO_WRITE); > } > > -#ifdef LIBRBD_SUPPORTS_AIO_FLUSH > static BlockAIOCB *qemu_rbd_aio_flush(BlockDriverState *bs, > BlockCompletionFunc *cb, > void *opaque) > @@ -1031,20 +954,6 @@ static BlockAIOCB *qemu_rbd_aio_flush(BlockDriverState *bs, > return rbd_start_aio(bs, 0, NULL, 0, cb, opaque, RBD_AIO_FLUSH); > } > > -#else > - > -static int qemu_rbd_co_flush(BlockDriverState *bs) > -{ > -#if LIBRBD_VERSION_CODE >= LIBRBD_VERSION(0, 1, 1) > - /* rbd_flush added in 0.1.1 */ > - BDRVRBDState *s = bs->opaque; > - return rbd_flush(s->image); > -#else > - return 0; > -#endif > -} > -#endif > - > static int qemu_rbd_getinfo(BlockDriverState *bs, BlockDriverInfo *bdi) > { > BDRVRBDState *s = bs->opaque; > @@ -1210,7 +1119,6 @@ static int qemu_rbd_snap_list(BlockDriverState *bs, > return snap_count; > } > > -#ifdef LIBRBD_SUPPORTS_DISCARD > static BlockAIOCB *qemu_rbd_aio_pdiscard(BlockDriverState *bs, > int64_t offset, > int bytes, > @@ -1220,9 +1128,7 @@ static BlockAIOCB *qemu_rbd_aio_pdiscard(BlockDriverState *bs, > return rbd_start_aio(bs, offset, NULL, bytes, cb, opaque, > RBD_AIO_DISCARD); > } > -#endif > > -#ifdef LIBRBD_SUPPORTS_INVALIDATE > static void coroutine_fn qemu_rbd_co_invalidate_cache(BlockDriverState *bs, > Error **errp) > { > @@ -1232,7 +1138,6 @@ static void coroutine_fn qemu_rbd_co_invalidate_cache(BlockDriverState *bs, > error_setg_errno(errp, -r, "Failed to invalidate the cache"); > } > } > -#endif > > static QemuOptsList qemu_rbd_create_opts = { > .name = "rbd-create-opts", > @@ -1290,23 +1195,14 @@ static BlockDriver bdrv_rbd = { > .bdrv_aio_preadv = qemu_rbd_aio_preadv, > .bdrv_aio_pwritev = qemu_rbd_aio_pwritev, > > -#ifdef LIBRBD_SUPPORTS_AIO_FLUSH > .bdrv_aio_flush = qemu_rbd_aio_flush, > -#else > - .bdrv_co_flush_to_disk = qemu_rbd_co_flush, > -#endif > - > -#ifdef LIBRBD_SUPPORTS_DISCARD > .bdrv_aio_pdiscard = qemu_rbd_aio_pdiscard, > -#endif > > .bdrv_snapshot_create = qemu_rbd_snap_create, > .bdrv_snapshot_delete = qemu_rbd_snap_remove, > .bdrv_snapshot_list = qemu_rbd_snap_list, > .bdrv_snapshot_goto = qemu_rbd_snap_rollback, > -#ifdef LIBRBD_SUPPORTS_INVALIDATE > .bdrv_co_invalidate_cache = qemu_rbd_co_invalidate_cache, > -#endif > > .strong_runtime_opts = qemu_rbd_strong_runtime_opts, > }; > diff --git a/meson.build b/meson.build > index 1559e8d873..644ef36476 100644 > --- a/meson.build > +++ b/meson.build > @@ -721,13 +721,16 @@ if not get_option('rbd').auto() or have_block > int main(void) { > rados_t cluster; > rados_create(&cluster, NULL); > + #if LIBRBD_VERSION_CODE < LIBRBD_VERSION(1, 12, 0) > + #error Hi Peter, Just a nit, but I would stick to the actual version of the library instead of the package version. librbd major version is 1, librados major version is 2 -- it shows up in ldd, when listing /usr/lib, etc. Something like #error librbd >= 1.12.0 required here > + #endif > return 0; > }''', dependencies: [librbd, librados]) > rbd = declare_dependency(dependencies: [librbd, librados]) > elif get_option('rbd').enabled() > - error('could not link librados') > + error('librados/librbd >= 12.0.0 required') and error('could not link librados/librbd') > else > - warning('could not link librados, disabling') > + warning('librados/librbd >= 12.0.0 not found, disabling rbd support') warning('could not link librados/librbd, disabling') here to avoid repeating the version. Thanks, Ilya
Am 16.06.21 um 14:26 schrieb Ilya Dryomov: > On Wed, May 19, 2021 at 4:26 PM Peter Lieven <pl@kamp.de> wrote: >> even luminous (version 12.2) is unmaintained for over 3 years now. >> Bump the requirement to get rid of the ifdef'ry in the code. >> Qemu 6.1 dropped the support for RHEL-7 which was the last supported >> OS that required an older librbd. >> >> Signed-off-by: Peter Lieven <pl@kamp.de> >> --- >> block/rbd.c | 120 ++++------------------------------------------------ >> meson.build | 7 ++- >> 2 files changed, 13 insertions(+), 114 deletions(-) >> >> diff --git a/block/rbd.c b/block/rbd.c >> index 26f64cce7c..6b1cbe1d75 100644 >> --- a/block/rbd.c >> +++ b/block/rbd.c >> @@ -55,24 +55,10 @@ >> * leading "\". >> */ >> >> -/* rbd_aio_discard added in 0.1.2 */ >> -#if LIBRBD_VERSION_CODE >= LIBRBD_VERSION(0, 1, 2) >> -#define LIBRBD_SUPPORTS_DISCARD >> -#else >> -#undef LIBRBD_SUPPORTS_DISCARD >> -#endif >> - >> #define OBJ_MAX_SIZE (1UL << OBJ_DEFAULT_OBJ_ORDER) >> >> #define RBD_MAX_SNAPS 100 >> >> -/* The LIBRBD_SUPPORTS_IOVEC is defined in librbd.h */ >> -#ifdef LIBRBD_SUPPORTS_IOVEC >> -#define LIBRBD_USE_IOVEC 1 >> -#else >> -#define LIBRBD_USE_IOVEC 0 >> -#endif >> - >> typedef enum { >> RBD_AIO_READ, >> RBD_AIO_WRITE, >> @@ -84,7 +70,6 @@ typedef struct RBDAIOCB { >> BlockAIOCB common; >> int64_t ret; >> QEMUIOVector *qiov; >> - char *bounce; >> RBDAIOCmd cmd; >> int error; >> struct BDRVRBDState *s; >> @@ -94,7 +79,6 @@ typedef struct RADOSCB { >> RBDAIOCB *acb; >> struct BDRVRBDState *s; >> int64_t size; >> - char *buf; >> int64_t ret; >> } RADOSCB; >> >> @@ -342,13 +326,9 @@ static int qemu_rbd_set_keypairs(rados_t cluster, const char *keypairs_json, >> >> static void qemu_rbd_memset(RADOSCB *rcb, int64_t offs) >> { >> - if (LIBRBD_USE_IOVEC) { >> - RBDAIOCB *acb = rcb->acb; >> - iov_memset(acb->qiov->iov, acb->qiov->niov, offs, 0, >> - acb->qiov->size - offs); >> - } else { >> - memset(rcb->buf + offs, 0, rcb->size - offs); >> - } >> + RBDAIOCB *acb = rcb->acb; >> + iov_memset(acb->qiov->iov, acb->qiov->niov, offs, 0, >> + acb->qiov->size - offs); >> } >> >> /* FIXME Deprecate and remove keypairs or make it available in QMP. */ >> @@ -504,13 +484,6 @@ static void qemu_rbd_complete_aio(RADOSCB *rcb) >> >> g_free(rcb); >> >> - if (!LIBRBD_USE_IOVEC) { >> - if (acb->cmd == RBD_AIO_READ) { >> - qemu_iovec_from_buf(acb->qiov, 0, acb->bounce, acb->qiov->size); >> - } >> - qemu_vfree(acb->bounce); >> - } >> - >> acb->common.cb(acb->common.opaque, (acb->ret > 0 ? 0 : acb->ret)); >> >> qemu_aio_unref(acb); >> @@ -878,28 +851,6 @@ static void rbd_finish_aiocb(rbd_completion_t c, RADOSCB *rcb) >> rbd_finish_bh, rcb); >> } >> >> -static int rbd_aio_discard_wrapper(rbd_image_t image, >> - uint64_t off, >> - uint64_t len, >> - rbd_completion_t comp) >> -{ >> -#ifdef LIBRBD_SUPPORTS_DISCARD >> - return rbd_aio_discard(image, off, len, comp); >> -#else >> - return -ENOTSUP; >> -#endif >> -} >> - >> -static int rbd_aio_flush_wrapper(rbd_image_t image, >> - rbd_completion_t comp) >> -{ >> -#ifdef LIBRBD_SUPPORTS_AIO_FLUSH >> - return rbd_aio_flush(image, comp); >> -#else >> - return -ENOTSUP; >> -#endif >> -} >> - >> static BlockAIOCB *rbd_start_aio(BlockDriverState *bs, >> int64_t off, >> QEMUIOVector *qiov, >> @@ -922,21 +873,6 @@ static BlockAIOCB *rbd_start_aio(BlockDriverState *bs, >> >> rcb = g_new(RADOSCB, 1); >> >> - if (!LIBRBD_USE_IOVEC) { >> - if (cmd == RBD_AIO_DISCARD || cmd == RBD_AIO_FLUSH) { >> - acb->bounce = NULL; >> - } else { >> - acb->bounce = qemu_try_blockalign(bs, qiov->size); >> - if (acb->bounce == NULL) { >> - goto failed; >> - } >> - } >> - if (cmd == RBD_AIO_WRITE) { >> - qemu_iovec_to_buf(acb->qiov, 0, acb->bounce, qiov->size); >> - } >> - rcb->buf = acb->bounce; >> - } >> - >> acb->ret = 0; >> acb->error = 0; >> acb->s = s; >> @@ -950,7 +886,7 @@ static BlockAIOCB *rbd_start_aio(BlockDriverState *bs, >> } >> >> switch (cmd) { >> - case RBD_AIO_WRITE: { >> + case RBD_AIO_WRITE: >> /* >> * RBD APIs don't allow us to write more than actual size, so in order >> * to support growing images, we resize the image before write >> @@ -962,25 +898,16 @@ static BlockAIOCB *rbd_start_aio(BlockDriverState *bs, >> goto failed_completion; >> } >> } >> -#ifdef LIBRBD_SUPPORTS_IOVEC >> - r = rbd_aio_writev(s->image, qiov->iov, qiov->niov, off, c); >> -#else >> - r = rbd_aio_write(s->image, off, size, rcb->buf, c); >> -#endif >> + r = rbd_aio_writev(s->image, qiov->iov, qiov->niov, off, c); >> break; >> - } >> case RBD_AIO_READ: >> -#ifdef LIBRBD_SUPPORTS_IOVEC >> - r = rbd_aio_readv(s->image, qiov->iov, qiov->niov, off, c); >> -#else >> - r = rbd_aio_read(s->image, off, size, rcb->buf, c); >> -#endif >> + r = rbd_aio_readv(s->image, qiov->iov, qiov->niov, off, c); >> break; >> case RBD_AIO_DISCARD: >> - r = rbd_aio_discard_wrapper(s->image, off, size, c); >> + r = rbd_aio_discard(s->image, off, size, c); >> break; >> case RBD_AIO_FLUSH: >> - r = rbd_aio_flush_wrapper(s->image, c); >> + r = rbd_aio_flush(s->image, c); >> break; >> default: >> r = -EINVAL; >> @@ -995,9 +922,6 @@ failed_completion: >> rbd_aio_release(c); >> failed: >> g_free(rcb); >> - if (!LIBRBD_USE_IOVEC) { >> - qemu_vfree(acb->bounce); >> - } >> >> qemu_aio_unref(acb); >> return NULL; >> @@ -1023,7 +947,6 @@ static BlockAIOCB *qemu_rbd_aio_pwritev(BlockDriverState *bs, >> RBD_AIO_WRITE); >> } >> >> -#ifdef LIBRBD_SUPPORTS_AIO_FLUSH >> static BlockAIOCB *qemu_rbd_aio_flush(BlockDriverState *bs, >> BlockCompletionFunc *cb, >> void *opaque) >> @@ -1031,20 +954,6 @@ static BlockAIOCB *qemu_rbd_aio_flush(BlockDriverState *bs, >> return rbd_start_aio(bs, 0, NULL, 0, cb, opaque, RBD_AIO_FLUSH); >> } >> >> -#else >> - >> -static int qemu_rbd_co_flush(BlockDriverState *bs) >> -{ >> -#if LIBRBD_VERSION_CODE >= LIBRBD_VERSION(0, 1, 1) >> - /* rbd_flush added in 0.1.1 */ >> - BDRVRBDState *s = bs->opaque; >> - return rbd_flush(s->image); >> -#else >> - return 0; >> -#endif >> -} >> -#endif >> - >> static int qemu_rbd_getinfo(BlockDriverState *bs, BlockDriverInfo *bdi) >> { >> BDRVRBDState *s = bs->opaque; >> @@ -1210,7 +1119,6 @@ static int qemu_rbd_snap_list(BlockDriverState *bs, >> return snap_count; >> } >> >> -#ifdef LIBRBD_SUPPORTS_DISCARD >> static BlockAIOCB *qemu_rbd_aio_pdiscard(BlockDriverState *bs, >> int64_t offset, >> int bytes, >> @@ -1220,9 +1128,7 @@ static BlockAIOCB *qemu_rbd_aio_pdiscard(BlockDriverState *bs, >> return rbd_start_aio(bs, offset, NULL, bytes, cb, opaque, >> RBD_AIO_DISCARD); >> } >> -#endif >> >> -#ifdef LIBRBD_SUPPORTS_INVALIDATE >> static void coroutine_fn qemu_rbd_co_invalidate_cache(BlockDriverState *bs, >> Error **errp) >> { >> @@ -1232,7 +1138,6 @@ static void coroutine_fn qemu_rbd_co_invalidate_cache(BlockDriverState *bs, >> error_setg_errno(errp, -r, "Failed to invalidate the cache"); >> } >> } >> -#endif >> >> static QemuOptsList qemu_rbd_create_opts = { >> .name = "rbd-create-opts", >> @@ -1290,23 +1195,14 @@ static BlockDriver bdrv_rbd = { >> .bdrv_aio_preadv = qemu_rbd_aio_preadv, >> .bdrv_aio_pwritev = qemu_rbd_aio_pwritev, >> >> -#ifdef LIBRBD_SUPPORTS_AIO_FLUSH >> .bdrv_aio_flush = qemu_rbd_aio_flush, >> -#else >> - .bdrv_co_flush_to_disk = qemu_rbd_co_flush, >> -#endif >> - >> -#ifdef LIBRBD_SUPPORTS_DISCARD >> .bdrv_aio_pdiscard = qemu_rbd_aio_pdiscard, >> -#endif >> >> .bdrv_snapshot_create = qemu_rbd_snap_create, >> .bdrv_snapshot_delete = qemu_rbd_snap_remove, >> .bdrv_snapshot_list = qemu_rbd_snap_list, >> .bdrv_snapshot_goto = qemu_rbd_snap_rollback, >> -#ifdef LIBRBD_SUPPORTS_INVALIDATE >> .bdrv_co_invalidate_cache = qemu_rbd_co_invalidate_cache, >> -#endif >> >> .strong_runtime_opts = qemu_rbd_strong_runtime_opts, >> }; >> diff --git a/meson.build b/meson.build >> index 1559e8d873..644ef36476 100644 >> --- a/meson.build >> +++ b/meson.build >> @@ -721,13 +721,16 @@ if not get_option('rbd').auto() or have_block >> int main(void) { >> rados_t cluster; >> rados_create(&cluster, NULL); >> + #if LIBRBD_VERSION_CODE < LIBRBD_VERSION(1, 12, 0) >> + #error > Hi Peter, > > Just a nit, but I would stick to the actual version of the library > instead of the package version. librbd major version is 1, librados > major version is 2 -- it shows up in ldd, when listing /usr/lib, etc. > Something like > > #error librbd >= 1.12.0 required > > here Okay. > >> + #endif >> return 0; >> }''', dependencies: [librbd, librados]) >> rbd = declare_dependency(dependencies: [librbd, librados]) >> elif get_option('rbd').enabled() >> - error('could not link librados') >> + error('librados/librbd >= 12.0.0 required') > and > > error('could not link librados/librbd') > >> else >> - warning('could not link librados, disabling') >> + warning('librados/librbd >= 12.0.0 not found, disabling rbd support') > warning('could not link librados/librbd, disabling') > > here to avoid repeating the version. I would only like to change it if the "#error" is visible in the actual configure output. Have not testet your proposal yet. If its visible I am fine to change it. Peter
On Fri, Jun 18, 2021 at 10:58 AM Peter Lieven <pl@kamp.de> wrote: > > Am 16.06.21 um 14:26 schrieb Ilya Dryomov: > > On Wed, May 19, 2021 at 4:26 PM Peter Lieven <pl@kamp.de> wrote: > >> even luminous (version 12.2) is unmaintained for over 3 years now. > >> Bump the requirement to get rid of the ifdef'ry in the code. > >> Qemu 6.1 dropped the support for RHEL-7 which was the last supported > >> OS that required an older librbd. > >> > >> Signed-off-by: Peter Lieven <pl@kamp.de> > >> --- > >> block/rbd.c | 120 ++++------------------------------------------------ > >> meson.build | 7 ++- > >> 2 files changed, 13 insertions(+), 114 deletions(-) > >> > >> diff --git a/block/rbd.c b/block/rbd.c > >> index 26f64cce7c..6b1cbe1d75 100644 > >> --- a/block/rbd.c > >> +++ b/block/rbd.c > >> @@ -55,24 +55,10 @@ > >> * leading "\". > >> */ > >> > >> -/* rbd_aio_discard added in 0.1.2 */ > >> -#if LIBRBD_VERSION_CODE >= LIBRBD_VERSION(0, 1, 2) > >> -#define LIBRBD_SUPPORTS_DISCARD > >> -#else > >> -#undef LIBRBD_SUPPORTS_DISCARD > >> -#endif > >> - > >> #define OBJ_MAX_SIZE (1UL << OBJ_DEFAULT_OBJ_ORDER) > >> > >> #define RBD_MAX_SNAPS 100 > >> > >> -/* The LIBRBD_SUPPORTS_IOVEC is defined in librbd.h */ > >> -#ifdef LIBRBD_SUPPORTS_IOVEC > >> -#define LIBRBD_USE_IOVEC 1 > >> -#else > >> -#define LIBRBD_USE_IOVEC 0 > >> -#endif > >> - > >> typedef enum { > >> RBD_AIO_READ, > >> RBD_AIO_WRITE, > >> @@ -84,7 +70,6 @@ typedef struct RBDAIOCB { > >> BlockAIOCB common; > >> int64_t ret; > >> QEMUIOVector *qiov; > >> - char *bounce; > >> RBDAIOCmd cmd; > >> int error; > >> struct BDRVRBDState *s; > >> @@ -94,7 +79,6 @@ typedef struct RADOSCB { > >> RBDAIOCB *acb; > >> struct BDRVRBDState *s; > >> int64_t size; > >> - char *buf; > >> int64_t ret; > >> } RADOSCB; > >> > >> @@ -342,13 +326,9 @@ static int qemu_rbd_set_keypairs(rados_t cluster, const char *keypairs_json, > >> > >> static void qemu_rbd_memset(RADOSCB *rcb, int64_t offs) > >> { > >> - if (LIBRBD_USE_IOVEC) { > >> - RBDAIOCB *acb = rcb->acb; > >> - iov_memset(acb->qiov->iov, acb->qiov->niov, offs, 0, > >> - acb->qiov->size - offs); > >> - } else { > >> - memset(rcb->buf + offs, 0, rcb->size - offs); > >> - } > >> + RBDAIOCB *acb = rcb->acb; > >> + iov_memset(acb->qiov->iov, acb->qiov->niov, offs, 0, > >> + acb->qiov->size - offs); > >> } > >> > >> /* FIXME Deprecate and remove keypairs or make it available in QMP. */ > >> @@ -504,13 +484,6 @@ static void qemu_rbd_complete_aio(RADOSCB *rcb) > >> > >> g_free(rcb); > >> > >> - if (!LIBRBD_USE_IOVEC) { > >> - if (acb->cmd == RBD_AIO_READ) { > >> - qemu_iovec_from_buf(acb->qiov, 0, acb->bounce, acb->qiov->size); > >> - } > >> - qemu_vfree(acb->bounce); > >> - } > >> - > >> acb->common.cb(acb->common.opaque, (acb->ret > 0 ? 0 : acb->ret)); > >> > >> qemu_aio_unref(acb); > >> @@ -878,28 +851,6 @@ static void rbd_finish_aiocb(rbd_completion_t c, RADOSCB *rcb) > >> rbd_finish_bh, rcb); > >> } > >> > >> -static int rbd_aio_discard_wrapper(rbd_image_t image, > >> - uint64_t off, > >> - uint64_t len, > >> - rbd_completion_t comp) > >> -{ > >> -#ifdef LIBRBD_SUPPORTS_DISCARD > >> - return rbd_aio_discard(image, off, len, comp); > >> -#else > >> - return -ENOTSUP; > >> -#endif > >> -} > >> - > >> -static int rbd_aio_flush_wrapper(rbd_image_t image, > >> - rbd_completion_t comp) > >> -{ > >> -#ifdef LIBRBD_SUPPORTS_AIO_FLUSH > >> - return rbd_aio_flush(image, comp); > >> -#else > >> - return -ENOTSUP; > >> -#endif > >> -} > >> - > >> static BlockAIOCB *rbd_start_aio(BlockDriverState *bs, > >> int64_t off, > >> QEMUIOVector *qiov, > >> @@ -922,21 +873,6 @@ static BlockAIOCB *rbd_start_aio(BlockDriverState *bs, > >> > >> rcb = g_new(RADOSCB, 1); > >> > >> - if (!LIBRBD_USE_IOVEC) { > >> - if (cmd == RBD_AIO_DISCARD || cmd == RBD_AIO_FLUSH) { > >> - acb->bounce = NULL; > >> - } else { > >> - acb->bounce = qemu_try_blockalign(bs, qiov->size); > >> - if (acb->bounce == NULL) { > >> - goto failed; > >> - } > >> - } > >> - if (cmd == RBD_AIO_WRITE) { > >> - qemu_iovec_to_buf(acb->qiov, 0, acb->bounce, qiov->size); > >> - } > >> - rcb->buf = acb->bounce; > >> - } > >> - > >> acb->ret = 0; > >> acb->error = 0; > >> acb->s = s; > >> @@ -950,7 +886,7 @@ static BlockAIOCB *rbd_start_aio(BlockDriverState *bs, > >> } > >> > >> switch (cmd) { > >> - case RBD_AIO_WRITE: { > >> + case RBD_AIO_WRITE: > >> /* > >> * RBD APIs don't allow us to write more than actual size, so in order > >> * to support growing images, we resize the image before write > >> @@ -962,25 +898,16 @@ static BlockAIOCB *rbd_start_aio(BlockDriverState *bs, > >> goto failed_completion; > >> } > >> } > >> -#ifdef LIBRBD_SUPPORTS_IOVEC > >> - r = rbd_aio_writev(s->image, qiov->iov, qiov->niov, off, c); > >> -#else > >> - r = rbd_aio_write(s->image, off, size, rcb->buf, c); > >> -#endif > >> + r = rbd_aio_writev(s->image, qiov->iov, qiov->niov, off, c); > >> break; > >> - } > >> case RBD_AIO_READ: > >> -#ifdef LIBRBD_SUPPORTS_IOVEC > >> - r = rbd_aio_readv(s->image, qiov->iov, qiov->niov, off, c); > >> -#else > >> - r = rbd_aio_read(s->image, off, size, rcb->buf, c); > >> -#endif > >> + r = rbd_aio_readv(s->image, qiov->iov, qiov->niov, off, c); > >> break; > >> case RBD_AIO_DISCARD: > >> - r = rbd_aio_discard_wrapper(s->image, off, size, c); > >> + r = rbd_aio_discard(s->image, off, size, c); > >> break; > >> case RBD_AIO_FLUSH: > >> - r = rbd_aio_flush_wrapper(s->image, c); > >> + r = rbd_aio_flush(s->image, c); > >> break; > >> default: > >> r = -EINVAL; > >> @@ -995,9 +922,6 @@ failed_completion: > >> rbd_aio_release(c); > >> failed: > >> g_free(rcb); > >> - if (!LIBRBD_USE_IOVEC) { > >> - qemu_vfree(acb->bounce); > >> - } > >> > >> qemu_aio_unref(acb); > >> return NULL; > >> @@ -1023,7 +947,6 @@ static BlockAIOCB *qemu_rbd_aio_pwritev(BlockDriverState *bs, > >> RBD_AIO_WRITE); > >> } > >> > >> -#ifdef LIBRBD_SUPPORTS_AIO_FLUSH > >> static BlockAIOCB *qemu_rbd_aio_flush(BlockDriverState *bs, > >> BlockCompletionFunc *cb, > >> void *opaque) > >> @@ -1031,20 +954,6 @@ static BlockAIOCB *qemu_rbd_aio_flush(BlockDriverState *bs, > >> return rbd_start_aio(bs, 0, NULL, 0, cb, opaque, RBD_AIO_FLUSH); > >> } > >> > >> -#else > >> - > >> -static int qemu_rbd_co_flush(BlockDriverState *bs) > >> -{ > >> -#if LIBRBD_VERSION_CODE >= LIBRBD_VERSION(0, 1, 1) > >> - /* rbd_flush added in 0.1.1 */ > >> - BDRVRBDState *s = bs->opaque; > >> - return rbd_flush(s->image); > >> -#else > >> - return 0; > >> -#endif > >> -} > >> -#endif > >> - > >> static int qemu_rbd_getinfo(BlockDriverState *bs, BlockDriverInfo *bdi) > >> { > >> BDRVRBDState *s = bs->opaque; > >> @@ -1210,7 +1119,6 @@ static int qemu_rbd_snap_list(BlockDriverState *bs, > >> return snap_count; > >> } > >> > >> -#ifdef LIBRBD_SUPPORTS_DISCARD > >> static BlockAIOCB *qemu_rbd_aio_pdiscard(BlockDriverState *bs, > >> int64_t offset, > >> int bytes, > >> @@ -1220,9 +1128,7 @@ static BlockAIOCB *qemu_rbd_aio_pdiscard(BlockDriverState *bs, > >> return rbd_start_aio(bs, offset, NULL, bytes, cb, opaque, > >> RBD_AIO_DISCARD); > >> } > >> -#endif > >> > >> -#ifdef LIBRBD_SUPPORTS_INVALIDATE > >> static void coroutine_fn qemu_rbd_co_invalidate_cache(BlockDriverState *bs, > >> Error **errp) > >> { > >> @@ -1232,7 +1138,6 @@ static void coroutine_fn qemu_rbd_co_invalidate_cache(BlockDriverState *bs, > >> error_setg_errno(errp, -r, "Failed to invalidate the cache"); > >> } > >> } > >> -#endif > >> > >> static QemuOptsList qemu_rbd_create_opts = { > >> .name = "rbd-create-opts", > >> @@ -1290,23 +1195,14 @@ static BlockDriver bdrv_rbd = { > >> .bdrv_aio_preadv = qemu_rbd_aio_preadv, > >> .bdrv_aio_pwritev = qemu_rbd_aio_pwritev, > >> > >> -#ifdef LIBRBD_SUPPORTS_AIO_FLUSH > >> .bdrv_aio_flush = qemu_rbd_aio_flush, > >> -#else > >> - .bdrv_co_flush_to_disk = qemu_rbd_co_flush, > >> -#endif > >> - > >> -#ifdef LIBRBD_SUPPORTS_DISCARD > >> .bdrv_aio_pdiscard = qemu_rbd_aio_pdiscard, > >> -#endif > >> > >> .bdrv_snapshot_create = qemu_rbd_snap_create, > >> .bdrv_snapshot_delete = qemu_rbd_snap_remove, > >> .bdrv_snapshot_list = qemu_rbd_snap_list, > >> .bdrv_snapshot_goto = qemu_rbd_snap_rollback, > >> -#ifdef LIBRBD_SUPPORTS_INVALIDATE > >> .bdrv_co_invalidate_cache = qemu_rbd_co_invalidate_cache, > >> -#endif > >> > >> .strong_runtime_opts = qemu_rbd_strong_runtime_opts, > >> }; > >> diff --git a/meson.build b/meson.build > >> index 1559e8d873..644ef36476 100644 > >> --- a/meson.build > >> +++ b/meson.build > >> @@ -721,13 +721,16 @@ if not get_option('rbd').auto() or have_block > >> int main(void) { > >> rados_t cluster; > >> rados_create(&cluster, NULL); > >> + #if LIBRBD_VERSION_CODE < LIBRBD_VERSION(1, 12, 0) > >> + #error > > Hi Peter, > > > > Just a nit, but I would stick to the actual version of the library > > instead of the package version. librbd major version is 1, librados > > major version is 2 -- it shows up in ldd, when listing /usr/lib, etc. > > Something like > > > > #error librbd >= 1.12.0 required > > > > here > > > Okay. > > > > > >> + #endif > >> return 0; > >> }''', dependencies: [librbd, librados]) > >> rbd = declare_dependency(dependencies: [librbd, librados]) > >> elif get_option('rbd').enabled() > >> - error('could not link librados') > >> + error('librados/librbd >= 12.0.0 required') > > and > > > > error('could not link librados/librbd') > > > >> else > >> - warning('could not link librados, disabling') > >> + warning('librados/librbd >= 12.0.0 not found, disabling rbd support') > > warning('could not link librados/librbd, disabling') > > > > here to avoid repeating the version. > > > I would only like to change it if the "#error" is visible in the actual configure output. Have not testet your proposal yet. Looks like it's not, only in build/meson-logs/meson-log.txt. In that case, just tweak the version numbers and since librados and librbd have different versions, don't mention librados. Thanks, Ilya
diff --git a/block/rbd.c b/block/rbd.c index 26f64cce7c..6b1cbe1d75 100644 --- a/block/rbd.c +++ b/block/rbd.c @@ -55,24 +55,10 @@ * leading "\". */ -/* rbd_aio_discard added in 0.1.2 */ -#if LIBRBD_VERSION_CODE >= LIBRBD_VERSION(0, 1, 2) -#define LIBRBD_SUPPORTS_DISCARD -#else -#undef LIBRBD_SUPPORTS_DISCARD -#endif - #define OBJ_MAX_SIZE (1UL << OBJ_DEFAULT_OBJ_ORDER) #define RBD_MAX_SNAPS 100 -/* The LIBRBD_SUPPORTS_IOVEC is defined in librbd.h */ -#ifdef LIBRBD_SUPPORTS_IOVEC -#define LIBRBD_USE_IOVEC 1 -#else -#define LIBRBD_USE_IOVEC 0 -#endif - typedef enum { RBD_AIO_READ, RBD_AIO_WRITE, @@ -84,7 +70,6 @@ typedef struct RBDAIOCB { BlockAIOCB common; int64_t ret; QEMUIOVector *qiov; - char *bounce; RBDAIOCmd cmd; int error; struct BDRVRBDState *s; @@ -94,7 +79,6 @@ typedef struct RADOSCB { RBDAIOCB *acb; struct BDRVRBDState *s; int64_t size; - char *buf; int64_t ret; } RADOSCB; @@ -342,13 +326,9 @@ static int qemu_rbd_set_keypairs(rados_t cluster, const char *keypairs_json, static void qemu_rbd_memset(RADOSCB *rcb, int64_t offs) { - if (LIBRBD_USE_IOVEC) { - RBDAIOCB *acb = rcb->acb; - iov_memset(acb->qiov->iov, acb->qiov->niov, offs, 0, - acb->qiov->size - offs); - } else { - memset(rcb->buf + offs, 0, rcb->size - offs); - } + RBDAIOCB *acb = rcb->acb; + iov_memset(acb->qiov->iov, acb->qiov->niov, offs, 0, + acb->qiov->size - offs); } /* FIXME Deprecate and remove keypairs or make it available in QMP. */ @@ -504,13 +484,6 @@ static void qemu_rbd_complete_aio(RADOSCB *rcb) g_free(rcb); - if (!LIBRBD_USE_IOVEC) { - if (acb->cmd == RBD_AIO_READ) { - qemu_iovec_from_buf(acb->qiov, 0, acb->bounce, acb->qiov->size); - } - qemu_vfree(acb->bounce); - } - acb->common.cb(acb->common.opaque, (acb->ret > 0 ? 0 : acb->ret)); qemu_aio_unref(acb); @@ -878,28 +851,6 @@ static void rbd_finish_aiocb(rbd_completion_t c, RADOSCB *rcb) rbd_finish_bh, rcb); } -static int rbd_aio_discard_wrapper(rbd_image_t image, - uint64_t off, - uint64_t len, - rbd_completion_t comp) -{ -#ifdef LIBRBD_SUPPORTS_DISCARD - return rbd_aio_discard(image, off, len, comp); -#else - return -ENOTSUP; -#endif -} - -static int rbd_aio_flush_wrapper(rbd_image_t image, - rbd_completion_t comp) -{ -#ifdef LIBRBD_SUPPORTS_AIO_FLUSH - return rbd_aio_flush(image, comp); -#else - return -ENOTSUP; -#endif -} - static BlockAIOCB *rbd_start_aio(BlockDriverState *bs, int64_t off, QEMUIOVector *qiov, @@ -922,21 +873,6 @@ static BlockAIOCB *rbd_start_aio(BlockDriverState *bs, rcb = g_new(RADOSCB, 1); - if (!LIBRBD_USE_IOVEC) { - if (cmd == RBD_AIO_DISCARD || cmd == RBD_AIO_FLUSH) { - acb->bounce = NULL; - } else { - acb->bounce = qemu_try_blockalign(bs, qiov->size); - if (acb->bounce == NULL) { - goto failed; - } - } - if (cmd == RBD_AIO_WRITE) { - qemu_iovec_to_buf(acb->qiov, 0, acb->bounce, qiov->size); - } - rcb->buf = acb->bounce; - } - acb->ret = 0; acb->error = 0; acb->s = s; @@ -950,7 +886,7 @@ static BlockAIOCB *rbd_start_aio(BlockDriverState *bs, } switch (cmd) { - case RBD_AIO_WRITE: { + case RBD_AIO_WRITE: /* * RBD APIs don't allow us to write more than actual size, so in order * to support growing images, we resize the image before write @@ -962,25 +898,16 @@ static BlockAIOCB *rbd_start_aio(BlockDriverState *bs, goto failed_completion; } } -#ifdef LIBRBD_SUPPORTS_IOVEC - r = rbd_aio_writev(s->image, qiov->iov, qiov->niov, off, c); -#else - r = rbd_aio_write(s->image, off, size, rcb->buf, c); -#endif + r = rbd_aio_writev(s->image, qiov->iov, qiov->niov, off, c); break; - } case RBD_AIO_READ: -#ifdef LIBRBD_SUPPORTS_IOVEC - r = rbd_aio_readv(s->image, qiov->iov, qiov->niov, off, c); -#else - r = rbd_aio_read(s->image, off, size, rcb->buf, c); -#endif + r = rbd_aio_readv(s->image, qiov->iov, qiov->niov, off, c); break; case RBD_AIO_DISCARD: - r = rbd_aio_discard_wrapper(s->image, off, size, c); + r = rbd_aio_discard(s->image, off, size, c); break; case RBD_AIO_FLUSH: - r = rbd_aio_flush_wrapper(s->image, c); + r = rbd_aio_flush(s->image, c); break; default: r = -EINVAL; @@ -995,9 +922,6 @@ failed_completion: rbd_aio_release(c); failed: g_free(rcb); - if (!LIBRBD_USE_IOVEC) { - qemu_vfree(acb->bounce); - } qemu_aio_unref(acb); return NULL; @@ -1023,7 +947,6 @@ static BlockAIOCB *qemu_rbd_aio_pwritev(BlockDriverState *bs, RBD_AIO_WRITE); } -#ifdef LIBRBD_SUPPORTS_AIO_FLUSH static BlockAIOCB *qemu_rbd_aio_flush(BlockDriverState *bs, BlockCompletionFunc *cb, void *opaque) @@ -1031,20 +954,6 @@ static BlockAIOCB *qemu_rbd_aio_flush(BlockDriverState *bs, return rbd_start_aio(bs, 0, NULL, 0, cb, opaque, RBD_AIO_FLUSH); } -#else - -static int qemu_rbd_co_flush(BlockDriverState *bs) -{ -#if LIBRBD_VERSION_CODE >= LIBRBD_VERSION(0, 1, 1) - /* rbd_flush added in 0.1.1 */ - BDRVRBDState *s = bs->opaque; - return rbd_flush(s->image); -#else - return 0; -#endif -} -#endif - static int qemu_rbd_getinfo(BlockDriverState *bs, BlockDriverInfo *bdi) { BDRVRBDState *s = bs->opaque; @@ -1210,7 +1119,6 @@ static int qemu_rbd_snap_list(BlockDriverState *bs, return snap_count; } -#ifdef LIBRBD_SUPPORTS_DISCARD static BlockAIOCB *qemu_rbd_aio_pdiscard(BlockDriverState *bs, int64_t offset, int bytes, @@ -1220,9 +1128,7 @@ static BlockAIOCB *qemu_rbd_aio_pdiscard(BlockDriverState *bs, return rbd_start_aio(bs, offset, NULL, bytes, cb, opaque, RBD_AIO_DISCARD); } -#endif -#ifdef LIBRBD_SUPPORTS_INVALIDATE static void coroutine_fn qemu_rbd_co_invalidate_cache(BlockDriverState *bs, Error **errp) { @@ -1232,7 +1138,6 @@ static void coroutine_fn qemu_rbd_co_invalidate_cache(BlockDriverState *bs, error_setg_errno(errp, -r, "Failed to invalidate the cache"); } } -#endif static QemuOptsList qemu_rbd_create_opts = { .name = "rbd-create-opts", @@ -1290,23 +1195,14 @@ static BlockDriver bdrv_rbd = { .bdrv_aio_preadv = qemu_rbd_aio_preadv, .bdrv_aio_pwritev = qemu_rbd_aio_pwritev, -#ifdef LIBRBD_SUPPORTS_AIO_FLUSH .bdrv_aio_flush = qemu_rbd_aio_flush, -#else - .bdrv_co_flush_to_disk = qemu_rbd_co_flush, -#endif - -#ifdef LIBRBD_SUPPORTS_DISCARD .bdrv_aio_pdiscard = qemu_rbd_aio_pdiscard, -#endif .bdrv_snapshot_create = qemu_rbd_snap_create, .bdrv_snapshot_delete = qemu_rbd_snap_remove, .bdrv_snapshot_list = qemu_rbd_snap_list, .bdrv_snapshot_goto = qemu_rbd_snap_rollback, -#ifdef LIBRBD_SUPPORTS_INVALIDATE .bdrv_co_invalidate_cache = qemu_rbd_co_invalidate_cache, -#endif .strong_runtime_opts = qemu_rbd_strong_runtime_opts, }; diff --git a/meson.build b/meson.build index 1559e8d873..644ef36476 100644 --- a/meson.build +++ b/meson.build @@ -721,13 +721,16 @@ if not get_option('rbd').auto() or have_block int main(void) { rados_t cluster; rados_create(&cluster, NULL); + #if LIBRBD_VERSION_CODE < LIBRBD_VERSION(1, 12, 0) + #error + #endif return 0; }''', dependencies: [librbd, librados]) rbd = declare_dependency(dependencies: [librbd, librados]) elif get_option('rbd').enabled() - error('could not link librados') + error('librados/librbd >= 12.0.0 required') else - warning('could not link librados, disabling') + warning('librados/librbd >= 12.0.0 not found, disabling rbd support') endif endif endif
even luminous (version 12.2) is unmaintained for over 3 years now. Bump the requirement to get rid of the ifdef'ry in the code. Qemu 6.1 dropped the support for RHEL-7 which was the last supported OS that required an older librbd. Signed-off-by: Peter Lieven <pl@kamp.de> --- block/rbd.c | 120 ++++------------------------------------------------ meson.build | 7 ++- 2 files changed, 13 insertions(+), 114 deletions(-)