[PATCH 3/4 V2] hw/scsi: add SCSI COMPARE_AND_WRITE support
This patch emulates COMPARE_AND_WRITE command with the
BDRV_REQ_COMPARE_AND_WRITE flag introduced by last patch. It matches
the SBC-4 standard except the FUA bit support, it'll be finished in
the next patch.
Note that cmd->xfer is set 2 * the number got by scsi_data_cdb_xfer
so we could touch the least code.
Signed-off-by: Yaowei Bai
---
hw/scsi/emulation.c | 1 +
hw/scsi/scsi-bus.c | 4 +++
hw/scsi/scsi-disk.c | 88 +
hw/scsi/trace-events| 1 +
include/hw/scsi/emulation.h | 3 ++
include/scsi/utils.h| 2 ++
scsi/utils.c| 5 +++
7 files changed, 104 insertions(+)
diff --git a/hw/scsi/emulation.c b/hw/scsi/emulation.c
index 06d62f3..1f53c4a 100644
--- a/hw/scsi/emulation.c
+++ b/hw/scsi/emulation.c
@@ -9,6 +9,7 @@ int scsi_emulate_block_limits(uint8_t *outbuf, const
SCSIBlockLimits *bl)
memset(outbuf, 0, 0x3c);
outbuf[0] = bl->wsnz; /* wsnz */
+outbuf[1] = MAX_COMPARE_AND_WRITE_LENGTH;
if (bl->max_io_sectors) {
/* optimal transfer length granularity. This field and the optimal
diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index 359d50d..a20eb11 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -1003,6 +1003,9 @@ static int scsi_req_xfer(SCSICommand *cmd, SCSIDevice
*dev, uint8_t *buf)
case WRITE_VERIFY_16:
cmd->xfer *= dev->blocksize;
break;
+case COMPARE_AND_WRITE:
+cmd->xfer *= 2 * dev->blocksize;
+break;
case READ_6:
case READ_REVERSE:
/* length 0 means 256 blocks */
@@ -1222,6 +1225,7 @@ static void scsi_cmd_xfer_mode(SCSICommand *cmd)
case PERSISTENT_RESERVE_OUT:
case MAINTENANCE_OUT:
case SET_WINDOW:
+case COMPARE_AND_WRITE:
case SCAN:
/* SCAN conflicts with START_STOP. START_STOP has cmd->xfer set to 0
for
* non-scanner devices, so we only get here for SCAN and not for
START_STOP.
diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index 07fb5eb..f9a0267 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -478,6 +478,9 @@ static bool scsi_handle_rw_error(SCSIDiskReq *r, int error,
bool acct_failed)
case ENOSPC:
scsi_check_condition(r, SENSE_CODE(SPACE_ALLOC_FAILED));
break;
+case EILSEQ:
+scsi_check_condition(r, SENSE_CODE(MISCOMPARE_DURING_VERIFY));
+break;
default:
scsi_check_condition(r, SENSE_CODE(IO_ERROR));
break;
@@ -1825,6 +1828,84 @@ static void scsi_disk_emulate_write_same(SCSIDiskReq *r,
uint8_t *inbuf)
scsi_write_same_complete, data);
}
+typedef struct CompareAndWriteCBData {
+SCSIDiskReq *r;
+int64_t sector;
+int nb_sectors;
+QEMUIOVector qiov;
+struct iovec iov;
+} CompareAndWriteCBData;
+
+static void scsi_compare_and_write_complete(void *opaque, int ret)
+{
+CompareAndWriteCBData *data = opaque;
+SCSIDiskReq *r = data->r;
+SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev);
+
+assert(r->req.aiocb != NULL);
+r->req.aiocb = NULL;
+aio_context_acquire(blk_get_aio_context(s->qdev.conf.blk));
+if (scsi_disk_req_check_error(r, ret, true)) {
+goto done;
+}
+
+block_acct_done(blk_get_stats(s->qdev.conf.blk), >acct);
+scsi_req_complete(>req, GOOD);
+
+done:
+scsi_req_unref(>req);
+qemu_vfree(data->iov.iov_base);
+g_free(data);
+aio_context_release(blk_get_aio_context(s->qdev.conf.blk));
+}
+
+static void scsi_disk_emulate_compare_and_write(SCSIDiskReq *r, uint8_t *inbuf)
+{
+SCSIRequest *req = >req;
+SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, req->dev);
+uint32_t nb_sectors = scsi_data_cdb_xfer(r->req.cmd.buf);
+CompareAndWriteCBData *data;
+uint8_t *buf;
+int i;
+
+if (nb_sectors > MAX_COMPARE_AND_WRITE_LENGTH) {
+scsi_check_condition(r, SENSE_CODE(INVALID_FIELD));
+return;
+}
+
+if (blk_is_read_only(s->qdev.conf.blk)) {
+scsi_check_condition(r, SENSE_CODE(WRITE_PROTECTED));
+return;
+}
+
+if (r->req.cmd.lba > s->qdev.max_lba ||
+!check_lba_range(s, r->req.cmd.lba, nb_sectors)) {
+scsi_check_condition(r, SENSE_CODE(LBA_OUT_OF_RANGE));
+return;
+}
+
+data = g_new0(CompareAndWriteCBData, 1);
+data->r = r;
+data->sector = r->req.cmd.lba * (s->qdev.blocksize / 512);
+data->nb_sectors = r->req.cmd.xfer * (s->qdev.blocksize / 512);
+data->iov.iov_len = data->nb_sectors;
+data->iov.iov_base = buf = blk_blockalign(s->qdev.conf.blk,
+ data->iov.iov_len);
+qemu_iovec_init_external(>qiov, >iov, 1);
+
+for (i = 0; i < data->iov.iov_len; i += s->qdev.blocksize)
[PATCH 2/4 V2] block/rbd: implement bdrv_aio_compare_and_write interface
This patch adds librbd's SCSI COMPARE_AND_WRITE command interface
support with bdrv_aio_compare_and_write function pointer. Note
currently when a miscompare happens a mismatch offset of 0 is
always reported rather than the actual mismatch offset. This
should not be a big issue contemporarily and will be fixed
accordingly in the future.
Signed-off-by: Yaowei Bai
---
block/raw-format.c | 3 ++-
block/rbd.c| 45 +++--
2 files changed, 45 insertions(+), 3 deletions(-)
diff --git a/block/raw-format.c b/block/raw-format.c
index 3a76ec7..e87cd44 100644
--- a/block/raw-format.c
+++ b/block/raw-format.c
@@ -439,7 +439,8 @@ static int raw_open(BlockDriverState *bs, QDict *options,
int flags,
bs->sg = bs->file->bs->sg;
bs->supported_write_flags = BDRV_REQ_WRITE_UNCHANGED |
-(BDRV_REQ_FUA & bs->file->bs->supported_write_flags);
+(BDRV_REQ_FUA & bs->file->bs->supported_write_flags) |
+(BDRV_REQ_COMPARE_AND_WRITE & bs->file->bs->supported_write_flags);
bs->supported_zero_flags = BDRV_REQ_WRITE_UNCHANGED |
((BDRV_REQ_FUA | BDRV_REQ_MAY_UNMAP | BDRV_REQ_NO_FALLBACK) &
bs->file->bs->supported_zero_flags);
diff --git a/block/rbd.c b/block/rbd.c
index 027cbcc..0e45bc3 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -73,11 +73,18 @@
#define LIBRBD_USE_IOVEC 0
#endif
+#ifdef LIBRBD_SUPPORTS_COMPARE_AND_WRITE
+#define LIBRBD_HAVE_COMPARE_AND_WRITE 1
+#else
+#define LIBRBD_HAVE_COMPARE_AND_WRITE 0
+#endif
+
typedef enum {
RBD_AIO_READ,
RBD_AIO_WRITE,
RBD_AIO_DISCARD,
-RBD_AIO_FLUSH
+RBD_AIO_FLUSH,
+RBD_AIO_COMPARE_AND_WRITE
} RBDAIOCmd;
typedef struct RBDAIOCB {
@@ -798,6 +805,10 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict
*options, int flags,
}
}
+if (LIBRBD_HAVE_COMPARE_AND_WRITE) {
+bs->supported_write_flags = BDRV_REQ_COMPARE_AND_WRITE;
+}
+
r = 0;
goto out;
@@ -933,7 +944,15 @@ static BlockAIOCB *rbd_start_aio(BlockDriverState *bs,
rcb = g_new(RADOSCB, 1);
-if (!LIBRBD_USE_IOVEC) {
+if (cmd == RBD_AIO_COMPARE_AND_WRITE) {
+acb->bounce = qemu_try_blockalign(bs, qiov->size);
+if (acb->bounce == NULL) {
+goto failed;
+}
+
+qemu_iovec_to_buf(acb->qiov, 0, acb->bounce, qiov->size);
+rcb->buf = acb->bounce;
+} else if (!LIBRBD_USE_IOVEC) {
if (cmd == RBD_AIO_DISCARD || cmd == RBD_AIO_FLUSH) {
acb->bounce = NULL;
} else {
@@ -993,6 +1012,10 @@ static BlockAIOCB *rbd_start_aio(BlockDriverState *bs,
case RBD_AIO_FLUSH:
r = rbd_aio_flush_wrapper(s->image, c);
break;
+case RBD_AIO_COMPARE_AND_WRITE:
+r = rbd_aio_compare_and_write(s->image, off, size / 2, rcb->buf,
+ (rcb->buf + size / 2), c, 0, 0);
+break;
default:
r = -EINVAL;
}
@@ -1056,6 +1079,20 @@ static int qemu_rbd_co_flush(BlockDriverState *bs)
}
#endif
+#ifdef LIBRBD_SUPPORTS_COMPARE_AND_WRITE
+static BlockAIOCB *qemu_rbd_aio_compare_and_write(BlockDriverState *bs,
+ uint64_t offset,
+ uint64_t bytes,
+ QEMUIOVector *qiov,
+ int flags,
+ BlockCompletionFunc *cb,
+ void *opaque)
+{
+return rbd_start_aio(bs, offset, qiov, bytes, cb, opaque,
+ RBD_AIO_COMPARE_AND_WRITE);
+}
+#endif
+
static int qemu_rbd_getinfo(BlockDriverState *bs, BlockDriverInfo *bdi)
{
BDRVRBDState *s = bs->opaque;
@@ -1310,6 +1347,10 @@ static BlockDriver bdrv_rbd = {
.bdrv_aio_pdiscard = qemu_rbd_aio_pdiscard,
#endif
+#ifdef LIBRBD_SUPPORTS_COMPARE_AND_WRITE
+.bdrv_aio_compare_and_write = qemu_rbd_aio_compare_and_write,
+#endif
+
.bdrv_snapshot_create = qemu_rbd_snap_create,
.bdrv_snapshot_delete = qemu_rbd_snap_remove,
.bdrv_snapshot_list = qemu_rbd_snap_list,
--
1.8.3.1
[PATCH 0/4 V2] SCSI COMPARE_AND_WRITE command support
Recently ceph/librbd added several interfaces to handle SCSI commands like COMPARE_AND_WRITE directly. However they were only be used in special scenarios, i.e. ISCSI. That involves more software components which makes the IO path longer and could bring more potential issues. Actually we're maintaining several ceph clusters with ISCSI protocal being used which, i have to say, is not an easy job. So supporting COMPARE_AND_WRITE in scsi-disk would be a reasonable solution and easy to implement with the help of the SCSI handlers in librbd, which could leave alone the ISCSI stuff and make the IO path shorter. This patchset implements it by reusing the blk_aio_pwritev interface and introducing a new BDRV_REQ_COMPARE_AND_WRITE element into BdrvRequestFlags to indicate a COMPARE_AND_WRITE request, rather than adding a new interface into block-backend.c. The FUA support is implemented in the blk_aio_pwritev's callback function similar to its emulation in scsi_write_do_fua function, maybe through BDRV_REQ_FUA is another doable way. This patchset is tested with the method of sg_compare_and_write.txt from sg3_utils. Link below: https://github.com/hreinecke/sg3_utils/blob/master/examples/sg_compare_and_write.txt v1->v2: fix checkpatch script complaints and cleanup Yaowei Bai (4): block: add SCSI COMPARE_AND_WRITE support block/rbd: implement bdrv_aio_compare_and_write interface hw/scsi: add SCSI COMPARE_AND_WRITE support scsi-disk: add FUA support for COMPARE_AND_WRITE block/io.c | 20 + block/raw-format.c | 3 +- block/rbd.c | 45 - hw/scsi/emulation.c | 1 + hw/scsi/scsi-bus.c | 4 ++ hw/scsi/scsi-disk.c | 98 + hw/scsi/trace-events| 1 + include/block/block.h | 5 ++- include/block/block_int.h | 3 ++ include/hw/scsi/emulation.h | 3 ++ include/scsi/utils.h| 2 + scsi/utils.c| 5 +++ 12 files changed, 185 insertions(+), 5 deletions(-) -- 1.8.3.1
[PATCH 1/4 V2] block: add SCSI COMPARE_AND_WRITE support
Some storages(i.e. librbd) already have interfaces to handle some SCSI
commands directly. This patch adds COMPARE_AND_WRITE command support
through the write path in the block io layer by introducing a new element
BDRV_REQ_COMPARE_AND_WRITE into BdrvRequestFlags which indicates a
COMPARE_AND_WRITE request. In this way we could easily extend to other
SCSI commands support like WRITE_SAME in the future.
Signed-off-by: Yaowei Bai
---
block/io.c| 20
include/block/block.h | 5 +++--
include/block/block_int.h | 3 +++
3 files changed, 26 insertions(+), 2 deletions(-)
diff --git a/block/io.c b/block/io.c
index f75777f..3507d71 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1186,6 +1186,26 @@ static int coroutine_fn
bdrv_driver_pwritev(BlockDriverState *bs,
goto emulate_flags;
}
+if (drv->bdrv_aio_compare_and_write &&
+ (flags & BDRV_REQ_COMPARE_AND_WRITE)) {
+BlockAIOCB *acb;
+CoroutineIOCompletion co = {
+.coroutine = qemu_coroutine_self(),
+};
+
+acb = drv->bdrv_aio_compare_and_write(bs, offset, bytes, qiov,
+flags & bs->supported_write_flags,
+bdrv_co_io_em_complete, );
+flags &= ~bs->supported_write_flags;
+if (acb == NULL) {
+ret = -EIO;
+} else {
+qemu_coroutine_yield();
+ret = co.ret;
+}
+goto emulate_flags;
+}
+
if (drv->bdrv_aio_pwritev) {
BlockAIOCB *acb;
CoroutineIOCompletion co = {
diff --git a/include/block/block.h b/include/block/block.h
index 1df9848..f71aa4f 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -92,9 +92,10 @@ typedef enum {
* on read request and means that caller doesn't really need data to be
* written to qiov parameter which may be NULL.
*/
-BDRV_REQ_PREFETCH = 0x200,
+BDRV_REQ_PREFETCH = 0x200,
+BDRV_REQ_COMPARE_AND_WRITE = 0x400,
/* Mask of valid flags */
-BDRV_REQ_MASK = 0x3ff,
+BDRV_REQ_MASK = 0x7ff,
} BdrvRequestFlags;
typedef struct BlockSizes {
diff --git a/include/block/block_int.h b/include/block/block_int.h
index dd033d0..96096e0 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -189,6 +189,9 @@ struct BlockDriver {
BlockAIOCB *(*bdrv_aio_pdiscard)(BlockDriverState *bs,
int64_t offset, int bytes,
BlockCompletionFunc *cb, void *opaque);
+BlockAIOCB *(*bdrv_aio_compare_and_write)(BlockDriverState *bs,
+uint64_t offset, uint64_t bytes, QEMUIOVector *qiov, int flags,
+BlockCompletionFunc *cb, void *opaque);
int coroutine_fn (*bdrv_co_readv)(BlockDriverState *bs,
int64_t sector_num, int nb_sectors, QEMUIOVector *qiov);
--
1.8.3.1
[PATCH 4/4 V2] scsi-disk: add FUA support for COMPARE_AND_WRITE
It is implemented in the blk_aio_pwritev's callback function in a way
similar to its emulation in scsi_write_do_fua function
Signed-off-by: Yaowei Bai
---
hw/scsi/scsi-disk.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index f9a0267..0731a3b 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -229,6 +229,7 @@ static bool scsi_is_cmd_fua(SCSICommand *cmd)
case WRITE_10:
case WRITE_12:
case WRITE_16:
+case COMPARE_AND_WRITE:
return (cmd->buf[1] & 8) != 0;
case VERIFY_10:
@@ -1850,10 +1851,17 @@ static void scsi_compare_and_write_complete(void
*opaque, int ret)
}
block_acct_done(blk_get_stats(s->qdev.conf.blk), >acct);
+if (r->need_fua_emulation) {
+block_acct_start(blk_get_stats(s->qdev.conf.blk), >acct, 0,
+ BLOCK_ACCT_FLUSH);
+r->req.aiocb = blk_aio_flush(s->qdev.conf.blk, scsi_aio_complete, r);
+goto free;
+}
scsi_req_complete(>req, GOOD);
done:
scsi_req_unref(>req);
+free:
qemu_vfree(data->iov.iov_base);
g_free(data);
aio_context_release(blk_get_aio_context(s->qdev.conf.blk));
@@ -1954,6 +1962,7 @@ static int32_t scsi_disk_emulate_command(SCSIRequest
*req, uint8_t *buf)
{
SCSIDiskReq *r = DO_UPCAST(SCSIDiskReq, req, req);
SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, req->dev);
+SCSIDiskClass *sdc = (SCSIDiskClass *) object_get_class(OBJECT(s));
uint64_t nb_sectors;
uint8_t *outbuf;
int buflen;
@@ -2209,6 +2218,7 @@ static int32_t scsi_disk_emulate_command(SCSIRequest
*req, uint8_t *buf)
return 0;
}
assert(!r->req.aiocb);
+r->need_fua_emulation = sdc->need_fua_emulation(>req.cmd);
r->iov.iov_len = MIN(r->buflen, req->cmd.xfer);
if (r->iov.iov_len == 0) {
scsi_req_complete(>req, GOOD);
--
1.8.3.1
Re: [PATCH 3/4] hw/scsi: add SCSI COMPARE_AND_WRITE support
On Fri, Oct 25, 2019 at 05:36:01PM +0800, Yaowei Bai wrote: > > diff --git a/include/tcmu/tcmu.h b/include/tcmu/tcmu.h > new file mode 100644 > index 000..656a545 > --- /dev/null > +++ b/include/tcmu/tcmu.h > @@ -0,0 +1,14 @@ > +#ifndef QEMU_TCMU_H > +#define QEMU_TCMU_H > + > +#include "qemu-common.h" > + > +typedef struct TCMUExport TCMUExport; > +extern QemuOptsList qemu_tcmu_export_opts; > + > +void qemu_tcmu_stop(void); > +void qemu_tcmu_start(const char *subtype, Error **errp); > +TCMUExport *tcmu_export_new(BlockBackend *blk, bool writable, Error **errp); > +int export_init_func(void *opaque, QemuOpts *all_opts, Error **errp); > + > +#endif Please ignore this odd part. > -- > 1.8.3.1 >
[PATCH 3/4] hw/scsi: add SCSI COMPARE_AND_WRITE support
This patch emulates COMPARE_AND_WRITE command with the
BDRV_REQ_COMPARE_AND_WRITE flag introduced by last patch. It matches
the SBC-4 standard except the FUA bit support, it'll be finished in
the next patch.
Note that cmd->xfer is set 2 * the number got by scsi_data_cdb_xfer
so we could touch the least code.
Signed-off-by: Yaowei Bai
---
hw/scsi/emulation.c | 1 +
hw/scsi/scsi-bus.c | 4 +++
hw/scsi/scsi-disk.c | 88 +
hw/scsi/trace-events| 1 +
include/hw/scsi/emulation.h | 3 ++
include/scsi/utils.h| 2 ++
include/tcmu/tcmu.h | 14
scsi/utils.c| 5 +++
8 files changed, 118 insertions(+)
create mode 100644 include/tcmu/tcmu.h
diff --git a/hw/scsi/emulation.c b/hw/scsi/emulation.c
index 06d62f3..1f53c4a 100644
--- a/hw/scsi/emulation.c
+++ b/hw/scsi/emulation.c
@@ -9,6 +9,7 @@ int scsi_emulate_block_limits(uint8_t *outbuf, const
SCSIBlockLimits *bl)
memset(outbuf, 0, 0x3c);
outbuf[0] = bl->wsnz; /* wsnz */
+outbuf[1] = MAX_COMPARE_AND_WRITE_LENGTH;
if (bl->max_io_sectors) {
/* optimal transfer length granularity. This field and the optimal
diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index bccb7cc..15aab8a 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -987,6 +987,9 @@ static int scsi_req_xfer(SCSICommand *cmd, SCSIDevice *dev,
uint8_t *buf)
case WRITE_VERIFY_16:
cmd->xfer *= dev->blocksize;
break;
+case COMPARE_AND_WRITE:
+cmd->xfer *= 2 * dev->blocksize;
+break;
case READ_6:
case READ_REVERSE:
/* length 0 means 256 blocks */
@@ -1206,6 +1209,7 @@ static void scsi_cmd_xfer_mode(SCSICommand *cmd)
case PERSISTENT_RESERVE_OUT:
case MAINTENANCE_OUT:
case SET_WINDOW:
+case COMPARE_AND_WRITE:
case SCAN:
/* SCAN conflicts with START_STOP. START_STOP has cmd->xfer set to 0
for
* non-scanner devices, so we only get here for SCAN and not for
START_STOP.
diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index 68b1675..4bff862 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -477,6 +477,9 @@ static bool scsi_handle_rw_error(SCSIDiskReq *r, int error,
bool acct_failed)
case ENOSPC:
scsi_check_condition(r, SENSE_CODE(SPACE_ALLOC_FAILED));
break;
+case EILSEQ:
+scsi_check_condition(r, SENSE_CODE(MISCOMPARE_DURING_VERIFY));
+break;
default:
scsi_check_condition(r, SENSE_CODE(IO_ERROR));
break;
@@ -1824,6 +1827,84 @@ static void scsi_disk_emulate_write_same(SCSIDiskReq *r,
uint8_t *inbuf)
scsi_write_same_complete, data);
}
+typedef struct CompareAndWriteCBData {
+SCSIDiskReq *r;
+int64_t sector;
+int nb_sectors;
+QEMUIOVector qiov;
+struct iovec iov;
+} CompareAndWriteCBData;
+
+static void scsi_compare_and_write_complete(void *opaque, int ret)
+{
+CompareAndWriteCBData *data = opaque;
+SCSIDiskReq *r = data->r;
+SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev);
+
+assert(r->req.aiocb != NULL);
+r->req.aiocb = NULL;
+aio_context_acquire(blk_get_aio_context(s->qdev.conf.blk));
+if (scsi_disk_req_check_error(r, ret, true)) {
+goto done;
+}
+
+block_acct_done(blk_get_stats(s->qdev.conf.blk), >acct);
+scsi_req_complete(>req, GOOD);
+
+done:
+scsi_req_unref(>req);
+qemu_vfree(data->iov.iov_base);
+g_free(data);
+aio_context_release(blk_get_aio_context(s->qdev.conf.blk));
+}
+
+static void scsi_disk_emulate_compare_and_write(SCSIDiskReq *r, uint8_t *inbuf)
+{
+SCSIRequest *req = >req;
+SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, req->dev);
+uint32_t nb_sectors = scsi_data_cdb_xfer(r->req.cmd.buf);
+CompareAndWriteCBData *data;
+uint8_t *buf;
+int i;
+
+if (nb_sectors > MAX_COMPARE_AND_WRITE_LENGTH) {
+scsi_check_condition(r, SENSE_CODE(INVALID_FIELD));
+return;
+}
+
+if (blk_is_read_only(s->qdev.conf.blk)) {
+scsi_check_condition(r, SENSE_CODE(WRITE_PROTECTED));
+return;
+}
+
+if (r->req.cmd.lba > s->qdev.max_lba ||
+!check_lba_range(s, r->req.cmd.lba, nb_sectors)) {
+scsi_check_condition(r, SENSE_CODE(LBA_OUT_OF_RANGE));
+return;
+}
+
+data = g_new0(CompareAndWriteCBData, 1);
+data->r = r;
+data->sector = r->req.cmd.lba * (s->qdev.blocksize / 512);
+data->nb_sectors = r->req.cmd.xfer * (s->qdev.blocksize / 512);
+data->iov.iov_len = data->nb_sectors;
+data->iov.iov_base = buf = blk_blockalign(s->qdev.conf.blk,
+ data->iov.iov_len);
+qemu_iovec_init_external(>qiov
[PATCH 0/4] SCSI COMPARE_AND_WRITE support
Recently ceph/librbd added several interfaces to handle SCSI commands like COMPARE_AND_WRITE directly. However they were only be used in special scenarios, i.e. ISCSI. That involves more software components which makes the IO path longer and could bring more potential issues. Actually we're maintaining several ceph clusters with ISCSI protocal being used which, i have to say, is not an easy job. So supporting COMPARE_AND_WRITE in scsi-disk would be a reasonable solution and easy to implement with the help of the SCSI handlers in librbd, which could leave alone the ISCSI stuff and make the IO path shorter. This patchset implements it by reusing the blk_aio_pwritev interface and introducing a new BDRV_REQ_COMPARE_AND_WRITE element into BdrvRequestFlags to indicate a COMPARE_AND_WRITE request, rather than adding a new interface into block-backend.c. The FUA support is implemented in the blk_aio_pwritev's callback function similar to its emulation in scsi_write_do_fua function, maybe through BDRV_REQ_FUA is another doable way. This patchset is tested with the method of sg_compare_and_write.txt from sg3_utils. Link below: https://github.com/hreinecke/sg3_utils/blob/master/examples/sg_compare_and_write.txt Yaowei Bai (4): block: add SCSI COMPARE_AND_WRITE support block/rbd: implement bdrv_aio_compare_and_write interface hw/scsi: add SCSI COMPARE_AND_WRITE support scsi-disk: add FUA support for COMPARE_AND_WRITE block/io.c | 20 + block/raw-format.c | 3 +- block/rbd.c | 41 ++- hw/scsi/emulation.c | 1 + hw/scsi/scsi-bus.c | 4 ++ hw/scsi/scsi-disk.c | 98 + hw/scsi/trace-events| 1 + include/block/block.h | 5 ++- include/block/block_int.h | 3 ++ include/hw/scsi/emulation.h | 3 ++ include/scsi/utils.h| 2 + include/tcmu/tcmu.h | 14 +++ scsi/utils.c| 5 +++ 13 files changed, 195 insertions(+), 5 deletions(-) create mode 100644 include/tcmu/tcmu.h -- 1.8.3.1
[PATCH 4/4] scsi-disk: add FUA support for COMPARE_AND_WRITE
It is implemented in the blk_aio_pwritev's callback function in a way
similar to its emulation in scsi_write_do_fua function
Signed-off-by: Yaowei Bai
---
hw/scsi/scsi-disk.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index 4bff862..ef9c257 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -228,6 +228,7 @@ static bool scsi_is_cmd_fua(SCSICommand *cmd)
case WRITE_10:
case WRITE_12:
case WRITE_16:
+case COMPARE_AND_WRITE:
return (cmd->buf[1] & 8) != 0;
case VERIFY_10:
@@ -1849,10 +1850,17 @@ static void scsi_compare_and_write_complete(void
*opaque, int ret)
}
block_acct_done(blk_get_stats(s->qdev.conf.blk), >acct);
+if (r->need_fua_emulation) {
+block_acct_start(blk_get_stats(s->qdev.conf.blk), >acct, 0,
+ BLOCK_ACCT_FLUSH);
+r->req.aiocb = blk_aio_flush(s->qdev.conf.blk, scsi_aio_complete, r);
+goto free;
+}
scsi_req_complete(>req, GOOD);
done:
scsi_req_unref(>req);
+free:
qemu_vfree(data->iov.iov_base);
g_free(data);
aio_context_release(blk_get_aio_context(s->qdev.conf.blk));
@@ -1953,6 +1961,7 @@ static int32_t scsi_disk_emulate_command(SCSIRequest
*req, uint8_t *buf)
{
SCSIDiskReq *r = DO_UPCAST(SCSIDiskReq, req, req);
SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, req->dev);
+SCSIDiskClass *sdc = (SCSIDiskClass *) object_get_class(OBJECT(s));
uint64_t nb_sectors;
uint8_t *outbuf;
int buflen;
@@ -2208,6 +2217,7 @@ static int32_t scsi_disk_emulate_command(SCSIRequest
*req, uint8_t *buf)
return 0;
}
assert(!r->req.aiocb);
+r->need_fua_emulation = sdc->need_fua_emulation(>req.cmd);
r->iov.iov_len = MIN(r->buflen, req->cmd.xfer);
if (r->iov.iov_len == 0) {
scsi_req_complete(>req, GOOD);
--
1.8.3.1
[PATCH 2/4] block/rbd: implement bdrv_aio_compare_and_write interface
This patch adds librbd's SCSI COMPARE_AND_WRITE command interface
support with bdrv_aio_compare_and_write function pointer. Note
currently when a miscompare happens a mismatch offset of 0 is
always reported rather than the actual mismatch offset. This
should not be a big issue contemporarily and will be fixed
accordingly in the future.
Signed-off-by: Yaowei Bai
---
block/raw-format.c | 3 ++-
block/rbd.c| 41 +++--
2 files changed, 41 insertions(+), 3 deletions(-)
diff --git a/block/raw-format.c b/block/raw-format.c
index 42c28cc..3d8f201 100644
--- a/block/raw-format.c
+++ b/block/raw-format.c
@@ -438,7 +438,8 @@ static int raw_open(BlockDriverState *bs, QDict *options,
int flags,
bs->sg = bs->file->bs->sg;
bs->supported_write_flags = BDRV_REQ_WRITE_UNCHANGED |
-(BDRV_REQ_FUA & bs->file->bs->supported_write_flags);
+(BDRV_REQ_FUA & bs->file->bs->supported_write_flags) |
+(BDRV_REQ_COMPARE_AND_WRITE & bs->file->bs->supported_write_flags);
bs->supported_zero_flags = BDRV_REQ_WRITE_UNCHANGED |
((BDRV_REQ_FUA | BDRV_REQ_MAY_UNMAP | BDRV_REQ_NO_FALLBACK) &
bs->file->bs->supported_zero_flags);
diff --git a/block/rbd.c b/block/rbd.c
index c71e45d..7065343 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -73,11 +73,18 @@
#define LIBRBD_USE_IOVEC 0
#endif
+#ifdef LIBRBD_SUPPORTS_COMPARE_AND_WRITE
+#define LIBRBD_HAVE_COMPARE_AND_WRITE 1
+#else
+#define LIBRBD_HAVE_COMPARE_AND_WRITE 0
+#endif
+
typedef enum {
RBD_AIO_READ,
RBD_AIO_WRITE,
RBD_AIO_DISCARD,
-RBD_AIO_FLUSH
+RBD_AIO_FLUSH,
+RBD_AIO_COMPARE_AND_WRITE
} RBDAIOCmd;
typedef struct RBDAIOCB {
@@ -798,6 +805,9 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict
*options, int flags,
}
}
+if (LIBRBD_HAVE_COMPARE_AND_WRITE)
+bs->supported_write_flags = BDRV_REQ_COMPARE_AND_WRITE;
+
r = 0;
goto out;
@@ -933,7 +943,15 @@ static BlockAIOCB *rbd_start_aio(BlockDriverState *bs,
rcb = g_new(RADOSCB, 1);
-if (!LIBRBD_USE_IOVEC) {
+if (cmd == RBD_AIO_COMPARE_AND_WRITE) {
+acb->bounce = qemu_try_blockalign(bs, qiov->size);
+if (acb->bounce == NULL) {
+goto failed;
+}
+
+qemu_iovec_to_buf(acb->qiov, 0, acb->bounce, qiov->size);
+rcb->buf = acb->bounce;
+} else if (!LIBRBD_USE_IOVEC) {
if (cmd == RBD_AIO_DISCARD || cmd == RBD_AIO_FLUSH) {
acb->bounce = NULL;
} else {
@@ -993,6 +1011,9 @@ static BlockAIOCB *rbd_start_aio(BlockDriverState *bs,
case RBD_AIO_FLUSH:
r = rbd_aio_flush_wrapper(s->image, c);
break;
+case RBD_AIO_COMPARE_AND_WRITE:
+r = rbd_aio_compare_and_write(s->image, off, size/2, rcb->buf,
(rcb->buf + size/2), c, 0, 0);
+break;
default:
r = -EINVAL;
}
@@ -1056,6 +1077,18 @@ static int qemu_rbd_co_flush(BlockDriverState *bs)
}
#endif
+#ifdef LIBRBD_SUPPORTS_COMPARE_AND_WRITE
+static BlockAIOCB *qemu_rbd_aio_compare_and_write(BlockDriverState *bs,
+ uint64_t offset, uint64_t
bytes,
+ QEMUIOVector *qiov, int
flags,
+ BlockCompletionFunc *cb,
+ void *opaque)
+{
+return rbd_start_aio(bs, offset, qiov, bytes, cb, opaque,
+ RBD_AIO_COMPARE_AND_WRITE);
+}
+#endif
+
static int qemu_rbd_getinfo(BlockDriverState *bs, BlockDriverInfo *bdi)
{
BDRVRBDState *s = bs->opaque;
@@ -1309,6 +1342,10 @@ static BlockDriver bdrv_rbd = {
.bdrv_aio_pdiscard = qemu_rbd_aio_pdiscard,
#endif
+#ifdef LIBRBD_SUPPORTS_COMPARE_AND_WRITE
+.bdrv_aio_compare_and_write = qemu_rbd_aio_compare_and_write,
+#endif
+
.bdrv_snapshot_create = qemu_rbd_snap_create,
.bdrv_snapshot_delete = qemu_rbd_snap_remove,
.bdrv_snapshot_list = qemu_rbd_snap_list,
--
1.8.3.1
[PATCH 1/4] block: add SCSI COMPARE_AND_WRITE support
Some storages(i.e. librbd) already have interfaces to handle some SCSI
commands directly. This patch adds COMPARE_AND_WRITE command support
through the write path in the block io layer by introducing a new element
BDRV_REQ_COMPARE_AND_WRITE into BdrvRequestFlags which indicates a
COMPARE_AND_WRITE request. In this way we could easily extend to other
SCSI commands support like WRITE_SAME in the future.
Signed-off-by: Yaowei Bai
---
block/io.c| 20
include/block/block.h | 5 +++--
include/block/block_int.h | 3 +++
3 files changed, 26 insertions(+), 2 deletions(-)
diff --git a/block/io.c b/block/io.c
index f0b86c1..667b5ea 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1168,6 +1168,26 @@ static int coroutine_fn
bdrv_driver_pwritev(BlockDriverState *bs,
goto emulate_flags;
}
+if (drv->bdrv_aio_compare_and_write &&
+ (flags & BDRV_REQ_COMPARE_AND_WRITE)) {
+BlockAIOCB *acb;
+CoroutineIOCompletion co = {
+.coroutine = qemu_coroutine_self(),
+};
+
+acb = drv->bdrv_aio_compare_and_write(bs, offset, bytes, qiov,
+flags & bs->supported_write_flags,
+bdrv_co_io_em_complete, );
+flags &= ~bs->supported_write_flags;
+if (acb == NULL) {
+ret = -EIO;
+} else {
+qemu_coroutine_yield();
+ret = co.ret;
+}
+goto emulate_flags;
+}
+
if (drv->bdrv_aio_pwritev) {
BlockAIOCB *acb;
CoroutineIOCompletion co = {
diff --git a/include/block/block.h b/include/block/block.h
index 89606bd..7159e03 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -92,9 +92,10 @@ typedef enum {
* on read request and means that caller doesn't really need data to be
* written to qiov parameter which may be NULL.
*/
-BDRV_REQ_PREFETCH = 0x200,
+BDRV_REQ_PREFETCH = 0x200,
+BDRV_REQ_COMPARE_AND_WRITE = 0x400,
/* Mask of valid flags */
-BDRV_REQ_MASK = 0x3ff,
+BDRV_REQ_MASK = 0x7ff,
} BdrvRequestFlags;
typedef struct BlockSizes {
diff --git a/include/block/block_int.h b/include/block/block_int.h
index ca4ccac..a039b68 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -189,6 +189,9 @@ struct BlockDriver {
BlockAIOCB *(*bdrv_aio_pdiscard)(BlockDriverState *bs,
int64_t offset, int bytes,
BlockCompletionFunc *cb, void *opaque);
+BlockAIOCB *(*bdrv_aio_compare_and_write)(BlockDriverState *bs,
+uint64_t offset, uint64_t bytes, QEMUIOVector *qiov, int flags,
+BlockCompletionFunc *cb, void *opaque);
int coroutine_fn (*bdrv_co_readv)(BlockDriverState *bs,
int64_t sector_num, int nb_sectors, QEMUIOVector *qiov);
--
1.8.3.1
Re: COMPARE_AND_WRITE support for rbd driver
On Fri, Sep 20, 2019 at 01:22:02PM +0200, Paolo Bonzini wrote: > On 19/09/19 07:36, Yaowei Bai wrote: > > > > Hey guys, > > > > I noticed that COMPARE_AND_WRITE had been supported by CEPH/librbd since > > v12.1.1. And in my company, we use this COMPARE_AND_WRITE support in > > CEPH with the ISCSI protocol. More precisely, we use tgt and CEPH with this > > COMPARE_AND_WRITE support as the SCSI target and export it to the remote > > hosts. And then VMs on remote hosts can use these SCSI targets through ISCSI > > initiator support in QEMU directly or as local SCSI disks. But > > unfortunately, > > there're some issues with this tgt case. So i think maybe we could also add > > this > > COMPARE_AND_WRITE support into the rbd driver in QEMU so we can leave the > > ISCSI/tgt alone and use this COMPARE_AND_WRITE support with the > > scsi-disk <--> virtio-scsi <--> rbd driver path. This can also apply to > > the WRITESAME support in CEPH/librbd. > > > > So is it suitable for doing this? > > Yes, it would be suitable. In a nutshell you would have to add support > for COMPARE_AND_WRITE to block/io.c (calling into a new BlockDriver > function pointer), block/rbd.c and hw/scsi/scsi-disk.c. OK, i'll try to implement this and send it out for review. Thanks Paolo. > > Paolo
[Qemu-block] COMPARE_AND_WRITE support for rbd driver
baiyao...@cmss.chinamobile.com Bcc: Subject: COMPARE_AND_WRITE support for rbd driver Reply-To: baiyao...@cmss.chinamobile.com Hey guys, I noticed that COMPARE_AND_WRITE had been supported by CEPH/librbd since v12.1.1. And in my company, we use this COMPARE_AND_WRITE support in CEPH with the ISCSI protocol. More precisely, we use tgt and CEPH with this COMPARE_AND_WRITE support as the SCSI target and export it to the remote hosts. And then VMs on remote hosts can use these SCSI targets through ISCSI initiator support in QEMU directly or as local SCSI disks. But unfortunately, there're some issues with this tgt case. So i think maybe we could also add this COMPARE_AND_WRITE support into the rbd driver in QEMU so we can leave the ISCSI/tgt alone and use this COMPARE_AND_WRITE support with the scsi-disk <--> virtio-scsi <--> rbd driver path. This can also apply to the WRITESAME support in CEPH/librbd. So is it suitable for doing this? I want to hear suggestions from you genius guys. Appreciate it.
Re: [Qemu-block] [PATCH] tcmu: Introduce qemu-tcmu utility
> > I'm not sure what check you mean. Case 2 would need to find an existing > export with the given name, of course, and would return an error if no > such export exists yet. > > But for care 1, isn't the image explicitly opened when the target is > configured? And if it can't be opened, -1 is returned to libtcmu? Yes it is. I misunderstood your meaning, forget about this. > > > Actually we thought about qemu-tcmu working like case2, but felt is's quite > > less > > flexible. With case2, e.g. when we want to export another new image file > > with > > one qemu-tcmu already running, we have to run another qemu-tcmu with > > the configuration of the new image file in commandline, or through QMP of > > the > > running qemu-tcmu, and then configure it with targetcli. So anyway, > > there's one more step for case2 to export a new image file. While for case1 > > you just need to run qemu-tcmu once and all other operations will be done > > within targetcli, which is more friendly to users i think. > > Some users may call it more convenient (even though it's really the > same, just moved to a different command, in the simple case that case 2 > supports). But it's actually not very flexible. > > If we implement case 1, you can define your configuration exactly as > with QEMU, including multiple -blockdev and -object options, and select > the exact block node that you want to export. > > In case 2, you only have a single string, which comes with many > downsides: > > * You cannot get the semantics of block nodes defined individually with > separate -blockdev options, but only a single tree that is managed as > a single unit. > > * Additional objects such as iothreads or secrets cannot be included in > the config string, so you must split the configuration and pass some > parts directly to qemu-tcmu and other parts to targetcli. This is > inconsistent. > > * You need a way to represent a possible options of a node in a string. > QEMU -drive already caused trouble by giving commas a special meaning. > This patch adds @ as a special character, without an option to escape > it. If you have a filename that contains @, there is no way to use it. > > * For the not yet implemented QMP part: You can export only images that > are newly opened. You cannot export images that are already opened and > in use. But exporting images that are in use by the VM is the main use > of even having a QMP interface. > > If I think a bit longer, I'm sure I can come up with more points. Case 2 > just doesn't feel right, it is like drives were configured in QEMU 0.10, > not in 4.0, and we moved away from it for many reasons, most of which > probably apply here, too. Thanks for explaining the background. It comes to my mind that actually we talked about these two cases with Fam a bit long time ago and decided to support both these two cases. The reason why we implement case2 first is currently we care more about exporting new opened images and it's a bit more convenient, exporting from a VM or QMP can be added in the later release. Do you think it's reasonable/acceptable that we support both cases and use case2 for normal new opened images and case1 for the circumstances you mention above? >
Re: [Qemu-block] [PATCH] tcmu: Introduce qemu-tcmu utility
> > > * The first priority should be adding an in-process iscsi target that > > > can be managed with QMP, similar to the built-in NBD server. > > > > Well, people used to manage iscsi targets through targetcli, a command > > line utility. Our intention is, with targetcli and qemu-tcmu, user can > > create/remove targets and backstores totally in just one place, so you > > don't need to create targets in targetcli and then turn to configure > > backstores in qemu-tcmu with QMP or command line, it's convenient. So > > we decide to implement QMP in the future release but it's definitely > > in our plan. > > I think QMP needs to come first to result in a clean design, because the > command line tool should only be a wrapper around the QMP code. Yeah, i agree this makes more sense from this point. Ok, i'll try to implement it in v2 as your suggestion. Thanks. > But anyway, I still think the change I had in mind is necessary. Maybe > you can see the difference best in an example. Your documentation says > to use this: > > # qemu-tcmu > # targetcli /backstores/user:qemu create qemulun 1G > @id=test@file=/root/test.file Let's call this one case1. > > I think it should work more like this: > > # qemu-tcmu -blockdev > driver=file,filename=/root/test.file,node-name=my_file \ > -export my_export,node=my_file > # targetcli /backstores/user:qemu create qemulun 1G my_export And this one case2. > > In fact, something like this is probably required if we want to export > existing block nodes (that may be in use by a guest) from a running QEMU > instance. In this case, you don't want to open a new image file, but > export the already opened image file. I think the main difference between these two cases is just how the configuration of exported image file is passed to qemu-tcmu. Case1 uses cfgstr while case2 uses command line. Case2 still needs one way to check whether the passed-in image file was opened, right? If so, case1 should also be able to use the same way to check that after extracting cfgstr. Actually we thought about qemu-tcmu working like case2, but felt is's quite less flexible. With case2, e.g. when we want to export another new image file with one qemu-tcmu already running, we have to run another qemu-tcmu with the configuration of the new image file in commandline, or through QMP of the running qemu-tcmu, and then configure it with targetcli. So anyway, there's one more step for case2 to export a new image file. While for case1 you just need to run qemu-tcmu once and all other operations will be done within targetcli, which is more friendly to users i think. So i think qemu-tcmu's quite different from qemu-nbd at this point. We can export a image file by NBD protocol just with qemu-nbd itself. While for qemu-tcmu we still need targetcli to complete other ISCSI target configurations to export a image file. So moving the configuration of image file from cmdline to targetcli should be reasonable in my opinion. > > (Also, can we somehow get rid of the 1G in the command line? qemu-tcmu > knows how big the image is and can provide this value.) Currently this size option is mandatory in targetcli, not all tools are so smart as QEMU. But maybe we could change it optional in the future.
Re: [Qemu-block] [PATCH] tcmu: Introduce qemu-tcmu utility
Add Fam. On Wed, Mar 06, 2019 at 10:56:33PM +0100, Kevin Wolf wrote: > Am 21.12.2018 um 11:16 hat Yaowei Bai geschrieben: > > This patch introduces a new utility, qemu-tcmu. Apart from the > > underlaying protocol it interacts with the world much like > > qemu-nbd. This patch bases on Fam's version. > > > > Qemu-tcmu handles SCSI commands which are passed through userspace > > from kernel by LIO subsystem using TCMU protocol. Libtcmu is the > > library for processing TCMU protocol in userspace. With qemu-tcmu, > > we can export images/formats like qcow2, rbd, etc. that qemu supports > > using iSCSI protocol or loopback for remote or local access. > > > > Currently qemu-tcmu implements several SCSI command helper functions > > to work. Our goal is to refactor and reuse SCSI code in scsi-disk. > > > > Please refer to docs/tcmu.txt to use qemu-tcmu. We test it on CentOS > > 7.3.(Please use 3.10.0-514 or lower version kernel, there's one issuse > > in higher kernel version we're resolving.) > > > > Cc: Mike Christie > > Cc: Amar Tumballi > > Cc: Prasanna Kalever > > Cc: Paolo Bonzini > > Signed-off-by: Fam Zheng > > Signed-off-by: Yaowei Bai > > Signed-off-by: Xiubo Li > > Sorry, with the email backlog after the Christmas break, this patch went > completely unnoticed on my side. > > I'm not going to review the code in detail yet, because I think there > are a few very high level points that need to be addressed first: > > * Patchew replied with a ton of coding style problems. This patch isn't > mergable without these problems fixed. These problems've been fixed in the V2 and it'll be sent out soon. Thanks. > > * The first priority should be adding an in-process iscsi target that > can be managed with QMP, similar to the built-in NBD server. Well, people used to manage iscsi targets through targetcli, a command line utility. Our intention is, with targetcli and qemu-tcmu, user can create/remove targets and backstores totally in just one place, so you don't need to create targets in targetcli and then turn to configure backstores in qemu-tcmu with QMP or command line, it's convenient. So we decide to implement QMP in the future release but it's definitely in our plan. > > * The standalone tool should configure its block backend using -blockdev > based code paths instead of duplicating legacy -drive code, which > cannot provide advanced functionality. OK, will turn into -blockdev based code paths. Thanks. > > * Even worse, you can't get the configuration from the iscsi initiator. > This would be a security nightmare. Instead, the user needs to > configure named exports either in QMP or on the command line for the > tool and the initiator then connects to an export name. So you mean we should configure exports through iscsi initiator? Sorry i don't understand the problems here, could you please explain more? > > * It should be considered if we can have a single standalone tool for > all export mechanisms (NBD, TCMU, vhost-user, fuse, and whatever new > ideas we will have in the future) that could have advanced > functionality like a QMP monitor instead of adding a minimal > specialised tool for each of them. That's a great and exciting idea so we don't need one qemu-nbd, one qemu-tcmu, or more qemu-xxx at the same time, there's just one standalone tool to use, maybe we can even instead add a new 'export' subcommand to qemu-img for this purpose. > > Kevin
Re: [Qemu-block] [PATCH] tcmu: Introduce qemu-tcmu utility
On Wed, Mar 06, 2019 at 10:56:33PM +0100, Kevin Wolf wrote: > Am 21.12.2018 um 11:16 hat Yaowei Bai geschrieben: > > This patch introduces a new utility, qemu-tcmu. Apart from the > > underlaying protocol it interacts with the world much like > > qemu-nbd. This patch bases on Fam's version. > > > > Qemu-tcmu handles SCSI commands which are passed through userspace > > from kernel by LIO subsystem using TCMU protocol. Libtcmu is the > > library for processing TCMU protocol in userspace. With qemu-tcmu, > > we can export images/formats like qcow2, rbd, etc. that qemu supports > > using iSCSI protocol or loopback for remote or local access. > > > > Currently qemu-tcmu implements several SCSI command helper functions > > to work. Our goal is to refactor and reuse SCSI code in scsi-disk. > > > > Please refer to docs/tcmu.txt to use qemu-tcmu. We test it on CentOS > > 7.3.(Please use 3.10.0-514 or lower version kernel, there's one issuse > > in higher kernel version we're resolving.) > > > > Cc: Mike Christie > > Cc: Amar Tumballi > > Cc: Prasanna Kalever > > Cc: Paolo Bonzini > > Signed-off-by: Fam Zheng > > Signed-off-by: Yaowei Bai > > Signed-off-by: Xiubo Li > > Sorry, with the email backlog after the Christmas break, this patch went > completely unnoticed on my side. > > I'm not going to review the code in detail yet, because I think there > are a few very high level points that need to be addressed first: > > * Patchew replied with a ton of coding style problems. This patch isn't > mergable without these problems fixed. These problems've been fixed in the V2 and it'll be sent out soon. Thanks. > > * The first priority should be adding an in-process iscsi target that > can be managed with QMP, similar to the built-in NBD server. Well, people used to manage iscsi targets through targetcli, a command line utility. Our intention is, with targetcli and qemu-tcmu, user can create/remove targets and backstores totally in just one place, so you don't need to create targets in targetcli and then turn to configure backstores in qemu-tcmu with QMP or command line, it's convenient. So we decide to implement QMP in the future release but it's definitely in our plan. > > * The standalone tool should configure its block backend using -blockdev > based code paths instead of duplicating legacy -drive code, which > cannot provide advanced functionality. OK, will turn into -blockdev based code paths. Thanks. > > * Even worse, you can't get the configuration from the iscsi initiator. > This would be a security nightmare. Instead, the user needs to > configure named exports either in QMP or on the command line for the > tool and the initiator then connects to an export name. So you mean we should configure exports through iscsi initiator? Sorry i don't understand the problems here, could you please explain more? > > * It should be considered if we can have a single standalone tool for > all export mechanisms (NBD, TCMU, vhost-user, fuse, and whatever new > ideas we will have in the future) that could have advanced > functionality like a QMP monitor instead of adding a minimal > specialised tool for each of them. That's a great and exciting idea so we don't need one qemu-nbd, one qemu-tcmu, or more qemu-xxx at the same time, there's just one standalone tool to use, maybe we can even instead add a new 'export' subcommand to qemu-img for this purpose. > > Kevin
[Qemu-block] (no subject)
baiyao...@cmss.chinamobile.com
Bcc:
Subject: Re: [Qemu-devel] [PATCH] tcmu: Introduce qemu-tcmu utility
Reply-To: baiyao...@cmss.chinamobile.com
In-Reply-To: <20190102015321.GA26514@byw>
Add Xiubo.
On Wed, Jan 02, 2019 at 09:53:21AM +0800, Yaowei Bai wrote:
> Ping.
>
> BTW, it should be update docker image to install glib to fix this.
>
> On Wed, Dec 26, 2018 at 12:19:48AM -0800, no-re...@patchew.org wrote:
> > Patchew URL:
> > https://patchew.org/QEMU/1545387387-9613-1-git-send-email-baiyao...@cmss.chinamobile.com/
> >
> >
> >
> > Hi,
> >
> > This series seems to have some coding style problems. See output below for
> > more information:
> >
> > Message-id: 1545387387-9613-1-git-send-email-baiyao...@cmss.chinamobile.com
> > Type: series
> > Subject: [Qemu-devel] [PATCH] tcmu: Introduce qemu-tcmu utility
> >
> > === TEST SCRIPT BEGIN ===
> > #!/bin/bash
> >
> > BASE=base
> > n=1
> > total=$(git log --oneline $BASE.. | wc -l)
> > failed=0
> >
> > git config --local diff.renamelimit 0
> > git config --local diff.renames True
> > git config --local diff.algorithm histogram
> >
> > commits="$(git log --format=%H --reverse $BASE..)"
> > for c in $commits; do
> > echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
> > if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -;
> > then
> > failed=1
> > echo
> > fi
> > n=$((n+1))
> > done
> >
> > exit $failed
> > === TEST SCRIPT END ===
> >
> > Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
> > Switched to a new branch 'test'
> > 52869e1 tcmu: Introduce qemu-tcmu utility
> >
> > === OUTPUT BEGIN ===
> > Checking PATCH 1/1: tcmu: Introduce qemu-tcmu utility...
> > WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
> > #157:
> > new file mode 100644
> >
> > ERROR: trailing whitespace
> > #329: FILE: qemu-tcmu.c:51:
> > +"Usage:\n" $
> >
> > WARNING: Block comments use a leading /* on a separate line
> > #466: FILE: qemu-tcmu.c:188:
> > +/* now when the initialization is (almost) complete, chdir("/")
> >
> > WARNING: Block comments use a trailing */ on a separate line
> > #467: FILE: qemu-tcmu.c:189:
> > + * to free any busy filesystems */
> >
> > ERROR: code indent should never use tabs
> > #525: FILE: tcmu/helper.c:16:
> > +^Iuint8_t *cdb,$
> >
> > ERROR: code indent should never use tabs
> > #526: FILE: tcmu/helper.c:17:
> > +^Istruct iovec *iovec,$
> >
> > ERROR: code indent should never use tabs
> > #527: FILE: tcmu/helper.c:18:
> > +^Isize_t iov_cnt)$
> >
> > ERROR: code indent should never use tabs
> > #529: FILE: tcmu/helper.c:20:
> > +^Iuint8_t buf[36];$
> >
> > ERROR: code indent should never use tabs
> > #531: FILE: tcmu/helper.c:22:
> > +^Imemset(buf, 0, sizeof(buf));$
> >
> > ERROR: code indent should never use tabs
> > #533: FILE: tcmu/helper.c:24:
> > +^Ibuf[2] = 0x05; /* SPC-3 */$
> >
> > ERROR: code indent should never use tabs
> > #534: FILE: tcmu/helper.c:25:
> > +^Ibuf[3] = 0x02; /* response data format */$
> >
> > ERROR: code indent should never use tabs
> > #536: FILE: tcmu/helper.c:27:
> > +^I/*$
> >
> > ERROR: code indent should never use tabs
> > #537: FILE: tcmu/helper.c:28:
> > +^I * A Third-Party Copy (3PC)$
> >
> > ERROR: code indent should never use tabs
> > #538: FILE: tcmu/helper.c:29:
> > +^I *$
> >
> > ERROR: code indent should never use tabs
> > #539: FILE: tcmu/helper.c:30:
> > +^I * Enable the XCOPY$
> >
> > ERROR: code indent should never use tabs
> > #540: FILE: tcmu/helper.c:31:
> > +^I */$
> >
> > ERROR: code indent should never use tabs
> > #541: FILE: tcmu/helper.c:32:
> > +^Ibuf[5] = 0x08;$
> >
> > ERROR: code indent should never use tabs
> > #543: FILE: tcmu/helper.c:34:
> > +^Ibuf[7] = 0x02; /* CmdQue */$
> >
> > ERROR: code indent should never use tabs
> > #545: FILE: tcmu/helper.c:36:
> > +^Imemcpy([8], "LIO-ORG ", 8);$
> >
> > ERROR: code indent should never use tabs
> > #546: FILE: tcmu/helper.c:37:
> > +^Imemset([16], 0x20, 16);$
> >
> > ERROR: code indent should never use tabs
> > #547: FILE: tcmu/helper.c:38:
> > +^Imemcpy([16], "TCMU device"
Re: [Qemu-block] [Qemu-devel] [PATCH] tcmu: Introduce qemu-tcmu utility
Ping.
BTW, it should be update docker image to install glib to fix this.
On Wed, Dec 26, 2018 at 12:19:48AM -0800, no-re...@patchew.org wrote:
> Patchew URL:
> https://patchew.org/QEMU/1545387387-9613-1-git-send-email-baiyao...@cmss.chinamobile.com/
>
>
>
> Hi,
>
> This series seems to have some coding style problems. See output below for
> more information:
>
> Message-id: 1545387387-9613-1-git-send-email-baiyao...@cmss.chinamobile.com
> Type: series
> Subject: [Qemu-devel] [PATCH] tcmu: Introduce qemu-tcmu utility
>
> === TEST SCRIPT BEGIN ===
> #!/bin/bash
>
> BASE=base
> n=1
> total=$(git log --oneline $BASE.. | wc -l)
> failed=0
>
> git config --local diff.renamelimit 0
> git config --local diff.renames True
> git config --local diff.algorithm histogram
>
> commits="$(git log --format=%H --reverse $BASE..)"
> for c in $commits; do
> echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
> if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -;
> then
> failed=1
> echo
> fi
> n=$((n+1))
> done
>
> exit $failed
> === TEST SCRIPT END ===
>
> Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
> Switched to a new branch 'test'
> 52869e1 tcmu: Introduce qemu-tcmu utility
>
> === OUTPUT BEGIN ===
> Checking PATCH 1/1: tcmu: Introduce qemu-tcmu utility...
> WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
> #157:
> new file mode 100644
>
> ERROR: trailing whitespace
> #329: FILE: qemu-tcmu.c:51:
> +"Usage:\n" $
>
> WARNING: Block comments use a leading /* on a separate line
> #466: FILE: qemu-tcmu.c:188:
> +/* now when the initialization is (almost) complete, chdir("/")
>
> WARNING: Block comments use a trailing */ on a separate line
> #467: FILE: qemu-tcmu.c:189:
> + * to free any busy filesystems */
>
> ERROR: code indent should never use tabs
> #525: FILE: tcmu/helper.c:16:
> +^Iuint8_t *cdb,$
>
> ERROR: code indent should never use tabs
> #526: FILE: tcmu/helper.c:17:
> +^Istruct iovec *iovec,$
>
> ERROR: code indent should never use tabs
> #527: FILE: tcmu/helper.c:18:
> +^Isize_t iov_cnt)$
>
> ERROR: code indent should never use tabs
> #529: FILE: tcmu/helper.c:20:
> +^Iuint8_t buf[36];$
>
> ERROR: code indent should never use tabs
> #531: FILE: tcmu/helper.c:22:
> +^Imemset(buf, 0, sizeof(buf));$
>
> ERROR: code indent should never use tabs
> #533: FILE: tcmu/helper.c:24:
> +^Ibuf[2] = 0x05; /* SPC-3 */$
>
> ERROR: code indent should never use tabs
> #534: FILE: tcmu/helper.c:25:
> +^Ibuf[3] = 0x02; /* response data format */$
>
> ERROR: code indent should never use tabs
> #536: FILE: tcmu/helper.c:27:
> +^I/*$
>
> ERROR: code indent should never use tabs
> #537: FILE: tcmu/helper.c:28:
> +^I * A Third-Party Copy (3PC)$
>
> ERROR: code indent should never use tabs
> #538: FILE: tcmu/helper.c:29:
> +^I *$
>
> ERROR: code indent should never use tabs
> #539: FILE: tcmu/helper.c:30:
> +^I * Enable the XCOPY$
>
> ERROR: code indent should never use tabs
> #540: FILE: tcmu/helper.c:31:
> +^I */$
>
> ERROR: code indent should never use tabs
> #541: FILE: tcmu/helper.c:32:
> +^Ibuf[5] = 0x08;$
>
> ERROR: code indent should never use tabs
> #543: FILE: tcmu/helper.c:34:
> +^Ibuf[7] = 0x02; /* CmdQue */$
>
> ERROR: code indent should never use tabs
> #545: FILE: tcmu/helper.c:36:
> +^Imemcpy([8], "LIO-ORG ", 8);$
>
> ERROR: code indent should never use tabs
> #546: FILE: tcmu/helper.c:37:
> +^Imemset([16], 0x20, 16);$
>
> ERROR: code indent should never use tabs
> #547: FILE: tcmu/helper.c:38:
> +^Imemcpy([16], "TCMU device", 11);$
>
> ERROR: code indent should never use tabs
> #548: FILE: tcmu/helper.c:39:
> +^Imemcpy([32], "0002", 4);$
>
> ERROR: code indent should never use tabs
> #549: FILE: tcmu/helper.c:40:
> +^Ibuf[4] = 31; /* Set additional length to 31 */$
>
> ERROR: code indent should never use tabs
> #551: FILE: tcmu/helper.c:42:
> +^Itcmu_memcpy_into_iovec(iovec, iov_cnt, buf, sizeof(buf));$
>
> ERROR: code indent should never use tabs
> #552: FILE: tcmu/helper.c:43:
> +^Ireturn TCMU_STS_OK;$
>
> ERROR: code indent should never use tabs
> #558: FILE: tcmu/helper.c:49:
> +^Iif (c >= '0' && c <= '9') {$
>
> ERROR: code indent should never use tabs
> #559: FILE: tcmu/helper.c:50:
> +^I^I*val = c - '0';$
>
> ERROR: code indent should never use tabs
> #560: FILE: tcmu/helper.c:51:
> +^I^Ireturn true;$
>
> ERROR: code indent should never use tabs
> #561: FILE: tcmu/helper.c:52:
> +^I}$
>
> ERROR: code indent should never use tabs
> #562: FILE: tcmu/helper.c:53:
> +^Iif (c >= 'a' && c <= 'f') {$
>
> ERROR: code indent should never use tabs
> #563: FILE: tcmu/helper.c:54:
> +^I^I*val = c - 'a' + 10;$
>
> ERROR: code indent should never use tabs
> #564: FILE: tcmu/helper.c:55:
> +^I^Ireturn true;$
>
> ERROR: code indent should never use tabs
> #565: FILE: tcmu/helper.c:56:
> +^I}$
>
> ERROR: code indent should never use tabs
> #566: FILE: tcmu/helper.c:57:
> +^Iif (c
[Qemu-block] [PATCH] tcmu: Introduce qemu-tcmu utility
This patch introduces a new utility, qemu-tcmu. Apart from the
underlaying protocol it interacts with the world much like
qemu-nbd. This patch bases on Fam's version.
Qemu-tcmu handles SCSI commands which are passed through userspace
from kernel by LIO subsystem using TCMU protocol. Libtcmu is the
library for processing TCMU protocol in userspace. With qemu-tcmu,
we can export images/formats like qcow2, rbd, etc. that qemu supports
using iSCSI protocol or loopback for remote or local access.
Currently qemu-tcmu implements several SCSI command helper functions
to work. Our goal is to refactor and reuse SCSI code in scsi-disk.
Please refer to docs/tcmu.txt to use qemu-tcmu. We test it on CentOS
7.3.(Please use 3.10.0-514 or lower version kernel, there's one issuse
in higher kernel version we're resolving.)
Cc: Mike Christie
Cc: Amar Tumballi
Cc: Prasanna Kalever
Cc: Paolo Bonzini
Signed-off-by: Fam Zheng
Signed-off-by: Yaowei Bai
Signed-off-by: Xiubo Li
---
Makefile| 1 +
Makefile.objs | 3 +-
configure | 45
docs/tcmu.txt | 91 +++
include/tcmu/tcmu.h | 14 +
qemu-tcmu.c | 214 +++
tcmu/Makefile.objs | 5 +
tcmu/helper.c | 741
tcmu/helper.h | 31 +++
tcmu/tcmu.c | 598 ++
tcmu/trace-events | 12 +
11 files changed, 1754 insertions(+), 1 deletion(-)
create mode 100644 docs/tcmu.txt
create mode 100644 include/tcmu/tcmu.h
create mode 100644 qemu-tcmu.c
create mode 100644 tcmu/Makefile.objs
create mode 100644 tcmu/helper.c
create mode 100644 tcmu/helper.h
create mode 100644 tcmu/tcmu.c
create mode 100644 tcmu/trace-events
diff --git a/Makefile b/Makefile
index 038780c..351e9d4 100644
--- a/Makefile
+++ b/Makefile
@@ -483,6 +483,7 @@ qemu-img.o: qemu-img-cmds.h
qemu-img$(EXESUF): qemu-img.o $(block-obj-y) $(crypto-obj-y) $(io-obj-y)
$(qom-obj-y) $(COMMON_LDADDS)
qemu-nbd$(EXESUF): qemu-nbd.o $(block-obj-y) $(crypto-obj-y) $(io-obj-y)
$(qom-obj-y) $(COMMON_LDADDS)
qemu-io$(EXESUF): qemu-io.o $(block-obj-y) $(crypto-obj-y) $(io-obj-y)
$(qom-obj-y) $(COMMON_LDADDS)
+qemu-tcmu$(EXESUF): qemu-tcmu.o $(block-obj-y) $(crypto-obj-y) $(io-obj-y)
$(qom-obj-y) $(COMMON_LDADDS)
qemu-bridge-helper$(EXESUF): qemu-bridge-helper.o $(COMMON_LDADDS)
diff --git a/Makefile.objs b/Makefile.objs
index 56af034..8f96c42 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -26,7 +26,7 @@ block-obj-y += block/ scsi/
block-obj-y += qemu-io-cmds.o
block-obj-$(CONFIG_REPLICATION) += replication.o
-block-obj-m = block/
+block-obj-m = block/ tcmu/
###
# crypto-obj-y is code used by both qemu system emulation and qemu-img
@@ -196,6 +196,7 @@ trace-events-subdirs += target/mips
trace-events-subdirs += target/ppc
trace-events-subdirs += target/s390x
trace-events-subdirs += target/sparc
+trace-events-subdirs += tcmu
trace-events-subdirs += ui
trace-events-subdirs += util
diff --git a/configure b/configure
index 224d307..d41e4e9 100755
--- a/configure
+++ b/configure
@@ -346,6 +346,7 @@ fdt=""
netmap="no"
sdl=""
sdlabi=""
+tcmu=""
virtfs=""
mpath=""
vnc="yes"
@@ -1034,6 +1035,10 @@ for opt do
# configure to be used by RPM and similar macros that set
# lots of directory switches by default.
;;
+ --enable-tcmu) tcmu="yes"
+ ;;
+ --disable-tcmu) tcmu="no"
+ ;;
--disable-sdl) sdl="no"
;;
--enable-sdl) sdl="yes"
@@ -3607,6 +3612,36 @@ else
fi
##
+# tcmu support probe
+
+if test "$tcmu" != "no"; then
+ # Sanity check for gio-unix-2.0 (part of glib2), cannot fail unless something
+ # is very wrong.
+ if ! $pkg_config gio-unix-2.0; then
+error_exit "glib is required to compile QEMU"
+ fi
+ cat > $TMPC <
+#include
+
+int main(int argc, char **argv)
+{
+ struct tcmulib_context *ctx = tcmulib_initialize(NULL, 0);
+ tcmulib_register(ctx);
+ return ctx != NULL;
+}
+EOF
+ if compile_prog "" "-ltcmu" ; then
+tcmu=yes
+tcmu_libs="-ltcmu"
+ elif test "$tcmu" == "yes"; then
+feature_not_found "libtcmu" "Install libtcmu devel (>=1.0.5)"
+ else
+tcmu=no
+ fi
+fi
+
+##
# libmpathpersist probe
if test "$mpath" != "no" ; then
@@ -5756,6 +5791,9 @@ if test "$want_tools" = "yes" ; then
if [ "$posix" = "yes" ] && [ "$curl" = "yes" ]; then
tools="elf2dmp $tools"
fi
+ if [ "$linux" = "yes" -a "$tcmu" = "yes" ] ; then
+tools="qemu-tcmu\$(EXESUF
Re: [Qemu-block] [PATCH] virtio-blk: fix comment for virtio_blk_rw_complete
Oh, sorry, this patch should go into trivial mail list.
On Sat, Jul 28, 2018 at 01:18:44PM +0800, Yaowei Bai wrote:
> Here should be submit_requests, there is no submit_merged_requests
> function.
>
> Signed-off-by: Yaowei Bai
> ---
> hw/block/virtio-blk.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
> index 50b5c86..91cbede 100644
> --- a/hw/block/virtio-blk.c
> +++ b/hw/block/virtio-blk.c
> @@ -97,8 +97,8 @@ static void virtio_blk_rw_complete(void *opaque, int ret)
>
> if (req->qiov.nalloc != -1) {
> /* If nalloc is != 1 req->qiov is a local copy of the original
> - * external iovec. It was allocated in submit_merged_requests
> - * to be able to merge requests. */
> + * external iovec. It was allocated in submit_requests to be
> + * able to merge requests. */
> qemu_iovec_destroy(>qiov);
> }
>
> --
> 1.8.3.1
>
[Qemu-block] [PATCH] virtio-blk: fix comment for virtio_blk_rw_complete
Here should be submit_requests, there is no submit_merged_requests
function.
Signed-off-by: Yaowei Bai
---
hw/block/virtio-blk.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 50b5c86..91cbede 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -97,8 +97,8 @@ static void virtio_blk_rw_complete(void *opaque, int ret)
if (req->qiov.nalloc != -1) {
/* If nalloc is != 1 req->qiov is a local copy of the original
- * external iovec. It was allocated in submit_merged_requests
- * to be able to merge requests. */
+ * external iovec. It was allocated in submit_requests to be
+ * able to merge requests. */
qemu_iovec_destroy(>qiov);
}
--
1.8.3.1
Re: [Qemu-block] [PATCH] gitignore: Ignore qapi/qapi-*-job.[ch]
On Thu, Jun 07, 2018 at 11:05:58AM -0400, Jeff Cody wrote: > On Thu, Jun 07, 2018 at 10:43:59AM -0400, Yaowei Bai wrote: > > They were introduced by commit bf42508f24ee(job: Introduce > > qapi/job.json) but forgot to ignore them in .gitignore. > > > > Signed-off-by: Yaowei Bai > > Good patch, but Eric Blake already sent an identical one that has two r-b's > on it already, so that is probably the one to pull in: OK, pls ignore this one. > > > [Qemu-block] [PATCH] block: Ignore generated job QAPI files > essage-Id: <20180531212435.165261-1-ebl...@redhat.com> > > Thanks, > Jeff > > > --- > > .gitignore | 4 > > 1 file changed, 4 insertions(+) > > > > diff --git a/.gitignore b/.gitignore > > index a178e4c..e4bacc6 100644 > > --- a/.gitignore > > +++ b/.gitignore > > @@ -36,6 +36,7 @@ > > /qapi/qapi-commands-common.[ch] > > /qapi/qapi-commands-crypto.[ch] > > /qapi/qapi-commands-introspect.[ch] > > +/qapi/qapi-commands-job.[ch] > > /qapi/qapi-commands-migration.[ch] > > /qapi/qapi-commands-misc.[ch] > > /qapi/qapi-commands-net.[ch] > > @@ -53,6 +54,7 @@ > > /qapi/qapi-events-common.[ch] > > /qapi/qapi-events-crypto.[ch] > > /qapi/qapi-events-introspect.[ch] > > +/qapi/qapi-events-job.[ch] > > /qapi/qapi-events-migration.[ch] > > /qapi/qapi-events-misc.[ch] > > /qapi/qapi-events-net.[ch] > > @@ -71,6 +73,7 @@ > > /qapi/qapi-types-common.[ch] > > /qapi/qapi-types-crypto.[ch] > > /qapi/qapi-types-introspect.[ch] > > +/qapi/qapi-types-job.[ch] > > /qapi/qapi-types-migration.[ch] > > /qapi/qapi-types-misc.[ch] > > /qapi/qapi-types-net.[ch] > > @@ -88,6 +91,7 @@ > > /qapi/qapi-visit-common.[ch] > > /qapi/qapi-visit-crypto.[ch] > > /qapi/qapi-visit-introspect.[ch] > > +/qapi/qapi-visit-job.[ch] > > /qapi/qapi-visit-migration.[ch] > > /qapi/qapi-visit-misc.[ch] > > /qapi/qapi-visit-net.[ch] > > -- > > 1.8.3.1 > > > > > >
[Qemu-block] [PATCH] gitignore: Ignore qapi/qapi-*-job.[ch]
They were introduced by commit bf42508f24ee(job: Introduce qapi/job.json) but forgot to ignore them in .gitignore. Signed-off-by: Yaowei Bai --- .gitignore | 4 1 file changed, 4 insertions(+) diff --git a/.gitignore b/.gitignore index a178e4c..e4bacc6 100644 --- a/.gitignore +++ b/.gitignore @@ -36,6 +36,7 @@ /qapi/qapi-commands-common.[ch] /qapi/qapi-commands-crypto.[ch] /qapi/qapi-commands-introspect.[ch] +/qapi/qapi-commands-job.[ch] /qapi/qapi-commands-migration.[ch] /qapi/qapi-commands-misc.[ch] /qapi/qapi-commands-net.[ch] @@ -53,6 +54,7 @@ /qapi/qapi-events-common.[ch] /qapi/qapi-events-crypto.[ch] /qapi/qapi-events-introspect.[ch] +/qapi/qapi-events-job.[ch] /qapi/qapi-events-migration.[ch] /qapi/qapi-events-misc.[ch] /qapi/qapi-events-net.[ch] @@ -71,6 +73,7 @@ /qapi/qapi-types-common.[ch] /qapi/qapi-types-crypto.[ch] /qapi/qapi-types-introspect.[ch] +/qapi/qapi-types-job.[ch] /qapi/qapi-types-migration.[ch] /qapi/qapi-types-misc.[ch] /qapi/qapi-types-net.[ch] @@ -88,6 +91,7 @@ /qapi/qapi-visit-common.[ch] /qapi/qapi-visit-crypto.[ch] /qapi/qapi-visit-introspect.[ch] +/qapi/qapi-visit-job.[ch] /qapi/qapi-visit-migration.[ch] /qapi/qapi-visit-misc.[ch] /qapi/qapi-visit-net.[ch] -- 1.8.3.1
Re: [Qemu-block] [PATCH 1/3] timer: fix misleading comment in timer.h
On Thu, Dec 01, 2016 at 04:23:17PM +0100, Paolo Bonzini wrote: > > > On 01/12/2016 14:50, Stefan Hajnoczi wrote: > > On Wed, Nov 30, 2016 at 11:30:38PM -0500, Yaowei Bai wrote: > >> It's timer to expire, not clock. > >> > >> Signed-off-by: Yaowei Bai <baiyao...@cmss.chinamobile.com> > >> --- > >> include/qemu/timer.h | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > > > > For the whole series: > > > > Reviewed-by: Stefan Hajnoczi <stefa...@redhat.com> > > > > PS: I suggest sending a cover letter "[PATCH 0/3]" in the future. This > > makes it easy for reviewers to indicate they have reviewed the whole > > series. Without a cover letter it's ambiguous whether my single > > Reviewed-by: applies to just this patch or to the whole series - and > > patch management tools will probably get it wrong too. > > > > I've queued the series for QEMU 2.9. This kind of patch can probably be > sent to qemu-triv...@nongnu.org, which will simplify their inclusion. > > Of course, this is not meant to diminish your contribution! "Trivial" > patches are important and good comments will also help the next person > studying QEMU's source code. Got it from both of you, will correct it in the future, thanks. > > Thanks, > > Paolo
[Qemu-block] [PATCH 3/3] block: drop remaining legacy aio functions in comment
Commit 87f68d318222563822b5c6b28192215fc4b4e441 (block: drop aio functions that operate on the main AioContext) drops qemu_aio_wait function references mostly while leaves these behind, clean up them. Signed-off-by: Yaowei Bai <baiyao...@cmss.chinamobile.com> --- include/block/aio.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/block/aio.h b/include/block/aio.h index 9583c9e..c1c57bb 100644 --- a/include/block/aio.h +++ b/include/block/aio.h @@ -193,8 +193,8 @@ QEMUBH *aio_bh_new(AioContext *ctx, QEMUBHFunc *cb, void *opaque); * aio_notify: Force processing of pending events. * * Similar to signaling a condition variable, aio_notify forces - * aio_wait to exit, so that the next call will re-examine pending events. - * The caller of aio_notify will usually call aio_wait again very soon, + * aio_poll to exit, so that the next call will re-examine pending events. + * The caller of aio_notify will usually call aio_poll again very soon, * or go through another iteration of the GLib main loop. Hence, aio_notify * also has the side effect of recalculating the sets of file descriptors * that the main loop waits for. -- 1.8.3.1
[Qemu-block] [PATCH 2/3] main-loop: update comment for qemu_mutex_lock/unlock_iothread
Commit 49cf57281b7 (vl: delay thread initialization after daemonization) makes the global mutex is taken after daemonization instead before daemonization by qemu_init_main_loop(). Signed-off-by: Yaowei Bai <baiyao...@cmss.chinamobile.com> --- include/qemu/main-loop.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/qemu/main-loop.h b/include/qemu/main-loop.h index 470f600..a9d4f23 100644 --- a/include/qemu/main-loop.h +++ b/include/qemu/main-loop.h @@ -238,7 +238,7 @@ bool qemu_mutex_iothread_locked(void); * qemu_mutex_lock_iothread: Lock the main loop mutex. * * This function locks the main loop mutex. The mutex is taken by - * qemu_init_main_loop and always taken except while waiting on + * main() in vl.c and always taken except while waiting on * external events (such as with select). The mutex should be taken * by threads other than the main loop thread when calling * qemu_bh_new(), qemu_set_fd_handler() and basically all other @@ -253,7 +253,7 @@ void qemu_mutex_lock_iothread(void); * qemu_mutex_unlock_iothread: Unlock the main loop mutex. * * This function unlocks the main loop mutex. The mutex is taken by - * qemu_init_main_loop and always taken except while waiting on + * main() in vl.c and always taken except while waiting on * external events (such as with select). The mutex should be unlocked * as soon as possible by threads other than the main loop thread, * because it prevents the main loop from processing callbacks, -- 1.8.3.1
[Qemu-block] [PATCH 1/3] timer: fix misleading comment in timer.h
It's timer to expire, not clock. Signed-off-by: Yaowei Bai <baiyao...@cmss.chinamobile.com> --- include/qemu/timer.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/qemu/timer.h b/include/qemu/timer.h index 309f3d0..c89ed2a 100644 --- a/include/qemu/timer.h +++ b/include/qemu/timer.h @@ -140,7 +140,7 @@ bool qemu_clock_has_timers(QEMUClockType type); * @type: the clock type * * Determines whether a clock's default timer list - * has an expired clock. + * has an expired timer. * * Returns: true if the clock's default timer list has * an expired timer -- 1.8.3.1
Re: [Qemu-block] [PATCH 2/2] aio: improve aio_poll performance by checking epoll only once
On Wed, Sep 14, 2016 at 05:40:17PM +0100, Stefan Hajnoczi wrote: > On Wed, Sep 14, 2016 at 07:03:39AM -0400, Yaowei Bai wrote: > > As epoll whether enabled or not is a global setting, we can just > > check it only once rather than checking it with every node iteration. > > Through this we can avoid a lot of checks when epoll is not enabled. > > > > Signed-off-by: Yaowei Bai <baiyao...@cmss.chinamobile.com> > > Reviewed-by: Xiubo Li <lixi...@cmss.chinamobile.com> > > --- > > aio-posix.c | 12 +++- > > 1 file changed, 7 insertions(+), 5 deletions(-) > > The commit message says "improve aio_poll performance" but no benchmark > results were provided. Therefore I can't take this patch as a > performance optimization. I'm still happy to merge the patch since it > makes the if statement simpler but I'll rename it to "aio-posix: avoid > unnecessary aio_epoll_enabled() calls". Sorry for replying late, i just came back from my vacation. And i'd like to say it's ok for me to rename the commit message. Thank you for doing it. > > I don't think this patch gives a measurable performance improvement. If Yes, i agree with you at this point in consideration of there's no too many fds to poll at most time. > you believe otherwise, please post benchmark results. Please let me > know what you think. > > Stefan
[Qemu-block] [PATCH 0/2] improve aio_poll performance
This patchset change to check epoll's enablement first before nodes iteration to improve aio_poll()'s performance. Also fix a wrong comment of mirror_start(). Yaowei Bai (2): block: mirror: fix wrong comment of mirror_start aio: improve aio_poll performance by checking epoll only once aio-posix.c | 12 +++- include/block/block_int.h | 2 +- 2 files changed, 8 insertions(+), 6 deletions(-) -- 1.8.3.1
[Qemu-block] [PATCH 2/2] aio: improve aio_poll performance by checking epoll only once
As epoll whether enabled or not is a global setting, we can just
check it only once rather than checking it with every node iteration.
Through this we can avoid a lot of checks when epoll is not enabled.
Signed-off-by: Yaowei Bai <baiyao...@cmss.chinamobile.com>
Reviewed-by: Xiubo Li <lixi...@cmss.chinamobile.com>
---
aio-posix.c | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/aio-posix.c b/aio-posix.c
index 43162a9..4ef34dd 100644
--- a/aio-posix.c
+++ b/aio-posix.c
@@ -431,11 +431,13 @@ bool aio_poll(AioContext *ctx, bool blocking)
assert(npfd == 0);
/* fill pollfds */
-QLIST_FOREACH(node, >aio_handlers, node) {
-if (!node->deleted && node->pfd.events
-&& !aio_epoll_enabled(ctx)
-&& aio_node_check(ctx, node->is_external)) {
-add_pollfd(node);
+
+if (!aio_epoll_enabled(ctx)) {
+QLIST_FOREACH(node, >aio_handlers, node) {
+if (!node->deleted && node->pfd.events
+&& aio_node_check(ctx, node->is_external)) {
+add_pollfd(node);
+}
}
}
--
1.8.3.1
[Qemu-block] [PATCH 1/2] block: mirror: fix wrong comment of mirror_start
Obviously, we should write to '@target'. Signed-off-by: Yaowei Bai <baiyao...@cmss.chinamobile.com> Reviewed-by: Xiubo Li <lixi...@cmss.chinamobile.com> --- include/block/block_int.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/block/block_int.h b/include/block/block_int.h index 1e939de..77d4e65 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -730,7 +730,7 @@ void commit_active_start(const char *job_id, BlockDriverState *bs, * @errp: Error object. * * Start a mirroring operation on @bs. Clusters that are allocated - * in @bs will be written to @bs until the job is cancelled or + * in @bs will be written to @target until the job is cancelled or * manually completed. At the end of a successful mirroring job, * @bs will be switched to read from @target. */ -- 1.8.3.1
